diff --git a/.github/workflows/solidity-foundry.yml b/.github/workflows/solidity-foundry.yml index 2d62ef864a5..5e2d95ea9d1 100644 --- a/.github/workflows/solidity-foundry.yml +++ b/.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 diff --git a/contracts/.changeset/small-paws-crash.md b/contracts/.changeset/small-paws-crash.md new file mode 100644 index 00000000000..3a3efa36805 --- /dev/null +++ b/contracts/.changeset/small-paws-crash.md @@ -0,0 +1,5 @@ +--- +"@chainlink/contracts": patch +--- + +Update operatorforwarder tests and pull out of dev/ diff --git a/contracts/GNUmakefile b/contracts/GNUmakefile index 4ec8057b975..ba11b396e89 100644 --- a/contracts/GNUmakefile +++ b/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` diff --git a/contracts/gas-snapshots/operatorforwarder.gas-snapshot b/contracts/gas-snapshots/operatorforwarder.gas-snapshot index 964c1a91b8d..ee6c063f2f3 100644 --- a/contracts/gas-snapshots/operatorforwarder.gas-snapshot +++ b/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) \ No newline at end of file +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) \ No newline at end of file diff --git a/contracts/scripts/native_solc_compile_all_operatorforwarder b/contracts/scripts/native_solc_compile_all_operatorforwarder index 2d455994813..c791d50c1d6 100755 --- a/contracts/scripts/native_solc_compile_all_operatorforwarder +++ b/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 diff --git a/contracts/src/v0.8/operatorforwarder/dev/AuthorizedForwarder.sol b/contracts/src/v0.8/operatorforwarder/AuthorizedForwarder.sol similarity index 97% rename from contracts/src/v0.8/operatorforwarder/dev/AuthorizedForwarder.sol rename to contracts/src/v0.8/operatorforwarder/AuthorizedForwarder.sol index 824ffce6f0f..ffb09248701 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/AuthorizedForwarder.sol +++ b/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"; diff --git a/contracts/src/v0.8/operatorforwarder/dev/AuthorizedReceiver.sol b/contracts/src/v0.8/operatorforwarder/AuthorizedReceiver.sol similarity index 93% rename from contracts/src/v0.8/operatorforwarder/dev/AuthorizedReceiver.sol rename to contracts/src/v0.8/operatorforwarder/AuthorizedReceiver.sol index b741118895f..919602b5acf 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/AuthorizedReceiver.sol +++ b/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; diff --git a/contracts/src/v0.8/operatorforwarder/dev/LinkTokenReceiver.sol b/contracts/src/v0.8/operatorforwarder/LinkTokenReceiver.sol similarity index 100% rename from contracts/src/v0.8/operatorforwarder/dev/LinkTokenReceiver.sol rename to contracts/src/v0.8/operatorforwarder/LinkTokenReceiver.sol diff --git a/contracts/src/v0.8/operatorforwarder/dev/Operator.sol b/contracts/src/v0.8/operatorforwarder/Operator.sol similarity index 96% rename from contracts/src/v0.8/operatorforwarder/dev/Operator.sol rename to contracts/src/v0.8/operatorforwarder/Operator.sol index e68df5fd075..64882e43cda 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/Operator.sol +++ b/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(); } diff --git a/contracts/src/v0.8/operatorforwarder/dev/OperatorFactory.sol b/contracts/src/v0.8/operatorforwarder/OperatorFactory.sol similarity index 100% rename from contracts/src/v0.8/operatorforwarder/dev/OperatorFactory.sol rename to contracts/src/v0.8/operatorforwarder/OperatorFactory.sol diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/operator.t.sol b/contracts/src/v0.8/operatorforwarder/dev/test/operator.t.sol deleted file mode 100644 index 96975a2baf4..00000000000 --- a/contracts/src/v0.8/operatorforwarder/dev/test/operator.t.sol +++ /dev/null @@ -1,100 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.19; - -import {Test} from "forge-std/Test.sol"; -import {Operator} from "../Operator.sol"; -import {ChainlinkClientHelper} from "./testhelpers/ChainlinkClientHelper.sol"; -import {LinkToken} from "../../../shared/token/ERC677/LinkToken.sol"; - -contract Operator_cancelRequest is Test { - address public s_link; - ChainlinkClientHelper public s_client; - Operator public s_operator; - - function setUp() public { - s_link = address(new LinkToken()); - s_client = new ChainlinkClientHelper(s_link); - - address[] memory auth = new address[](1); - auth[0] = address(this); - s_operator = new Operator(s_link, address(this)); - s_operator.setAuthorizedSenders(auth); - } - - function test_Success(uint96 payment) public { - payment = uint96(bound(payment, 1, type(uint96).max)); - deal(s_link, address(s_client), payment); - // We're going to cancel one request and fulfil the other - bytes32 requestIdToCancel = s_client.sendRequest(address(s_operator), payment); - - // Nothing withdrawable - // 1 payment in escrow - // Client has zero link - assertEq(s_operator.withdrawable(), 0); - assertEq(LinkToken(s_link).balanceOf(address(s_operator)), payment); - assertEq(LinkToken(s_link).balanceOf(address(s_client)), 0); - - // Advance time so we can cancel - uint256 expiration = block.timestamp + s_operator.EXPIRYTIME(); - vm.warp(expiration + 1); - s_client.cancelRequest(requestIdToCancel, payment, expiration); - - // 1 payment has been returned due to the cancellation. - assertEq(s_operator.withdrawable(), 0); - assertEq(LinkToken(s_link).balanceOf(address(s_operator)), 0); - assertEq(LinkToken(s_link).balanceOf(address(s_client)), payment); - } - - function test_afterSuccessfulRequestSucess(uint96 payment) public { - payment = uint96(bound(payment, 1, type(uint96).max) / 2); - deal(s_link, address(s_client), 2 * payment); - - // Initial state, client has 2 payments, zero in escrow, zero in the operator, zeero withdrawable - assertEq(s_operator.withdrawable(), 0); - assertEq(LinkToken(s_link).balanceOf(address(s_operator)), 0); - assertEq(LinkToken(s_link).balanceOf(address(s_client)), 2 * payment); - - // We're going to cancel one request and fulfil the other - bytes32 requestId = s_client.sendRequest(address(s_operator), payment); - bytes32 requestIdToCancel = s_client.sendRequest(address(s_operator), payment); - - // Nothing withdrawable - // Operator now has the 2 payments in escrow - // Client has zero payments - assertEq(s_operator.withdrawable(), 0); - assertEq(LinkToken(s_link).balanceOf(address(s_operator)), 2 * payment); - assertEq(LinkToken(s_link).balanceOf(address(s_client)), 0); - - // Fulfill one request - uint256 expiration = block.timestamp + s_operator.EXPIRYTIME(); - s_operator.fulfillOracleRequest( - requestId, - payment, - address(s_client), - s_client.FULFILSELECTOR(), - expiration, - bytes32(hex"01") - ); - // 1 payment withdrawable from fulfilling `requestId`, 1 payment in escrow - assertEq(s_operator.withdrawable(), payment); - assertEq(LinkToken(s_link).balanceOf(address(s_operator)), 2 * payment); - assertEq(LinkToken(s_link).balanceOf(address(s_client)), 0); - - // Advance time so we can cancel - vm.warp(expiration + 1); - s_client.cancelRequest(requestIdToCancel, payment, expiration); - - // 1 payment has been returned due to the cancellation, 1 payment should be withdrawable - assertEq(s_operator.withdrawable(), payment); - assertEq(LinkToken(s_link).balanceOf(address(s_operator)), payment); - assertEq(LinkToken(s_link).balanceOf(address(s_client)), payment); - - // Withdraw the remaining payment - s_operator.withdraw(address(s_client), payment); - - // End state is exactly the same as the initial state. - assertEq(s_operator.withdrawable(), 0); - assertEq(LinkToken(s_link).balanceOf(address(s_operator)), 0); - assertEq(LinkToken(s_link).balanceOf(address(s_client)), 2 * payment); - } -} diff --git a/contracts/src/v0.8/operatorforwarder/dev/interfaces/AuthorizedReceiverInterface.sol b/contracts/src/v0.8/operatorforwarder/interfaces/IAuthorizedReceiver.sol similarity index 87% rename from contracts/src/v0.8/operatorforwarder/dev/interfaces/AuthorizedReceiverInterface.sol rename to contracts/src/v0.8/operatorforwarder/interfaces/IAuthorizedReceiver.sol index 28b20b14f33..78140d58682 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/interfaces/AuthorizedReceiverInterface.sol +++ b/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); diff --git a/contracts/src/v0.8/operatorforwarder/dev/interfaces/WithdrawalInterface.sol b/contracts/src/v0.8/operatorforwarder/interfaces/IWithdrawal.sol similarity index 93% rename from contracts/src/v0.8/operatorforwarder/dev/interfaces/WithdrawalInterface.sol rename to contracts/src/v0.8/operatorforwarder/interfaces/IWithdrawal.sol index c064b0627b5..433738d406f 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/interfaces/WithdrawalInterface.sol +++ b/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 diff --git a/contracts/src/v0.8/operatorforwarder/test/Factory.t.sol b/contracts/src/v0.8/operatorforwarder/test/Factory.t.sol new file mode 100644 index 00000000000..d54dc620460 --- /dev/null +++ b/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); + } +} diff --git a/contracts/src/v0.8/operatorforwarder/test/Forwarder.t.sol b/contracts/src/v0.8/operatorforwarder/test/Forwarder.t.sol new file mode 100644 index 00000000000..ba6ce1c17c1 --- /dev/null +++ b/contracts/src/v0.8/operatorforwarder/test/Forwarder.t.sol @@ -0,0 +1,183 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.0; + +import {Deployer} from "./testhelpers/Deployer.t.sol"; +import {AuthorizedForwarder} from "../AuthorizedForwarder.sol"; + +contract ForwarderTest is Deployer { + AuthorizedForwarder internal s_forwarder; + + function setUp() public { + _setUp(); + + vm.prank(ALICE); + s_forwarder = AuthorizedForwarder(s_factory.deployNewForwarder()); + } + + function test_SetAuthorizedSenders_Success() public { + address[] memory senders; + + // Expect a revert when trying to set an empty list of authorized senders + vm.expectRevert("Cannot set authorized senders"); + s_forwarder.setAuthorizedSenders(senders); + + vm.prank(ALICE); + // Expect a revert because the sender list is empty + vm.expectRevert("Must have at least 1 sender"); + s_forwarder.setAuthorizedSenders(senders); + + // Create a list with two identical sender addresses + senders = new address[](2); + senders[0] = SENDER_1; + senders[1] = SENDER_1; + + vm.prank(ALICE); + // Expect a revert because the sender list has duplicates + vm.expectRevert("Must not have duplicate senders"); + s_forwarder.setAuthorizedSenders(senders); + + // Set the second sender to a different address + senders[1] = SENDER_2; + + vm.prank(ALICE); + // Update the authorized senders list + s_forwarder.setAuthorizedSenders(senders); + + // Check if both SENDER_1 and SENDER_2 are now authorized + assertTrue(s_forwarder.isAuthorizedSender(SENDER_1)); + assertTrue(s_forwarder.isAuthorizedSender(SENDER_2)); + + // Fetch the authorized senders and verify they match the set addresses + address[] memory returnedSenders = s_forwarder.getAuthorizedSenders(); + require(returnedSenders[0] == senders[0]); + require(returnedSenders[1] == senders[1]); + + // Create a new list with only SENDER_3 + senders = new address[](1); + senders[0] = SENDER_3; + + // Prank 'alice' and update the authorized senders to just SENDER_3 + vm.prank(ALICE); + s_forwarder.setAuthorizedSenders(senders); + + // Ensure SENDER_1 and SENDER_2 are no longer authorized + assertFalse(s_forwarder.isAuthorizedSender(SENDER_1)); + assertFalse(s_forwarder.isAuthorizedSender(SENDER_2)); + + // Check that SENDER_3 is now the only authorized sender + assertTrue(s_forwarder.isAuthorizedSender(SENDER_3)); + returnedSenders = s_forwarder.getAuthorizedSenders(); + require(returnedSenders[0] == senders[0]); + } + + function test_Forward_Success(uint256 _value) public { + _addSenders(); + + vm.expectRevert("Not authorized sender"); + s_forwarder.forward(address(0), new bytes(0)); + + vm.prank(SENDER_1); + vm.expectRevert("Cannot forward to Link token"); + s_forwarder.forward(address(s_link), new bytes(0)); + + vm.prank(SENDER_1); + vm.expectRevert("Must forward to a contract"); + s_forwarder.forward(address(0), new bytes(0)); + + vm.prank(SENDER_1); + vm.expectRevert("Forwarded call reverted without reason"); + s_forwarder.forward(address(s_mockReceiver), new bytes(0)); + + vm.prank(SENDER_1); + vm.expectRevert("test revert message"); + s_forwarder.forward(address(s_mockReceiver), abi.encodeWithSignature("revertMessage()")); + + vm.prank(SENDER_1); + s_forwarder.forward(address(s_mockReceiver), abi.encodeWithSignature("receiveData(uint256)", _value)); + + require(s_mockReceiver.getValue() == _value); + } + + function test_MultiForward_Success(uint256 _value1, uint256 _value2) public { + _addSenders(); + + address[] memory tos; + bytes[] memory datas; + + vm.expectRevert("Not authorized sender"); + s_forwarder.multiForward(tos, datas); + + tos = new address[](2); + datas = new bytes[](1); + + vm.prank(SENDER_1); + vm.expectRevert("Arrays must have the same length"); + s_forwarder.multiForward(tos, datas); + + datas = new bytes[](2); + + vm.prank(SENDER_1); + vm.expectRevert("Must forward to a contract"); + s_forwarder.multiForward(tos, datas); + + tos[0] = address(s_mockReceiver); + tos[1] = address(s_link); + + vm.prank(SENDER_1); + vm.expectRevert("Forwarded call reverted without reason"); + s_forwarder.multiForward(tos, datas); + + datas[0] = abi.encodeWithSignature("receiveData(uint256)", _value1); + datas[1] = abi.encodeWithSignature("receiveData(uint256)", _value2); + + vm.prank(SENDER_1); + vm.expectRevert("Cannot forward to Link token"); + s_forwarder.multiForward(tos, datas); + + tos[1] = address(s_mockReceiver); + + vm.prank(SENDER_1); + s_forwarder.multiForward(tos, datas); + + require(s_mockReceiver.getValue() == _value2); + } + + function test_OwnerForward_Success() public { + vm.expectRevert("Only callable by owner"); + s_forwarder.ownerForward(address(0), new bytes(0)); + + vm.prank(ALICE); + vm.expectRevert("Forwarded call reverted without reason"); + s_forwarder.ownerForward(address(s_link), new bytes(0)); + + vm.prank(ALICE); + s_forwarder.ownerForward(address(s_link), abi.encodeWithSignature("balanceOf(address)", address(0))); + } + + function test_TransferOwnershipWithMessage_Success() public { + vm.prank(BOB); + vm.expectRevert("Only callable by owner"); + s_forwarder.transferOwnershipWithMessage(BOB, new bytes(0)); + + vm.prank(ALICE); + s_forwarder.transferOwnershipWithMessage(BOB, new bytes(0)); + + vm.expectRevert("Must be proposed owner"); + s_forwarder.acceptOwnership(); + + vm.prank(BOB); + s_forwarder.acceptOwnership(); + + require(s_forwarder.owner() == BOB); + } + + function _addSenders() internal { + address[] memory senders = new address[](3); + senders[0] = SENDER_1; + senders[1] = SENDER_2; + senders[2] = SENDER_3; + + vm.prank(ALICE); + s_forwarder.setAuthorizedSenders(senders); + } +} diff --git a/contracts/src/v0.8/operatorforwarder/test/operator.t.sol b/contracts/src/v0.8/operatorforwarder/test/operator.t.sol new file mode 100644 index 00000000000..6c4a7c2ae1a --- /dev/null +++ b/contracts/src/v0.8/operatorforwarder/test/operator.t.sol @@ -0,0 +1,291 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.19; + +import {Operator} from "../Operator.sol"; +import {Callback} from "./testhelpers/Callback.sol"; +import {ChainlinkClientHelper} from "./testhelpers/ChainlinkClientHelper.sol"; +import {Deployer} from "./testhelpers/Deployer.t.sol"; + +contract OperatorTest is Deployer { + ChainlinkClientHelper private s_client; + Callback private s_callback; + Operator private s_operator; + + function setUp() public { + _setUp(); + s_client = new ChainlinkClientHelper(address(s_link)); + + address[] memory auth = new address[](1); + auth[0] = address(this); + s_operator = new Operator(address(s_link), address(this)); + s_operator.setAuthorizedSenders(auth); + + s_callback = new Callback(address(s_operator)); + } + + function test_SendRequest_Success(uint96 payment) public { + vm.assume(payment > 0); + deal(address(s_link), address(s_client), payment); + // We're going to cancel one request and fulfill the other + bytes32 requestIdToCancel = s_client.sendRequest(address(s_operator), payment); + + // Nothing withdrawable + // 1 payment in escrow + // Client has zero link + assertEq(s_operator.withdrawable(), 0); + assertEq(s_link.balanceOf(address(s_operator)), payment); + assertEq(s_link.balanceOf(address(s_client)), 0); + + // Advance time so we can cancel + uint256 expiration = block.timestamp + s_operator.EXPIRYTIME(); + vm.warp(expiration + 1); + s_client.cancelRequest(requestIdToCancel, payment, expiration); + + // 1 payment has been returned due to the cancellation. + assertEq(s_operator.withdrawable(), 0); + assertEq(s_link.balanceOf(address(s_operator)), 0); + assertEq(s_link.balanceOf(address(s_client)), payment); + } + + function test_SendRequestAndCancelRequest_Success(uint96 payment) public { + vm.assume(payment > 1); + payment /= payment; + + deal(address(s_link), address(s_client), 2 * payment); + + // Initial state, client has 2 payments, zero in escrow, zero in the operator, zeero withdrawable + assertEq(s_operator.withdrawable(), 0); + assertEq(s_link.balanceOf(address(s_operator)), 0); + assertEq(s_link.balanceOf(address(s_client)), 2 * payment); + + // We're going to cancel one request and fulfill the other + bytes32 requestId = s_client.sendRequest(address(s_operator), payment); + bytes32 requestIdToCancel = s_client.sendRequest(address(s_operator), payment); + + // Nothing withdrawable + // Operator now has the 2 payments in escrow + // Client has zero payments + assertEq(s_operator.withdrawable(), 0); + assertEq(s_link.balanceOf(address(s_operator)), 2 * payment); + assertEq(s_link.balanceOf(address(s_client)), 0); + + // Fulfill one request + uint256 expiration = block.timestamp + s_operator.EXPIRYTIME(); + s_operator.fulfillOracleRequest( + requestId, + payment, + address(s_client), + s_client.FULFILL_SELECTOR(), + expiration, + bytes32(hex"01") + ); + // 1 payment withdrawable from fulfilling `requestId`, 1 payment in escrow + assertEq(s_operator.withdrawable(), payment); + assertEq(s_link.balanceOf(address(s_operator)), 2 * payment); + assertEq(s_link.balanceOf(address(s_client)), 0); + + // Advance time so we can cancel + vm.warp(expiration + 1); + s_client.cancelRequest(requestIdToCancel, payment, expiration); + + // 1 payment has been returned due to the cancellation, 1 payment should be withdrawable + assertEq(s_operator.withdrawable(), payment); + assertEq(s_link.balanceOf(address(s_operator)), payment); + assertEq(s_link.balanceOf(address(s_client)), payment); + + // Withdraw the remaining payment + s_operator.withdraw(address(s_client), payment); + + // End state is exactly the same as the initial state. + assertEq(s_operator.withdrawable(), 0); + assertEq(s_link.balanceOf(address(s_operator)), 0); + assertEq(s_link.balanceOf(address(s_client)), 2 * payment); + } + + function test_OracleRequest_Success() public { + // Define some mock values + bytes32 specId = keccak256("testSpec"); + bytes4 callbackFunctionId = bytes4(keccak256("callback(bytes32)")); + uint256 nonce = 0; + uint256 dataVersion = 1; + bytes memory data = ""; + + uint256 initialLinkBalance = s_link.balanceOf(address(s_operator)); + uint256 payment = 1 ether; // Mock payment value + + uint256 withdrawableBefore = s_operator.withdrawable(); + + // Send LINK tokens to the Operator contract using `transferAndCall` + deal(address(s_link), ALICE, payment); + assertEq(s_link.balanceOf(ALICE), 1 ether, "balance update failed"); + + vm.prank(ALICE); + s_link.transferAndCall( + address(s_operator), + payment, + abi.encodeWithSignature( + "oracleRequest(address,uint256,bytes32,address,bytes4,uint256,uint256,bytes)", + address(this), + payment, + specId, + address(s_callback), + callbackFunctionId, + nonce, + dataVersion, + data + ) + ); + + // Check that the LINK tokens were transferred to the Operator contract + assertEq(s_link.balanceOf(address(s_operator)), initialLinkBalance + payment); + // No withdrawable LINK as it's all locked + assertEq(s_operator.withdrawable(), withdrawableBefore); + } + + function test_FulfillOracleRequest_Success() public { + // This test file is the callback target and actual sender contract + // so we should enable it to set Authorised senders to itself + address[] memory senders = new address[](2); + senders[0] = address(this); + senders[0] = BOB; + + s_operator.setAuthorizedSenders(senders); + + uint256 withdrawableBefore = s_operator.withdrawable(); + + // Define mock values for creating a new oracle request + bytes32 specId = keccak256("testSpecForFulfill"); + bytes4 callbackFunctionId = bytes4(keccak256("callback(bytes32)")); + uint256 nonce = 1; + uint256 dataVersion = 1; + bytes memory dataBytes = ""; + uint256 payment = 1 ether; + uint256 expiration = block.timestamp + 5 minutes; + + // Convert bytes to bytes32 + bytes32 data = bytes32(keccak256(dataBytes)); + + // Send LINK tokens to the Operator contract using `transferAndCall` + deal(address(s_link), BOB, payment); + vm.prank(BOB); + s_link.transferAndCall( + address(s_operator), + payment, + abi.encodeWithSignature( + "oracleRequest(address,uint256,bytes32,address,bytes4,uint256,uint256,bytes)", + address(this), + payment, + specId, + address(s_callback), + callbackFunctionId, + nonce, + dataVersion, + dataBytes + ) + ); + + // Fulfill the request using the operator + bytes32 requestId = keccak256(abi.encodePacked(BOB, nonce)); + vm.prank(BOB); + s_operator.fulfillOracleRequest(requestId, payment, address(s_callback), callbackFunctionId, expiration, data); + + assertEq(s_callback.getCallbacksReceived(), 1, "Oracle request was not fulfilled"); + + // Withdrawable balance + assertEq(s_operator.withdrawable(), withdrawableBefore + payment, "Internal accounting not updated correctly"); + } + + function test_CancelOracleRequest_Success() public { + // Define mock values for creating a new oracle request + bytes32 specId = keccak256("testSpecForCancel"); + bytes4 callbackFunctionId = bytes4(keccak256("callback(bytes32)")); + uint256 nonce = 2; + uint256 dataVersion = 1; + bytes memory dataBytes = ""; + uint256 payment = 1 ether; + uint256 expiration = block.timestamp + 5 minutes; + + uint256 withdrawableBefore = s_operator.withdrawable(); + + // Send LINK tokens to the Operator contract using `transferAndCall` + deal(address(s_link), BOB, payment); + vm.prank(BOB); + s_link.transferAndCall( + address(s_operator), + payment, + abi.encodeWithSignature( + "oracleRequest(address,uint256,bytes32,address,bytes4,uint256,uint256,bytes)", + BOB, + payment, + specId, + BOB, + callbackFunctionId, + nonce, + dataVersion, + dataBytes + ) + ); + + // No withdrawable balance as it's all locked + assertEq(s_operator.withdrawable(), withdrawableBefore, "Internal accounting not updated correctly"); + + bytes32 requestId = keccak256(abi.encodePacked(BOB, nonce)); + + vm.startPrank(ALICE); + vm.expectRevert(bytes("Params do not match request ID")); + s_operator.cancelOracleRequest(requestId, payment, callbackFunctionId, expiration); + + vm.startPrank(BOB); + vm.expectRevert(bytes("Request is not expired")); + s_operator.cancelOracleRequest(requestId, payment, callbackFunctionId, expiration); + + vm.warp(expiration); + s_operator.cancelOracleRequest(requestId, payment, callbackFunctionId, expiration); + + // Check if the LINK tokens were refunded to the sender (bob in this case) + assertEq(s_link.balanceOf(BOB), 1 ether, "Oracle request was not canceled properly"); + + assertEq(s_operator.withdrawable(), withdrawableBefore, "Internal accounting not updated correctly"); + } + + function test_NotAuthorizedSender_Revert() public { + bytes32 specId = keccak256("unauthorizedFulfillSpec"); + bytes4 callbackFunctionId = bytes4(keccak256("callback(bytes32)")); + uint256 nonce = 5; + uint256 dataVersion = 1; + bytes memory dataBytes = ""; + uint256 payment = 1 ether; + uint256 expiration = block.timestamp + 5 minutes; + + deal(address(s_link), ALICE, payment); + vm.prank(ALICE); + s_link.transferAndCall( + address(s_operator), + payment, + abi.encodeWithSignature( + "oracleRequest(address,uint256,bytes32,address,bytes4,uint256,uint256,bytes)", + ALICE, + payment, + specId, + address(s_callback), + callbackFunctionId, + nonce, + dataVersion, + dataBytes + ) + ); + + bytes32 requestId = keccak256(abi.encodePacked(ALICE, nonce)); + + vm.prank(BOB); + vm.expectRevert(bytes("Not authorized sender")); + s_operator.fulfillOracleRequest( + requestId, + payment, + address(s_callback), + callbackFunctionId, + expiration, + bytes32(keccak256(dataBytes)) + ); + } +} diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/BasicConsumer.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/BasicConsumer.sol similarity index 100% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/BasicConsumer.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/BasicConsumer.sol diff --git a/contracts/src/v0.8/operatorforwarder/test/testhelpers/Callback.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/Callback.sol new file mode 100644 index 00000000000..9dccfed428a --- /dev/null +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/Callback.sol @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +contract Callback { + address private s_operator; + uint256 private s_callbacksReceived = 0; + + constructor(address _operator) { + s_operator = _operator; + } + + // Callback function for oracle request fulfillment + function callback(bytes32) public { + require(msg.sender == s_operator, "Only Operator can call this function"); + s_callbacksReceived += 1; + } + + function getCallbacksReceived() public view returns (uint256) { + return s_callbacksReceived; + } +} diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/ChainlinkClientHelper.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/ChainlinkClientHelper.sol similarity index 76% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/ChainlinkClientHelper.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/ChainlinkClientHelper.sol index d15eb07c8c9..9b6ba6bb432 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/ChainlinkClientHelper.sol +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/ChainlinkClientHelper.sol @@ -1,17 +1,17 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import {ChainlinkClient} from "../../../../ChainlinkClient.sol"; +import {ChainlinkClient} from "../../../ChainlinkClient.sol"; contract ChainlinkClientHelper is ChainlinkClient { - bytes4 public constant FULFILSELECTOR = this.fulfill.selector; + bytes4 public constant FULFILL_SELECTOR = this.fulfill.selector; constructor(address link) { _setChainlinkToken(link); } function sendRequest(address op, uint256 payment) external returns (bytes32) { - return _sendChainlinkRequestTo(op, _buildOperatorRequest(bytes32(hex"10"), FULFILSELECTOR), payment); + return _sendChainlinkRequestTo(op, _buildOperatorRequest(bytes32(hex"10"), FULFILL_SELECTOR), payment); } function cancelRequest(bytes32 requestId, uint256 payment, uint256 expiration) external { diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/Chainlinked.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/Chainlinked.sol similarity index 98% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/Chainlinked.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/Chainlinked.sol index 86dc474e8a6..dba5d407623 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/Chainlinked.sol +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/Chainlinked.sol @@ -1,6 +1,6 @@ pragma solidity ^0.8.0; -import {ChainlinkClient, Chainlink} from "../../../../ChainlinkClient.sol"; +import {ChainlinkClient, Chainlink} from "../../../ChainlinkClient.sol"; /** * @title The Chainlinked contract diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/Consumer.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/Consumer.sol similarity index 87% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/Consumer.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/Consumer.sol index 0d01778e19e..82709d3def8 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/Consumer.sol +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/Consumer.sol @@ -1,14 +1,14 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import {ChainlinkClient, ChainlinkRequestInterface, LinkTokenInterface} from "../../../../ChainlinkClient.sol"; -import {Chainlink} from "../../../../Chainlink.sol"; +import {ChainlinkClient, ChainlinkRequestInterface, LinkTokenInterface} from "../../../ChainlinkClient.sol"; +import {Chainlink} from "../../../Chainlink.sol"; contract Consumer is ChainlinkClient { using Chainlink for Chainlink.Request; bytes32 internal s_specId; - bytes32 public currentPrice; + bytes32 internal s_currentPrice; event RequestFulfilled( bytes32 indexed requestId, // User-defined ID @@ -50,6 +50,10 @@ contract Consumer is ChainlinkClient { function fulfill(bytes32 _requestId, bytes32 _price) public recordChainlinkFulfillment(_requestId) { emit RequestFulfilled(_requestId, _price); - currentPrice = _price; + s_currentPrice = _price; + } + + function getCurrentPrice() public view returns (bytes32) { + return s_currentPrice; } } diff --git a/contracts/src/v0.8/operatorforwarder/test/testhelpers/Deployer.t.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/Deployer.t.sol new file mode 100644 index 00000000000..da746c7ff8c --- /dev/null +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/Deployer.t.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.0; + +import {Test} from "forge-std/Test.sol"; + +import {MockReceiver} from "./MockReceiver.sol"; +import {AuthorizedForwarder} from "../../AuthorizedForwarder.sol"; +import {Operator} from "../../Operator.sol"; +import {OperatorFactory} from "../../OperatorFactory.sol"; +import {LinkToken} from "../../../shared/token/ERC677/LinkToken.sol"; + +abstract contract Deployer is Test { + OperatorFactory internal s_factory; + LinkToken internal s_link; + MockReceiver internal s_mockReceiver; + + address internal constant ALICE = address(0x101); + address internal constant BOB = address(0x102); + address internal constant SENDER_1 = address(0x103); + address internal constant SENDER_2 = address(0x104); + address internal constant SENDER_3 = address(0x105); + + function _setUp() internal { + _deploy(); + } + + function _deploy() internal { + s_link = new LinkToken(); + s_factory = new OperatorFactory(address(s_link)); + + s_mockReceiver = new MockReceiver(); + } +} diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/EmptyOracle.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/EmptyOracle.sol similarity index 83% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/EmptyOracle.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/EmptyOracle.sol index 2abe393151e..f278791d2bb 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/EmptyOracle.sol +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/EmptyOracle.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import {ChainlinkRequestInterface} from "../../../../interfaces/ChainlinkRequestInterface.sol"; -import {OracleInterface} from "../../../../interfaces/OracleInterface.sol"; +import {ChainlinkRequestInterface} from "../../../interfaces/ChainlinkRequestInterface.sol"; +import {OracleInterface} from "../../../interfaces/OracleInterface.sol"; /* solhint-disable no-empty-blocks */ contract EmptyOracle is ChainlinkRequestInterface, OracleInterface { diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/GasGuzzlingConsumer.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/GasGuzzlingConsumer.sol similarity index 96% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/GasGuzzlingConsumer.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/GasGuzzlingConsumer.sol index 54ff0e30e66..029102018b0 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/GasGuzzlingConsumer.sol +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/GasGuzzlingConsumer.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; import {Consumer} from "./Consumer.sol"; -import {Chainlink} from "../../../../Chainlink.sol"; +import {Chainlink} from "../../../Chainlink.sol"; contract GasGuzzlingConsumer is Consumer { using Chainlink for Chainlink.Request; diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/GetterSetter.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/GetterSetter.sol similarity index 71% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/GetterSetter.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/GetterSetter.sol index 494da582e1b..722362bc4aa 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/GetterSetter.sol +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/GetterSetter.sol @@ -3,10 +3,10 @@ pragma solidity ^0.8.0; // GetterSetter is a contract to aid debugging and testing during development. // solhint-disable contract GetterSetter { - bytes32 public getBytes32; - uint256 public getUint256; - bytes32 public requestId; - bytes public getBytes; + bytes32 private s_getBytes32; + uint256 private s_getUint256; + bytes32 private s_requestId; + bytes private s_getBytes; event SetBytes32(address indexed from, bytes32 indexed value); event SetUint256(address indexed from, uint256 indexed value); @@ -15,32 +15,36 @@ contract GetterSetter { event Output(bytes32 b32, uint256 u256, bytes32 b322); function setBytes32(bytes32 _value) public { - getBytes32 = _value; + s_getBytes32 = _value; emit SetBytes32(msg.sender, _value); } function requestedBytes32(bytes32 _requestId, bytes32 _value) public { - requestId = _requestId; + s_requestId = _requestId; setBytes32(_value); } function setBytes(bytes memory _value) public { - getBytes = _value; + s_getBytes = _value; emit SetBytes(msg.sender, _value); } + function getBytes() public view returns (bytes memory _value) { + return s_getBytes; + } + function requestedBytes(bytes32 _requestId, bytes memory _value) public { - requestId = _requestId; + s_requestId = _requestId; setBytes(_value); } function setUint256(uint256 _value) public { - getUint256 = _value; + s_getUint256 = _value; emit SetUint256(msg.sender, _value); } function requestedUint256(bytes32 _requestId, uint256 _value) public { - requestId = _requestId; + s_requestId = _requestId; setUint256(_value); } } diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousChainlink.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousChainlink.sol similarity index 91% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousChainlink.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousChainlink.sol index 5cc343aa7f4..11c863fbb98 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousChainlink.sol +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousChainlink.sol @@ -1,7 +1,7 @@ pragma solidity ^0.8.0; -import {CBORChainlink as CBOR_Chainlink} from "../../../../vendor/CBORChainlink.sol"; -import {BufferChainlink as Buffer_Chainlink} from "../../../../vendor/BufferChainlink.sol"; +import {CBORChainlink as CBOR_Chainlink} from "../../../vendor/CBORChainlink.sol"; +import {BufferChainlink as Buffer_Chainlink} from "../../../vendor/BufferChainlink.sol"; // solhint-disable library MaliciousChainlink { diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousChainlinked.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousChainlinked.sol similarity index 80% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousChainlinked.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousChainlinked.sol index 722fbdd599b..989c39c18be 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousChainlinked.sol +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousChainlinked.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; import {MaliciousChainlink} from "./MaliciousChainlink.sol"; import {Chainlinked, Chainlink} from "./Chainlinked.sol"; -import {LinkTokenInterface} from "../../../../shared/interfaces/LinkTokenInterface.sol"; +import {LinkTokenInterface} from "../../../shared/interfaces/LinkTokenInterface.sol"; // solhint-disable contract MaliciousChainlinked is Chainlinked { @@ -10,8 +10,8 @@ contract MaliciousChainlinked is Chainlinked { using MaliciousChainlink for MaliciousChainlink.WithdrawRequest; using Chainlink for Chainlink.Request; - uint256 private maliciousRequests = 1; - mapping(bytes32 => address) private maliciousPendingRequests; + uint256 private s_maliciousRequests = 1; + mapping(bytes32 => address) private s_maliciousPendingRequests; function newWithdrawRequest( bytes32 _specId, @@ -27,31 +27,31 @@ contract MaliciousChainlinked is Chainlinked { Chainlink.Request memory _req, uint256 _amount ) internal returns (bytes32 requestId) { - requestId = keccak256(abi.encodePacked(_target, maliciousRequests)); - _req.nonce = maliciousRequests; - maliciousPendingRequests[requestId] = oracleAddress(); + requestId = keccak256(abi.encodePacked(_target, s_maliciousRequests)); + _req.nonce = s_maliciousRequests; + s_maliciousPendingRequests[requestId] = oracleAddress(); emit ChainlinkRequested(requestId); LinkTokenInterface link = LinkTokenInterface(chainlinkToken()); require( link.transferAndCall(oracleAddress(), _amount, encodeTargetRequest(_req)), "Unable to transferAndCall to oracle" ); - maliciousRequests += 1; + s_maliciousRequests += 1; return requestId; } function chainlinkPriceRequest(Chainlink.Request memory _req, uint256 _amount) internal returns (bytes32 requestId) { - requestId = keccak256(abi.encodePacked(this, maliciousRequests)); - _req.nonce = maliciousRequests; - maliciousPendingRequests[requestId] = oracleAddress(); + requestId = keccak256(abi.encodePacked(this, s_maliciousRequests)); + _req.nonce = s_maliciousRequests; + s_maliciousPendingRequests[requestId] = oracleAddress(); emit ChainlinkRequested(requestId); LinkTokenInterface link = LinkTokenInterface(chainlinkToken()); require( link.transferAndCall(oracleAddress(), _amount, encodePriceRequest(_req)), "Unable to transferAndCall to oracle" ); - maliciousRequests += 1; + s_maliciousRequests += 1; return requestId; } @@ -60,16 +60,16 @@ contract MaliciousChainlinked is Chainlinked { MaliciousChainlink.WithdrawRequest memory _req, uint256 _wei ) internal returns (bytes32 requestId) { - requestId = keccak256(abi.encodePacked(this, maliciousRequests)); - _req.nonce = maliciousRequests; - maliciousPendingRequests[requestId] = oracleAddress(); + requestId = keccak256(abi.encodePacked(this, s_maliciousRequests)); + _req.nonce = s_maliciousRequests; + s_maliciousPendingRequests[requestId] = oracleAddress(); emit ChainlinkRequested(requestId); LinkTokenInterface link = LinkTokenInterface(chainlinkToken()); require( link.transferAndCall(oracleAddress(), _wei, encodeWithdrawRequest(_req)), "Unable to transferAndCall to oracle" ); - maliciousRequests += 1; + s_maliciousRequests += 1; return requestId; } diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousConsumer.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousConsumer.sol similarity index 92% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousConsumer.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousConsumer.sol index 003e628880f..842eec90542 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousConsumer.sol +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousConsumer.sol @@ -5,7 +5,7 @@ import {Chainlinked, Chainlink} from "./Chainlinked.sol"; // solhint-disable contract MaliciousConsumer is Chainlinked { uint256 private constant ORACLE_PAYMENT = 1 ether; - uint256 private expiration; + uint256 private s_expiration; constructor(address _link, address _oracle) public payable { setLinkToken(_link); @@ -16,7 +16,7 @@ contract MaliciousConsumer is Chainlinked { function requestData(bytes32 _id, bytes memory _callbackFunc) public { Chainlink.Request memory req = newRequest(_id, address(this), bytes4(keccak256(_callbackFunc))); - expiration = block.timestamp + 5 minutes; + s_expiration = block.timestamp + 5 minutes; chainlinkRequest(req, ORACLE_PAYMENT); } @@ -25,7 +25,7 @@ contract MaliciousConsumer is Chainlinked { } function cancelRequestOnFulfill(bytes32 _requestId, bytes32) public { - _cancelChainlinkRequest(_requestId, ORACLE_PAYMENT, this.cancelRequestOnFulfill.selector, expiration); + _cancelChainlinkRequest(_requestId, ORACLE_PAYMENT, this.cancelRequestOnFulfill.selector, s_expiration); } function remove() public { diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousMultiWordConsumer.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousMultiWordConsumer.sol similarity index 94% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousMultiWordConsumer.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousMultiWordConsumer.sol index 272361f2dda..d9d14cb3d43 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousMultiWordConsumer.sol +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousMultiWordConsumer.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import {ChainlinkClient} from "../../../../ChainlinkClient.sol"; -import {Chainlink} from "../../../../Chainlink.sol"; +import {ChainlinkClient} from "../../../ChainlinkClient.sol"; +import {Chainlink} from "../../../Chainlink.sol"; contract MaliciousMultiWordConsumer is ChainlinkClient { uint256 private constant ORACLE_PAYMENT = 1 ether; diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousRequester.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousRequester.sol similarity index 95% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousRequester.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousRequester.sol index 9b19653722c..c01c8a60bb7 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousRequester.sol +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MaliciousRequester.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; import {MaliciousChainlink} from "./MaliciousChainlink.sol"; import {MaliciousChainlinked, Chainlink} from "./MaliciousChainlinked.sol"; -import {ChainlinkRequestInterface} from "../../../../interfaces/ChainlinkRequestInterface.sol"; +import {ChainlinkRequestInterface} from "../../../interfaces/ChainlinkRequestInterface.sol"; contract MaliciousRequester is MaliciousChainlinked { uint256 private constant ORACLE_PAYMENT = 1 ether; diff --git a/contracts/src/v0.8/operatorforwarder/test/testhelpers/MockReceiver.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MockReceiver.sol new file mode 100644 index 00000000000..4e825b4505f --- /dev/null +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MockReceiver.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.0; + +contract MockReceiver { + uint256 private s_value; + + function receiveData(uint256 _value) public { + s_value = _value; + } + + function revertMessage() public pure { + revert("test revert message"); + } + + function getValue() external view returns (uint256) { + return s_value; + } +} diff --git a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MultiWordConsumer.sol b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MultiWordConsumer.sol similarity index 81% rename from contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MultiWordConsumer.sol rename to contracts/src/v0.8/operatorforwarder/test/testhelpers/MultiWordConsumer.sol index ce2bf1907c5..fe249831fef 100644 --- a/contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/MultiWordConsumer.sol +++ b/contracts/src/v0.8/operatorforwarder/test/testhelpers/MultiWordConsumer.sol @@ -1,21 +1,21 @@ pragma solidity ^0.8.0; -import {ChainlinkClient, ChainlinkRequestInterface, LinkTokenInterface} from "../../../../ChainlinkClient.sol"; -import {Chainlink} from "../../../../Chainlink.sol"; +import {ChainlinkClient, ChainlinkRequestInterface, LinkTokenInterface} from "../../../ChainlinkClient.sol"; +import {Chainlink} from "../../../Chainlink.sol"; contract MultiWordConsumer is ChainlinkClient { using Chainlink for Chainlink.Request; bytes32 internal s_specId; - bytes public currentPrice; + bytes internal s_currentPrice; - bytes32 public usd; - bytes32 public eur; - bytes32 public jpy; + bytes32 private s_usd; + bytes32 private s_eur; + bytes32 private s_jpy; - uint256 public usdInt; - uint256 public eurInt; - uint256 public jpyInt; + uint256 private s_usdInt; + uint256 private s_eurInt; + uint256 private s_jpyInt; event RequestFulfilled( bytes32 indexed requestId, // User-defined ID @@ -100,9 +100,9 @@ contract MultiWordConsumer is ChainlinkClient { bytes32 _jpy ) public recordChainlinkFulfillment(_requestId) { emit RequestMultipleFulfilled(_requestId, _usd, _eur, _jpy); - usd = _usd; - eur = _eur; - jpy = _jpy; + s_usd = _usd; + s_eur = _eur; + s_jpy = _jpy; } function fulfillMultipleParametersWithCustomURLs( @@ -112,17 +112,33 @@ contract MultiWordConsumer is ChainlinkClient { uint256 _jpy ) public recordChainlinkFulfillment(_requestId) { emit RequestMultipleFulfilledWithCustomURLs(_requestId, _usd, _eur, _jpy); - usdInt = _usd; - eurInt = _eur; - jpyInt = _jpy; + s_usdInt = _usd; + s_eurInt = _eur; + s_jpyInt = _jpy; } function fulfillBytes(bytes32 _requestId, bytes memory _price) public recordChainlinkFulfillment(_requestId) { emit RequestFulfilled(_requestId, _price); - currentPrice = _price; + s_currentPrice = _price; } function publicGetNextRequestCount() external view returns (uint256) { return _getNextRequestCount(); } + + function getCurrentPrice() public view returns (bytes memory _value) { + return s_currentPrice; + } + + function usd() public view returns (bytes32 _value) { + return s_usd; + } + + function eur() public view returns (bytes32 _value) { + return s_eur; + } + + function jpy() public view returns (bytes32 _value) { + return s_jpy; + } } diff --git a/contracts/test/v0.8/ChainlinkClient.test.ts b/contracts/test/v0.8/ChainlinkClient.test.ts index b483e890a6b..c5691211c1a 100644 --- a/contracts/test/v0.8/ChainlinkClient.test.ts +++ b/contracts/test/v0.8/ChainlinkClient.test.ts @@ -27,15 +27,15 @@ before(async () => { roles.defaultAccount, ) emptyOracleFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/test/testhelpers/EmptyOracle.sol:EmptyOracle', + 'src/v0.8/operatorforwarder/test/testhelpers/EmptyOracle.sol:EmptyOracle', roles.defaultAccount, ) getterSetterFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/test/testhelpers/GetterSetter.sol:GetterSetter', + 'src/v0.8/operatorforwarder/test/testhelpers/GetterSetter.sol:GetterSetter', roles.defaultAccount, ) operatorFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/Operator.sol:Operator', + 'src/v0.8/operatorforwarder/Operator.sol:Operator', roles.defaultAccount, ) linkTokenFactory = await ethers.getContractFactory( diff --git a/contracts/test/v0.8/operatorforwarder/AuthorizedForwarder.test.ts b/contracts/test/v0.8/operatorforwarder/AuthorizedForwarder.test.ts index 2d6329e221d..d4e1918c976 100644 --- a/contracts/test/v0.8/operatorforwarder/AuthorizedForwarder.test.ts +++ b/contracts/test/v0.8/operatorforwarder/AuthorizedForwarder.test.ts @@ -18,7 +18,7 @@ before(async () => { roles = users.roles getterSetterFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/test/testhelpers/GetterSetter.sol:GetterSetter', + 'src/v0.8/operatorforwarder/test/testhelpers/GetterSetter.sol:GetterSetter', roles.defaultAccount, ) brokenFactory = await ethers.getContractFactory( @@ -26,7 +26,7 @@ before(async () => { roles.defaultAccount, ) forwarderFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/AuthorizedForwarder.sol:AuthorizedForwarder', + 'src/v0.8/operatorforwarder/AuthorizedForwarder.sol:AuthorizedForwarder', roles.defaultAccount, ) linkTokenFactory = await ethers.getContractFactory( diff --git a/contracts/test/v0.8/operatorforwarder/Operator.test.ts b/contracts/test/v0.8/operatorforwarder/Operator.test.ts index 0d75d8530a4..6fb8768f54a 100644 --- a/contracts/test/v0.8/operatorforwarder/Operator.test.ts +++ b/contracts/test/v0.8/operatorforwarder/Operator.test.ts @@ -50,31 +50,31 @@ before(async () => { roles = users.roles basicConsumerFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/test/testhelpers/BasicConsumer.sol:BasicConsumer', + 'src/v0.8/operatorforwarder/test/testhelpers/BasicConsumer.sol:BasicConsumer', ) multiWordConsumerFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/test/testhelpers/MultiWordConsumer.sol:MultiWordConsumer', + 'src/v0.8/operatorforwarder/test/testhelpers/MultiWordConsumer.sol:MultiWordConsumer', ) gasGuzzlingConsumerFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/test/testhelpers/GasGuzzlingConsumer.sol:GasGuzzlingConsumer', + 'src/v0.8/operatorforwarder/test/testhelpers/GasGuzzlingConsumer.sol:GasGuzzlingConsumer', ) getterSetterFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/test/testhelpers/GetterSetter.sol:GetterSetter', + 'src/v0.8/operatorforwarder/test/testhelpers/GetterSetter.sol:GetterSetter', ) maliciousRequesterFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousRequester.sol:MaliciousRequester', + 'src/v0.8/operatorforwarder/test/testhelpers/MaliciousRequester.sol:MaliciousRequester', ) maliciousConsumerFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousConsumer.sol:MaliciousConsumer', + 'src/v0.8/operatorforwarder/test/testhelpers/MaliciousConsumer.sol:MaliciousConsumer', ) maliciousMultiWordConsumerFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousMultiWordConsumer.sol:MaliciousMultiWordConsumer', + 'src/v0.8/operatorforwarder/test/testhelpers/MaliciousMultiWordConsumer.sol:MaliciousMultiWordConsumer', ) operatorFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/Operator.sol:Operator', + 'src/v0.8/operatorforwarder/Operator.sol:Operator', ) forwarderFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/AuthorizedForwarder.sol:AuthorizedForwarder', + 'src/v0.8/operatorforwarder/AuthorizedForwarder.sol:AuthorizedForwarder', ) linkTokenFactory = await ethers.getContractFactory( 'src/v0.8/shared/test/helpers/LinkTokenTestHelper.sol:LinkTokenTestHelper', @@ -1076,7 +1076,7 @@ describe('Operator', () => { .connect(roles.oracleNode) .fulfillOracleRequest(...convertFufillParams(request, response)) - const currentValue = await basicConsumer.currentPrice() + const currentValue = await basicConsumer.getCurrentPrice() assert.equal(response, ethers.utils.parseBytes32String(currentValue)) }) @@ -1105,7 +1105,7 @@ describe('Operator', () => { .fulfillOracleRequest(...convertFufillParams(request, response2)), ) - const currentValue = await basicConsumer.currentPrice() + const currentValue = await basicConsumer.getCurrentPrice() assert.equal(response, ethers.utils.parseBytes32String(currentValue)) }) }) @@ -1419,7 +1419,7 @@ describe('Operator', () => { }) describe('#fulfillOracleRequest2', () => { - describe('single word fulfils', () => { + describe('single word fulfills', () => { const response = 'Hi mom!' const responseTypes = ['bytes32'] const responseValues = [toBytes32String(response)] @@ -1528,7 +1528,7 @@ describe('Operator', () => { ), ) - const currentValue = await basicConsumer.currentPrice() + const currentValue = await basicConsumer.getCurrentPrice() assert.equal( response, ethers.utils.parseBytes32String(currentValue), @@ -1576,7 +1576,7 @@ describe('Operator', () => { ), ) - const currentValue = await basicConsumer.currentPrice() + const currentValue = await basicConsumer.getCurrentPrice() assert.equal( response, ethers.utils.parseBytes32String(currentValue), @@ -2024,7 +2024,7 @@ describe('Operator', () => { }) }) - describe('multi word fulfils', () => { + describe('multi word fulfills', () => { describe('one bytes parameter', () => { const response = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.\ @@ -2149,7 +2149,7 @@ describe('Operator', () => { ), ) - const currentValue = await multiConsumer.currentPrice() + const currentValue = await multiConsumer.getCurrentPrice() assert.equal(response, ethers.utils.toUtf8String(currentValue)) }) @@ -2195,7 +2195,7 @@ describe('Operator', () => { ), ) - const currentValue = await multiConsumer.currentPrice() + const currentValue = await multiConsumer.getCurrentPrice() assert.equal(response, ethers.utils.toUtf8String(currentValue)) }) }) diff --git a/contracts/test/v0.8/operatorforwarder/OperatorFactory.test.ts b/contracts/test/v0.8/operatorforwarder/OperatorFactory.test.ts index b9a0fe508b0..b54a75c2c0d 100644 --- a/contracts/test/v0.8/operatorforwarder/OperatorFactory.test.ts +++ b/contracts/test/v0.8/operatorforwarder/OperatorFactory.test.ts @@ -20,15 +20,15 @@ before(async () => { roles.defaultAccount, ) operatorGeneratorFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/OperatorFactory.sol:OperatorFactory', + 'src/v0.8/operatorforwarder/OperatorFactory.sol:OperatorFactory', roles.defaultAccount, ) operatorFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/Operator.sol:Operator', + 'src/v0.8/operatorforwarder/Operator.sol:Operator', roles.defaultAccount, ) forwarderFactory = await ethers.getContractFactory( - 'src/v0.8/operatorforwarder/dev/AuthorizedForwarder.sol:AuthorizedForwarder', + 'src/v0.8/operatorforwarder/AuthorizedForwarder.sol:AuthorizedForwarder', roles.defaultAccount, ) })