Enumberable set cleanup (#12885)

* support decimals * address comments * properly cleanup the enumberable set * add unit test and remove in reverse order
smartcontractkit · Apr 23, 2024 · 394844e · 394844e
1 parent 8efaa2f
commit 394844e
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 18 deletions.
diff --git a/.changeset/sour-needles-collect.md b/.changeset/sour-needles-collect.md
@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+cleanup set #bugfix
diff --git a/contracts/.changeset/hungry-days-occur.md b/contracts/.changeset/hungry-days-occur.md
@@ -0,0 +1,5 @@
+---
+"@chainlink/contracts": patch
+---
+
+set cleanup #bugfix
diff --git a/contracts/src/v0.8/automation/dev/test/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/dev/test/AutomationRegistry2_3.t.sol
@@ -5,7 +5,7 @@ import {Vm} from "forge-std/Test.sol";
 import {BaseTest} from "./BaseTest.t.sol";
 import {AutomationRegistryBase2_3 as AutoBase} from "../v2_3/AutomationRegistryBase2_3.sol";
 import {AutomationRegistrar2_3 as Registrar} from "../v2_3/AutomationRegistrar2_3.sol";
-import {IAutomationRegistryMaster2_3 as Registry, AutomationRegistryBase2_3} from "../interfaces/v2_3/IAutomationRegistryMaster2_3.sol";
+import {IAutomationRegistryMaster2_3 as Registry, AutomationRegistryBase2_3, IAutomationV21PlusCommon} from "../interfaces/v2_3/IAutomationRegistryMaster2_3.sol";
 import {ChainModuleBase} from "../../chains/ChainModuleBase.sol";
 import {IERC20Metadata as IERC20} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/extensions/IERC20Metadata.sol";
 import {IWrappedNative} from "../interfaces/v2_3/IWrappedNative.sol";
@@ -334,6 +334,7 @@ contract SetConfig is SetUp {
   );
 
   address module = address(new ChainModuleBase());
+
   AutomationRegistryBase2_3.OnchainConfig cfg =
     AutomationRegistryBase2_3.OnchainConfig({
       checkGasLimit: 5_000_000,
@@ -347,7 +348,7 @@ contract SetConfig is SetUp {
       fallbackLinkPrice: 2_000_000_000, // $20
       fallbackNativePrice: 400_000_000_000, // $4,000
       transcoder: 0xB1e66855FD67f6e85F0f0fA38cd6fBABdf00923c,
-      registrars: new address[](0),
+      registrars: _getRegistrars(),
       upkeepPrivilegeManager: PRIVILEGE_MANAGER,
       chainModule: module,
       reorgProtectionEnabled: true,
@@ -507,8 +508,22 @@ contract SetConfig is SetUp {
       decimals: 18
     });
 
+    // the first time uses the default onchain config with 2 registrars
     bytes memory onchainConfigBytesWithBilling1 = abi.encode(cfg, billingTokens1, billingConfigs1);
 
+    // set config once
+    registry.setConfig(
+      SIGNERS,
+      TRANSMITTERS,
+      F,
+      onchainConfigBytesWithBilling1,
+      OFFCHAIN_CONFIG_VERSION,
+      offchainConfigBytes
+    );
+
+    (, IAutomationV21PlusCommon.OnchainConfigLegacy memory onchainConfig1, , , ) = registry.getState();
+    assertEq(onchainConfig1.registrars.length, 2);
+
     // BillingConfig2
     address billingTokenAddress2 = address(usdToken18);
     address[] memory billingTokens2 = new address[](1);
@@ -524,17 +539,33 @@ contract SetConfig is SetUp {
       decimals: 18
     });
 
-    bytes memory onchainConfigBytesWithBilling2 = abi.encode(cfg, billingTokens2, billingConfigs2);
+    address[] memory newRegistrars = new address[](3);
+    newRegistrars[0] = address(uint160(uint256(keccak256("newRegistrar1"))));
+    newRegistrars[1] = address(uint160(uint256(keccak256("newRegistrar2"))));
+    newRegistrars[2] = address(uint160(uint256(keccak256("newRegistrar3"))));
 
-    // set config once
-    registry.setConfig(
-      SIGNERS,
-      TRANSMITTERS,
-      F,
-      onchainConfigBytesWithBilling1,
-      OFFCHAIN_CONFIG_VERSION,
-      offchainConfigBytes
-    );
+    // new onchain config with 3 new registrars, all other fields stay the same as the default
+    AutomationRegistryBase2_3.OnchainConfig memory cfg2 = AutomationRegistryBase2_3.OnchainConfig({
+      checkGasLimit: 5_000_000,
+      stalenessSeconds: 90_000,
+      gasCeilingMultiplier: 0,
+      maxPerformGas: 10_000_000,
+      maxCheckDataSize: 5_000,
+      maxPerformDataSize: 5_000,
+      maxRevertDataSize: 5_000,
+      fallbackGasPrice: 20_000_000_000,
+      fallbackLinkPrice: 2_000_000_000, // $20
+      fallbackNativePrice: 400_000_000_000, // $4,000
+      transcoder: 0xB1e66855FD67f6e85F0f0fA38cd6fBABdf00923c,
+      registrars: newRegistrars,
+      upkeepPrivilegeManager: PRIVILEGE_MANAGER,
+      chainModule: module,
+      reorgProtectionEnabled: true,
+      financeAdmin: FINANCE_ADMIN
+    });
+
+    // the second time uses the new onchain config with 3 new registrars and also new billing tokens/configs
+    bytes memory onchainConfigBytesWithBilling2 = abi.encode(cfg2, billingTokens2, billingConfigs2);
 
     // set config twice
     registry.setConfig(
@@ -546,8 +577,18 @@ contract SetConfig is SetUp {
       offchainConfigBytes
     );
 
-    (, , address[] memory signers, address[] memory transmitters, uint8 f) = registry.getState();
-
+    (
+      ,
+      IAutomationV21PlusCommon.OnchainConfigLegacy memory onchainConfig2,
+      address[] memory signers,
+      address[] memory transmitters,
+      uint8 f
+    ) = registry.getState();
+
+    assertEq(onchainConfig2.registrars.length, 3);
+    for (uint256 i = 0; i < newRegistrars.length; i++) {
+      assertEq(newRegistrars[i], onchainConfig2.registrars[i]);
+    }
     assertEq(signers, SIGNERS);
     assertEq(transmitters, TRANSMITTERS);
     assertEq(f, F);
@@ -766,6 +807,13 @@ contract SetConfig is SetUp {
     assertEq(transmitters, NEW_TRANSMITTERS);
   }
 
+  function _getRegistrars() private pure returns (address[] memory) {
+    address[] memory registrars = new address[](2);
+    registrars[0] = address(uint160(uint256(keccak256("registrar1"))));
+    registrars[1] = address(uint160(uint256(keccak256("registrar2"))));
+    return registrars;
+  }
+
   function _configDigestFromConfigData(
     uint256 chainId,
     address contractAddress,

diff --git a/contracts/src/v0.8/automation/dev/v2_3/AutomationRegistry2_3.sol b/contracts/src/v0.8/automation/dev/v2_3/AutomationRegistry2_3.sol
@@ -355,7 +355,9 @@ contract AutomationRegistry2_3 is AutomationRegistryBase2_3, OCR2Abstract, Chain
       offchainConfig
     );
 
-    delete s_registrars;
+    for (uint256 idx = s_registrars.length(); idx > 0; idx--) {
+      s_registrars.remove(s_registrars.at(idx - 1));
+    }
 
     for (uint256 idx = 0; idx < onchainConfig.registrars.length; idx++) {
       s_registrars.add(onchainConfig.registrars[idx]);

diff --git a/contracts/src/v0.8/automation/dev/v2_3/AutomationRegistryLogicC2_3.sol b/contracts/src/v0.8/automation/dev/v2_3/AutomationRegistryLogicC2_3.sol
@@ -184,7 +184,10 @@ contract AutomationRegistryLogicC2_3 is AutomationRegistryBase2_3 {
       payments[i + activeTransmittersLength] = transmitter.balance;
       s_transmitters[deactivatedAddr].balance = 0;
     }
-    delete s_deactivatedTransmitters;
+
+    for (uint256 idx = s_deactivatedTransmitters.length(); idx > 0; idx--) {
+      s_deactivatedTransmitters.remove(s_deactivatedTransmitters.at(idx - 1));
+    }
 
     emit NOPsSettledOffchain(payees, payments);
   }

diff --git a/...gethwrappers/generated/automation_registry_wrapper_2_3/automation_registry_wrapper_2_3.go b/...gethwrappers/generated/automation_registry_wrapper_2_3/automation_registry_wrapper_2_3.go
diff --git a/core/gethwrappers/generation/generated-wrapper-dependency-versions-do-not-edit.txt b/core/gethwrappers/generation/generated-wrapper-dependency-versions-do-not-edit.txt
@@ -14,7 +14,7 @@ automation_registry_logic_a_wrapper_2_3: ../../contracts/solc/v0.8.19/Automation
 automation_registry_logic_b_wrapper_2_2: ../../contracts/solc/v0.8.19/AutomationRegistryLogicB2_2/AutomationRegistryLogicB2_2.abi ../../contracts/solc/v0.8.19/AutomationRegistryLogicB2_2/AutomationRegistryLogicB2_2.bin a6d33dfbbfb0ff253eb59a51f4f6d6d4c22ea5ec95aae52d25d49a312b37a22f
 automation_registry_logic_b_wrapper_2_3: ../../contracts/solc/v0.8.19/AutomationRegistryLogicB2_3/AutomationRegistryLogicB2_3.abi ../../contracts/solc/v0.8.19/AutomationRegistryLogicB2_3/AutomationRegistryLogicB2_3.bin 2d0f45d2087f6f3c8bfa0a16b26a1c8c1d5c64b89859478c609201535c96eeed
 automation_registry_wrapper_2_2: ../../contracts/solc/v0.8.19/AutomationRegistry2_2/AutomationRegistry2_2.abi ../../contracts/solc/v0.8.19/AutomationRegistry2_2/AutomationRegistry2_2.bin de60f69878e9b32a291a001c91fc8636544c2cfbd9b507c8c1a4873b602bfb62
-automation_registry_wrapper_2_3: ../../contracts/solc/v0.8.19/AutomationRegistry2_3/AutomationRegistry2_3.abi ../../contracts/solc/v0.8.19/AutomationRegistry2_3/AutomationRegistry2_3.bin 2f9db5da86183eaf4f78f726458e5a928d37f7c90c4024923847b25186b644c5
+automation_registry_wrapper_2_3: ../../contracts/solc/v0.8.19/AutomationRegistry2_3/AutomationRegistry2_3.abi ../../contracts/solc/v0.8.19/AutomationRegistry2_3/AutomationRegistry2_3.bin 68acf720890c89512a43c3468712f5da6c1c9613bc7c462b51599293f0cce9f4
 automation_utils_2_1: ../../contracts/solc/v0.8.16/AutomationUtils2_1/AutomationUtils2_1.abi ../../contracts/solc/v0.8.16/AutomationUtils2_1/AutomationUtils2_1.bin 815b17b63f15d26a0274b962eefad98cdee4ec897ead58688bbb8e2470e585f5
 automation_utils_2_2: ../../contracts/solc/v0.8.19/AutomationUtils2_2/AutomationUtils2_2.abi ../../contracts/solc/v0.8.19/AutomationUtils2_2/AutomationUtils2_2.bin 8743f6231aaefa3f2a0b2d484258070d506e2d0860690e66890dccc3949edb2e
 automation_utils_2_3: ../../contracts/solc/v0.8.19/AutomationUtils2_3/AutomationUtils2_3.abi ../../contracts/solc/v0.8.19/AutomationUtils2_3/AutomationUtils2_3.bin 11e2b481dc9a4d936e3443345d45d2cc571164459d214917b42a8054b295393b