From 3f5466a73af34a715590d96a060eadaf0b9f7ea8 Mon Sep 17 00:00:00 2001 From: Borja Aranda Date: Thu, 14 Dec 2023 10:56:40 +0100 Subject: [PATCH 1/8] Add onRamps support to LinkMon --- .../upkeeps/LinkAvailableBalanceMonitor.sol | 94 ++++++- .../v4.8.3/contracts/access/AccessControl.sol | 247 ++++++++++++++++++ .../contracts/access/IAccessControl.sol | 88 +++++++ .../contracts/utils/introspection/ERC165.sol | 29 ++ .../LinkAvailableBalanceMonitor.test.ts | 184 ++++++++++++- 5 files changed, 622 insertions(+), 20 deletions(-) create mode 100644 contracts/src/v0.8/vendor/openzeppelin-solidity/v4.8.3/contracts/access/AccessControl.sol create mode 100644 contracts/src/v0.8/vendor/openzeppelin-solidity/v4.8.3/contracts/access/IAccessControl.sol create mode 100644 contracts/src/v0.8/vendor/openzeppelin-solidity/v4.8.3/contracts/utils/introspection/ERC165.sol diff --git a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol index 9b9dc2d6b7d..ddfdb923238 100644 --- a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol +++ b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.19; import {AutomationCompatibleInterface} from "../interfaces/AutomationCompatibleInterface.sol"; +import {AccessControl} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/access/AccessControl.sol"; import {ConfirmedOwner} from "../../shared/access/ConfirmedOwner.sol"; import {IERC20} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/IERC20.sol"; import {Pausable} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/security/Pausable.sol"; @@ -33,7 +34,7 @@ interface ILinkAvailable { /// this is a "trusless" upkeep, meaning it does not trust the caller of performUpkeep; /// we could save a fair amount of gas and re-write this upkeep for use with Automation v2.0+, /// which has significantly different trust assumptions -contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AutomationCompatibleInterface { +contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AccessControl, AutomationCompatibleInterface { event BalanceUpdated(address indexed addr, uint256 oldBalance, uint256 newBalance); event FundsWithdrawn(uint256 amountWithdrawn, address payee); event UpkeepIntervalSet(uint256 oldUpkeepInterval, uint256 newUpkeepInterval); @@ -63,16 +64,23 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AutomationComp bool isActive; } + bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE"); + bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE"); + uint96 private constant DEFAULT_TOP_UP_AMOUNT_JULES = 9000000000000000000; + uint96 private constant DEFAULT_MIN_BALANCE_JULES = 1000000000000000000; IERC20 private immutable LINK_TOKEN; + uint256 private s_minWaitPeriodSeconds; uint16 private s_maxPerform; uint16 private s_maxCheck; uint8 private s_upkeepInterval; address[] private s_watchList; mapping(address targetAddress => MonitoredAddress targetProperties) internal s_targets; + mapping(uint64 dstChainSelector => address onRamp) internal s_onRampAddresses; /// @param linkTokenAddress the LINK token address constructor( + address admin, address linkTokenAddress, uint256 minWaitPeriodSeconds, uint16 maxPerform, @@ -80,6 +88,9 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AutomationComp uint8 upkeepInterval ) ConfirmedOwner(msg.sender) { if (linkTokenAddress == address(0)) revert InvalidLinkTokenAddress(linkTokenAddress); + _setRoleAdmin(ADMIN_ROLE, ADMIN_ROLE); + _setRoleAdmin(EXECUTOR_ROLE, ADMIN_ROLE); + _grantRole(ADMIN_ROLE, admin); LINK_TOKEN = IERC20(linkTokenAddress); setMinWaitPeriodSeconds(minWaitPeriodSeconds); setMaxPerform(maxPerform); @@ -95,7 +106,7 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AutomationComp address[] calldata addresses, uint96[] calldata minBalances, uint96[] calldata topUpAmounts - ) external onlyOwner { + ) external onlyRoleOrAdminRole(EXECUTOR_ROLE) { if (addresses.length != minBalances.length || addresses.length != topUpAmounts.length) { revert InvalidWatchList(); } @@ -118,6 +129,53 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AutomationComp emit WatchlistUpdated(); } + /// @notice Adds a new address to the watchlist + /// @param targetAddress the address to be added to the watchlist + /// @param dstChainSelector carries a non-zero value in case the targetAddress is an onRamp, otherwise it carries a 0 + /// @dev this function has to be compatible with the event onRampSet(address, dstChainSelector) emitted by + /// the CCIP router. Important detail to know is this event is also emitted when an onRamp is decomissioned, + /// in which case it will carry the proper dstChainSelector along with the 0x0 address + function addToWatchListOrDecomission( + address targetAddress, + uint64 dstChainSelector + ) public onlyRoleOrAdminRole(EXECUTOR_ROLE) { + if (s_targets[targetAddress].isActive) revert DuplicateAddress(targetAddress); + address oldAddress = s_onRampAddresses[dstChainSelector]; + // if targetAddress is an existing onRamp, there's a need of cleaning the previous onRamp associated to this dstChainSelector + // there's no need to remove any other address that's not an onRamp + if (dstChainSelector > 0 && oldAddress != address(0)) { + removeFromWatchList(oldAddress); + } + // only add the new address if it's not 0x0 + if (targetAddress != address(0)) { + s_onRampAddresses[dstChainSelector] = targetAddress; + s_targets[targetAddress] = MonitoredAddress({ + isActive: true, + minBalance: DEFAULT_MIN_BALANCE_JULES, + topUpAmount: DEFAULT_TOP_UP_AMOUNT_JULES, + lastTopUpTimestamp: 0 + }); + s_watchList.push(targetAddress); + } else { + // if the address is 0x0, it means the onRamp has ben decomissioned and has to be cleaned + delete s_onRampAddresses[dstChainSelector]; + } + } + + /// @notice Delete an address from the watchlist and sets the target to inactive + /// @param targetAddress the address to be deleted + function removeFromWatchList(address targetAddress) public onlyRoleOrAdminRole(EXECUTOR_ROLE) returns (bool) { + s_targets[targetAddress].isActive = false; + for (uint256 i; i < s_watchList.length; i++) { + if (s_watchList[i] == targetAddress) { + s_watchList[i] = s_watchList[s_watchList.length - 1]; + s_watchList.pop(); + return true; + } + } + return false; + } + /// @notice Gets a list of proxies that are underfunded, up to the s_maxPerform size /// @dev the function starts at a random index in the list to avoid biasing the first /// addresses in the list over latter ones. @@ -201,7 +259,9 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AutomationComp } try target.linkAvailableForPayment() returns (int256 balance) { if ( - balance < int256(minBalance) && addressToCheck.lastTopUpTimestamp + s_minWaitPeriodSeconds <= block.timestamp + balance < int256(minBalance) && + addressToCheck.lastTopUpTimestamp + s_minWaitPeriodSeconds <= block.timestamp && + addressToCheck.isActive ) { return true; } @@ -231,14 +291,14 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AutomationComp /// @notice Withdraws the contract balance in the LINK token. /// @param amount the amount of the LINK to withdraw /// @param payee the address to pay - function withdraw(uint256 amount, address payable payee) external onlyOwner { + function withdraw(uint256 amount, address payable payee) external onlyRoleOrAdminRole(EXECUTOR_ROLE) { if (payee == address(0)) revert InvalidAddress(payee); LINK_TOKEN.transfer(payee, amount); emit FundsWithdrawn(amount, payee); } /// @notice Sets the minimum balance for the given target address - function setMinBalance(address target, uint96 minBalance) external onlyOwner { + function setMinBalance(address target, uint96 minBalance) external onlyRole(ADMIN_ROLE) { if (target == address(0)) revert InvalidAddress(target); if (minBalance == 0) revert InvalidMinBalance(minBalance); if (!s_targets[target].isActive) revert InvalidWatchList(); @@ -248,7 +308,7 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AutomationComp } /// @notice Sets the minimum balance for the given target address - function setTopUpAmount(address target, uint96 topUpAmount) external onlyOwner { + function setTopUpAmount(address target, uint96 topUpAmount) external onlyRole(ADMIN_ROLE) { if (target == address(0)) revert InvalidAddress(target); if (topUpAmount == 0) revert InvalidTopUpAmount(topUpAmount); if (!s_targets[target].isActive) revert InvalidWatchList(); @@ -258,25 +318,25 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AutomationComp } /// @notice Update s_maxPerform - function setMaxPerform(uint16 maxPerform) public onlyOwner { + function setMaxPerform(uint16 maxPerform) public onlyRole(ADMIN_ROLE) { s_maxPerform = maxPerform; emit MaxPerformSet(s_maxPerform, maxPerform); } /// @notice Update s_maxCheck - function setMaxCheck(uint16 maxCheck) public onlyOwner { + function setMaxCheck(uint16 maxCheck) public onlyRole(ADMIN_ROLE) { s_maxCheck = maxCheck; emit MaxCheckSet(s_maxCheck, maxCheck); } /// @notice Sets the minimum wait period (in seconds) for addresses between funding - function setMinWaitPeriodSeconds(uint256 minWaitPeriodSeconds) public onlyOwner { + function setMinWaitPeriodSeconds(uint256 minWaitPeriodSeconds) public onlyRole(ADMIN_ROLE) { s_minWaitPeriodSeconds = minWaitPeriodSeconds; emit MinWaitPeriodSet(s_minWaitPeriodSeconds, minWaitPeriodSeconds); } /// @notice Update s_upkeepInterval - function setUpkeepInterval(uint8 upkeepInterval) public onlyOwner { + function setUpkeepInterval(uint8 upkeepInterval) public onlyRole(ADMIN_ROLE) { if (upkeepInterval > 255) revert InvalidUpkeepInterval(upkeepInterval); s_upkeepInterval = upkeepInterval; emit UpkeepIntervalSet(s_upkeepInterval, upkeepInterval); @@ -315,13 +375,23 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AutomationComp return (target.isActive, target.minBalance, target.topUpAmount); } + /// @dev Modifier to make a function callable only by a certain role or the + /// admin role. + modifier onlyRoleOrAdminRole(bytes32 role) { + address sender = _msgSender(); + if (!hasRole(ADMIN_ROLE, sender)) { + _checkRole(role, sender); + } + _; + } + /// @notice Pause the contract, which prevents executing performUpkeep - function pause() external onlyOwner { + function pause() external onlyRole(ADMIN_ROLE) { _pause(); } /// @notice Unpause the contract - function unpause() external onlyOwner { + function unpause() external onlyRole(ADMIN_ROLE) { _unpause(); } } diff --git a/contracts/src/v0.8/vendor/openzeppelin-solidity/v4.8.3/contracts/access/AccessControl.sol b/contracts/src/v0.8/vendor/openzeppelin-solidity/v4.8.3/contracts/access/AccessControl.sol new file mode 100644 index 00000000000..4e388f9d83d --- /dev/null +++ b/contracts/src/v0.8/vendor/openzeppelin-solidity/v4.8.3/contracts/access/AccessControl.sol @@ -0,0 +1,247 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts (last updated v4.8.0) (access/AccessControl.sol) + +pragma solidity ^0.8.0; + +import "./IAccessControl.sol"; +import "../utils/Context.sol"; +import "../utils/Strings.sol"; +import "../utils/introspection/ERC165.sol"; + +/** + * @dev Contract module that allows children to implement role-based access + * control mechanisms. This is a lightweight version that doesn't allow enumerating role + * members except through off-chain means by accessing the contract event logs. Some + * applications may benefit from on-chain enumerability, for those cases see + * {AccessControlEnumerable}. + * + * Roles are referred to by their `bytes32` identifier. These should be exposed + * in the external API and be unique. The best way to achieve this is by + * using `public constant` hash digests: + * + * ``` + * bytes32 public constant MY_ROLE = keccak256("MY_ROLE"); + * ``` + * + * Roles can be used to represent a set of permissions. To restrict access to a + * function call, use {hasRole}: + * + * ``` + * function foo() public { + * require(hasRole(MY_ROLE, msg.sender)); + * ... + * } + * ``` + * + * Roles can be granted and revoked dynamically via the {grantRole} and + * {revokeRole} functions. Each role has an associated admin role, and only + * accounts that have a role's admin role can call {grantRole} and {revokeRole}. + * + * By default, the admin role for all roles is `DEFAULT_ADMIN_ROLE`, which means + * that only accounts with this role will be able to grant or revoke other + * roles. More complex role relationships can be created by using + * {_setRoleAdmin}. + * + * WARNING: The `DEFAULT_ADMIN_ROLE` is also its own admin: it has permission to + * grant and revoke this role. Extra precautions should be taken to secure + * accounts that have been granted it. + */ +abstract contract AccessControl is Context, IAccessControl, ERC165 { + struct RoleData { + mapping(address => bool) members; + bytes32 adminRole; + } + + mapping(bytes32 => RoleData) private _roles; + + bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00; + + /** + * @dev Modifier that checks that an account has a specific role. Reverts + * with a standardized message including the required role. + * + * The format of the revert reason is given by the following regular expression: + * + * /^AccessControl: account (0x[0-9a-f]{40}) is missing role (0x[0-9a-f]{64})$/ + * + * _Available since v4.1._ + */ + modifier onlyRole(bytes32 role) { + _checkRole(role); + _; + } + + /** + * @dev See {IERC165-supportsInterface}. + */ + function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) { + return interfaceId == type(IAccessControl).interfaceId || super.supportsInterface(interfaceId); + } + + /** + * @dev Returns `true` if `account` has been granted `role`. + */ + function hasRole(bytes32 role, address account) public view virtual override returns (bool) { + return _roles[role].members[account]; + } + + /** + * @dev Revert with a standard message if `_msgSender()` is missing `role`. + * Overriding this function changes the behavior of the {onlyRole} modifier. + * + * Format of the revert message is described in {_checkRole}. + * + * _Available since v4.6._ + */ + function _checkRole(bytes32 role) internal view virtual { + _checkRole(role, _msgSender()); + } + + /** + * @dev Revert with a standard message if `account` is missing `role`. + * + * The format of the revert reason is given by the following regular expression: + * + * /^AccessControl: account (0x[0-9a-f]{40}) is missing role (0x[0-9a-f]{64})$/ + */ + function _checkRole(bytes32 role, address account) internal view virtual { + if (!hasRole(role, account)) { + revert( + string( + abi.encodePacked( + "AccessControl: account ", + Strings.toHexString(account), + " is missing role ", + Strings.toHexString(uint256(role), 32) + ) + ) + ); + } + } + + /** + * @dev Returns the admin role that controls `role`. See {grantRole} and + * {revokeRole}. + * + * To change a role's admin, use {_setRoleAdmin}. + */ + function getRoleAdmin(bytes32 role) public view virtual override returns (bytes32) { + return _roles[role].adminRole; + } + + /** + * @dev Grants `role` to `account`. + * + * If `account` had not been already granted `role`, emits a {RoleGranted} + * event. + * + * Requirements: + * + * - the caller must have ``role``'s admin role. + * + * May emit a {RoleGranted} event. + */ + function grantRole(bytes32 role, address account) public virtual override onlyRole(getRoleAdmin(role)) { + _grantRole(role, account); + } + + /** + * @dev Revokes `role` from `account`. + * + * If `account` had been granted `role`, emits a {RoleRevoked} event. + * + * Requirements: + * + * - the caller must have ``role``'s admin role. + * + * May emit a {RoleRevoked} event. + */ + function revokeRole(bytes32 role, address account) public virtual override onlyRole(getRoleAdmin(role)) { + _revokeRole(role, account); + } + + /** + * @dev Revokes `role` from the calling account. + * + * Roles are often managed via {grantRole} and {revokeRole}: this function's + * purpose is to provide a mechanism for accounts to lose their privileges + * if they are compromised (such as when a trusted device is misplaced). + * + * If the calling account had been revoked `role`, emits a {RoleRevoked} + * event. + * + * Requirements: + * + * - the caller must be `account`. + * + * May emit a {RoleRevoked} event. + */ + function renounceRole(bytes32 role, address account) public virtual override { + require(account == _msgSender(), "AccessControl: can only renounce roles for self"); + + _revokeRole(role, account); + } + + /** + * @dev Grants `role` to `account`. + * + * If `account` had not been already granted `role`, emits a {RoleGranted} + * event. Note that unlike {grantRole}, this function doesn't perform any + * checks on the calling account. + * + * May emit a {RoleGranted} event. + * + * [WARNING] + * ==== + * This function should only be called from the constructor when setting + * up the initial roles for the system. + * + * Using this function in any other way is effectively circumventing the admin + * system imposed by {AccessControl}. + * ==== + * + * NOTE: This function is deprecated in favor of {_grantRole}. + */ + function _setupRole(bytes32 role, address account) internal virtual { + _grantRole(role, account); + } + + /** + * @dev Sets `adminRole` as ``role``'s admin role. + * + * Emits a {RoleAdminChanged} event. + */ + function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual { + bytes32 previousAdminRole = getRoleAdmin(role); + _roles[role].adminRole = adminRole; + emit RoleAdminChanged(role, previousAdminRole, adminRole); + } + + /** + * @dev Grants `role` to `account`. + * + * Internal function without access restriction. + * + * May emit a {RoleGranted} event. + */ + function _grantRole(bytes32 role, address account) internal virtual { + if (!hasRole(role, account)) { + _roles[role].members[account] = true; + emit RoleGranted(role, account, _msgSender()); + } + } + + /** + * @dev Revokes `role` from `account`. + * + * Internal function without access restriction. + * + * May emit a {RoleRevoked} event. + */ + function _revokeRole(bytes32 role, address account) internal virtual { + if (hasRole(role, account)) { + _roles[role].members[account] = false; + emit RoleRevoked(role, account, _msgSender()); + } + } +} \ No newline at end of file diff --git a/contracts/src/v0.8/vendor/openzeppelin-solidity/v4.8.3/contracts/access/IAccessControl.sol b/contracts/src/v0.8/vendor/openzeppelin-solidity/v4.8.3/contracts/access/IAccessControl.sol new file mode 100644 index 00000000000..efb82a3cb5e --- /dev/null +++ b/contracts/src/v0.8/vendor/openzeppelin-solidity/v4.8.3/contracts/access/IAccessControl.sol @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts v4.4.1 (access/IAccessControl.sol) + +pragma solidity ^0.8.0; + +/** + * @dev External interface of AccessControl declared to support ERC165 detection. + */ +interface IAccessControl { + /** + * @dev Emitted when `newAdminRole` is set as ``role``'s admin role, replacing `previousAdminRole` + * + * `DEFAULT_ADMIN_ROLE` is the starting admin for all roles, despite + * {RoleAdminChanged} not being emitted signaling this. + * + * _Available since v3.1._ + */ + event RoleAdminChanged(bytes32 indexed role, bytes32 indexed previousAdminRole, bytes32 indexed newAdminRole); + + /** + * @dev Emitted when `account` is granted `role`. + * + * `sender` is the account that originated the contract call, an admin role + * bearer except when using {AccessControl-_setupRole}. + */ + event RoleGranted(bytes32 indexed role, address indexed account, address indexed sender); + + /** + * @dev Emitted when `account` is revoked `role`. + * + * `sender` is the account that originated the contract call: + * - if using `revokeRole`, it is the admin role bearer + * - if using `renounceRole`, it is the role bearer (i.e. `account`) + */ + event RoleRevoked(bytes32 indexed role, address indexed account, address indexed sender); + + /** + * @dev Returns `true` if `account` has been granted `role`. + */ + function hasRole(bytes32 role, address account) external view returns (bool); + + /** + * @dev Returns the admin role that controls `role`. See {grantRole} and + * {revokeRole}. + * + * To change a role's admin, use {AccessControl-_setRoleAdmin}. + */ + function getRoleAdmin(bytes32 role) external view returns (bytes32); + + /** + * @dev Grants `role` to `account`. + * + * If `account` had not been already granted `role`, emits a {RoleGranted} + * event. + * + * Requirements: + * + * - the caller must have ``role``'s admin role. + */ + function grantRole(bytes32 role, address account) external; + + /** + * @dev Revokes `role` from `account`. + * + * If `account` had been granted `role`, emits a {RoleRevoked} event. + * + * Requirements: + * + * - the caller must have ``role``'s admin role. + */ + function revokeRole(bytes32 role, address account) external; + + /** + * @dev Revokes `role` from the calling account. + * + * Roles are often managed via {grantRole} and {revokeRole}: this function's + * purpose is to provide a mechanism for accounts to lose their privileges + * if they are compromised (such as when a trusted device is misplaced). + * + * If the calling account had been granted `role`, emits a {RoleRevoked} + * event. + * + * Requirements: + * + * - the caller must be `account`. + */ + function renounceRole(bytes32 role, address account) external; +} \ No newline at end of file diff --git a/contracts/src/v0.8/vendor/openzeppelin-solidity/v4.8.3/contracts/utils/introspection/ERC165.sol b/contracts/src/v0.8/vendor/openzeppelin-solidity/v4.8.3/contracts/utils/introspection/ERC165.sol new file mode 100644 index 00000000000..c682b07a65b --- /dev/null +++ b/contracts/src/v0.8/vendor/openzeppelin-solidity/v4.8.3/contracts/utils/introspection/ERC165.sol @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts v4.4.1 (utils/introspection/ERC165.sol) + +pragma solidity ^0.8.0; + +import "./IERC165.sol"; + +/** + * @dev Implementation of the {IERC165} interface. + * + * Contracts that want to implement ERC165 should inherit from this contract and override {supportsInterface} to check + * for the additional interface id that will be supported. For example: + * + * ```solidity + * function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) { + * return interfaceId == type(MyInterface).interfaceId || super.supportsInterface(interfaceId); + * } + * ``` + * + * Alternatively, {ERC165Storage} provides an easier to use but more expensive implementation. + */ +abstract contract ERC165 is IERC165 { + /** + * @dev See {IERC165-supportsInterface}. + */ + function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) { + return interfaceId == type(IERC165).interfaceId; + } +} \ No newline at end of file diff --git a/contracts/test/v0.8/automation/LinkAvailableBalanceMonitor.test.ts b/contracts/test/v0.8/automation/LinkAvailableBalanceMonitor.test.ts index 76a3dcfff1b..847acd70f2f 100644 --- a/contracts/test/v0.8/automation/LinkAvailableBalanceMonitor.test.ts +++ b/contracts/test/v0.8/automation/LinkAvailableBalanceMonitor.test.ts @@ -26,8 +26,6 @@ const TARGET_PERFORM_GAS_LIMIT = 2_000_000 const TARGET_CHECK_GAS_LIMIT = 3_500_000 // // ////////////////////////////////////////////////////////////////////////////////////////////////// - -const OWNABLE_ERR = 'Only callable by owner' const INVALID_WATCHLIST_ERR = `InvalidWatchList()` const PAUSED_ERR = 'Pausable: paused' @@ -35,7 +33,6 @@ const zeroLINK = ethers.utils.parseEther('0') const oneLINK = ethers.utils.parseEther('1') const twoLINK = ethers.utils.parseEther('2') const fourLINK = ethers.utils.parseEther('4') -const fiveLINK = ethers.utils.parseEther('5') const tenLINK = ethers.utils.parseEther('10') const oneHundredLINK = ethers.utils.parseEther('100') @@ -152,6 +149,7 @@ const setup = async () => { lt = (await ltFactory.deploy()) as LinkToken labm = await labmFactory.deploy( + owner.address, lt.address, minWaitPeriodSeconds, maxPerform, @@ -223,6 +221,42 @@ describe('LinkAvailableBalanceMonitor', () => { }) }) + describe('setMaxPerform()', () => { + it('configures the MaxPerform', async () => { + await labm.connect(owner).setMaxPerform(BigNumber.from(100)) + const report = await labm.getMaxPerform() + assert.equal(report.toString(), '100') + }) + + it('is only callable by the owner', async () => { + await expect(labm.connect(stranger).setMaxPerform(100)).to.be.reverted + }) + }) + + describe('setMaxCheck()', () => { + it('configures the MaxCheck', async () => { + await labm.connect(owner).setMaxCheck(BigNumber.from(100)) + const report = await labm.getMaxCheck() + assert.equal(report.toString(), '100') + }) + + it('is only callable by the owner', async () => { + await expect(labm.connect(stranger).setMaxCheck(100)).to.be.reverted + }) + }) + + describe('setUpkeepInterval()', () => { + it('configures the UpkeepInterval', async () => { + await labm.connect(owner).setUpkeepInterval(BigNumber.from(100)) + const report = await labm.getUpkeepInterval() + assert.equal(report.toString(), '100') + }) + + it('is only callable by the owner', async () => { + await expect(labm.connect(stranger).setUpkeepInterval(100)).to.be.reverted + }) + }) + describe('withdraw()', () => { beforeEach(async () => { const tx = await lt.connect(owner).transfer(labm.address, oneLINK) @@ -260,7 +294,7 @@ describe('LinkAvailableBalanceMonitor', () => { it('Should not allow strangers to withdraw', async () => { const tx = labm.connect(stranger).withdraw(oneLINK, owner.address) - await expect(tx).to.be.revertedWith(OWNABLE_ERR) + await expect(tx).to.be.reverted }) }) @@ -274,15 +308,15 @@ describe('LinkAvailableBalanceMonitor', () => { it('Should not allow strangers to pause / unpause', async () => { const pauseTxStranger = labm.connect(stranger).pause() - await expect(pauseTxStranger).to.be.revertedWith(OWNABLE_ERR) + await expect(pauseTxStranger).to.be.reverted const pauseTxOwner = await labm.connect(owner).pause() await pauseTxOwner.wait() const unpauseTxStranger = labm.connect(stranger).unpause() - await expect(unpauseTxStranger).to.be.revertedWith(OWNABLE_ERR) + await expect(unpauseTxStranger).to.be.reverted }) }) - describe('setWatchList() / addToWatchList() / removeFromWatchlist() / getWatchList()', () => { + describe('setWatchList() / addToWatchListOrDecomissionOrDecomission() / removeFromWatchlist() / getWatchList()', () => { const watchAddress1 = randAddr() const watchAddress2 = randAddr() const watchAddress3 = randAddr() @@ -342,7 +376,7 @@ describe('LinkAvailableBalanceMonitor', () => { const setTxStranger = labm .connect(stranger) .setWatchList([watchAddress1], [oneLINK], [oneLINK]) - await expect(setTxStranger).to.be.revertedWith(OWNABLE_ERR) + await expect(setTxStranger).to.be.reverted }) it('Should revert if any of the addresses are empty', async () => { @@ -355,6 +389,139 @@ describe('LinkAvailableBalanceMonitor', () => { ) await expect(tx).to.be.revertedWith(INVALID_WATCHLIST_ERR) }) + + it('Should allow owner to add multiple addresses with dstChainSelector 0 to the watchlist', async () => { + let tx = await labm + .connect(owner) + .addToWatchListOrDecomission(watchAddress1, 0) + await tx.wait + let watchList = await labm.getWatchList() + assert.deepEqual(watchList[0], watchAddress1) + + tx = await labm + .connect(owner) + .addToWatchListOrDecomission(watchAddress2, 0) + await tx.wait + watchList = await labm.getWatchList() + assert.deepEqual(watchList[0], watchAddress1) + assert.deepEqual(watchList[1], watchAddress2) + + tx = await labm + .connect(owner) + .addToWatchListOrDecomission(watchAddress3, 0) + await tx.wait + watchList = await labm.getWatchList() + assert.deepEqual(watchList[0], watchAddress1) + assert.deepEqual(watchList[1], watchAddress2) + assert.deepEqual(watchList[2], watchAddress3) + }) + + it('Should allow owner to add only one address with an unique non-zero dstChainSelector 0 to the watchlist', async () => { + let tx = await labm + .connect(owner) + .addToWatchListOrDecomission(watchAddress1, 1) + await tx.wait + let watchList = await labm.getWatchList() + assert.deepEqual(watchList[0], watchAddress1) + + // 1 is active + let report = await labm.getAccountInfo(watchAddress1) + assert.isTrue(report.isActive) + + tx = await labm + .connect(owner) + .addToWatchListOrDecomission(watchAddress2, 1) + await tx.wait + watchList = await labm.getWatchList() + assert.deepEqual(watchList[0], watchAddress2) + + // 2 is active, 1 should be false + report = await labm.getAccountInfo(watchAddress2) + assert.isTrue(report.isActive) + report = await labm.getAccountInfo(watchAddress1) + assert.isFalse(report.isActive) + + tx = await labm + .connect(owner) + .addToWatchListOrDecomission(watchAddress3, 1) + await tx.wait + watchList = await labm.getWatchList() + assert.deepEqual(watchList[0], watchAddress3) + + // 3 is active, 1 and 2 should be false + report = await labm.getAccountInfo(watchAddress3) + assert.isTrue(report.isActive) + report = await labm.getAccountInfo(watchAddress2) + assert.isFalse(report.isActive) + report = await labm.getAccountInfo(watchAddress1) + assert.isFalse(report.isActive) + }) + + it('Should not add address 0 to the watchlist', async () => { + await labm + .connect(owner) + .addToWatchListOrDecomission(ethers.constants.AddressZero, 1) + expect(await labm.getWatchList()).to.not.contain( + ethers.constants.AddressZero, + ) + }) + + it('Should not allow stangers to add addresses to the watchlist', async () => { + await expect( + labm.connect(stranger).addToWatchListOrDecomission(watchAddress1, 1), + ).to.be.reverted + }) + + it('Should allow owner to remove addresses from the watchlist', async () => { + let tx = await labm + .connect(owner) + .addToWatchListOrDecomission(watchAddress1, 1) + await tx.wait + let watchList = await labm.getWatchList() + assert.deepEqual(watchList[0], watchAddress1) + let report = await labm.getAccountInfo(watchAddress1) + assert.isTrue(report.isActive) + + // remove address + tx = await labm.connect(owner).removeFromWatchList(watchAddress1) + + // address should be false + report = await labm.getAccountInfo(watchAddress1) + assert.isFalse(report.isActive) + + watchList = await labm.getWatchList() + assert.deepEqual(watchList, []) + }) + + it('Should allow only one address per dstChainSelector', async () => { + // add address1 + await labm.connect(owner).addToWatchListOrDecomission(watchAddress1, 1) + expect(await labm.getWatchList()).to.contain(watchAddress1) + + // add address2 + await labm.connect(owner).addToWatchListOrDecomission(watchAddress2, 1) + + // only address2 has to be in the watchlist + const watchlist = await labm.getWatchList() + expect(watchlist).to.not.contain(watchAddress1) + expect(watchlist).to.contain(watchAddress2) + }) + + it('Should delete the onRamp address on a zero-address with same dstChainSelector', async () => { + // add address1 + await labm.connect(owner).addToWatchListOrDecomission(watchAddress1, 1) + expect(await labm.getWatchList()).to.contain(watchAddress1) + + // simulates an onRampSet(zeroAddress, same dstChainSelector) + await labm + .connect(owner) + .addToWatchListOrDecomission(ethers.constants.AddressZero, 1) + + // address1 should be cleaned + const watchlist = await labm.getWatchList() + expect(watchlist).to.not.contain(watchAddress1) + assert.deepEqual(watchlist, []) + }) }) describe('checkUpkeep() / sampleUnderfundedAddresses() [ @skip-coverage ]', () => { @@ -528,6 +695,7 @@ describe('LinkAvailableBalanceMonitor', () => { await labm.connect(owner).pause() const performTx = labm.connect(keeperRegistry).performUpkeep(validPayload) await expect(performTx).to.be.revertedWith(PAUSED_ERR) + console.log(performTx) }) it('Should fund the appropriate addresses', async () => { From 5a959892a069ff4f9f6a0f14b700885a81e83690 Mon Sep 17 00:00:00 2001 From: Borja Aranda Date: Fri, 15 Dec 2023 12:47:11 +0100 Subject: [PATCH 2/8] apply audit --- .../upkeeps/LinkAvailableBalanceMonitor.sol | 26 +++++++++---------- .../LinkAvailableBalanceMonitor.test.ts | 1 - 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol index ddfdb923238..55cc4d2a8c5 100644 --- a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol +++ b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol @@ -4,7 +4,6 @@ pragma solidity 0.8.19; import {AutomationCompatibleInterface} from "../interfaces/AutomationCompatibleInterface.sol"; import {AccessControl} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/access/AccessControl.sol"; -import {ConfirmedOwner} from "../../shared/access/ConfirmedOwner.sol"; import {IERC20} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/IERC20.sol"; import {Pausable} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/security/Pausable.sol"; @@ -34,7 +33,7 @@ interface ILinkAvailable { /// this is a "trusless" upkeep, meaning it does not trust the caller of performUpkeep; /// we could save a fair amount of gas and re-write this upkeep for use with Automation v2.0+, /// which has significantly different trust assumptions -contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AccessControl, AutomationCompatibleInterface { +contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInterface, Pausable { event BalanceUpdated(address indexed addr, uint256 oldBalance, uint256 newBalance); event FundsWithdrawn(uint256 amountWithdrawn, address payee); event UpkeepIntervalSet(uint256 oldUpkeepInterval, uint256 newUpkeepInterval); @@ -68,7 +67,7 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AccessControl, bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE"); uint96 private constant DEFAULT_TOP_UP_AMOUNT_JULES = 9000000000000000000; uint96 private constant DEFAULT_MIN_BALANCE_JULES = 1000000000000000000; - IERC20 private immutable LINK_TOKEN; + IERC20 private immutable i_linkToken; uint256 private s_minWaitPeriodSeconds; uint16 private s_maxPerform; @@ -86,12 +85,13 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AccessControl, uint16 maxPerform, uint16 maxCheck, uint8 upkeepInterval - ) ConfirmedOwner(msg.sender) { + ) { if (linkTokenAddress == address(0)) revert InvalidLinkTokenAddress(linkTokenAddress); _setRoleAdmin(ADMIN_ROLE, ADMIN_ROLE); _setRoleAdmin(EXECUTOR_ROLE, ADMIN_ROLE); _grantRole(ADMIN_ROLE, admin); - LINK_TOKEN = IERC20(linkTokenAddress); + _grantRole(DEFAULT_ADMIN_ROLE, msg.sender); + i_linkToken = IERC20(linkTokenAddress); setMinWaitPeriodSeconds(minWaitPeriodSeconds); setMaxPerform(maxPerform); setMaxCheck(maxCheck); @@ -216,15 +216,15 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AccessControl, function topUp(address[] memory targetAddresses) public whenNotPaused { MonitoredAddress memory target; - uint256 localBalance = LINK_TOKEN.balanceOf(address(this)); + uint256 localBalance = i_linkToken.balanceOf(address(this)); for (uint256 idx = 0; idx < targetAddresses.length; idx++) { address targetAddress = targetAddresses[idx]; target = s_targets[targetAddress]; if (localBalance >= target.topUpAmount && _needsFunding(targetAddress, target.minBalance)) { - bool success = LINK_TOKEN.transfer(targetAddress, target.topUpAmount); + bool success = i_linkToken.transfer(targetAddress, target.topUpAmount); if (success) { localBalance -= target.topUpAmount; - target.lastTopUpTimestamp = uint56(block.timestamp); + s_targets[targetAddress].lastTopUpTimestamp = uint56(block.timestamp); emit TopUpSucceeded(targetAddress); } else { emit TopUpFailed(targetAddress); @@ -293,7 +293,7 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AccessControl, /// @param payee the address to pay function withdraw(uint256 amount, address payable payee) external onlyRoleOrAdminRole(EXECUTOR_ROLE) { if (payee == address(0)) revert InvalidAddress(payee); - LINK_TOKEN.transfer(payee, amount); + i_linkToken.transfer(payee, amount); emit FundsWithdrawn(amount, payee); } @@ -319,27 +319,27 @@ contract LinkAvailableBalanceMonitor is ConfirmedOwner, Pausable, AccessControl, /// @notice Update s_maxPerform function setMaxPerform(uint16 maxPerform) public onlyRole(ADMIN_ROLE) { - s_maxPerform = maxPerform; emit MaxPerformSet(s_maxPerform, maxPerform); + s_maxPerform = maxPerform; } /// @notice Update s_maxCheck function setMaxCheck(uint16 maxCheck) public onlyRole(ADMIN_ROLE) { - s_maxCheck = maxCheck; emit MaxCheckSet(s_maxCheck, maxCheck); + s_maxCheck = maxCheck; } /// @notice Sets the minimum wait period (in seconds) for addresses between funding function setMinWaitPeriodSeconds(uint256 minWaitPeriodSeconds) public onlyRole(ADMIN_ROLE) { - s_minWaitPeriodSeconds = minWaitPeriodSeconds; emit MinWaitPeriodSet(s_minWaitPeriodSeconds, minWaitPeriodSeconds); + s_minWaitPeriodSeconds = minWaitPeriodSeconds; } /// @notice Update s_upkeepInterval function setUpkeepInterval(uint8 upkeepInterval) public onlyRole(ADMIN_ROLE) { if (upkeepInterval > 255) revert InvalidUpkeepInterval(upkeepInterval); - s_upkeepInterval = upkeepInterval; emit UpkeepIntervalSet(s_upkeepInterval, upkeepInterval); + s_upkeepInterval = upkeepInterval; } /// @notice Gets maxPerform diff --git a/contracts/test/v0.8/automation/LinkAvailableBalanceMonitor.test.ts b/contracts/test/v0.8/automation/LinkAvailableBalanceMonitor.test.ts index 847acd70f2f..1cab0fa010e 100644 --- a/contracts/test/v0.8/automation/LinkAvailableBalanceMonitor.test.ts +++ b/contracts/test/v0.8/automation/LinkAvailableBalanceMonitor.test.ts @@ -695,7 +695,6 @@ describe('LinkAvailableBalanceMonitor', () => { await labm.connect(owner).pause() const performTx = labm.connect(keeperRegistry).performUpkeep(validPayload) await expect(performTx).to.be.revertedWith(PAUSED_ERR) - console.log(performTx) }) it('Should fund the appropriate addresses', async () => { From 9cfa7984ef9ca351af0860a62e22c7ddafc917c8 Mon Sep 17 00:00:00 2001 From: Borja Aranda Date: Tue, 19 Dec 2023 11:57:04 +0100 Subject: [PATCH 3/8] review changes --- .../upkeeps/LinkAvailableBalanceMonitor.sol | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol index 55cc4d2a8c5..754f3553038 100644 --- a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol +++ b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol @@ -75,9 +75,17 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter uint8 private s_upkeepInterval; address[] private s_watchList; mapping(address targetAddress => MonitoredAddress targetProperties) internal s_targets; + + /// @notice s_onRampAddresses represents a list of CCIP onRamp addresses watched on this contract + /// There has to be only one onRamp per dstChainSelector. mapping(uint64 dstChainSelector => address onRamp) internal s_onRampAddresses; + /// @param admin is the administrator address of this contract /// @param linkTokenAddress the LINK token address + /// @param minWaitPeriodSeconds represents the amount of time that has to wait a contract to be funded + /// @param maxPerform maximum amount of contracts to fund + /// @param maxCheck maximum amount of contracts to check + /// @param upkeepInterval randomizes the check for underfunded contracts constructor( address admin, address linkTokenAddress, @@ -90,7 +98,6 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter _setRoleAdmin(ADMIN_ROLE, ADMIN_ROLE); _setRoleAdmin(EXECUTOR_ROLE, ADMIN_ROLE); _grantRole(ADMIN_ROLE, admin); - _grantRole(DEFAULT_ADMIN_ROLE, msg.sender); i_linkToken = IERC20(linkTokenAddress); setMinWaitPeriodSeconds(minWaitPeriodSeconds); setMaxPerform(maxPerform); @@ -106,7 +113,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter address[] calldata addresses, uint96[] calldata minBalances, uint96[] calldata topUpAmounts - ) external onlyRoleOrAdminRole(EXECUTOR_ROLE) { + ) external onlyAdminOrExecutor { if (addresses.length != minBalances.length || addresses.length != topUpAmounts.length) { revert InvalidWatchList(); } @@ -135,10 +142,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter /// @dev this function has to be compatible with the event onRampSet(address, dstChainSelector) emitted by /// the CCIP router. Important detail to know is this event is also emitted when an onRamp is decomissioned, /// in which case it will carry the proper dstChainSelector along with the 0x0 address - function addToWatchListOrDecomission( - address targetAddress, - uint64 dstChainSelector - ) public onlyRoleOrAdminRole(EXECUTOR_ROLE) { + function addToWatchListOrDecomission(address targetAddress, uint64 dstChainSelector) public onlyAdminOrExecutor { if (s_targets[targetAddress].isActive) revert DuplicateAddress(targetAddress); address oldAddress = s_onRampAddresses[dstChainSelector]; // if targetAddress is an existing onRamp, there's a need of cleaning the previous onRamp associated to this dstChainSelector @@ -164,7 +168,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter /// @notice Delete an address from the watchlist and sets the target to inactive /// @param targetAddress the address to be deleted - function removeFromWatchList(address targetAddress) public onlyRoleOrAdminRole(EXECUTOR_ROLE) returns (bool) { + function removeFromWatchList(address targetAddress) public onlyAdminOrExecutor returns (bool) { s_targets[targetAddress].isActive = false; for (uint256 i; i < s_watchList.length; i++) { if (s_watchList[i] == targetAddress) { @@ -214,6 +218,8 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter return targetsToFund; } + /// @notice tries to fund an array of target addresses, checking if they're underfunded in the process + /// @param targetAddresses is an array of contract addresses to be funded in case they're underfunded function topUp(address[] memory targetAddresses) public whenNotPaused { MonitoredAddress memory target; uint256 localBalance = i_linkToken.balanceOf(address(this)); @@ -291,7 +297,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter /// @notice Withdraws the contract balance in the LINK token. /// @param amount the amount of the LINK to withdraw /// @param payee the address to pay - function withdraw(uint256 amount, address payable payee) external onlyRoleOrAdminRole(EXECUTOR_ROLE) { + function withdraw(uint256 amount, address payable payee) external onlyAdminOrExecutor { if (payee == address(0)) revert InvalidAddress(payee); i_linkToken.transfer(payee, amount); emit FundsWithdrawn(amount, payee); @@ -375,12 +381,12 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter return (target.isActive, target.minBalance, target.topUpAmount); } - /// @dev Modifier to make a function callable only by a certain role or the + /// @dev Modifier to make a function callable only by executor role or the /// admin role. - modifier onlyRoleOrAdminRole(bytes32 role) { + modifier onlyAdminOrExecutor() { address sender = _msgSender(); if (!hasRole(ADMIN_ROLE, sender)) { - _checkRole(role, sender); + _checkRole(EXECUTOR_ROLE, sender); } _; } From fde3af632d3848084fd9605bed41e122ff021804 Mon Sep 17 00:00:00 2001 From: Borja Aranda Date: Wed, 20 Dec 2023 17:09:14 +0100 Subject: [PATCH 4/8] add dstChainSelector to setWatchList --- .../upkeeps/LinkAvailableBalanceMonitor.sol | 33 ++++++++++++--- .../LinkAvailableBalanceMonitor.test.ts | 40 ++++++++++++++++--- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol index 754f3553038..5ab3e74d0d7 100644 --- a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol +++ b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol @@ -54,6 +54,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter error InvalidUpkeepInterval(uint8 upkeepInterval); error InvalidLinkTokenAddress(address lt); error InvalidWatchList(); + error InvalidChainSelector(); error DuplicateAddress(address duplicate); struct MonitoredAddress { @@ -73,11 +74,19 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter uint16 private s_maxPerform; uint16 private s_maxCheck; uint8 private s_upkeepInterval; + + /// @notice s_watchList contains all the addresses watched by this monitor + /// It mainly provides the length() function address[] private s_watchList; + + /// @notice s_targets contains all the addresses watched by this monitor + /// Each key points to a MonitoredAddress with all the needed metadata mapping(address targetAddress => MonitoredAddress targetProperties) internal s_targets; /// @notice s_onRampAddresses represents a list of CCIP onRamp addresses watched on this contract /// There has to be only one onRamp per dstChainSelector. + /// dstChainSelector is needed as we have to track the live onRamp, and delete the onRamp + /// whenever a new one is deployed with the same dstChainSelector. mapping(uint64 dstChainSelector => address onRamp) internal s_onRampAddresses; /// @param admin is the administrator address of this contract @@ -112,9 +121,14 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter function setWatchList( address[] calldata addresses, uint96[] calldata minBalances, - uint96[] calldata topUpAmounts + uint96[] calldata topUpAmounts, + uint64[] calldata dstChainSelectors ) external onlyAdminOrExecutor { - if (addresses.length != minBalances.length || addresses.length != topUpAmounts.length) { + if ( + addresses.length != minBalances.length || + addresses.length != topUpAmounts.length || + addresses.length != dstChainSelectors.length + ) { revert InvalidWatchList(); } for (uint256 idx = 0; idx < s_watchList.length; idx++) { @@ -122,8 +136,8 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter } for (uint256 idx = 0; idx < addresses.length; idx++) { address targetAddress = addresses[idx]; - if (s_targets[targetAddress].isActive) revert DuplicateAddress(addresses[idx]); - if (addresses[idx] == address(0)) revert InvalidWatchList(); + if (s_targets[targetAddress].isActive) revert DuplicateAddress(targetAddress); + if (targetAddress == address(0)) revert InvalidWatchList(); if (topUpAmounts[idx] == 0) revert InvalidWatchList(); s_targets[targetAddress] = MonitoredAddress({ isActive: true, @@ -131,6 +145,9 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter topUpAmount: topUpAmounts[idx], lastTopUpTimestamp: 0 }); + if (dstChainSelectors[idx] > 0) { + s_onRampAddresses[dstChainSelectors[idx]] = targetAddress; + } } s_watchList = addresses; emit WatchlistUpdated(); @@ -169,7 +186,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter /// @notice Delete an address from the watchlist and sets the target to inactive /// @param targetAddress the address to be deleted function removeFromWatchList(address targetAddress) public onlyAdminOrExecutor returns (bool) { - s_targets[targetAddress].isActive = false; + delete s_targets[targetAddress]; for (uint256 i; i < s_watchList.length; i++) { if (s_watchList[i] == targetAddress) { s_watchList[i] = s_watchList[s_watchList.length - 1]; @@ -373,6 +390,12 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter return s_watchList; } + /// @notice Gets the onRamp address with the specified dstChainSelector + function getOnRampAddressAtChainSelector(uint64 dstChainSelector) external view returns (address) { + if (dstChainSelector > 0) revert InvalidChainSelector(); + return s_onRampAddresses[dstChainSelector]; + } + /// @notice Gets configuration information for an address on the watchlist function getAccountInfo( address targetAddress diff --git a/contracts/test/v0.8/automation/LinkAvailableBalanceMonitor.test.ts b/contracts/test/v0.8/automation/LinkAvailableBalanceMonitor.test.ts index 1cab0fa010e..2627bee34a3 100644 --- a/contracts/test/v0.8/automation/LinkAvailableBalanceMonitor.test.ts +++ b/contracts/test/v0.8/automation/LinkAvailableBalanceMonitor.test.ts @@ -58,6 +58,7 @@ let directTarget2: MockContract let watchListAddresses: string[] let watchListMinBalances: BigNumber[] let watchListTopUpAmounts: BigNumber[] +let watchListDstChainSelectors: number[] async function assertContractLinkBalances( balance1: BigNumber, @@ -120,6 +121,7 @@ const setup = async () => { ] watchListMinBalances = [oneLINK, oneLINK, oneLINK, twoLINK, twoLINK] watchListTopUpAmounts = [twoLINK, twoLINK, twoLINK, twoLINK, twoLINK] + watchListDstChainSelectors = [1, 2, 3, 4, 5] await proxy1.mock.aggregator.returns(aggregator1.address) await proxy2.mock.aggregator.returns(aggregator2.address) @@ -169,6 +171,7 @@ const setup = async () => { watchListAddresses, watchListMinBalances, watchListTopUpAmounts, + watchListDstChainSelectors, ) await setTx.wait() } @@ -323,7 +326,7 @@ describe('LinkAvailableBalanceMonitor', () => { beforeEach(async () => { // reset watchlist to empty before running these tests - await labm.connect(owner).setWatchList([], [], []) + await labm.connect(owner).setWatchList([], [], [], []) const watchList = await labm.getWatchList() assert.deepEqual(watchList, []) }) @@ -332,7 +335,7 @@ describe('LinkAvailableBalanceMonitor', () => { // add first watchlist let tx = await labm .connect(owner) - .setWatchList([watchAddress1], [oneLINK], [oneLINK]) + .setWatchList([watchAddress1], [oneLINK], [oneLINK], [0]) let watchList = await labm.getWatchList() assert.deepEqual(watchList[0], watchAddress1) // add more to watchlist @@ -342,6 +345,7 @@ describe('LinkAvailableBalanceMonitor', () => { [watchAddress1, watchAddress2, watchAddress3], [oneLINK, oneLINK, oneLINK], [oneLINK, oneLINK, oneLINK], + [1, 2, 3], ) await tx.wait() watchList = await labm.getWatchList() @@ -356,6 +360,7 @@ describe('LinkAvailableBalanceMonitor', () => { [watchAddress1, watchAddress2, watchAddress1], [oneLINK, oneLINK], [oneLINK, oneLINK], + [1, 2], ) await expect(tx).to.be.revertedWith(errMsg) }) @@ -368,6 +373,7 @@ describe('LinkAvailableBalanceMonitor', () => { [watchAddress1, watchAddress2, watchAddress1], [oneLINK, oneLINK, oneLINK], [oneLINK, oneLINK, oneLINK], + [1, 2, 3], ) await expect(tx).to.be.revertedWith(errMsg) }) @@ -375,7 +381,7 @@ describe('LinkAvailableBalanceMonitor', () => { it('Should not allow strangers to set the watchlist', async () => { const setTxStranger = labm .connect(stranger) - .setWatchList([watchAddress1], [oneLINK], [oneLINK]) + .setWatchList([watchAddress1], [oneLINK], [oneLINK], [0]) await expect(setTxStranger).to.be.reverted }) @@ -386,6 +392,7 @@ describe('LinkAvailableBalanceMonitor', () => { [watchAddress1, ethers.constants.AddressZero], [oneLINK, oneLINK], [oneLINK, oneLINK], + [1, 2], ) await expect(tx).to.be.revertedWith(INVALID_WATCHLIST_ERR) }) @@ -535,6 +542,7 @@ describe('LinkAvailableBalanceMonitor', () => { watchListAddresses, watchListMinBalances, watchListTopUpAmounts, + watchListDstChainSelectors, ) const [should, payload] = await labm.checkUpkeep('0x') @@ -560,6 +568,7 @@ describe('LinkAvailableBalanceMonitor', () => { [aggregator2.address, directTarget1.address, directTarget2.address], [oneLINK, twoLINK, twoLINK], [oneLINK, oneLINK, oneLINK], + [1, 2, 3], ) // all of them are underfunded, return 3 @@ -608,6 +617,7 @@ describe('LinkAvailableBalanceMonitor', () => { let minBalances: BigNumber[] let topUpAmount: BigNumber[] let aggregators: MockContract[] + let dstChainSelectors: number[] beforeEach(async () => { MAX_PERFORM = await labm.getMaxPerform() @@ -616,6 +626,7 @@ describe('LinkAvailableBalanceMonitor', () => { minBalances = [] topUpAmount = [] aggregators = [] + dstChainSelectors = [] const numAggregators = MAX_CHECK + 50 for (let idx = 0; idx < numAggregators; idx++) { const proxy = await deployMockContract( @@ -632,8 +643,14 @@ describe('LinkAvailableBalanceMonitor', () => { minBalances.push(oneLINK) topUpAmount.push(oneLINK) aggregators.push(aggregator) + dstChainSelectors.push(0) } - await labm.setWatchList(proxyAddresses, minBalances, topUpAmount) + await labm.setWatchList( + proxyAddresses, + minBalances, + topUpAmount, + dstChainSelectors, + ) let watchlist = await labm.getWatchList() expect(watchlist).to.deep.equalInAnyOrder(proxyAddresses) assert.equal(watchlist.length, minBalances.length) @@ -688,6 +705,7 @@ describe('LinkAvailableBalanceMonitor', () => { watchListAddresses, watchListMinBalances, watchListTopUpAmounts, + watchListDstChainSelectors, ) }) @@ -730,6 +748,7 @@ describe('LinkAvailableBalanceMonitor', () => { const proxyAddresses = [] const minBalances = [] const topUpAmount = [] + const dstChainSelectors = [] for (let idx = 0; idx < MAX_PERFORM; idx++) { const proxy = await deployMockContract( owner, @@ -744,8 +763,14 @@ describe('LinkAvailableBalanceMonitor', () => { proxyAddresses.push(proxy.address) minBalances.push(oneLINK) topUpAmount.push(oneLINK) + dstChainSelectors.push(0) } - await labm.setWatchList(proxyAddresses, minBalances, topUpAmount) + await labm.setWatchList( + proxyAddresses, + minBalances, + topUpAmount, + dstChainSelectors, + ) let watchlist = await labm.getWatchList() expect(watchlist).to.deep.equalInAnyOrder(proxyAddresses) assert.equal(watchlist.length, minBalances.length) @@ -858,6 +883,7 @@ describe('LinkAvailableBalanceMonitor', () => { [proxy1.address, directTarget1.address], [oneLINK, oneLINK], [oneLINK, oneLINK], + [1, 2], ) const tx = await labm .connect(keeperRegistry) @@ -895,6 +921,7 @@ describe('LinkAvailableBalanceMonitor', () => { [proxy1.address, proxy4.address], [oneLINK, oneLINK], [oneLINK, oneLINK], + [1, 2], ) const tx = await labm .connect(keeperRegistry) @@ -913,6 +940,7 @@ describe('LinkAvailableBalanceMonitor', () => { [proxy1.address, proxy4.address], [oneLINK, oneLINK], [oneLINK, oneLINK], + [1, 2], ) const tx = await labm .connect(keeperRegistry) @@ -932,6 +960,7 @@ describe('LinkAvailableBalanceMonitor', () => { [proxy1.address, proxy4.address], [oneLINK, oneLINK], [oneLINK, oneLINK], + [1, 2], ) const tx = await labm .connect(keeperRegistry) @@ -950,6 +979,7 @@ describe('LinkAvailableBalanceMonitor', () => { [proxy1.address, directTarget1.address], [oneLINK, oneLINK], [oneLINK, oneLINK], + [1, 2], ) const tx = await labm .connect(keeperRegistry) From 339e66e6e387d1d67afb4f5b170ea99bad8e88ec Mon Sep 17 00:00:00 2001 From: Borja Aranda Date: Wed, 20 Dec 2023 18:52:37 +0100 Subject: [PATCH 5/8] transform onRampAddresses into an enumerableMap --- .../upkeeps/LinkAvailableBalanceMonitor.sol | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol index 5ab3e74d0d7..bdf3706bddf 100644 --- a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol +++ b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.19; import {AutomationCompatibleInterface} from "../interfaces/AutomationCompatibleInterface.sol"; import {AccessControl} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/access/AccessControl.sol"; +import {EnumerableMap} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/structs/EnumerableMap.sol"; import {IERC20} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/IERC20.sol"; import {Pausable} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/security/Pausable.sol"; @@ -34,6 +35,8 @@ interface ILinkAvailable { /// we could save a fair amount of gas and re-write this upkeep for use with Automation v2.0+, /// which has significantly different trust assumptions contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInterface, Pausable { + using EnumerableMap for EnumerableMap.UintToAddressMap; + event BalanceUpdated(address indexed addr, uint256 oldBalance, uint256 newBalance); event FundsWithdrawn(uint256 amountWithdrawn, address payee); event UpkeepIntervalSet(uint256 oldUpkeepInterval, uint256 newUpkeepInterval); @@ -87,7 +90,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter /// There has to be only one onRamp per dstChainSelector. /// dstChainSelector is needed as we have to track the live onRamp, and delete the onRamp /// whenever a new one is deployed with the same dstChainSelector. - mapping(uint64 dstChainSelector => address onRamp) internal s_onRampAddresses; + EnumerableMap.UintToAddressMap internal s_onRampAddresses; /// @param admin is the administrator address of this contract /// @param linkTokenAddress the LINK token address @@ -134,6 +137,10 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter for (uint256 idx = 0; idx < s_watchList.length; idx++) { delete s_targets[s_watchList[idx]]; } + for (uint256 idx = 0; idx < s_onRampAddresses.length(); idx++) { + (uint256 key, ) = s_onRampAddresses.at(idx); + s_onRampAddresses.remove(key); + } for (uint256 idx = 0; idx < addresses.length; idx++) { address targetAddress = addresses[idx]; if (s_targets[targetAddress].isActive) revert DuplicateAddress(targetAddress); @@ -146,7 +153,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter lastTopUpTimestamp: 0 }); if (dstChainSelectors[idx] > 0) { - s_onRampAddresses[dstChainSelectors[idx]] = targetAddress; + s_onRampAddresses.set(dstChainSelectors[idx], targetAddress); } } s_watchList = addresses; @@ -161,15 +168,16 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter /// in which case it will carry the proper dstChainSelector along with the 0x0 address function addToWatchListOrDecomission(address targetAddress, uint64 dstChainSelector) public onlyAdminOrExecutor { if (s_targets[targetAddress].isActive) revert DuplicateAddress(targetAddress); - address oldAddress = s_onRampAddresses[dstChainSelector]; + bool onRampExists = s_onRampAddresses.contains(dstChainSelector); // if targetAddress is an existing onRamp, there's a need of cleaning the previous onRamp associated to this dstChainSelector // there's no need to remove any other address that's not an onRamp - if (dstChainSelector > 0 && oldAddress != address(0)) { + if (dstChainSelector > 0 && onRampExists) { + address oldAddress = s_onRampAddresses.get(dstChainSelector); removeFromWatchList(oldAddress); } // only add the new address if it's not 0x0 if (targetAddress != address(0)) { - s_onRampAddresses[dstChainSelector] = targetAddress; + s_onRampAddresses.set(dstChainSelector, targetAddress); s_targets[targetAddress] = MonitoredAddress({ isActive: true, minBalance: DEFAULT_MIN_BALANCE_JULES, @@ -179,18 +187,18 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter s_watchList.push(targetAddress); } else { // if the address is 0x0, it means the onRamp has ben decomissioned and has to be cleaned - delete s_onRampAddresses[dstChainSelector]; + s_onRampAddresses.remove(dstChainSelector); } } /// @notice Delete an address from the watchlist and sets the target to inactive /// @param targetAddress the address to be deleted function removeFromWatchList(address targetAddress) public onlyAdminOrExecutor returns (bool) { - delete s_targets[targetAddress]; for (uint256 i; i < s_watchList.length; i++) { if (s_watchList[i] == targetAddress) { s_watchList[i] = s_watchList[s_watchList.length - 1]; s_watchList.pop(); + delete s_targets[targetAddress]; return true; } } @@ -393,7 +401,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter /// @notice Gets the onRamp address with the specified dstChainSelector function getOnRampAddressAtChainSelector(uint64 dstChainSelector) external view returns (address) { if (dstChainSelector > 0) revert InvalidChainSelector(); - return s_onRampAddresses[dstChainSelector]; + return s_onRampAddresses.get(dstChainSelector); } /// @notice Gets configuration information for an address on the watchlist From 794906d67feab19c4b9aad8b856145959e8b2783 Mon Sep 17 00:00:00 2001 From: Borja Aranda Date: Thu, 21 Dec 2023 15:39:15 +0100 Subject: [PATCH 6/8] move watchList to EnumerableSet --- .../upkeeps/LinkAvailableBalanceMonitor.sol | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol index bdf3706bddf..1a2430dd10d 100644 --- a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol +++ b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol @@ -5,6 +5,7 @@ pragma solidity 0.8.19; import {AutomationCompatibleInterface} from "../interfaces/AutomationCompatibleInterface.sol"; import {AccessControl} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/access/AccessControl.sol"; import {EnumerableMap} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/structs/EnumerableMap.sol"; +import {EnumerableSet} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/structs/EnumerableSet.sol"; import {IERC20} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/IERC20.sol"; import {Pausable} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/security/Pausable.sol"; @@ -36,6 +37,7 @@ interface ILinkAvailable { /// which has significantly different trust assumptions contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInterface, Pausable { using EnumerableMap for EnumerableMap.UintToAddressMap; + using EnumerableSet for EnumerableSet.AddressSet; event BalanceUpdated(address indexed addr, uint256 oldBalance, uint256 newBalance); event FundsWithdrawn(uint256 amountWithdrawn, address payee); @@ -80,7 +82,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter /// @notice s_watchList contains all the addresses watched by this monitor /// It mainly provides the length() function - address[] private s_watchList; + EnumerableSet.AddressSet private s_watchList; /// @notice s_targets contains all the addresses watched by this monitor /// Each key points to a MonitoredAddress with all the needed metadata @@ -134,9 +136,13 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter ) { revert InvalidWatchList(); } - for (uint256 idx = 0; idx < s_watchList.length; idx++) { - delete s_targets[s_watchList[idx]]; + for (uint256 idx = s_watchList.length(); idx > 0; idx--) { + address member = s_watchList.at(idx - 1); + s_watchList.remove(member); + delete s_targets[member]; } + // s_onRampAddresses is not the same length as s_watchList, so it has + // to be clean in a separate loop for (uint256 idx = 0; idx < s_onRampAddresses.length(); idx++) { (uint256 key, ) = s_onRampAddresses.at(idx); s_onRampAddresses.remove(key); @@ -155,8 +161,8 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter if (dstChainSelectors[idx] > 0) { s_onRampAddresses.set(dstChainSelectors[idx], targetAddress); } + s_watchList.add(targetAddress); } - s_watchList = addresses; emit WatchlistUpdated(); } @@ -184,7 +190,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter topUpAmount: DEFAULT_TOP_UP_AMOUNT_JULES, lastTopUpTimestamp: 0 }); - s_watchList.push(targetAddress); + s_watchList.add(targetAddress); } else { // if the address is 0x0, it means the onRamp has ben decomissioned and has to be cleaned s_onRampAddresses.remove(dstChainSelector); @@ -194,13 +200,9 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter /// @notice Delete an address from the watchlist and sets the target to inactive /// @param targetAddress the address to be deleted function removeFromWatchList(address targetAddress) public onlyAdminOrExecutor returns (bool) { - for (uint256 i; i < s_watchList.length; i++) { - if (s_watchList[i] == targetAddress) { - s_watchList[i] = s_watchList[s_watchList.length - 1]; - s_watchList.pop(); - delete s_targets[targetAddress]; - return true; - } + if (s_watchList.remove(targetAddress)) { + delete s_targets[targetAddress]; + return true; } return false; } @@ -214,7 +216,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter function sampleUnderfundedAddresses() public view returns (address[] memory) { uint16 maxPerform = s_maxPerform; uint16 maxCheck = s_maxCheck; - uint256 numTargets = s_watchList.length; + uint256 numTargets = s_watchList.length(); uint256 idx = uint256(blockhash(block.number - (block.number % s_upkeepInterval) - 1)) % numTargets; uint256 numToCheck = numTargets < maxCheck ? numTargets : maxCheck; uint256 numFound = 0; @@ -225,7 +227,7 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter numChecked < numToCheck; (idx, numChecked) = ((idx + 1) % numTargets, numChecked + 1) ) { - address targetAddress = s_watchList[idx]; + address targetAddress = s_watchList.at(idx); target = s_targets[targetAddress]; if (_needsFunding(targetAddress, target.minBalance)) { targetsToFund[numFound] = targetAddress; @@ -395,12 +397,12 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter /// @notice Gets the list of subscription ids being watched function getWatchList() external view returns (address[] memory) { - return s_watchList; + return s_watchList.values(); } /// @notice Gets the onRamp address with the specified dstChainSelector function getOnRampAddressAtChainSelector(uint64 dstChainSelector) external view returns (address) { - if (dstChainSelector > 0) revert InvalidChainSelector(); + if (dstChainSelector == 0) revert InvalidChainSelector(); return s_onRampAddresses.get(dstChainSelector); } From 6086c82204ae7fcd211df32a18be6d9786541efa Mon Sep 17 00:00:00 2001 From: Borja Aranda Date: Wed, 10 Jan 2024 17:28:59 +0100 Subject: [PATCH 7/8] moving vars from internal to private --- .../upkeeps/LinkAvailableBalanceMonitor.sol | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol index 1a2430dd10d..b1836b333b9 100644 --- a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol +++ b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol @@ -69,8 +69,8 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter bool isActive; } - bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE"); - bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE"); + bytes32 private constant ADMIN_ROLE = keccak256("ADMIN_ROLE"); + bytes32 private constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE"); uint96 private constant DEFAULT_TOP_UP_AMOUNT_JULES = 9000000000000000000; uint96 private constant DEFAULT_MIN_BALANCE_JULES = 1000000000000000000; IERC20 private immutable i_linkToken; @@ -80,39 +80,38 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter uint16 private s_maxCheck; uint8 private s_upkeepInterval; - /// @notice s_watchList contains all the addresses watched by this monitor + /// @dev s_watchList contains all the addresses watched by this monitor /// It mainly provides the length() function EnumerableSet.AddressSet private s_watchList; /// @notice s_targets contains all the addresses watched by this monitor /// Each key points to a MonitoredAddress with all the needed metadata - mapping(address targetAddress => MonitoredAddress targetProperties) internal s_targets; + mapping(address targetAddress => MonitoredAddress targetProperties) private s_targets; /// @notice s_onRampAddresses represents a list of CCIP onRamp addresses watched on this contract /// There has to be only one onRamp per dstChainSelector. /// dstChainSelector is needed as we have to track the live onRamp, and delete the onRamp /// whenever a new one is deployed with the same dstChainSelector. - EnumerableMap.UintToAddressMap internal s_onRampAddresses; + EnumerableMap.UintToAddressMap private s_onRampAddresses; /// @param admin is the administrator address of this contract - /// @param linkTokenAddress the LINK token address + /// @param linkToken the LINK token address /// @param minWaitPeriodSeconds represents the amount of time that has to wait a contract to be funded /// @param maxPerform maximum amount of contracts to fund /// @param maxCheck maximum amount of contracts to check /// @param upkeepInterval randomizes the check for underfunded contracts constructor( address admin, - address linkTokenAddress, + IERC20 linkToken, uint256 minWaitPeriodSeconds, uint16 maxPerform, uint16 maxCheck, uint8 upkeepInterval ) { - if (linkTokenAddress == address(0)) revert InvalidLinkTokenAddress(linkTokenAddress); _setRoleAdmin(ADMIN_ROLE, ADMIN_ROLE); _setRoleAdmin(EXECUTOR_ROLE, ADMIN_ROLE); _grantRole(ADMIN_ROLE, admin); - i_linkToken = IERC20(linkTokenAddress); + i_linkToken = linkToken; setMinWaitPeriodSeconds(minWaitPeriodSeconds); setMaxPerform(maxPerform); setMaxCheck(maxCheck); From 8769faaa19997def8b0a73372167974a1437adb0 Mon Sep 17 00:00:00 2001 From: Borja Aranda Date: Wed, 10 Jan 2024 18:16:05 +0100 Subject: [PATCH 8/8] fix natspec --- .../v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol index b1836b333b9..b858800d73a 100644 --- a/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol +++ b/contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol @@ -80,8 +80,8 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter uint16 private s_maxCheck; uint8 private s_upkeepInterval; - /// @dev s_watchList contains all the addresses watched by this monitor - /// It mainly provides the length() function + /// @notice s_watchList contains all the addresses watched by this monitor + /// @dev It mainly provides the length() function EnumerableSet.AddressSet private s_watchList; /// @notice s_targets contains all the addresses watched by this monitor