refactor test lib and add tests (#12664)

contracts/gas-snapshots/operatorforwarder.gas-snapshot

@@ -0,0 +1,2 @@
+Operator_cancelRequest:test_Success(uint96) (runs: 256, μ: 306103, ~: 306096)
+Operator_cancelRequest:test_afterSuccessfulRequestSucess(uint96) (runs: 256, μ: 384781, ~: 389554)

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

@@ -336,7 +336,7 @@ contract Operator is AuthorizedReceiver, ConfirmedOwner, LinkTokenReceiver, Oper
     uint256 payment,
     bytes4 callbackFunc,
     uint256 expiration
-  ) external override {
+  ) public override {
     bytes31 paramsHash = _buildParamsHash(payment, msg.sender, callbackFunc, expiration);
     require(s_commitments[requestId].paramsHash == paramsHash, "Params do not match request ID");
     // solhint-disable-next-line not-rely-on-time
@@ -345,6 +345,8 @@ contract Operator is AuthorizedReceiver, ConfirmedOwner, LinkTokenReceiver, Oper
     delete s_commitments[requestId];
     emit CancelOracleRequest(requestId);
 
+    // Free up the escrowed funds, as we're sending them back to the requester
+    s_tokensInEscrow -= payment;
     i_linkToken.transfer(msg.sender, payment);
   }
 
@@ -362,16 +364,7 @@ contract Operator is AuthorizedReceiver, ConfirmedOwner, LinkTokenReceiver, Oper
     bytes4 callbackFunc,
     uint256 expiration
   ) external {
-    bytes32 requestId = keccak256(abi.encodePacked(msg.sender, nonce));
-    bytes31 paramsHash = _buildParamsHash(payment, msg.sender, callbackFunc, expiration);
-    require(s_commitments[requestId].paramsHash == paramsHash, "Params do not match request ID");
-    // solhint-disable-next-line not-rely-on-time
-    require(expiration <= block.timestamp, "Request is not expired");
-
-    delete s_commitments[requestId];
-    emit CancelOracleRequest(requestId);
-
-    i_linkToken.transfer(msg.sender, payment);
+    cancelOracleRequest(keccak256(abi.encodePacked(msg.sender, nonce)), payment, callbackFunc, expiration);
   }
 
   // @notice Returns the address of the LINK token

contracts/src/v0.8/operatorforwarder/dev/test/operator.t.sol

@@ -0,0 +1,100 @@
+// 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);
+  }
+}

contracts/src/v0.8/operatorforwarder/dev/test/testhelpers/ChainlinkClientHelper.sol

@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+import {ChainlinkClient} from "../../../../ChainlinkClient.sol";
+
+contract ChainlinkClientHelper is ChainlinkClient {
+  bytes4 public constant FULFILSELECTOR = 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);
+  }
+
+  function cancelRequest(bytes32 requestId, uint256 payment, uint256 expiration) external {
+    _cancelChainlinkRequest(requestId, payment, this.fulfill.selector, expiration);
+  }
+
+  function fulfill(bytes32) external {}
+}

contracts/test/v0.8/ChainlinkClient.test.ts

@@ -27,11 +27,11 @@ before(async () => {
     roles.defaultAccount,
   )
   emptyOracleFactory = await ethers.getContractFactory(
-    'src/v0.8/operatorforwarder/dev/tests/testhelpers/EmptyOracle.sol:EmptyOracle',
+    'src/v0.8/operatorforwarder/dev/test/testhelpers/EmptyOracle.sol:EmptyOracle',
     roles.defaultAccount,
   )
   getterSetterFactory = await ethers.getContractFactory(
-    'src/v0.8/operatorforwarder/dev/tests/testhelpers/GetterSetter.sol:GetterSetter',
+    'src/v0.8/operatorforwarder/dev/test/testhelpers/GetterSetter.sol:GetterSetter',
     roles.defaultAccount,
   )
   operatorFactory = await ethers.getContractFactory(

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/tests/testhelpers/GetterSetter.sol:GetterSetter',
+    'src/v0.8/operatorforwarder/dev/test/testhelpers/GetterSetter.sol:GetterSetter',
     roles.defaultAccount,
   )
   brokenFactory = await ethers.getContractFactory(

contracts/test/v0.8/operatorforwarder/Operator.test.ts

@@ -50,25 +50,25 @@ before(async () => {
 
   roles = users.roles
   basicConsumerFactory = await ethers.getContractFactory(
-    'src/v0.8/operatorforwarder/dev/tests/testhelpers/BasicConsumer.sol:BasicConsumer',
+    'src/v0.8/operatorforwarder/dev/test/testhelpers/BasicConsumer.sol:BasicConsumer',
   )
   multiWordConsumerFactory = await ethers.getContractFactory(
-    'src/v0.8/operatorforwarder/dev/tests/testhelpers/MultiWordConsumer.sol:MultiWordConsumer',
+    'src/v0.8/operatorforwarder/dev/test/testhelpers/MultiWordConsumer.sol:MultiWordConsumer',
   )
   gasGuzzlingConsumerFactory = await ethers.getContractFactory(
-    'src/v0.8/operatorforwarder/dev/tests/testhelpers/GasGuzzlingConsumer.sol:GasGuzzlingConsumer',
+    'src/v0.8/operatorforwarder/dev/test/testhelpers/GasGuzzlingConsumer.sol:GasGuzzlingConsumer',
   )
   getterSetterFactory = await ethers.getContractFactory(
-    'src/v0.8/operatorforwarder/dev/tests/testhelpers/GetterSetter.sol:GetterSetter',
+    'src/v0.8/operatorforwarder/dev/test/testhelpers/GetterSetter.sol:GetterSetter',
   )
   maliciousRequesterFactory = await ethers.getContractFactory(
-    'src/v0.8/operatorforwarder/dev/tests/testhelpers/MaliciousRequester.sol:MaliciousRequester',
+    'src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousRequester.sol:MaliciousRequester',
   )
   maliciousConsumerFactory = await ethers.getContractFactory(
-    'src/v0.8/operatorforwarder/dev/tests/testhelpers/MaliciousConsumer.sol:MaliciousConsumer',
+    'src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousConsumer.sol:MaliciousConsumer',
   )
   maliciousMultiWordConsumerFactory = await ethers.getContractFactory(
-    'src/v0.8/operatorforwarder/dev/tests/testhelpers/MaliciousMultiWordConsumer.sol:MaliciousMultiWordConsumer',
+    'src/v0.8/operatorforwarder/dev/test/testhelpers/MaliciousMultiWordConsumer.sol:MaliciousMultiWordConsumer',
   )
   operatorFactory = await ethers.getContractFactory(
     'src/v0.8/operatorforwarder/dev/Operator.sol:Operator',