Commit-Reveal contract still vulnerable to front running. How can it be improved?

Question

I'm writing a contract and I want to shield it from front-running as best as it can possible be done (obviously, fully preventing front running would be ideal). My implementation is very similar to the Raffle implementation from @Ismael here so I will be using that:

contract Raffle {

    mapping(address => bytes32) commitments;
    mapping(uint256 => address) reserved;

    event Reserved(uint256 value, address owner);
    event Committed(bytes32 hash);

    function commit(bytes32 hash) public {
        require(commitments[msg.sender] == bytes32(0), "Already committed");
        commitments[msg.sender] = hash;
        emit Committed(hash); // Added this event for similarity
    }

    function reveal(uint256 nonce, uint256 value) public {
        bytes32 d = digest(nonce, value, msg.sender);
        require(commitments[msg.sender] == d, "Invalid data");
        require(reserved[value] == address(0), "Already reserved");
        reserved[value] = msg.sender;
        emit Reserved(value, msg.sender);
    }

    function digest(uint256 nonce, uint256 value, address sender) public pure returns (bytes32) {
        return keccak256(abi.encodePacked(nonce, value, sender));
    }
}

This contract is vulnerable to the following front running scenario:

1. Honest user makes a commit. Commit is a hash so there is nothing the attacker can do at this point.
2. Eventually honest user will make transaction to use the reveal method. At this point both the value and nonce are made public.
3. Before the honest user's transaction is picked from the mempool, the attacker get's the now revealed parameters value and nonce, sends a high gas transaction to make his commit.
4. In sequence frontruns the reveal transaction from honest user. (3 and 4 may even be done in the same block)

What measures can I take to prevent/mitigate this behavior?

• Just like in the contract above I need both commit and reveal methods to be permanently open for interactions (Meaning I can't separate the methods in commit phase and reveal phase, as it would be done in a sealed bid auction)

---

My current idea for mitigation is to register the block.number with the commit, and in the reveal method only reserve the value if an arbitrary number of blocks has been mined since the commit was made. E.g: You made the commit on block 100, you need to wait to block 110 to call reval (otherwise the transaction is reverted). This gives a 10 blocks "advantage" to the honest user, as attacker will need to wait 10 blocks to try to frontrun.

The cons:

• It doesn't solve the problem, the reveal transaction may stay X blocks (X being the arbitrary number of blocks to wait) on the mempool, due to clogged blockchain, low gas, or other reasons. Long enough for the attacker to frontrun.
• If the honest user mistakenly calls the reveal function before the number of blocks is mined he will have a failed transaction and the parameters will be revealed, cutting his advantage short.
• Hard to determine a reasonable number of blocks.

Answer 1

I also propose a solution based on the block.number, which is safer than using block.timestamp, but with a different approach. Rather than using it to secure a reservation after several blocks, I would use it as an ordering criterion in case that multiple users go for the same number.

When a user commits a hash, the current block number is captured in the commitment struct. Afterwards, when a user reveals his/her number, the reveal function will determine if the number was:

a) Free => it will be assigned to the current user

b) Not free => it will compare the block numbers between the current user and the previously assigned user, and will update to the user with the oldest block (the first user that committed that number)

The updated code would look like this one:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.0;

contract Raffle {
    struct Commitments {
        bytes32 commitment;
        uint256 blockNumber;
    }

    mapping(address => Commitments) commitments;
    mapping(uint256 => address) reserved;

    function commit(bytes32 hash) external {
        require(commitments[msg.sender].commitment == bytes32(0), "Already committed");
        commitments[msg.sender] = Commitments(hash, block.number);
    }

    function reveal(uint256 nonce, uint256 value) external {
        bytes32 d = digest(nonce, value, msg.sender);
        require(commitments[msg.sender].commitment == d, "Invalid data");
        if (reserved[value] == address(0)) {
            reserved[value] = msg.sender;
        } else if (commitments[reserved[value]].blockNumber > commitments[msg.sender].blockNumber) {
            reserved[value] = msg.sender;
        } else {
            revert('Already reserved');
        }
    }

    function digest(uint256 nonce, uint256 value, address sender) public pure returns (bytes32) {
        return keccak256(abi.encodePacked(nonce, value, sender));
    }
}

Answer 2

Update the commit function such that a particular hash can only be submitted/accepted once.

  mapping(bytes32 => bool) committedHash;

  function commit(bytes32 hash) public {
    require(commitments[msg.sender] == bytes32(0), "Already committed");
    require(committedHash[hash] == false, "this hash is already used");
    committedHash[hash] = true;
    commitments[msg.sender] = hash;
    emit Committed(hash); // Added this event for similarity
}

Answer 3

This should solve your issue. I added comments on the lines that I changed so you can spot them easily.

Let me know if it works for your use case!

contract Raffle {

    mapping(address => bytes32) commitments;
    mapping(uint256 => address) reserved;
    mapping(bytes32 => address) commitOwners; // ADD THIS

    event Reserved(uint256 value, address owner);
    event Committed(bytes32 hash);

    function commit(bytes32 hash) public {
        require(commitments[msg.sender] == bytes32(0), "Already committed");
        require(commitOwners[hash] == address(0), "I HATE YOU FRONT RUNNER!"); // THIS MAKES IT IMPOSSIBLE TO COMMIT A NONCE/VALUE HASH THAT WAS ALREADY COMMITTED
        commitments[msg.sender] = hash;
        commitOwners[hash] = msg.sender;
        emit Committed(hash);
    }

    function reveal(uint256 nonce, uint256 value) public {
        bytes32 d = digest(nonce, value); // REMOVE MSG.SENDER
        require(commitments[msg.sender] == d, "Invalid data");
        require(reserved[value] == address(0), "Already reserved");
        reserved[value] = msg.sender;
        emit Reserved(value, msg.sender);
    }

    // REMOVE SENDER IN THIS FUNCTION
    function digest(uint256 nonce, uint256 value) public pure returns (bytes32) {
        return keccak256(abi.encodePacked(nonce, value));
    }
}