From ef14f59121be841740a3412cd303d755d71eca3c Mon Sep 17 00:00:00 2001 From: Francisco Rolotti <102830313+franrolotti@users.noreply.github.com> Date: Fri, 18 Sep 2026 17:58:23 +0200 Subject: [PATCH] refactor: resolve claim target member index without fetching the member list _executeAutomatedClaimTarget copied the whole member array across an external call just to find one index. Read it with the O(1) SavingCircles.memberIndex getter instead (now part of ISavingCircles), and drop the redundant NotMember pre-check since isMemberWithdrawable and withdrawFor already reject non-members. A non-member target now fails with NotWithdrawable instead of NotMember. --- src/contracts/AutomaticSavingCircles.sol | 24 +++--------------------- src/contracts/SavingCircles.sol | 3 ++- src/interfaces/ISavingCircles.sol | 10 ++++++++++ test/unit/AutomaticSavingCircles.t.sol | 23 +++++++++++++++++++++++ 4 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/contracts/AutomaticSavingCircles.sol b/src/contracts/AutomaticSavingCircles.sol index 4d8eefa..9366157 100644 --- a/src/contracts/AutomaticSavingCircles.sol +++ b/src/contracts/AutomaticSavingCircles.sol @@ -259,10 +259,9 @@ contract AutomaticSavingCircles is IAutomaticSavingCircles, Ownable, ReentrancyG // Check opt-in before fetching members while retaining the specific error for an invalid circle. if (!automaticClaimsEnabled[_circleId][_member]) revert AutomaticClaimsDisabled(); - address[] memory members = SAVING_CIRCLES.getCircleMembers(_circleId); - (bool isMember, uint256 memberIndex) = _getMemberIndex(members, _member); - - if (!isMember) revert ISavingCircles.NotMember(); + // A non-member resolves to index 0 here, but `isMemberWithdrawable` inside the eligibility check + // rejects them, so no separate membership lookup is needed. + uint256 memberIndex = SAVING_CIRCLES.memberIndex(_circleId, _member); if (!_isEligibleForAutomatedClaim(_circleId, _circle, _member, memberIndex)) { revert ISavingCircles.NotWithdrawable(); } @@ -512,21 +511,4 @@ contract AutomaticSavingCircles is IAutomaticSavingCircles, Ownable, ReentrancyG return (true, _balances[i]); } } - - /** - * @dev Looks up whether a member belongs to the supplied member snapshot and returns their index - * @param _members Snapshot of circle members - * @param _member Member being searched for - * @return isMember Whether the member was found in the snapshot - * @return memberIndex The member's zero-based index in the circle - */ - function _getMemberIndex( - address[] memory _members, - address _member - ) internal pure returns (bool isMember, uint256 memberIndex) { - for (uint256 i = 0; i < _members.length; i++) { - if (_members[i] != _member) continue; - return (true, i); - } - } } diff --git a/src/contracts/SavingCircles.sol b/src/contracts/SavingCircles.sol index e1cb42b..0d4473c 100644 --- a/src/contracts/SavingCircles.sol +++ b/src/contracts/SavingCircles.sol @@ -376,7 +376,8 @@ contract SavingCircles is ISavingCircles, ReentrancyGuardUpgradeable, OwnableUpg return _memberStates[_id][_member].lastDepositRound; } - function memberIndex(uint256 _id, address _member) external view returns (uint256 index) { + /// @inheritdoc ISavingCircles + function memberIndex(uint256 _id, address _member) external view override returns (uint256 index) { return _memberStates[_id][_member].memberIndex; } diff --git a/src/interfaces/ISavingCircles.sol b/src/interfaces/ISavingCircles.sol index 392b3ff..b636db7 100644 --- a/src/interfaces/ISavingCircles.sol +++ b/src/interfaces/ISavingCircles.sol @@ -425,6 +425,16 @@ interface ISavingCircles { */ function isMemberWithdrawable(uint256 id, address member) external view returns (bool withdrawable); + /** + * @notice Get a member's zero-based payout round in a circle + * @dev Returns 0 for addresses that are not members, indistinguishable from the first member. + * Callers that need to tell them apart must also check membership. + * @param id The ID of the circle + * @param member The member to look up + * @return index The member's zero-based position in the payout order + */ + function memberIndex(uint256 id, address member) external view returns (uint256 index); + /** * @notice Get the address of the withdrawable by * @param id The ID of the circle diff --git a/test/unit/AutomaticSavingCircles.t.sol b/test/unit/AutomaticSavingCircles.t.sol index 68cc05d..e3c1683 100644 --- a/test/unit/AutomaticSavingCircles.t.sol +++ b/test/unit/AutomaticSavingCircles.t.sol @@ -623,6 +623,29 @@ contract AutomaticSavingCirclesUnit is SavingCirclesTestBase { assertFalse(savingCircles.hasClaimed(baseCircleId, alice)); } + function test_BatchExecuteAutomatedClaims_ReportsNotWithdrawableForNonMember() external { + address outsider = makeAddr('outsider'); + _depositRound(baseCircleId); + _enableAutomaticClaim(baseCircleId, outsider); + + ISavingCircles.Circle memory circle = savingCircles.getCircle(baseCircleId); + vm.warp(circle.effectiveCircleStartTime + DEPOSIT_INTERVAL); + + uint256[] memory circleIds = new uint256[](1); + address[] memory targetMembers = new address[](1); + circleIds[0] = baseCircleId; + targetMembers[0] = outsider; + + vm.prank(automationExecutor); + vm.expectEmit(true, true, false, true, address(automaticSavingCircles)); + emit IAutomaticSavingCircles.AutomatedClaimFailed( + baseCircleId, outsider, abi.encodeWithSelector(ISavingCircles.NotWithdrawable.selector) + ); + automaticSavingCircles.batchExecuteAutomatedClaims(circleIds, targetMembers); + + assertEq(token.balanceOf(outsider), 0); + } + function test_BatchExecuteAutomatedClaims_ReportsUnknownCircleBeforeDisabledClaim() external { uint256 invalidCircleId = savingCircles.nextId(); uint256[] memory circleIds = new uint256[](1);