Fix getAccounts (#565)

fix getAccounts
thirdweb-dev · Oct 31, 2023 · d6bba75 · d6bba75
1 parent bd54c96
commit d6bba75
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 8 deletions.
diff --git a/contracts/prebuilts/account/utils/BaseAccountFactory.sol b/contracts/prebuilts/account/utils/BaseAccountFactory.sol
@@ -119,17 +119,16 @@ abstract contract BaseAccountFactory is IAccountFactory, Multicall {
 
     /// @notice Returns all accounts between the given indices.
     function getAccounts(uint256 _start, uint256 _end) external view returns (address[] memory accounts) {
-        uint256 len = _baseAccountFactoryStorage().allAccounts.length();
-        require(_start < _end && _end <= len, "BaseAccountFactory: invalid indices");
-
-        if (len == 0) {
-            return new address[](0);
-        }
+        require(
+            _start < _end && _end <= _baseAccountFactoryStorage().allAccounts.length(),
+            "BaseAccountFactory: invalid indices"
+        );
 
+        uint256 len = _end - _start;
         accounts = new address[](_end - _start);
 
-        for (uint256 i = _start; i < _end; i += 1) {
-            accounts[i] = _baseAccountFactoryStorage().allAccounts.at(i);
+        for (uint256 i = 0; i < len; i += 1) {
+            accounts[i] = _baseAccountFactoryStorage().allAccounts.at(i + _start);
         }
     }
 

diff --git a/src/test/smart-wallet/Account.t.sol b/src/test/smart-wallet/Account.t.sol
@@ -322,6 +322,14 @@ contract SimpleAccountTest is BaseTest {
 
     /// @dev Create more than one accounts with the same admin.
     function test_state_createAccount_viaEntrypoint_multipleAccountSameAdmin() public {
+        uint256 start = 0;
+        uint256 end = 0;
+
+        assertEq(accountFactory.totalAccounts(), 0);
+
+        vm.expectRevert("BaseAccountFactory: invalid indices");
+        address[] memory accs = accountFactory.getAccounts(start, end);
+
         uint256 amount = 100;
 
         for (uint256 i = 0; i < amount; i += 1) {
@@ -365,6 +373,50 @@ contract SimpleAccountTest is BaseTest {
                 )
             );
         }
+
+        start = 25;
+        end = 75;
+
+        address[] memory accountsPaginatedOne = accountFactory.getAccounts(start, end);
+
+        for (uint256 i = 0; i < (end - start); i += 1) {
+            assertEq(
+                accountsPaginatedOne[i],
+                Clones.predictDeterministicAddress(
+                    accountFactory.accountImplementation(),
+                    _generateSalt(accountAdmin, bytes(abi.encode(start + i))),
+                    address(accountFactory)
+                )
+            );
+        }
+
+        start = 0;
+        end = amount;
+
+        address[] memory accountsPaginatedTwo = accountFactory.getAccounts(start, end);
+
+        for (uint256 i = 0; i < (end - start); i += 1) {
+            assertEq(
+                accountsPaginatedTwo[i],
+                Clones.predictDeterministicAddress(
+                    accountFactory.accountImplementation(),
+                    _generateSalt(accountAdmin, bytes(abi.encode(start + i))),
+                    address(accountFactory)
+                )
+            );
+        }
+
+        start = 75;
+        end = 25;
+
+        vm.expectRevert("BaseAccountFactory: invalid indices");
+        accs = accountFactory.getAccounts(start, end);
+
+        start = 25;
+        end = amount + 1;
+
+        vm.expectRevert("BaseAccountFactory: invalid indices");
+        accs = accountFactory.getAccounts(start, end);
     }
 
     /*///////////////////////////////////////////////////////////////