diff --git a/.github/workflows/solidity-foundry.yml b/.github/workflows/solidity-foundry.yml index 7c930207fe7..47f9571eea3 100644 --- a/.github/workflows/solidity-foundry.yml +++ b/.github/workflows/solidity-foundry.yml @@ -32,7 +32,7 @@ jobs: strategy: fail-fast: false matrix: - product: [vrf, automation, llo-feeds, l2ep, functions, shared] + product: [vrf, automation, llo-feeds, l2ep, functions, keystone, shared] needs: [changes] name: Foundry Tests ${{ matrix.product }} # See https://github.com/foundry-rs/foundry/issues/3827 diff --git a/.github/workflows/solidity-hardhat.yml b/.github/workflows/solidity-hardhat.yml index da9d7daccbb..f07c9f8fdca 100644 --- a/.github/workflows/solidity-hardhat.yml +++ b/.github/workflows/solidity-hardhat.yml @@ -25,7 +25,7 @@ jobs: with: filters: | src: - - 'contracts/src/!(v0.8/(llo-feeds|ccip)/**)/**/*' + - 'contracts/src/!(v0.8/(llo-feeds|keystone|ccip)/**)/**/*' - 'contracts/test/**/*' - 'contracts/package.json' - 'contracts/pnpm-lock.yaml' diff --git a/contracts/gas-snapshots/keystone.gas-snapshot b/contracts/gas-snapshots/keystone.gas-snapshot new file mode 100644 index 00000000000..11897d62913 --- /dev/null +++ b/contracts/gas-snapshots/keystone.gas-snapshot @@ -0,0 +1,2 @@ +KeystoneForwarderTest:test_abi_partial_decoding_works() (gas: 2068) +KeystoneForwarderTest:test_it_works() (gas: 1026676) \ No newline at end of file diff --git a/contracts/src/v0.8/keystone/KeystoneForwarder.sol b/contracts/src/v0.8/keystone/KeystoneForwarder.sol index 22e5654b456..056f5630687 100644 --- a/contracts/src/v0.8/keystone/KeystoneForwarder.sol +++ b/contracts/src/v0.8/keystone/KeystoneForwarder.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.19; import {IForwarder} from "./interfaces/IForwarder.sol"; import {ConfirmedOwner} from "../shared/access/ConfirmedOwner.sol"; import {TypeAndVersionInterface} from "../interfaces/TypeAndVersionInterface.sol"; +import {Utils} from "./libraries/Utils.sol"; // solhint-disable custom-errors, no-unused-vars contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterface { @@ -19,46 +20,14 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac constructor() ConfirmedOwner(msg.sender) {} - // solhint-disable avoid-low-level-calls, chainlink-solidity/explicit-returns - function splitSignature(bytes memory sig) public pure returns (bytes32 r, bytes32 s, uint8 v) { - require(sig.length == 65, "invalid signature length"); - - assembly { - /* - First 32 bytes stores the length of the signature - - add(sig, 32) = pointer of sig + 32 - effectively, skips first 32 bytes of signature - - mload(p) loads next 32 bytes starting at the memory address p into memory - */ - - // first 32 bytes, after the length prefix - r := mload(add(sig, 32)) - // second 32 bytes - s := mload(add(sig, 64)) - // final byte (first byte of the next 32 bytes) - v := byte(0, mload(add(sig, 96))) - } - - // implicitly return (r, s, v) - } - - // solhint-disable avoid-low-level-calls, chainlink-solidity/explicit-returns - function splitReport(bytes memory rawReport) public pure returns (bytes32 workflowId, bytes32 workflowExecutionId) { - require(rawReport.length > 64, "invalid report length"); - assembly { - workflowId := mload(add(rawReport, 4)) - workflowExecutionId := mload(add(rawReport, 36)) // 4 + 32 - } - } - // send a report to targetAddress function report( address targetAddress, bytes calldata data, bytes[] calldata signatures ) external nonReentrant returns (bool) { + require(data.length > 4 + 64, "invalid data length"); + // data is an encoded call with the selector prefixed: (bytes4 selector, bytes report, ...) // we are able to partially decode just the first param, since we don't know the rest bytes memory rawReport = abi.decode(data[4:], (bytes)); @@ -70,12 +39,12 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac // validate signatures for (uint256 i = 0; i < signatures.length; i++) { // TODO: is libocr-style multiple bytes32 arrays more optimal? - (bytes32 r, bytes32 s, uint8 v) = splitSignature(signatures[i]); + (bytes32 r, bytes32 s, uint8 v) = Utils.splitSignature(signatures[i]); address signer = ecrecover(hash, v, r, s); // TODO: we need to store oracle cluster similar to aggregator then, to validate valid signer list } - (bytes32 workflowId, bytes32 workflowExecutionId) = splitReport(rawReport); + (bytes32 workflowId, bytes32 workflowExecutionId) = Utils.splitReport(rawReport); // report was already processed if (s_reports[workflowExecutionId] != address(0)) { diff --git a/contracts/src/v0.8/keystone/libraries/Utils.sol b/contracts/src/v0.8/keystone/libraries/Utils.sol new file mode 100644 index 00000000000..6ec6dcb89a0 --- /dev/null +++ b/contracts/src/v0.8/keystone/libraries/Utils.sol @@ -0,0 +1,37 @@ +// solhint-disable custom-errors +library Utils { + // solhint-disable avoid-low-level-calls, chainlink-solidity/explicit-returns + function splitSignature(bytes memory sig) public pure returns (bytes32 r, bytes32 s, uint8 v) { + require(sig.length == 65, "invalid signature length"); + + assembly { + /* + First 32 bytes stores the length of the signature + + add(sig, 32) = pointer of sig + 32 + effectively, skips first 32 bytes of signature + + mload(p) loads next 32 bytes starting at the memory address p into memory + */ + + // first 32 bytes, after the length prefix + r := mload(add(sig, 32)) + // second 32 bytes + s := mload(add(sig, 64)) + // final byte (first byte of the next 32 bytes) + v := byte(0, mload(add(sig, 96))) + } + + // implicitly return (r, s, v) + } + + // solhint-disable avoid-low-level-calls, chainlink-solidity/explicit-returns + function splitReport(bytes memory rawReport) public pure returns (bytes32 workflowId, bytes32 workflowExecutionId) { + require(rawReport.length > 64, "invalid report length"); + assembly { + // skip first 32 bytes, contains length of the report + workflowId := mload(add(rawReport, 32)) + workflowExecutionId := mload(add(rawReport, 64)) + } + } +} diff --git a/contracts/src/v0.8/keystone/test/KeystoneForwarder.t.sol b/contracts/src/v0.8/keystone/test/KeystoneForwarder.t.sol index 26e8291f9ed..1c47d697c33 100644 --- a/contracts/src/v0.8/keystone/test/KeystoneForwarder.t.sol +++ b/contracts/src/v0.8/keystone/test/KeystoneForwarder.t.sol @@ -4,6 +4,21 @@ pragma solidity ^0.8.19; import "forge-std/Test.sol"; import "../KeystoneForwarder.sol"; +import {Utils} from "../libraries/Utils.sol"; + +contract Receiver { + event MessageReceived(bytes32 indexed workflowId, bytes32 indexed workflowExecutionId, bytes[] mercuryReports); + + constructor() {} + + function foo(bytes calldata rawReport) external { + // decode metadata + (bytes32 workflowId, bytes32 workflowExecutionId) = Utils.splitReport(rawReport); + // parse actual report + bytes[] memory mercuryReports = abi.decode(rawReport[64:], (bytes[])); + emit MessageReceived(workflowId, workflowExecutionId, mercuryReports); + } +} contract KeystoneForwarderTest is Test { function setUp() public virtual {} @@ -11,9 +26,41 @@ contract KeystoneForwarderTest is Test { function test_abi_partial_decoding_works() public { bytes memory report = hex"0102"; uint256 amount = 1; - // bytes memory payload = abi.encodeWithSignature("transfer(bytes,uint256)", report, amount); bytes memory payload = abi.encode(report, amount); bytes memory decodedReport = abi.decode(payload, (bytes)); assertEq(decodedReport, report, "not equal"); } + + function test_it_works() public { + KeystoneForwarder forwarder = new KeystoneForwarder(); + Receiver receiver = new Receiver(); + + // taken from https://github.com/smartcontractkit/chainlink/blob/2390ec7f3c56de783ef4e15477e99729f188c524/core/services/relay/evm/cap_encoder_test.go#L42-L55 + bytes + memory report = hex"6d795f69640000000000000000000000000000000000000000000000000000006d795f657865637574696f6e5f696400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000301020300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004aabbccdd00000000000000000000000000000000000000000000000000000000"; + bytes memory data = abi.encodeWithSignature("foo(bytes)", report); + bytes[] memory signatures = new bytes[](0); + + vm.expectCall(address(receiver), data); + vm.recordLogs(); + + bool delivered1 = forwarder.report(address(receiver), data, signatures); + assertTrue(delivered1, "report not delivered"); + + Vm.Log[] memory entries = vm.getRecordedLogs(); + assertEq(entries[0].emitter, address(receiver)); + // validate workflow id and workflow execution id + bytes32 workflowId = hex"6d795f6964000000000000000000000000000000000000000000000000000000"; + bytes32 executionId = hex"6d795f657865637574696f6e5f69640000000000000000000000000000000000"; + assertEq(entries[0].topics[1], workflowId); + assertEq(entries[0].topics[2], executionId); + bytes[] memory mercuryReports = abi.decode(entries[0].data, (bytes[])); + assertEq(mercuryReports.length, 2); + assertEq(mercuryReports[0], hex"010203"); + assertEq(mercuryReports[1], hex"aabbccdd"); + + // doesn't deliver the same report more than once + bool delivered2 = forwarder.report(address(receiver), data, signatures); + assertFalse(delivered2, "report redelivered"); + } }