Operator Forwarder Production Readiness (#12983)

* move oepratorforwarder out of dev/ and add more test cases

* Update test locations

* Update gas snapshot

* Update solc compile

* Changeset

* Prettier linting + hardhat test updates

* Interface naming and test updates

* Update Deployer.sol => Deployer.t.sol

* Rework OperatorForwarder tests

* Update gas snapshot

* prettier

* Fix Hardhat tests

* More style maintenance

* Update constant variables in Deployer.t.sol

* Update gas snapshot

.github/workflows/solidity-foundry.yml

@@ -31,7 +31,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        product: [vrf, automation, llo-feeds, l2ep, functions, keystone, shared]
+        product: [automation, functions, keystone, l2ep, llo-feeds, operatorforwarder, shared, vrf]
     needs: [changes]
     name: Foundry Tests ${{ matrix.product }}
     # See https://github.com/foundry-rs/foundry/issues/3827

contracts/.changeset/small-paws-crash.md

@@ -0,0 +1,5 @@
+---
+"@chainlink/contracts": patch
+---
+
+Update operatorforwarder tests and pull out of dev/

contracts/GNUmakefile

@@ -1,6 +1,6 @@
 # ALL_FOUNDRY_PRODUCTS contains a list of all products that have a foundry
 # profile defined and use the Foundry snapshots. 
-ALL_FOUNDRY_PRODUCTS = l2ep llo-feeds functions keystone shared transmission
+ALL_FOUNDRY_PRODUCTS = functions keystone l2ep llo-feeds operatorforwarder shared transmission
 
 # To make a snapshot for a specific product, either set the `FOUNDRY_PROFILE` env var
 # or call the target with `FOUNDRY_PROFILE=product`

contracts/gas-snapshots/operatorforwarder.gas-snapshot

@@ -1,2 +1,21 @@
-Operator_cancelRequest:test_Success(uint96) (runs: 256, μ: 306103, ~: 306096)
-Operator_cancelRequest:test_afterSuccessfulRequestSucess(uint96) (runs: 256, μ: 384781, ~: 389554)
+FactoryTest:test_DeployNewForwarderAndTransferOwnership_Success() (gas: 1059722)
+FactoryTest:test_DeployNewForwarder_Success() (gas: 1048209)
+FactoryTest:test_DeployNewOperatorAndForwarder_Success() (gas: 4069305)
+FactoryTest:test_DeployNewOperator_Success() (gas: 3020464)
+ForwarderTest:test_Forward_Success(uint256) (runs: 256, μ: 226200, ~: 227289)
+ForwarderTest:test_MultiForward_Success(uint256,uint256) (runs: 256, μ: 257876, ~: 259120)
+ForwarderTest:test_OwnerForward_Success() (gas: 30118)
+ForwarderTest:test_SetAuthorizedSenders_Success() (gas: 160524)
+ForwarderTest:test_TransferOwnershipWithMessage_Success() (gas: 35123)
+OperatorTest:test_CancelOracleRequest_Success() (gas: 274436)
+OperatorTest:test_CancelOracleRequest_Success() (gas: 274436)
+OperatorTest:test_FulfillOracleRequest_Success() (gas: 330603)
+OperatorTest:test_FulfillOracleRequest_Success() (gas: 330603)
+OperatorTest:test_NotAuthorizedSender_Revert() (gas: 246716)
+OperatorTest:test_NotAuthorizedSender_Revert() (gas: 246716)
+OperatorTest:test_OracleRequest_Success() (gas: 250019)
+OperatorTest:test_OracleRequest_Success() (gas: 250019)
+OperatorTest:test_SendRequestAndCancelRequest_Success(uint96) (runs: 256, μ: 387120, ~: 387124)
+OperatorTest:test_SendRequestAndCancelRequest_Success(uint96) (runs: 256, μ: 387120, ~: 387124)
+OperatorTest:test_SendRequest_Success(uint96) (runs: 256, μ: 303611, ~: 303620)
+OperatorTest:test_SendRequest_Success(uint96) (runs: 256, μ: 303611, ~: 303620)

contracts/scripts/native_solc_compile_all_operatorforwarder

@@ -28,8 +28,9 @@ compileContract () {
 }
 
 # Contracts
-compileContract operatorforwarder/dev/AuthorizedForwarder.sol
-compileContract operatorforwarder/dev/AuthorizedReceiver.sol
-compileContract operatorforwarder/dev/Operator.sol
-compileContract operatorforwarder/dev/OperatorFactory.sol
+compileContract operatorforwarder/AuthorizedForwarder.sol
+compileContract operatorforwarder/AuthorizedReceiver.sol
+compileContract operatorforwarder/LinkTokenReceiver.sol
+compileContract operatorforwarder/Operator.sol
+compileContract operatorforwarder/OperatorFactory.sol
 

contracts/src/v0.8/operatorforwarder/dev/AuthorizedForwarder.sol → contracts/src/v0.8/operatorforwarder/AuthorizedForwarder.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.19;
 
-import {ConfirmedOwnerWithProposal} from "../../shared/access/ConfirmedOwnerWithProposal.sol";
+import {ConfirmedOwnerWithProposal} from "../shared/access/ConfirmedOwnerWithProposal.sol";
 import {AuthorizedReceiver} from "./AuthorizedReceiver.sol";
 import {Address} from "@openzeppelin/contracts/utils/Address.sol";
 

contracts/src/v0.8/operatorforwarder/dev/AuthorizedReceiver.sol → contracts/src/v0.8/operatorforwarder/AuthorizedReceiver.sol

@@ -1,10 +1,10 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.19;
 
-import {AuthorizedReceiverInterface} from "./interfaces/AuthorizedReceiverInterface.sol";
+import {IAuthorizedReceiver} from "./interfaces/IAuthorizedReceiver.sol";
 
 // solhint-disable gas-custom-errors
-abstract contract AuthorizedReceiver is AuthorizedReceiverInterface {
+abstract contract AuthorizedReceiver is IAuthorizedReceiver {
   mapping(address sender => bool authorized) private s_authorizedSenders;
   address[] private s_authorizedSenderList;
 

contracts/src/v0.8/operatorforwarder/dev/Operator.sol → contracts/src/v0.8/operatorforwarder/Operator.sol

@@ -3,19 +3,19 @@ pragma solidity 0.8.19;
 
 import {AuthorizedReceiver} from "./AuthorizedReceiver.sol";
 import {LinkTokenReceiver} from "./LinkTokenReceiver.sol";
-import {ConfirmedOwner} from "../../shared/access/ConfirmedOwner.sol";
-import {LinkTokenInterface} from "../../shared/interfaces/LinkTokenInterface.sol";
-import {AuthorizedReceiverInterface} from "./interfaces/AuthorizedReceiverInterface.sol";
-import {OperatorInterface} from "../../interfaces/OperatorInterface.sol";
-import {IOwnable} from "../../shared/interfaces/IOwnable.sol";
-import {WithdrawalInterface} from "./interfaces/WithdrawalInterface.sol";
-import {OracleInterface} from "../../interfaces/OracleInterface.sol";
-import {SafeCast} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/math/SafeCast.sol";
+import {ConfirmedOwner} from "../shared/access/ConfirmedOwner.sol";
+import {LinkTokenInterface} from "../shared/interfaces/LinkTokenInterface.sol";
+import {IAuthorizedReceiver} from "./interfaces/IAuthorizedReceiver.sol";
+import {OperatorInterface} from "../interfaces/OperatorInterface.sol";
+import {IOwnable} from "../shared/interfaces/IOwnable.sol";
+import {IWithdrawal} from "./interfaces/IWithdrawal.sol";
+import {OracleInterface} from "../interfaces/OracleInterface.sol";
+import {SafeCast} from "../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/math/SafeCast.sol";
 
 // @title The Chainlink Operator contract
 // @notice Node operators can deploy this contract to fulfill requests sent to them
 // solhint-disable gas-custom-errors
-contract Operator is AuthorizedReceiver, ConfirmedOwner, LinkTokenReceiver, OperatorInterface, WithdrawalInterface {
+contract Operator is AuthorizedReceiver, ConfirmedOwner, LinkTokenReceiver, OperatorInterface, IWithdrawal {
   struct Commitment {
     bytes31 paramsHash;
     uint8 dataVersion;
@@ -241,7 +241,7 @@ contract Operator is AuthorizedReceiver, ConfirmedOwner, LinkTokenReceiver, Oper
     emit TargetsUpdatedAuthorizedSenders(targets, senders, msg.sender);
 
     for (uint256 i = 0; i < targets.length; ++i) {
-      AuthorizedReceiverInterface(targets[i]).setAuthorizedSenders(senders);
+      IAuthorizedReceiver(targets[i]).setAuthorizedSenders(senders);
     }
   }
 
@@ -266,14 +266,14 @@ contract Operator is AuthorizedReceiver, ConfirmedOwner, LinkTokenReceiver, Oper
   function withdraw(
     address recipient,
     uint256 amount
-  ) external override(OracleInterface, WithdrawalInterface) onlyOwner validateAvailableFunds(amount) {
+  ) external override(OracleInterface, IWithdrawal) onlyOwner validateAvailableFunds(amount) {
     assert(i_linkToken.transfer(recipient, amount));
   }
 
   // @notice Displays the amount of LINK that is available for the node operator to withdraw
   // @dev We use `ONE_FOR_CONSISTENT_GAS_COST` in place of 0 in storage
   // @return The amount of withdrawable LINK on the contract
-  function withdrawable() external view override(OracleInterface, WithdrawalInterface) returns (uint256) {
+  function withdrawable() external view override(OracleInterface, IWithdrawal) returns (uint256) {
     return _fundsAvailable();
   }
 

contracts/src/v0.8/operatorforwarder/dev/interfaces/AuthorizedReceiverInterface.sol → contracts/src/v0.8/operatorforwarder/interfaces/IAuthorizedReceiver.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.0;
 
-interface AuthorizedReceiverInterface {
+interface IAuthorizedReceiver {
   function isAuthorizedSender(address sender) external view returns (bool);
 
   function getAuthorizedSenders() external returns (address[] memory);

contracts/src/v0.8/operatorforwarder/dev/interfaces/WithdrawalInterface.sol → contracts/src/v0.8/operatorforwarder/interfaces/IWithdrawal.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.0;
 
-interface WithdrawalInterface {
+interface IWithdrawal {
   // @notice transfer LINK held by the contract belonging to msg.sender to
   // another address
   // @param recipient is the address to send the LINK to

contracts/src/v0.8/operatorforwarder/test/Factory.t.sol

@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.0;
+
+import {Deployer} from "./testhelpers/Deployer.t.sol";
+import {AuthorizedForwarder} from "../AuthorizedForwarder.sol";
+import {Operator} from "../Operator.sol";
+
+contract FactoryTest is Deployer {
+  function setUp() public {
+    _setUp();
+
+    vm.startPrank(ALICE);
+  }
+
+  function test_DeployNewOperator_Success() public {
+    // Deploy a new operator using the factory.
+    address newOperator = s_factory.deployNewOperator();
+    // Assert that the new operator was indeed created by the factory.
+    assertTrue(s_factory.created(newOperator));
+    // Ensure that Alice is the owner of the newly deployed operator.
+    require(Operator(newOperator).owner() == ALICE);
+  }
+
+  function test_DeployNewOperatorAndForwarder_Success() public {
+    // Deploy both a new operator and a new forwarder using the factory.
+    (address newOperator, address newForwarder) = s_factory.deployNewOperatorAndForwarder();
+
+    // Assert that the new operator and the new forwarder were indeed created by the factory.
+    assertTrue(s_factory.created(newOperator));
+    assertTrue(s_factory.created(newForwarder));
+    // Ensure that Alice is the owner of the newly deployed operator.
+    require(Operator(newOperator).owner() == ALICE);
+
+    //Operator to accept ownership
+    vm.startPrank(newOperator);
+    AuthorizedForwarder(newForwarder).acceptOwnership();
+
+    // Ensure that the newly deployed operator is the owner of the newly deployed forwarder.
+    require(AuthorizedForwarder(newForwarder).owner() == newOperator, "operator is not the owner");
+  }
+
+  function test_DeployNewForwarder_Success() public {
+    // Deploy a new forwarder using the factory.
+    address newForwarder = s_factory.deployNewForwarder();
+    // Assert that the new forwarder was indeed created by the factory.
+    assertTrue(s_factory.created(newForwarder));
+    // Ensure that Alice is the owner of the newly deployed forwarder.
+    require(AuthorizedForwarder(newForwarder).owner() == ALICE);
+  }
+
+  function test_DeployNewForwarderAndTransferOwnership_Success() public {
+    // Deploy a new forwarder with a proposal to transfer its ownership to Bob.
+    address newForwarder = s_factory.deployNewForwarderAndTransferOwnership(BOB, new bytes(0));
+    // Assert that the new forwarder was indeed created by the factory.
+    assertTrue(s_factory.created(newForwarder));
+    // Ensure that Alice is still the current owner of the newly deployed forwarder.
+    require(AuthorizedForwarder(newForwarder).owner() == ALICE);
+
+    // Only proposed owner can call acceptOwnership()
+    vm.expectRevert("Must be proposed owner");
+    AuthorizedForwarder(newForwarder).acceptOwnership();
+
+    vm.startPrank(BOB);
+    // Let Bob accept the ownership.
+    AuthorizedForwarder(newForwarder).acceptOwnership();
+    // Ensure that Bob is now the owner of the forwarder after the transfer.
+    require(AuthorizedForwarder(newForwarder).owner() == BOB);
+  }
+}