LinkBalMonV2(minPerform) and CCIPWatchlist AutoUpdate Contract #13605

poke1994 · 2024-06-18T13:22:52Z

Adding 2 files.

I added a minPerform() function to the LinkBalMonV2 contract and tested it on Sepolia. The purpose of the minPerform function is to establish a minimum batch size required for the sampleUnderfunded address array. If the array size is less than minPerform, no top-up operation will be performed. This optimization is crucial for deploying the BalanceMonitor contract on Ethereum, which consumes significant gas.
Creation of a CCIPOnRampWatchlistUpdate contract to allow auto update of the watchlist on the LinkBalMonV2 when deployed to fund CCIP On-Ramps

Testing File Link - https://docs.google.com/spreadsheets/d/1zk4J4xQrhSJhIEt7l1bpsG6_jrl6qTQLNa5EZia3JzU/edit?gid=829494086#gid=829494086 Specification File - https://docs.google.com/document/d/1p4naJOlrdPmZ5xsZkZE7FDw1cEQqWcQ3hgguhI21SNQ/edit#heading=h.3t9vh460y192

Ticket

Requires Dependencies

Resolves Dependencies

Adding 2 files. 1. I added a minPerform() function to the LinkBalMonV2 contract and tested it on Sepolia. The purpose of the minPerform function is to establish a minimum batch size required for the sampleUnderfunded address array. If the array size is less than minPerform, no top-up operation will be performed. This optimization is crucial for deploying the BalanceMonitor contract on Ethereum, which consumes significant gas. 2. Creation of a CCIPOnRampWatchlistUpdate contract to allow auto update of the watchlist on the LinkBalMonV2 when deployed to fund CCIP On-Ramps Testing File Link - https://docs.google.com/spreadsheets/d/1zk4J4xQrhSJhIEt7l1bpsG6_jrl6qTQLNa5EZia3JzU/edit?gid=829494086#gid=829494086 Specification File - https://docs.google.com/document/d/1p4naJOlrdPmZ5xsZkZE7FDw1cEQqWcQ3hgguhI21SNQ/edit#heading=h.3t9vh460y192

github-actions · 2024-06-18T13:23:10Z

I see you updated files related to contracts. Please run pnpm changeset in the contracts directory to add a changeset.

FelixFan1992 · 2024-06-19T14:39:54Z

contracts/src/v0.8/automation/upkeeps/CCIPAutoWatchlist.sol

+import {LinkAvailableBalanceMonitor} from "../upkeeps/LinkAvailableBalanceMonitor.sol";
+
+
+struct Log {


can you import this and ILogAutomation from the existing ILogAutomation.sol interface?

FelixFan1992 · 2024-06-19T14:41:06Z

contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol

@@ -333,7 +337,8 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter
    bytes calldata
  ) external view override whenNotPaused returns (bool upkeepNeeded, bytes memory performData) {
    address[] memory needsFunding = sampleUnderfundedAddresses();
-    if (needsFunding.length == 0) {
+    uint16 minPerform = s_minPerform;
+    if (needsFunding.length <= minPerform) {


nit: it's probably more idiomatic to do <

FelixFan1992 · 2024-06-19T14:41:46Z

contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol

@@ -333,7 +337,8 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter
    bytes calldata
  ) external view override whenNotPaused returns (bool upkeepNeeded, bytes memory performData) {
    address[] memory needsFunding = sampleUnderfundedAddresses();
-    if (needsFunding.length == 0) {
+    uint16 minPerform = s_minPerform;


no need to create a new variable unless it will be read again in this function

FelixFan1992 · 2024-06-19T14:44:25Z

contracts/src/v0.8/automation/upkeeps/CCIPAutoWatchlist.sol

+    require(routerAddress != address(0), "Router address not set");
+
+    // Define the event signature for OnRampSet(uint64,address)
+    bytes32 eventSignature = keccak256(abi.encodePacked("OnRampSet(uint64,address)"));


since this event sig is known in advance, u can encode it and set it as a constant

FelixFan1992 · 2024-06-19T14:45:10Z

contracts/src/v0.8/automation/upkeeps/CCIPAutoWatchlist.sol

+    function checkLog(
+    Log calldata log,
+    bytes memory checkData
+) external view override returns (bool upkeepNeeded, bytes memory performData) {


you can try to run
pnpm prettier:write to format all the code

FelixFan1992 · 2024-06-19T14:46:58Z

contracts/src/v0.8/automation/upkeeps/CCIPAutoWatchlist.sol

+
+    event setWatchlistOnMonitor(uint64 indexed _dstChainSelector,address indexed _onRamp);
+
+    LinkAvailableBalanceMonitor public LinkAvailableBalanceMonitorAddress;


nit: variable starts with lowercase and it should be called linkAvailableBalanceMonitor or monitor bc it's not an address anymore

FelixFan1992 · 2024-06-19T14:50:16Z

contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol

@@ -389,6 +394,12 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter
    s_maxPerform = maxPerform;
  }

+  /// @notice Update s_minPerform
+  function setMinPerform(uint16 minPerform) public onlyRole(ADMIN_ROLE) {
+    emit MinPerformSet(s_minPerform, minPerform);


nit: emit after the variable is set

FelixFan1992 · 2024-06-19T14:53:35Z

contracts/src/v0.8/automation/upkeeps/CCIPAutoWatchlist.sol

+    if (log.source == routerAddress && log.topics.length > 0 && log.topics[0] == eventSignature) {
+        // Extract the indexed parameter from the log
+        uint64 destChainSelector = uint64(uint256(log.topics[1])); // cast to uint64
+        address onRamp = address(uint160(uint256(bytes32(log.data)))); // extract from log.data


depending on the possible values of destChainSelector and onRamp, maybe add some sanity checks for these values?

FelixFan1992 · 2024-06-19T14:54:21Z

contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol

@@ -389,6 +394,12 @@ contract LinkAvailableBalanceMonitor is AccessControl, AutomationCompatibleInter
    s_maxPerform = maxPerform;
  }

+  /// @notice Update s_minPerform
+  function setMinPerform(uint16 minPerform) public onlyRole(ADMIN_ROLE) {


this can be external

Made changed as per Felix's comments

poke1994

Made changes to CCIPAutoWatchlist contract and LinkAvailableBalanceMonitor contract

contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol

poke1994

Made changes to LinkBalanceMonitorContract as suggested

FelixFan1992 · 2024-06-23T21:13:21Z

contracts/src/v0.8/automation/upkeeps/CCIPAutoWatchlist.sol

@@ -6,89 +6,67 @@ import "../../shared/access/ConfirmedOwner.sol";
 import {AutomationCompatibleInterface} from "../interfaces/AutomationCompatibleInterface.sol";
 import {AccessControl} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/access/AccessControl.sol";
 import {LinkAvailableBalanceMonitor} from "../upkeeps/LinkAvailableBalanceMonitor.sol";
+import "../interfaces/ILogAutomation.sol";


let's import specific struct / interface

contracts/src/v0.8/automation/upkeeps/CCIPAutoWatchlist.sol

Address comments to the updateWatchlist function of the code Imported specific parts of ILogAutomation

cl-sonarqube-production · 2024-06-25T15:33:31Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

RyanRHall

Sorry, but I can't sign off on these without unit tests! From your testing file it looks like you have a good understanding of what needs coverage, but those test cases need to manifest in code :)

chainchad · 2024-11-08T14:17:09Z

Going to close this for now as it seems to have gotten stale. Please re-open when ready.

poke1994 requested a review from a team as a code owner June 18, 2024 13:22

poke1994 temporarily deployed to sdlc June 18, 2024 13:22 — with GitHub Actions Inactive

Update LinkAvailableBalanceMonitor.sol

57ac523

poke1994 temporarily deployed to sdlc June 18, 2024 13:23 — with GitHub Actions Inactive

Update LinkAvailableBalanceMonitor.sol

85fcb70

poke1994 temporarily deployed to sdlc June 18, 2024 13:27 — with GitHub Actions Inactive

poke1994 assigned FelixFan1992 Jun 18, 2024

poke1994 changed the title ~~Add files via upload~~ LinkBalMonV2(minPerform) and CCIPWatchlist AutoUpdate Contract Jun 18, 2024

FelixFan1992 reviewed Jun 19, 2024

View reviewed changes

FelixFan1992 requested a review from RyanRHall June 19, 2024 15:01

Update LinkAvailableBalanceMonitor.sol

0b034e3

Made changed as per Felix's comments

poke1994 temporarily deployed to sdlc June 19, 2024 18:53 — with GitHub Actions Inactive

Update CCIPAutoWatchlist.sol

d2f2a63

poke1994 temporarily deployed to sdlc June 21, 2024 16:34 — with GitHub Actions Inactive

poke1994 commented Jun 21, 2024

View reviewed changes

contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol Show resolved Hide resolved

contracts/src/v0.8/automation/upkeeps/LinkAvailableBalanceMonitor.sol Show resolved Hide resolved

poke1994 commented Jun 21, 2024

View reviewed changes

FelixFan1992 reviewed Jun 23, 2024

View reviewed changes

contracts/src/v0.8/automation/upkeeps/CCIPAutoWatchlist.sol Show resolved Hide resolved

Update CCIPAutoWatchlist.sol

95f46c9

Address comments to the updateWatchlist function of the code Imported specific parts of ILogAutomation

poke1994 temporarily deployed to sdlc June 25, 2024 15:18 — with GitHub Actions Inactive

FelixFan1992 approved these changes Jun 26, 2024

View reviewed changes

RyanRHall suggested changes Jun 26, 2024

View reviewed changes

chainchad closed this Nov 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LinkBalMonV2(minPerform) and CCIPWatchlist AutoUpdate Contract #13605

LinkBalMonV2(minPerform) and CCIPWatchlist AutoUpdate Contract #13605

poke1994 commented Jun 18, 2024

github-actions bot commented Jun 18, 2024

FelixFan1992 Jun 19, 2024

FelixFan1992 Jun 19, 2024

FelixFan1992 Jun 19, 2024

FelixFan1992 Jun 19, 2024

FelixFan1992 Jun 19, 2024

FelixFan1992 Jun 19, 2024

FelixFan1992 Jun 19, 2024

FelixFan1992 Jun 19, 2024

FelixFan1992 Jun 19, 2024

poke1994 left a comment

poke1994 left a comment

FelixFan1992 Jun 23, 2024

cl-sonarqube-production bot commented Jun 25, 2024

RyanRHall left a comment

chainchad commented Nov 8, 2024

		import {LinkAvailableBalanceMonitor} from "../upkeeps/LinkAvailableBalanceMonitor.sol";


		struct Log {


		event setWatchlistOnMonitor(uint64 indexed _dstChainSelector,address indexed _onRamp);

		LinkAvailableBalanceMonitor public LinkAvailableBalanceMonitorAddress;

LinkBalMonV2(minPerform) and CCIPWatchlist AutoUpdate Contract #13605

LinkBalMonV2(minPerform) and CCIPWatchlist AutoUpdate Contract #13605

Conversation

poke1994 commented Jun 18, 2024

Ticket

Requires Dependencies

Resolves Dependencies

github-actions bot commented Jun 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poke1994 left a comment

Choose a reason for hiding this comment

poke1994 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cl-sonarqube-production bot commented Jun 25, 2024

Quality Gate passed

RyanRHall left a comment

Choose a reason for hiding this comment

chainchad commented Nov 8, 2024