fix: add commit-reveal delay to prevent front-running (#15)

* fix: update foundry settings

* fix: add commit-reveal delay to prevent frontrunning

Author: varunsrin

.gas-snapshot

@@ -19,52 +19,53 @@ AccountRegistryTest:testRegister() (gas: 89924)
 AccountRegistryTest:testRequestRecovery() (gas: 144852)
 AccountRegistryTest:testSetRecoveryAddress() (gas: 95427)
 AccountRegistryTest:testTransfer() (gas: 75750)
-NameSpaceTest:testBidAfterOneStep() (gas: 217603)
-NameSpaceTest:testBidAndOverpay() (gas: 213134)
-NameSpaceTest:testBidFlatRate() (gas: 215981)
-NameSpaceTest:testBidImmediately() (gas: 229318)
-NameSpaceTest:testBidOnHundredthStep() (gas: 215959)
-NameSpaceTest:testBidOnPenultimateStep() (gas: 215971)
-NameSpaceTest:testBidShouldClearRecovery() (gas: 259941)
-NameSpaceTest:testCancelRecoveryFromCustodyAddress() (gas: 221316)
-NameSpaceTest:testCancelRecoveryFromRecoveryAddress() (gas: 220922)
-NameSpaceTest:testCannotBidIfRegistrable() (gas: 30961)
-NameSpaceTest:testCannotBidUnlessBiddable() (gas: 173313)
-NameSpaceTest:testCannotCancelRecoveryIfNotStarted() (gas: 185540)
-NameSpaceTest:testCannotCancelRecoveryIfUnauthorized() (gas: 232109)
-NameSpaceTest:testCannotCommitWithInvalidName() (gas: 22905)
-NameSpaceTest:testCannotCompleteRecoveryIfExpired() (gas: 244138)
-NameSpaceTest:testCannotCompleteRecoveryIfNotStarted() (gas: 189819)
-NameSpaceTest:testCannotCompleteRecoveryIfUnauthorized() (gas: 217405)
-NameSpaceTest:testCannotCompleteRecoveryWhenInEscrow() (gas: 233743)
+NameSpaceTest:testBidAfterOneStep() (gas: 220057)
+NameSpaceTest:testBidAndOverpay() (gas: 215676)
+NameSpaceTest:testBidFlatRate() (gas: 218435)
+NameSpaceTest:testBidImmediately() (gas: 231728)
+NameSpaceTest:testBidOnHundredthStep() (gas: 218435)
+NameSpaceTest:testBidOnPenultimateStep() (gas: 218425)
+NameSpaceTest:testBidShouldClearRecovery() (gas: 261975)
+NameSpaceTest:testCancelRecoveryFromCustodyAddress() (gas: 223880)
+NameSpaceTest:testCancelRecoveryFromRecoveryAddress() (gas: 223486)
+NameSpaceTest:testCannotBidIfRegistrable() (gas: 30917)
+NameSpaceTest:testCannotBidUnlessBiddable() (gas: 175767)
+NameSpaceTest:testCannotCancelRecoveryIfNotStarted() (gas: 188104)
+NameSpaceTest:testCannotCancelRecoveryIfUnauthorized() (gas: 234673)
+NameSpaceTest:testCannotCommitWithInvalidName() (gas: 22838)
+NameSpaceTest:testCannotCompleteRecoveryIfExpired() (gas: 246680)
+NameSpaceTest:testCannotCompleteRecoveryIfNotStarted() (gas: 192361)
+NameSpaceTest:testCannotCompleteRecoveryIfUnauthorized() (gas: 219947)
+NameSpaceTest:testCannotCompleteRecoveryWhenInEscrow() (gas: 236285)
 NameSpaceTest:testCannotReclaimUnlessMinted() (gas: 17407)
-NameSpaceTest:testCannotRegisterWithoutPayment() (gas: 85851)
-NameSpaceTest:testCannotRenewIfBiddable() (gas: 162869)
-NameSpaceTest:testCannotRenewIfRegistered() (gas: 162953)
+NameSpaceTest:testCannotRegisterBeforeDelay() (gas: 90808)
+NameSpaceTest:testCannotRegisterWithoutPayment() (gas: 85835)
+NameSpaceTest:testCannotRenewIfBiddable() (gas: 165411)
+NameSpaceTest:testCannotRenewIfRegistered() (gas: 165495)
 NameSpaceTest:testCannotRenewIfRegistrable() (gas: 31908)
 NameSpaceTest:testCannotRenewIfRegistrable2() (gas: 31950)
-NameSpaceTest:testCannotRenewWithoutPayment() (gas: 156012)
+NameSpaceTest:testCannotRenewWithoutPayment() (gas: 158554)
 NameSpaceTest:testCannotRequestRecoveryIfRegistrable() (gas: 26459)
-NameSpaceTest:testCannotRequestRecoveryToZeroAddr() (gas: 185400)
-NameSpaceTest:testCannotRequestRecoveryUnlessAuthorized() (gas: 163808)
-NameSpaceTest:testCannotSetRecoveryIfExpired() (gas: 164996)
+NameSpaceTest:testCannotRequestRecoveryToZeroAddr() (gas: 187942)
+NameSpaceTest:testCannotRequestRecoveryUnlessAuthorized() (gas: 166372)
+NameSpaceTest:testCannotSetRecoveryIfExpired() (gas: 167538)
 NameSpaceTest:testCannotSetRecoveryIfRegistrable() (gas: 20896)
-NameSpaceTest:testCannotSetRecoveryUnlessOwner() (gas: 158592)
+NameSpaceTest:testCannotSetRecoveryUnlessOwner() (gas: 161134)
 NameSpaceTest:testCurrYear() (gas: 78247)
 NameSpaceTest:testCurrYearPayment() (gas: 45881)
 NameSpaceTest:testGenerateCommit() (gas: 40350)
-NameSpaceTest:testOwnerOfRevertsIfExpired() (gas: 152771)
+NameSpaceTest:testOwnerOfRevertsIfExpired() (gas: 155313)
 NameSpaceTest:testOwnerOfRevertsIfRegistrable() (gas: 12872)
-NameSpaceTest:testReclaimBiddableNames() (gas: 181528)
-NameSpaceTest:testReclaimRegisteredNames() (gas: 190207)
-NameSpaceTest:testReclaimRenewableNames() (gas: 181560)
-NameSpaceTest:testReclaimResetsRecoveryState() (gas: 226069)
-NameSpaceTest:testRecoveryCompletion() (gas: 245134)
-NameSpaceTest:testRegister() (gas: 255587)
-NameSpaceTest:testRenewOther() (gas: 174360)
-NameSpaceTest:testRenewSelf() (gas: 171777)
-NameSpaceTest:testRenewWithOverpayment() (gas: 177327)
-NameSpaceTest:testRequestRecovery() (gas: 241593)
-NameSpaceTest:testSetRecoveryAddress() (gas: 189689)
-NameSpaceTest:testTransferFromCannotTransferExpiredName() (gas: 171463)
-NameSpaceTest:testTransferFromResetsRecovery() (gas: 225429)
+NameSpaceTest:testReclaimBiddableNames() (gas: 184070)
+NameSpaceTest:testReclaimRegisteredNames() (gas: 192240)
+NameSpaceTest:testReclaimRenewableNames() (gas: 184124)
+NameSpaceTest:testReclaimResetsRecoveryState() (gas: 228103)
+NameSpaceTest:testRecoveryCompletion() (gas: 247168)
+NameSpaceTest:testRegister() (gas: 259089)
+NameSpaceTest:testRenewOther() (gas: 176902)
+NameSpaceTest:testRenewSelf() (gas: 174319)
+NameSpaceTest:testRenewWithOverpayment() (gas: 179869)
+NameSpaceTest:testRequestRecovery() (gas: 244135)
+NameSpaceTest:testSetRecoveryAddress() (gas: 192253)
+NameSpaceTest:testTransferFromCannotTransferExpiredName() (gas: 173939)
+NameSpaceTest:testTransferFromResetsRecovery() (gas: 227445)

foundry.toml

@@ -1,4 +1,4 @@
-[default]
+[profile.default]
 src = 'src'
 out = 'out'
 libs = ['lib']

src/Namespace.sol

@@ -54,8 +54,8 @@ contract Namespace is ERC721, Owned {
                         REGISTRATION STORAGE
     //////////////////////////////////////////////////////////////*/
 
-    // Mapping from commitment hash to state
-    mapping(bytes32 => bool) public stateOf;
+    // Mapping from commitment hash to block number
+    mapping(bytes32 => uint256) public blockOf;
 
     // Mapping from tokenID to expiration year
     mapping(uint256 => uint256) public expiryOf;
@@ -162,12 +162,15 @@ contract Namespace is ERC721, Owned {
      * @param commit the commitment hash to be persisted on-chain
      */
     function makeCommit(bytes32 commit) public {
-        stateOf[commit] = true;
+        blockOf[commit] = block.timestamp;
     }
 
     /**
      * @notice Mint a new username if a commitment was made previously and send it to the owner.
      *
+     * @dev The registration must be made at least 5 blocks after commit to minimize front-running,
+     * or approximately 1 minute after commit.
+     *
      * @param username the username to register
      * @param owner the address that will claim the username
      * @param secret the secret that protects the commitment
@@ -182,8 +185,9 @@ contract Namespace is ERC721, Owned {
         uint256 _currYearFee = currYearFee();
         if (msg.value < _currYearFee) revert InsufficientFunds();
 
-        if (!stateOf[commit]) revert InvalidCommit();
-        delete stateOf[commit];
+        uint256 _commitBlock = blockOf[commit];
+        if (_commitBlock == 0 || _commitBlock + 60 > block.timestamp) revert InvalidCommit();
+        delete blockOf[commit];
 
         uint256 tokenId = uint256(bytes32(username));
         if (expiryOf[tokenId] != 0) revert NotRegistrable();

test/Namespace.t.sol

@@ -31,6 +31,7 @@ contract NameSpaceTest is Test {
     address david = address(0x531);
 
     uint256 escrowPeriod = 3 days;
+    uint256 commitRegisterDelay = 60;
 
     uint256 timestamp2023 = 1672531200; // Sun, Jan 1, 2023 0:00:00 GMT
     uint256 timestamp2024 = 1704067200; // Sun, Jan 1, 2024 0:00:00 GMT
@@ -109,6 +110,7 @@ contract NameSpaceTest is Test {
         namespace.makeCommit(commitHash);
 
         // 3. Register the name alice, and deliver it to bob
+        vm.warp(block.timestamp + commitRegisterDelay);
         vm.expectEmit(true, true, true, false);
         emit Transfer(address(0), bob, uint256(bytes32("alice")));
         uint256 balance = alice.balance;
@@ -122,11 +124,14 @@ contract NameSpaceTest is Test {
         // 5. Check that comitting and minting again fails
         namespace.makeCommit(commitHash);
         vm.expectRevert(NotRegistrable.selector);
+        vm.warp(block.timestamp + commitRegisterDelay);
         namespace.register{value: 0.01 ether}("alice", bob, "secret");
 
         // 6. Check that alice can still mint another name to bob
         bytes32 commitHashMorty = namespace.generateCommit("morty", bob, "secret");
         namespace.makeCommit(commitHashMorty);
+        vm.warp(block.timestamp + commitRegisterDelay);
+
         namespace.register{value: 0.01 ether}("morty", bob, "secret");
         assertEq(namespace.ownerOf(uint256(bytes32("morty"))), bob);
         assertEq(namespace.balanceOf(bob), 2);
@@ -181,6 +186,24 @@ contract NameSpaceTest is Test {
         vm.stopPrank();
     }
 
+    function testCannotRegisterBeforeDelay() public {
+        // 1. Give alice money and fast forward to 2022 to begin registration.
+        vm.deal(alice, 10_000 ether);
+        vm.warp(aliceRegisterTs);
+
+        // 2. Make the commitment to register the name alice, but deliver it to bob
+        vm.startPrank(alice);
+        bytes32 commitHash = namespace.generateCommit("alice", bob, "secret");
+        namespace.makeCommit(commitHash);
+
+        // 3. Try to register the name and fail
+        vm.warp(block.timestamp + commitRegisterDelay - 1);
+        vm.expectRevert(InvalidCommit.selector);
+        namespace.register{value: 0.01 ether}("alice", bob, "secret");
+
+        vm.stopPrank();
+    }
+
     function testCannotRegisterWithInvalidNames(address owner, bytes32 secret) public {
         bytes16 incorrectUsername = "al{ce";
         bytes32 invalidCommit = keccak256(abi.encode(incorrectUsername, owner, secret));
@@ -1062,6 +1085,7 @@ contract NameSpaceTest is Test {
         vm.startPrank(alice);
         bytes32 commitHash = namespace.generateCommit("alice", alice, "secret");
         namespace.makeCommit(commitHash);
+        vm.warp(block.timestamp + commitRegisterDelay);
 
         namespace.register{value: namespace.fee()}("alice", alice, "secret");
         assertEq(namespace.expiryOf(aliceTokenId), timestamp2023);