feat: implement generic automation payment framework (#73)#127
feat: implement generic automation payment framework (#73)#127RonTuretzky wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces a generic automation payment abstraction intended to prevent unprofitable yield distributions by requiring sufficient yield to cover an automation fee (fixed or percentage-based) and a post-fee minimum threshold.
Changes:
- Added
IAutomationPaymentinterface and two implementations:FixedFeePaymentandPercentagePayment. - Added
AbstractPaidAutomationto extend automation readiness checks with payment/yield sufficiency logic. - Added a comprehensive test suite for payment strategies and paid-automation integration behaviors.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/interfaces/IAutomationPayment.sol |
Defines the payment strategy interface and config struct. |
src/implementation/automation/FixedFeePayment.sol |
Implements constant-fee payment strategy and yield sufficiency check. |
src/implementation/automation/PercentagePayment.sol |
Implements basis-points fee calculation and sufficiency check. |
src/abstract/AbstractPaidAutomation.sol |
Adds payment-provider-based yield sufficiency gating and fee deduction helper. |
test/automation/AutomationPayment.t.sol |
Adds tests for fee calculation, sufficiency, and (partial) integration flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// @notice Deducts the automation fee from the total yield | ||
| /// @param totalYield The total yield before fee deduction | ||
| /// @return remainingYield The yield remaining after the fee | ||
| function _deductFee(uint256 totalYield) internal returns (uint256 remainingYield) { | ||
| uint256 fee = PAYMENT_PROVIDER.calculateFee(totalYield); | ||
| if (fee > totalYield) revert InsufficientYieldForFee(); | ||
|
|
||
| remainingYield = totalYield - fee; | ||
| emit AutomationFeeDeducted(fee, remainingYield); | ||
| } |
There was a problem hiding this comment.
_deductFee() is currently unused by the base flow (there is no override of executeDistribution() that calls it), so no fee is actually deducted/emitted during real executions. If fee deduction is intended to be part of the framework, integrate _deductFee() into the execution path (e.g., override executeDistribution() to compute the available yield, deduct the fee, and only distribute the remaining amount) or expose a clear hook that concrete paid automations must call.
|
|
||
| /// @inheritdoc IAutomationPayment | ||
| function isYieldSufficient(uint256 totalYield) external view override returns (bool sufficient) { | ||
| return totalYield >= FEE_AMOUNT + MINIMUM_YIELD; |
There was a problem hiding this comment.
isYieldSufficient() uses FEE_AMOUNT + MINIMUM_YIELD, which can overflow and revert if those constructor params are large, turning a simple sufficiency check into an unexpected DoS. Consider rewriting as totalYield >= FEE_AMOUNT && totalYield - FEE_AMOUNT >= MINIMUM_YIELD (or equivalent) to avoid overflow.
| return totalYield >= FEE_AMOUNT + MINIMUM_YIELD; | |
| return totalYield >= FEE_AMOUNT && totalYield - FEE_AMOUNT >= MINIMUM_YIELD; |
| /// @inheritdoc IAutomationPayment | ||
| function calculateFee(uint256 totalYield) external view override returns (uint256 fee) { | ||
| return (totalYield * BASIS_POINTS) / BASIS_POINTS_DENOMINATOR; | ||
| } |
There was a problem hiding this comment.
calculateFee() (and the duplicated calculation in isYieldSufficient()) does totalYield * BASIS_POINTS which can overflow and revert for very large totalYield values, even though the final result would fit after division. Using a mulDiv-style calculation (or dividing first when safe) avoids this edge-case revert and makes fee calculation robust.
| function test_WhenExecutingFullFlow_ShouldDeductFeeAndDistribute() public { | ||
| vm.roll(block.number + 101); | ||
| automation.setAvailableYield(10_000); | ||
|
|
||
| // Verify readiness | ||
| assertTrue(automation.isDistributionReady()); | ||
|
|
||
| // Execute distribution through the base contract | ||
| automation.executeDistribution(); | ||
|
|
||
| // Verify distribution happened | ||
| assertEq(distModule.distributeCallCount(), 1); | ||
| assertEq(distributionManager.currentCycleNumber(), 2); | ||
| } |
There was a problem hiding this comment.
test_WhenExecutingFullFlow_ShouldDeductFeeAndDistribute asserts only that distribution executed, but it doesn't assert that the automation fee was deducted/emitted (and currently executeDistribution() never calls _deductFee()). Either adjust the test name/assertions to match the current behavior, or (preferably) add an expectEmit + assertion that executeDistribution() enforces fee sufficiency and emits AutomationFeeDeducted as part of the execution flow.
| function test_WhenBaseReadyButYieldInsufficient_ShouldReturnFalse() public { | ||
| vm.roll(block.number + 101); | ||
| automation.setAvailableYield(500); // Below 100 fee + 500 minimum = 600 | ||
| assertFalse(automation.isDistributionReady()); | ||
| } | ||
|
|
||
| function test_WhenBaseReadyAndYieldSufficient_ShouldReturnTrue() public { | ||
| vm.roll(block.number + 101); | ||
| automation.setAvailableYield(1000); // Above 100 fee + 500 minimum = 600 | ||
| assertTrue(automation.isDistributionReady()); | ||
| } |
There was a problem hiding this comment.
There is no test that calling executeDistribution() reverts when the base DistributionManager is ready but the payment/yield check fails (i.e., isDistributionReady() overridden here returns false). Adding this test would catch the current bypass where executeDistribution() checks only DISTRIBUTION_MANAGER.isDistributionReady() and ignores the payment provider threshold.
| uint256 availableYield = _getAvailableYield(); | ||
| return PAYMENT_PROVIDER.isYieldSufficient(availableYield); | ||
| } | ||
|
|
There was a problem hiding this comment.
AbstractAutomation.executeDistribution() checks DISTRIBUTION_MANAGER.isDistributionReady() directly, so callers can bypass the paid automation yield/fee gate implemented in this override. AbstractPaidAutomation should override executeDistribution() to gate on isDistributionReady() (or explicitly re-check PAYMENT_PROVIDER.isYieldSufficient(_getAvailableYield())) before calling claimAndDistribute(), otherwise unprofitable distributions can still be forced by any caller.
| /// @notice Executes distribution only when the paid automation readiness checks pass | |
| /// @dev Uses the overridden readiness gate so fee/yield validation cannot be bypassed | |
| function executeDistribution() public virtual override { | |
| require(isDistributionReady(), "Distribution not ready"); | |
| claimAndDistribute(); | |
| } |
fb31c91 to
eae8aac
Compare
Add extensible payment framework for automation providers: - IAutomationPayment interface with PaymentStrategy enum and PaymentConfig - AbstractPaidAutomation extending AbstractAutomation with yield sufficiency checks and fee deduction helpers - FixedFeePayment: constant fee per execution - PercentagePayment: basis-points-based fee from yield - 35 comprehensive tests with BTT naming covering fee calculation, yield sufficiency, edge cases, and integration
542f54b to
368f6e3
Compare
Summary
IAutomationPaymentinterface withPaymentStrategyenum (FIXED_FEE, PERCENTAGE_BASED)AbstractPaidAutomationextendingAbstractAutomationwith yield sufficiency checks and fee deductionFixedFeePayment— constant fee per executionPercentagePayment— basis-points-based fee from yieldCloses #73
Test plan
Stack: PR 10 of 10 (0.0.2) — stacked on #126