fix: correct account registry invariant docs, remove unnecessary check for recoveryAddress (#16)

* docs: explain more explicitly where the invariant holds

* opt: remove account registry check for recoveryAddress != custodyAddress

* fix: gas snapshot

Author: varunsrin

.gas-snapshot

@@ -1,23 +1,22 @@
-AccountRegistryTest:testCancelRecoveryFromCustodyAddress() (gas: 134666)
-AccountRegistryTest:testCancelRecoveryFromRecoveryAddress() (gas: 134831)
-AccountRegistryTest:testCannotCancelRecoveryIfNotStarted() (gas: 92950)
-AccountRegistryTest:testCannotCancelRecoveryIfUnauthorized() (gas: 143175)
-AccountRegistryTest:testCannotCompleteRecoveryIfNotStarted() (gas: 103929)
-AccountRegistryTest:testCannotCompleteRecoveryIfUnauthorized() (gas: 145990)
-AccountRegistryTest:testCannotCompleteRecoveryToAddressThatOwnsAnId() (gas: 170123)
-AccountRegistryTest:testCannotCompleteRecoveryWhenInEscrow() (gas: 143704)
-AccountRegistryTest:testCannotRegisterTwice() (gas: 59173)
-AccountRegistryTest:testCannotRequestRecoveryToAddressThatOwnsAnId() (gas: 115853)
-AccountRegistryTest:testCannotRequestRecoveryUnlessAuthorized() (gas: 69275)
-AccountRegistryTest:testCannotRequestRecoveryUnlessIssued() (gas: 17819)
-AccountRegistryTest:testCannotSetRecoveryAddressWithoutId() (gas: 18914)
-AccountRegistryTest:testCannotSetSelfAsRecovery() (gas: 61534)
-AccountRegistryTest:testCannotTransferIfNoId() (gas: 21711)
-AccountRegistryTest:testCannotTransferToAddressWithId() (gas: 90483)
-AccountRegistryTest:testRecoveryCompletion() (gas: 133092)
+AccountRegistryTest:testCancelRecoveryFromCustodyAddress() (gas: 134604)
+AccountRegistryTest:testCancelRecoveryFromRecoveryAddress() (gas: 134769)
+AccountRegistryTest:testCannotCancelRecoveryIfNotStarted() (gas: 92888)
+AccountRegistryTest:testCannotCancelRecoveryIfUnauthorized() (gas: 143224)
+AccountRegistryTest:testCannotCompleteRecoveryIfNotStarted() (gas: 103867)
+AccountRegistryTest:testCannotCompleteRecoveryIfUnauthorized() (gas: 145950)
+AccountRegistryTest:testCannotCompleteRecoveryToAddressThatOwnsAnId() (gas: 170083)
+AccountRegistryTest:testCannotCompleteRecoveryWhenInEscrow() (gas: 143753)
+AccountRegistryTest:testCannotRegisterTwice() (gas: 59151)
+AccountRegistryTest:testCannotRequestRecoveryToAddressThatOwnsAnId() (gas: 115813)
+AccountRegistryTest:testCannotRequestRecoveryUnlessAuthorized() (gas: 69253)
+AccountRegistryTest:testCannotRequestRecoveryUnlessIssued() (gas: 17797)
+AccountRegistryTest:testCannotSetRecoveryAddressWithoutId() (gas: 18880)
+AccountRegistryTest:testCannotTransferIfNoId() (gas: 21689)
+AccountRegistryTest:testCannotTransferToAddressWithId() (gas: 90461)
+AccountRegistryTest:testRecoveryCompletion() (gas: 133042)
 AccountRegistryTest:testRegister() (gas: 89924)
-AccountRegistryTest:testRequestRecovery() (gas: 144852)
-AccountRegistryTest:testSetRecoveryAddress() (gas: 95427)
+AccountRegistryTest:testRequestRecovery() (gas: 144812)
+AccountRegistryTest:testSetRecoveryAddress() (gas: 95325)
 AccountRegistryTest:testTransfer() (gas: 75750)
 NameSpaceTest:testBidAfterOneStep() (gas: 220057)
 NameSpaceTest:testBidAndOverpay() (gas: 215676)

src/AccountRegistry.sol

@@ -131,7 +131,8 @@ contract AccountRegistry {
     //////////////////////////////////////////////////////////////*/
 
     /**
-     * INVARIANT 1:  idOf[address] != 0 if msg.sender == recoveryOf[idOf[address]]
+     * INVARIANT 1:  idOf[address] != 0 if msg.sender == recoveryOf[idOf[address]] during
+     * invocation of requestRecovery, completeRecovery and cancelRecovery
      *
      * recoveryOf[idOf[address]] != address(0) only if idOf[address] != 0 [setRecoveryAddress]
      * when idOf[address] == 0, recoveryof[idOf[address]] also == address(0) [_unsafeTransfer]
@@ -157,7 +158,6 @@ contract AccountRegistry {
         uint256 id = idOf[msg.sender];
 
         if (id == 0) revert ZeroId();
-        if (recoveryAddress == msg.sender) revert InvalidRecoveryAddr();
 
         recoveryOf[id] = recoveryAddress;
         emit SetRecoveryAddress(recoveryAddress, id);

test/AccountRegistry.t.sol

@@ -141,19 +141,6 @@ contract AccountRegistryTest is Test {
         assertEq(accountRegistry.recoveryOf(1), charlie);
     }
 
-    function testCannotSetSelfAsRecovery() public {
-        // 1. alice registers id 1
-        vm.startPrank(alice);
-        accountRegistry.register();
-
-        // 2. alice sets herself as the recovery address, which fails
-        vm.expectRevert(InvalidRecoveryAddr.selector);
-        accountRegistry.setRecoveryAddress(alice);
-        vm.stopPrank();
-
-        assertEq(accountRegistry.recoveryOf(1), address(0));
-    }
-
     function testCannotSetRecoveryAddressWithoutId() public {
         vm.startPrank(alice);
         vm.expectRevert(ZeroId.selector);