Merge pull request #60 from coinbase/wilson/ownerCount

add ownerCount method

.gas-snapshot

@@ -1,72 +1,72 @@
-AddOwnerAddressTest:testEmitsAddOwner() (gas: 92402)
-AddOwnerAddressTest:testIncreasesOwnerIndex() (gas: 90676)
-AddOwnerAddressTest:testRevertsIfAlreadyOwner() (gas: 93191)
-AddOwnerAddressTest:testRevertsIfCalledByNonOwner() (gas: 11764)
-AddOwnerAddressTest:testSetsIsOwner() (gas: 90470)
-AddOwnerAddressTest:testSetsOwnerAtIndex() (gas: 98970)
-AddOwnerPublicKeyTest:testEmitsAddOwner() (gas: 115537)
-AddOwnerPublicKeyTest:testFuzzIsOwnerPublicKey(bytes32,bytes32) (runs: 256, μ: 115328, ~: 115328)
-AddOwnerPublicKeyTest:testRevertsIfAlreadyOwner() (gas: 116355)
-AddOwnerPublicKeyTest:testRevertsIfCalledByNonOwner() (gas: 11754)
-AddOwnerPublicKeyTest:testSetsIsOwner() (gas: 113769)
-AddOwnerPublicKeyTest:testSetsOwnerAtIndex() (gas: 128616)
-CoinbaseSmartWallet1271InputGeneratorTest:testGetReplaySafeHashForDeployedAccount() (gas: 308446)
-CoinbaseSmartWallet1271InputGeneratorTest:testGetReplaySafeHashForUndeployedAccount() (gas: 291475)
-CoinbaseSmartWalletFactoryTest:testDeployDeterministicPassValues() (gas: 267250)
-CoinbaseSmartWalletFactoryTest:test_CreateAccount_ReturnsPredeterminedAddress_WhenAccountAlreadyExists() (gas: 286914)
-CoinbaseSmartWalletFactoryTest:test_RevertsIfLength32ButLargerThanAddress() (gas: 298636)
-CoinbaseSmartWalletFactoryTest:test_constructor_setsImplementation(address) (runs: 256, μ: 314355, ~: 314355)
-CoinbaseSmartWalletFactoryTest:test_createAccountDeploysToPredeterminedAddress() (gas: 268601)
-CoinbaseSmartWalletFactoryTest:test_createAccountSetsOwnersCorrectly() (gas: 277291)
-CoinbaseSmartWalletFactoryTest:test_exitIfAccountIsAlreadyInitialized() (gas: 267718)
+AddOwnerAddressTest:testEmitsAddOwner() (gas: 92424)
+AddOwnerAddressTest:testIncreasesOwnerIndex() (gas: 90698)
+AddOwnerAddressTest:testRevertsIfAlreadyOwner() (gas: 93235)
+AddOwnerAddressTest:testRevertsIfCalledByNonOwner() (gas: 11786)
+AddOwnerAddressTest:testSetsIsOwner() (gas: 90492)
+AddOwnerAddressTest:testSetsOwnerAtIndex() (gas: 98992)
+AddOwnerPublicKeyTest:testEmitsAddOwner() (gas: 115559)
+AddOwnerPublicKeyTest:testFuzzIsOwnerPublicKey(bytes32,bytes32) (runs: 256, μ: 115372, ~: 115372)
+AddOwnerPublicKeyTest:testRevertsIfAlreadyOwner() (gas: 116399)
+AddOwnerPublicKeyTest:testRevertsIfCalledByNonOwner() (gas: 11776)
+AddOwnerPublicKeyTest:testSetsIsOwner() (gas: 113813)
+AddOwnerPublicKeyTest:testSetsOwnerAtIndex() (gas: 128638)
+CoinbaseSmartWallet1271InputGeneratorTest:testGetReplaySafeHashForDeployedAccount() (gas: 308479)
+CoinbaseSmartWallet1271InputGeneratorTest:testGetReplaySafeHashForUndeployedAccount() (gas: 291508)
+CoinbaseSmartWalletFactoryTest:testDeployDeterministicPassValues() (gas: 267283)
+CoinbaseSmartWalletFactoryTest:test_CreateAccount_ReturnsPredeterminedAddress_WhenAccountAlreadyExists() (gas: 286935)
+CoinbaseSmartWalletFactoryTest:test_RevertsIfLength32ButLargerThanAddress() (gas: 298669)
+CoinbaseSmartWalletFactoryTest:test_constructor_setsImplementation(address) (runs: 256, μ: 313355, ~: 313355)
+CoinbaseSmartWalletFactoryTest:test_createAccountDeploysToPredeterminedAddress() (gas: 268634)
+CoinbaseSmartWalletFactoryTest:test_createAccountSetsOwnersCorrectly() (gas: 277324)
+CoinbaseSmartWalletFactoryTest:test_exitIfAccountIsAlreadyInitialized() (gas: 267739)
 CoinbaseSmartWalletFactoryTest:test_implementation_returnsExpectedAddress() (gas: 7676)
 CoinbaseSmartWalletFactoryTest:test_initCodeHash() (gas: 7912)
 CoinbaseSmartWalletFactoryTest:test_revertsIfNoOwners() (gas: 29232)
-ERC1271Test:test_returnsExpectedDomainHashWhenProxy() (gas: 29198)
-ERC1271Test:test_static() (gas: 3235744)
+ERC1271Test:test_returnsExpectedDomainHashWhenProxy() (gas: 29243)
+ERC1271Test:test_static() (gas: 3238027)
 MultiOwnableInitializeTest:testRevertsIfLength32ButLargerThanAddress() (gas: 81111)
 MultiOwnableInitializeTest:testRevertsIfLength32NotAddress() (gas: 81092)
 MultiOwnableInitializeTest:testRevertsIfLengthNot32Or64() (gas: 103395)
-RemoveLastOwnerTest:test_emitsRemoveOwner() (gas: 50337)
-RemoveLastOwnerTest:test_removesOwner() (gas: 49609)
-RemoveLastOwnerTest:test_removesOwnerAtIndex() (gas: 49562)
+RemoveLastOwnerTest:test_emitsRemoveOwner() (gas: 50363)
+RemoveLastOwnerTest:test_removesOwner() (gas: 49657)
+RemoveLastOwnerTest:test_removesOwnerAtIndex() (gas: 49588)
 RemoveLastOwnerTest:test_revert_whenCalledByNonOwner(address) (runs: 256, μ: 19094, ~: 19094)
-RemoveLastOwnerTest:test_revert_whenNoOwnerAtIndex() (gas: 48379)
-RemoveLastOwnerTest:test_revert_whenWrongOwnerAtIndex() (gas: 34153)
-RemoveLastOwnerTest:test_reverts_whenNotLastOwner() (gas: 123412)
-RemoveOwnerAtIndexTest:test_emitsRemoveOwner() (gas: 55524)
-RemoveOwnerAtIndexTest:test_removesOwner() (gas: 54784)
-RemoveOwnerAtIndexTest:test_removesOwnerAtIndex() (gas: 54540)
+RemoveLastOwnerTest:test_revert_whenNoOwnerAtIndex() (gas: 48407)
+RemoveLastOwnerTest:test_revert_whenWrongOwnerAtIndex() (gas: 34181)
+RemoveLastOwnerTest:test_reverts_whenNotLastOwner() (gas: 123462)
+RemoveOwnerAtIndexTest:test_emitsRemoveOwner() (gas: 55549)
+RemoveOwnerAtIndexTest:test_removesOwner() (gas: 54828)
+RemoveOwnerAtIndexTest:test_removesOwnerAtIndex() (gas: 54566)
 RemoveOwnerAtIndexTest:test_revert_whenCalledByNonOwner(address) (runs: 256, μ: 19145, ~: 19145)
-RemoveOwnerAtIndexTest:test_revert_whenNoOwnerAtIndex() (gas: 33372)
-RemoveOwnerAtIndexTest:test_revert_whenWrongOwnerAtIndex() (gas: 36694)
-RemoveOwnerAtIndexTest:test_reverts_ifIsLastOwner() (gas: 7471805)
+RemoveOwnerAtIndexTest:test_revert_whenNoOwnerAtIndex() (gas: 33406)
+RemoveOwnerAtIndexTest:test_revert_whenWrongOwnerAtIndex() (gas: 36728)
+RemoveOwnerAtIndexTest:test_reverts_ifIsLastOwner() (gas: 7647492)
 TestCanSkipChainIdValidation:test_approvedSelectorsReturnTrue() (gas: 17781)
 TestCanSkipChainIdValidation:test_otherSelectorsReturnFalse() (gas: 12579)
-TestExecuteWithoutChainIdValidation:testExecute() (gas: 424837)
-TestExecuteWithoutChainIdValidation:testExecuteBatch() (gas: 728897)
-TestExecuteWithoutChainIdValidation:testExecuteBatch(uint256) (runs: 256, μ: 3592913, ~: 3463925)
-TestExecuteWithoutChainIdValidation:test__codesize() (gas: 49920)
-TestExecuteWithoutChainIdValidation:test__codesize() (gas: 50155)
+TestExecuteWithoutChainIdValidation:testExecute() (gas: 424882)
+TestExecuteWithoutChainIdValidation:testExecuteBatch() (gas: 728942)
+TestExecuteWithoutChainIdValidation:testExecuteBatch(uint256) (runs: 256, μ: 3590200, ~: 3466169)
+TestExecuteWithoutChainIdValidation:test__codesize() (gas: 49931)
+TestExecuteWithoutChainIdValidation:test__codesize() (gas: 50166)
 TestExecuteWithoutChainIdValidation:test_revertsWithReservedNonce() (gas: 81983)
-TestExecuteWithoutChainIdValidation:test_reverts_whenCallerNotEntryPoint() (gas: 11053)
-TestExecuteWithoutChainIdValidation:test_reverts_whenOneCallReverts() (gas: 467715)
-TestExecuteWithoutChainIdValidation:test_reverts_whenOneSelectorNotApproved() (gas: 179684)
-TestExecuteWithoutChainIdValidation:test_reverts_whenSelectorNotApproved() (gas: 106758)
-TestExecuteWithoutChainIdValidation:test_succeeds_whenSelectorAllowed() (gas: 423916)
-TestImplementation:testImplementation() (gas: 12623)
-TestInitialize:testInitialize() (gas: 21034)
-TestInitialize:test_cannotInitImplementation() (gas: 2885066)
-TestIsValidSignature:testReturnsInvalidIfPasskeySigButWrongOwnerLength() (gas: 40187)
-TestIsValidSignature:testRevertsIfEthereumSignatureButWrongOwnerLength() (gas: 24040)
-TestIsValidSignature:testRevertsIfOwnerIsInvalidEthereumAddress() (gas: 22021)
-TestIsValidSignature:testSmartWalletSigner() (gas: 3166717)
-TestIsValidSignature:testValidateSignatureWithEOASigner() (gas: 24922)
-TestIsValidSignature:testValidateSignatureWithEOASignerFailsWithWrongSigner() (gas: 23877)
-TestIsValidSignature:testValidateSignatureWithPasskeySigner() (gas: 423238)
-TestIsValidSignature:testValidateSignatureWithPasskeySignerFailsBadOwnerIndex() (gas: 35664)
-TestIsValidSignature:testValidateSignatureWithPasskeySignerFailsWithWrongBadSignature() (gas: 430704)
-TestUpgradeToAndCall:testUpgradeToAndCall() (gas: 25477)
+TestExecuteWithoutChainIdValidation:test_reverts_whenCallerNotEntryPoint() (gas: 11031)
+TestExecuteWithoutChainIdValidation:test_reverts_whenOneCallReverts() (gas: 467783)
+TestExecuteWithoutChainIdValidation:test_reverts_whenOneSelectorNotApproved() (gas: 179662)
+TestExecuteWithoutChainIdValidation:test_reverts_whenSelectorNotApproved() (gas: 106736)
+TestExecuteWithoutChainIdValidation:test_succeeds_whenSelectorAllowed() (gas: 423939)
+TestImplementation:testImplementation() (gas: 12558)
+TestInitialize:testInitialize() (gas: 21012)
+TestInitialize:test_cannotInitImplementation() (gas: 2887326)
+TestIsValidSignature:testReturnsInvalidIfPasskeySigButWrongOwnerLength() (gas: 40165)
+TestIsValidSignature:testRevertsIfEthereumSignatureButWrongOwnerLength() (gas: 24018)
+TestIsValidSignature:testRevertsIfOwnerIsInvalidEthereumAddress() (gas: 21999)
+TestIsValidSignature:testSmartWalletSigner() (gas: 3168925)
+TestIsValidSignature:testValidateSignatureWithEOASigner() (gas: 24900)
+TestIsValidSignature:testValidateSignatureWithEOASignerFailsWithWrongSigner() (gas: 23855)
+TestIsValidSignature:testValidateSignatureWithPasskeySigner() (gas: 423216)
+TestIsValidSignature:testValidateSignatureWithPasskeySignerFailsBadOwnerIndex() (gas: 35642)
+TestIsValidSignature:testValidateSignatureWithPasskeySignerFailsWithWrongBadSignature() (gas: 430682)
+TestUpgradeToAndCall:testUpgradeToAndCall() (gas: 25499)
 TestValidateUserOp:test_reverts_whenReplayableNonceKeyInvalidForSelector() (gas: 14187)
 TestValidateUserOp:test_reverts_whenSelectorInvalidForReplayableNonceKey() (gas: 14452)
 TestValidateUserOp:test_succeedsWithEOASigner() (gas: 447938)

src/MultiOwnable.sol

@@ -120,8 +120,7 @@ contract MultiOwnable {
     /// @param index The index of the owner to be removed.
     /// @param owner The ABI encoded bytes of the owner to be removed.
     function removeOwnerAtIndex(uint256 index, bytes calldata owner) external virtual onlyOwner {
-        MultiOwnableStorage storage $ = _getMultiOwnableStorage();
-        if ($.nextOwnerIndex - $.removedOwnersCount == 1) {
+        if (ownerCount() == 1) {
             revert LastOwner();
         }
 
@@ -137,8 +136,7 @@ contract MultiOwnable {
     /// @param index The index of the owner to be removed.
     /// @param owner The ABI encoded bytes of the owner to be removed.
     function removeLastOwner(uint256 index, bytes calldata owner) external virtual onlyOwner {
-        MultiOwnableStorage storage $ = _getMultiOwnableStorage();
-        uint256 ownersRemaining = $.nextOwnerIndex - $.removedOwnersCount;
+        uint256 ownersRemaining = ownerCount();
         if (ownersRemaining > 1) {
             revert NotLastOwner(ownersRemaining);
         }
@@ -190,6 +188,14 @@ contract MultiOwnable {
         return _getMultiOwnableStorage().nextOwnerIndex;
     }
 
+    /// @notice Returns the current number of owners
+    ///
+    /// @return The current owner count
+    function ownerCount() public view virtual returns (uint256) {
+        MultiOwnableStorage storage $ = _getMultiOwnableStorage();
+        return $.nextOwnerIndex - $.removedOwnersCount;
+    }
+
     /// @notice Tracks the number of owners removed, used with nextOwnerIndex to avoid removing all owners
     ///
     /// @return The number of owners that have been removed.

test/MultiOwnable/RemoveOwnerAtIndex.t.sol

@@ -14,14 +14,17 @@ contract RemoveOwnerAtIndexTest is RemoveOwnerBaseTest {
         mock.init(initialOwners);
         assertEq(mock.nextOwnerIndex(), 1);
         assertEq(mock.removedOwnersCount(), 0);
+        assertEq(mock.ownerCount(), 1);
         vm.startPrank(firstOnwer);
         for (uint256 i; i < owners; i++) {
             mock.addOwnerAddress(makeAddr(string(abi.encodePacked(i))));
             assertEq(mock.nextOwnerIndex(), i + 2);
+            assertEq(mock.ownerCount(), i + 2);
         }
         for (uint256 i = 1; i < owners + 1; i++) {
             mock.removeOwnerAtIndex(i, abi.encode(makeAddr(string(abi.encodePacked(i - 1)))));
             assertEq(mock.removedOwnersCount(), i);
+            assertEq(mock.ownerCount(), owners - i + 1);
         }
         vm.expectRevert(MultiOwnable.LastOwner.selector);
         mock.removeOwnerAtIndex(0, abi.encode(firstOnwer));