Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 186 additions & 1 deletion src/contracts/AutomaticSavingCircles.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
* @param _savingCircles Address of the main SavingCircles contract
* @param _owner Owner of the automatic deposits extension
*/
constructor(address _savingCircles, address _owner) Ownable(_owner) {

Check warning on line 49 in src/contracts/AutomaticSavingCircles.sol

View workflow job for this annotation

GitHub Actions / Lint Commit Messages

Variable "_owner" is unused

Check warning on line 49 in src/contracts/AutomaticSavingCircles.sol

View workflow job for this annotation

GitHub Actions / Lint Commit Messages

Variable "_owner" is unused
SAVING_CIRCLES = ISavingCircles(_savingCircles);
}

Expand All @@ -72,6 +72,15 @@
_executeAutomatedDepositTarget(_circleId, _member);
}

/**
* @dev External trampoline used to isolate single-target failures with try/catch during batch claim execution
* @param _circleId Circle to process
* @param _member Member to claim for
*/
function executeAutomatedClaimTarget(uint256 _circleId, address _member) external onlySelf {
Comment thread
bagelface marked this conversation as resolved.
_executeAutomatedClaimTarget(_circleId, _member);
}

/// @inheritdoc IAutomaticSavingCircles
function batchExecuteAutomatedDeposits(
uint256[] calldata _circleIds,
Expand All @@ -87,13 +96,28 @@
}
}

/// @inheritdoc IAutomaticSavingCircles
function batchExecuteAutomatedClaims(
uint256[] calldata _circleIds,
address[] calldata _members
) external override nonReentrant onlyAutomationExecutor {
if (_circleIds.length != _members.length) revert ArrayLengthMismatch();

for (uint256 i = 0; i < _circleIds.length; i++) {
try this.executeAutomatedClaimTarget(_circleIds[i], _members[i]) {}
catch (bytes memory reason) {
emit AutomatedClaimFailed(_circleIds[i], _members[i], reason);
}
}
}

/// @inheritdoc IAutomaticSavingCircles
function isAutomaticDepositsEnabled(address _member) external view override returns (bool) {
return automaticDepositsEnabled[_member];
}

/// @inheritdoc IAutomaticSavingCircles
function checker() external view override returns (bool canExec, bytes memory execPayload) {
function depositChecker() external view override returns (bool canExec, bytes memory execPayload) {
uint256[] memory circleIds;
address[] memory members;

Expand All @@ -108,6 +132,22 @@
execPayload = abi.encodeCall(IAutomaticSavingCircles.batchExecuteAutomatedDeposits, (circleIds, members));
}

/// @inheritdoc IAutomaticSavingCircles
function claimChecker() external view override returns (bool canExec, bytes memory execPayload) {
uint256[] memory circleIds;
address[] memory members;

if (automationExecutor != address(0)) {
(circleIds, members) = getEligibleAutomatedClaims();
canExec = circleIds.length > 0;
} else {
circleIds = new uint256[](0);
members = new address[](0);
}

execPayload = abi.encodeCall(IAutomaticSavingCircles.batchExecuteAutomatedClaims, (circleIds, members));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When automationExecutor == address(0), canExec is false but execPayload is still a valid (empty-arrays) encoded call. This is safe (the call would revert on onlyAutomationExecutor), but Gelato's own resolver examples use execPayload = "" when canExec = false. Low priority, but worth aligning for consistency with Gelato conventions.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah makes sense. Since this is safe and both checkers currently use the same empty batch payload pattern, we could leave as is. for this PR unless you’d prefer we align both with Gelato’s bytes("") convention

@bagelface

}

/// @inheritdoc IAutomaticSavingCircles
function getEligibleAutomatedDeposits()
public
Expand All @@ -131,6 +171,29 @@
}
}

/// @inheritdoc IAutomaticSavingCircles
function getEligibleAutomatedClaims()
public
view
override
returns (uint256[] memory circleIds, address[] memory members)
{
uint256 circleCount = SAVING_CIRCLES.nextId();
uint256 eligibleCount = 0;

for (uint256 circleId = 0; circleId < circleCount; circleId++) {
eligibleCount += _countEligibleAutomatedClaimsForCircle(circleId);
}

circleIds = new uint256[](eligibleCount);
members = new address[](eligibleCount);

uint256 index = 0;
for (uint256 circleId = 0; circleId < circleCount; circleId++) {
index = _populateEligibleAutomatedClaimsForCircle(circleId, circleIds, members, index);
}
}

/**
* @dev Executes a single automated deposit target after re-validating all onchain constraints
* @param _circleId Circle to process
Expand Down Expand Up @@ -163,6 +226,24 @@
SAVING_CIRCLES.depositFor(_circleId, requiredAmount, _member);
}

/**
* @dev Executes a single automated claim target after re-validating all onchain constraints
* @param _circleId Circle to process
* @param _member Member to claim for
*/
function _executeAutomatedClaimTarget(uint256 _circleId, address _member) internal {
ISavingCircles.Circle memory _circle = SAVING_CIRCLES.getCircle(_circleId);
address[] memory members = SAVING_CIRCLES.getCircleMembers(_circleId);
(bool isMember, uint256 memberIndex) = _getMemberIndex(members, _member);

if (!isMember) revert ISavingCircles.NotMember();
if (!_isEligibleForAutomatedClaim(_circleId, _circle, _member, memberIndex)) {
revert ISavingCircles.NotWithdrawable();
}

SAVING_CIRCLES.withdrawFor(_circleId, _member);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_executeAutomatedClaimTarget fetches getCircleMembers and calls _getMemberIndex to check membership, then calls _isEligibleForAutomatedClaim, which calls isMemberWithdrawable → _claimable → _activeClaimableCheck → _getMemberIndex — so membership is verified twice with two separate getCircleMembers calls. Since withdrawFor → _withdraw has onlyMember(_id, _member) as a hard guard, the explicit NotMember revert at line 239 is also redundant. Consider dropping the pre-check and relying on withdrawFor to enforce it, or at least share the member list with _isEligibleForAutomatedClaim to avoid the duplicate fetch.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, fair. We still need the member index for the round time check, so I don’t think I can just drop the lookup without changing the invalid member path. I can either keep the pre-check, or refactor this so membership/index is resolved once and reused more cleanly. wdyt? @bagelface

}

/**
* @dev Returns the current round index for a circle using the live block timestamp
* @param _circle Circle configuration to evaluate
Expand All @@ -179,6 +260,16 @@
return (block.timestamp - _circle.effectiveCircleStartTime) / _circle.depositInterval;
}

/**
* @dev Returns the ending timestamp for a round
* @param _circle Circle configuration to evaluate
* @param _round Zero-based round index
* @return Timestamp when the round's time condition has passed
*/
function _roundEndTime(ISavingCircles.Circle memory _circle, uint256 _round) internal pure returns (uint256) {
return _circle.effectiveCircleStartTime + (_circle.depositInterval * (_round + 1));
}

/**
* @dev Returns whether a member currently satisfies the offchain selection criteria for automated deposit
* @param _circle Circle configuration to evaluate
Expand All @@ -186,7 +277,7 @@
* @param _currentBalance Amount already deposited by the member in the active round
* @return Whether the member can be included in the automation payload
*/
function _isEligibleForAutomatedDeposit(

Check warning on line 280 in src/contracts/AutomaticSavingCircles.sol

View workflow job for this annotation

GitHub Actions / Lint Commit Messages

Function order is incorrect, internal view function can not go after internal pure function (line 269)

Check warning on line 280 in src/contracts/AutomaticSavingCircles.sol

View workflow job for this annotation

GitHub Actions / Lint Commit Messages

Function order is incorrect, internal view function can not go after internal pure function (line 269)
ISavingCircles.Circle memory _circle,
address _member,
uint256 _currentBalance
Expand Down Expand Up @@ -221,6 +312,27 @@
}
}

/**
* @dev Counts how many members in a circle are currently eligible for automated claim
* @param _circleId Circle to inspect
* @return eligibleCount Number of eligible claim targets found
*/
function _countEligibleAutomatedClaimsForCircle(uint256 _circleId) internal view returns (uint256 eligibleCount) {
try SAVING_CIRCLES.getCircle(_circleId) returns (ISavingCircles.Circle memory _circle) {
if (!SAVING_CIRCLES.isActive(_circleId)) return 0;
if (SAVING_CIRCLES.isDecommissionable(_circleId)) return 0;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isActive and isDecommissionable are checked here (and again in _populateEligibleAutomatedClaimsForCircle) before calling _isEligibleForAutomatedClaim, which already checks both conditions (lines 438–440). The early-exit guards are redundant and could diverge from the authoritative check over time. Consider removing them from _countEligibleAutomatedClaimsForCircle and _populateEligibleAutomatedClaimsForCircle and letting _isEligibleForAutomatedClaim be the single gate.

Copy link
Copy Markdown
Collaborator

@bagelface bagelface Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally agree with this comment, but assuming the early-exit guards aren't too expensive it can be worth leaving in just to future proof. isDecommissionable loops over members, so this is sort of a heavy check.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skipping this one too


address[] memory members = SAVING_CIRCLES.getCircleMembers(_circleId);
for (uint256 i = 0; i < members.length; i++) {
if (_isEligibleForAutomatedClaim(_circleId, _circle, members[i], i)) {
eligibleCount++;
}
}
} catch {
return 0;
}
}

/**
* @dev Appends the eligible targets for a circle into the output arrays used by the selector
* @param _circleId Circle to inspect
Expand Down Expand Up @@ -254,6 +366,39 @@
}
}

/**
* @dev Appends the eligible claim targets for a circle into the output arrays used by the selector
* @param _circleId Circle to inspect
* @param _circleIds Output array of eligible circle IDs
* @param _members Output array of eligible members
* @param _index Current write index in the output arrays
* @return nextIndex Updated write index after appending any eligible targets
*/
function _populateEligibleAutomatedClaimsForCircle(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming of this function is a bit confusing, since it implies it's populating an array but really it's just returning the next index. The comments and function name prescribe a use to the function, which I think isn't ideal. Consider renaming to something like "_getNextEligibleAutomationClaimIndexForCircle"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdyt about _appendEligibleAutomatedClaimsForCircle? since the helper does write into the output arrays and then returns the next index, that name feels a bit closer to what it’s doing

@bagelface

uint256 _circleId,
uint256[] memory _circleIds,
address[] memory _members,
uint256 _index
) internal view returns (uint256 nextIndex) {
nextIndex = _index;

try SAVING_CIRCLES.getCircle(_circleId) returns (ISavingCircles.Circle memory _circle) {
if (!SAVING_CIRCLES.isActive(_circleId)) return nextIndex;
if (SAVING_CIRCLES.isDecommissionable(_circleId)) return nextIndex;

address[] memory members = SAVING_CIRCLES.getCircleMembers(_circleId);
for (uint256 i = 0; i < members.length; i++) {
if (!_isEligibleForAutomatedClaim(_circleId, _circle, members[i], i)) continue;

_circleIds[nextIndex] = _circleId;
_members[nextIndex] = members[i];
nextIndex++;
}
} catch {
return nextIndex;
}
}

/**
* @dev Returns whether a circle is in a state where automation can process deposits
* @param _circleId Circle identifier
Expand All @@ -275,6 +420,29 @@
return true;
}

/**
* @dev Returns whether a member currently satisfies the automated claim criteria.
* Gelato should only claim after every member deposited for the member's round and that round's time has passed.
* @param _circleId Circle identifier
* @param _circle Circle configuration to evaluate
* @param _member Member being checked
* @param _memberIndex The member's zero-based payout round
* @return Whether the member can be included in the claim automation payload
*/
function _isEligibleForAutomatedClaim(
uint256 _circleId,
ISavingCircles.Circle memory _circle,
address _member,
uint256 _memberIndex
) internal view returns (bool) {
if (!SAVING_CIRCLES.isActive(_circleId)) return false;
if (_circle.effectiveCircleStartTime == 0) return false;
if (SAVING_CIRCLES.isDecommissionable(_circleId)) return false;
if (block.timestamp < _roundEndTime(_circle, _memberIndex)) return false;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes _memberIndex (the position in getCircleMembers()) equals the member's payout round index. That invariant holds as long as member order in the array is the canonical payout order, but it is implicit. Worth adding a comment asserting this assumption so a future refactor of how member order is stored does not silently break claim eligibility.

Copy link
Copy Markdown
Collaborator

@bagelface bagelface Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we make this assumption throughout the contract, so can probably skip this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skipping this one @bagelface


return SAVING_CIRCLES.isMemberWithdrawable(_circleId, _member);
}

/**
* @dev Looks up whether a member belongs to the supplied balances snapshot and returns their current balance
* @param _members Snapshot of circle members
Expand All @@ -293,4 +461,21 @@
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);
}
}
}
7 changes: 3 additions & 4 deletions src/contracts/SavingCircles.sol
Original file line number Diff line number Diff line change
Expand Up @@ -376,11 +376,10 @@ contract SavingCircles is ISavingCircles, ReentrancyGuardUpgradeable, OwnableUpg
}

/**
* @dev Make a withdrawal from a specified circle
* Permissionless: anyone can trigger the payout for the member whose turn it is to withdraw
* A withdrawal must be made by a member of the circle, even if it is for another member.
* @dev Make a withdrawal from a specified circle.
* Anyone can trigger the payout for a valid member whose turn is withdrawable.
*/
function _withdraw(uint256 _id, address _member) internal onlyMember(_id, msg.sender) {
function _withdraw(uint256 _id, address _member) internal onlyMember(_id, _member) {
Circle storage _circle = circles[_id];

if (!_claimable(_id, _member)) revert NotWithdrawable();
Expand Down
31 changes: 30 additions & 1 deletion src/interfaces/IAutomaticSavingCircles.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ interface IAutomaticSavingCircles {
*/
event AutomatedDepositFailed(uint256 indexed circleId, address indexed member, bytes reason);

/**
* @notice Emitted when an automated claim target fails during batch execution
* @param circleId The circle that failed
* @param member The member that failed
* @param reason The raw revert data returned by the failed execution
*/
event AutomatedClaimFailed(uint256 indexed circleId, address indexed member, bytes reason);

/**
* @notice Thrown when a non-Gelato caller attempts an automated execution
*/
Expand Down Expand Up @@ -73,6 +81,13 @@ interface IAutomaticSavingCircles {
*/
function batchExecuteAutomatedDeposits(uint256[] calldata circleIds, address[] calldata members) external;

/**
* @notice Execute automated claims for precomputed targets
* @param circleIds Circle IDs to process
* @param members Members to process for each circle ID
*/
function batchExecuteAutomatedClaims(uint256[] calldata circleIds, address[] calldata members) external;

/**
* @notice The configured Gelato dedicated msg.sender
* @return The automation executor address
Expand All @@ -93,10 +108,24 @@ interface IAutomaticSavingCircles {
*/
function getEligibleAutomatedDeposits() external view returns (uint256[] memory circleIds, address[] memory members);

/**
* @notice Return every member/circle pair currently eligible for automated claim
* @return circleIds Circle IDs with pending automated claims
* @return members Members eligible to claim in each circle
*/
function getEligibleAutomatedClaims() external view returns (uint256[] memory circleIds, address[] memory members);

/**
* @notice Gelato resolver-style checker for automatic deposits across every circle
* @return canExec Whether Gelato should execute the sweep
* @return execPayload Encoded calldata for the automated deposit execution
*/
function checker() external view returns (bool canExec, bytes memory execPayload);
function depositChecker() external view returns (bool canExec, bytes memory execPayload);

/**
* @notice Gelato resolver-style checker for automatic claims across every circle
* @return canExec Whether Gelato can execute the claims
* @return execPayload Encoded calldata for the automated claim execution
*/
function claimChecker() external view returns (bool canExec, bytes memory execPayload);
}
Loading
Loading