Centralize the issuance of new tokens

Hi everyone, I have the goal of centralizing the issuance of new tokens using meta-transactions but I must be sure if what I've achieve is correct.

Goal: a user mints X tokens in my contract ONLY if the owner (me) generates a valid hash in my backend.
The general idea of this process is:

  • my backend generates a valid hash using keccak256 like so:
function generateValidHash(tokens) {
        const privateKey = '0xajwdiuahdaiuhd';
        const address = account1;
        let random = parseInt(Math.random() * 100000000);
        let message = EthCrypto.hash.keccak256(
            [
                { type: "uint256", value: random },
                { type: "address", value: address },
                { type: "uint256", value: tokens }
            ]
        );

        const signature = EthCrypto.sign(privateKey, message);
        return {
            signature: signature,
            hash: message,
            nonce: random,
            tokens: tokens
        }
    }
  • the client receives the values returned by the "generateValidHash" function
  • the client calls the Smart Contract function "mintTokens"
  • the Smart Contract checks if it can rebuild the hash with the given parameters, if it succeed it checks if the hash was signed by the owner. If everything works then the smart contract mints the tokens to the msg.sender (more about the importance of msg.sender below).

IMPORTANCE OF THE MSG.SENDER
Normally a miner could take the hash that the client is passing to the Smart Contract and mint the tokens to his address. So I hashed the address of the user that will be able to receive the tokens because the msg.sender can't be changed by the miner when executing a transaction. So nobody will be able to rebuild the correct hash but the correct user with the address that I hashed

I developed a Smart Contract and a few tests that mimic this goal.
This is the function that mints the tokens.

 /**
     * @dev This function must mint the tokens given as param only if the 
     *      hashed message is correctly rebuilt and the hash was signed by
     *      the owner of the contract.
     * @notice Without using the msg.sender in the reconstruction of the 
     *      signed message a miner could take that hash and use it. 
     *      Using the msg.sender we're making sure that only the one who's calling
     *      this function can rebuild the original message.
     */
    function mintFunction(
        bytes32 _hash,
        bytes memory _signature,
        uint256 _nonce,
        uint256 _amount
    ) public {
        bytes32 message = keccak256(
            abi.encodePacked(_nonce, msg.sender, _amount)
        );
        require(!hashes[_hash], "This hash was already used");
        require(
            message == _hash,
            "I could not reconstruct the original message, hence it is not valid"
        );
        require(ECDSA.recover(_hash, _signature) == owner, "This message was not signed by the owner of the contract");

        balance[msg.sender] += _amount;
        hashes[_hash] = true;
        hashesUsed.push(_hash);
    }

This is the full testing unit:

const HashRegistry = artifacts.require('./Hashing.sol');
const EthCrypto = require('eth-crypto');

contract('HashRegistry', async accounts => {
    const account1 = accounts[0];
    const account2 = accounts[1];

    let hashRegistryInstance;

    function generateInvalidHash(tokens) {
        const privateKey = 'privateKey1';
        const address = account2;
        let random = parseInt(Math.random() * 100000000);
        let message = EthCrypto.hash.keccak256(
            [
                { type: "uint256", value: random },
                { type: "address", value: address },
                { type: "uint256", value: tokens }
            ]
        );

        const signature = EthCrypto.sign(privateKey, message);
        return {
            signature: signature,
            hash: message,
            nonce: random,
            tokens: tokens
        }
    }

    function generateValidHash(tokens) {
        const privateKey = 'privateKey2';
        const address = account1;
        let random = parseInt(Math.random() * 100000000);
        let message = EthCrypto.hash.keccak256(
            [
                { type: "uint256", value: random },
                { type: "address", value: address },
                { type: "uint256", value: tokens }
            ]
        );

        const signature = EthCrypto.sign(privateKey, message);
        return {
            signature: signature,
            hash: message,
            nonce: random,
            tokens: tokens
        }
    }
    before(async function () {
        try {
            await web3.eth.personal.unlockAccount(accounts[0], "", 10000);
            await web3.eth.personal.unlockAccount(accounts[1], "", 10000);
            hashRegistryInstance = await HashRegistry.deployed();
        } catch (error) {
            console.warn(error);
        }
    });
    describe('A client generates correct hashes with a private key', () => {
        it('generates a valid hash and check that the hash was registered in the contract', async () => {
            const tokens = 9000;
            const { signature, hash, nonce } = generateValidHash(tokens);

            await hashRegistryInstance.mintFunction(
                hash,
                signature,
                nonce,
                tokens,
                { from: account1 }
            );
            const hashes = await hashRegistryInstance.getHashes();
            const balance = await hashRegistryInstance.getBalance(account1);
            assert.equal(hashes.length > 0, true);
            assert.equal(hashes[0], hash);
            assert.equal(balance.toNumber(), tokens);
        })
        it('reverts because another user tries to use the message that was signed by the winner of the tokens', async () => {
            let didThrow = false;
            const tokens = 9000;
            const { signature, hash, nonce } = generateValidHash(tokens);
            
            try {
                await hashRegistryInstance.mintFunction(
                    hash,
                    signature,
                    nonce,
                    tokens,
                    { from: account2 }
                );
            } catch (error) {
                didThrow = true;
                assert.equal(error.data.reason.includes('I could not reconstruct the original message, hence it is not valid'), true);
            }
            assert.equal(didThrow, true);
        })
        it('throws because the legit user try to cheat and change the tokens to redeem', async () => {
            let didThrow = false;
            let tokens = 9000;
            const { signature, hash, nonce } = generateValidHash(tokens);
            tokens = 1000000;
            
            try {
                await hashRegistryInstance.mintFunction(
                    hash,
                    signature,
                    nonce,
                    tokens,
                    { from: account1 }
                );
            } catch (error) {
                didThrow = true;
                assert.equal(error.data.reason.includes('I could not reconstruct the original message, hence it is not valid'), true);
            }
            assert.equal(didThrow, true);
        })
        it('throws because the a user generated an invalid hash', async () => {
            let didThrow = false;
            let tokens = 9000;
            const { signature, hash, nonce } = generateInvalidHash(tokens);
            
            try {
                await hashRegistryInstance.mintFunction(
                    hash,
                    signature,
                    nonce,
                    tokens,
                    { from: account2 }
                );
            } catch (error) {
                didThrow = true;
                assert.equal(error.data.reason.includes('This message was not signed by the owner of the contract'), true);
            }
            assert.equal(didThrow, true);
        })
        it('throws because the a user generates an invalid hash but uses a valid signature', async () => {
            let didThrow = false;
            let tokens = 9000;
            const invalidData = generateInvalidHash(tokens);
            const validData = generateValidHash(tokens);
            
            try {
                await hashRegistryInstance.mintFunction(
                    invalidData.hash,
                    validData.signature,
                    validData.nonce,
                    tokens,
                    { from: account1 }
                );
            } catch (error) {
                didThrow = true;
                assert.equal(error.data.reason.includes('I could not reconstruct the original message, hence it is not valid'), true);
                console.error(error);
            }
            assert.equal(didThrow, true);
        })
    })
})

I hope to find someone who can help me figure out if what I'm doing is safe or not.

EDIT1: I forgot to tell that I'm developing using ganache and truffle. All the tests you see pass correctly

I would still use the nonce in case the signed transaction was not submitted and executed at a later time.

You would also want signature that are only valid on a specific chain.

Take a look at EIP712