Upkeep Balance Monitor #11180
Upkeep Balance Monitor #11180
Conversation
|
I see that you haven't updated any CHANGELOG files. Would it make sense to do so? |
Support getMinBalanceForUpkeep
RyanRHall
force-pushed
the
upkeepbalancemonitor
branch
from
10a6831 to
f73ab09
Compare
RyanRHall
force-pushed
the
upkeepbalancemonitor
branch
from
fa154d8 to
975f015
Compare
| uint8 maxBatchSize; | ||
| uint24 minPercentage; | ||
| uint24 targetPercentage; | ||
| uint96 maxTopUpAmount; |
There was a problem hiding this comment.
Just curious, any specific reason of using those int types? I guess uint8 should be enough for percentage?
| /// @notice Sets the list of upkeeps to watch and their funding parameters. | ||
| /// @param watchlist the list of subscription ids to watch | ||
| function setWatchList(uint256[] calldata watchlist) external onlyOwner { | ||
| s_watchList = watchlist; |
There was a problem hiding this comment.
do we need to check if the upkeep belongs to this owner?
| /// @notice Called by the keeper to send funds to underfunded addresses. | ||
| /// @param performData the abi encoded list of addresses to fund | ||
| function performUpkeep(bytes calldata performData) external whenNotPaused { | ||
| (uint256[] memory upkeepIDs, uint96[] memory topUpAmounts) = abi.decode(performData, (uint256[], uint96[])); |
There was a problem hiding this comment.
should the uint256[], uint96[] align with uint256[] memory needsFunding, uint256[] memory topUpAmounts as in L137? Since you tested this, I guess it doesn't matter? But curious to know the reasoning behind those types :)
| /// secure the output of getUnderfundedUpkeeps() as the input to topUp(), and we are treating the owner | ||
| /// as a privileged user that can perform arbitrary top-ups to any upkeepID. | ||
| function topUp(uint256[] memory upkeepIDs, uint96[] memory topUpAmounts) public { | ||
| IAutomationForwarder forwarder = s_forwarder; |
There was a problem hiding this comment.
I dont see the setForwarder function is invoked anywhere in this contract, how is forwarder set?
| continue; | ||
| } | ||
| } catch {} | ||
| emit TopUpFailed(upkeepIDs[i]); |
There was a problem hiding this comment.
so, if there is any upkeep fails to topUp, we log this event and carry on.
Nit: is it more readable to have this emit TopUpFailed(upkeepIDs[i]); in the catch clause?
| IAutomationForwarder forwarder = s_forwarder; | ||
| if (msg.sender != address(s_forwarder) && msg.sender != owner()) revert OnlyForwarderOrOwner(); | ||
| if (upkeepIDs.length != topUpAmounts.length) revert InvalidTopUpData(); | ||
| address registryAddress = address(forwarder.getRegistry()); |
There was a problem hiding this comment.
nit: instead of forwarder.getRegistry(), just use getRegistry()?
| if (count < numUpkeeps) { | ||
| assembly { | ||
| mstore(needsFunding, count) | ||
| mstore(topUpAmounts, count) |
There was a problem hiding this comment.
n00b, what does this mean? we are setting the value of count to the address of topUpAmounts? isn't topUpAmounts of type uint256[]?
There was a problem hiding this comment.
these arrays are created by the size of config.maxBatchSize
however, we only have count number of underfunded upkeeps
this basically cuts the array size to count and saves the gas using inline assembly.
| uint256[] memory topUpAmounts = new uint256[](numUpkeeps); | ||
| Config memory config = s_config; | ||
| IAutomationRegistryConsumer registry = getRegistry(); | ||
| uint256 availableFunds = LINK_TOKEN.balanceOf(address(this)); |
There was a problem hiding this comment.
So this means this current contract holds LINK which will be used to auto fund the upkeeps. Is it possible to give a warning when this availableFunds is lower than topUpAmount as in L83?
|
SonarQube Quality Gate
|
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* write UpkeepBalanceMonWithBuffer * Update AutomationRegistryInterface2_0.sol Support getMinBalanceForUpkeep * reformat * remove modifier * add typeAndVersion check in constructor * restore AutomationRegistryInterface2_0.sol function * update import paths * update comments * cleanup * get rid of redundant REGISTRY variable * cleanup * rename * cleanup * remove target, min wait period, switch to min/target percentages * refactor getUnderfundedUpkeeps() to return top up amounts * refactor topUp() function * switch to max batch size * add max top up amount * add maxTopUpAmount * whitelist performUpkeep to forwarder * cleanup * rename * bring topUp() back * write initial test suite * fix solhint errors * update test for getUnderfundedUpkeeps() * add tests for owner only functions and events * add topUp and performUpkeep tests * rearrange functions on contract * add support for multiple registries * cleanup * change topUpAmounts to uint96[] * move pausable to topUp(); add length check to topUp() --------- Co-authored-by: De Clercq Wentzel <[email protected]>
No description provided.