Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Operator Forwarder Production Readiness #12983

Merged
merged 20 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/solidity-foundry.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions contracts/.changeset/small-paws-crash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@chainlink/contracts": patch
---

Update operatorforwarder tests and pull out of dev/
2 changes: 1 addition & 1 deletion contracts/GNUmakefile
Original file line number Diff line number Diff line change
@@ -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`
Expand Down
23 changes: 21 additions & 2 deletions contracts/gas-snapshots/operatorforwarder.gas-snapshot
Original file line number Diff line number Diff line change
@@ -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)
9 changes: 5 additions & 4 deletions contracts/scripts/native_solc_compile_all_operatorforwarder
Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
@@ -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";

Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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();
}

Expand Down
100 changes: 0 additions & 100 deletions contracts/src/v0.8/operatorforwarder/dev/test/operator.t.sol

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
69 changes: 69 additions & 0 deletions contracts/src/v0.8/operatorforwarder/test/Factory.t.sol
Original file line number Diff line number Diff line change
@@ -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);
}
}
Loading
Loading