Fix tests

smartcontractkit · Jun 5, 2024 · 3ce2f6d · 3ce2f6d
1 parent 8ed3224
commit 3ce2f6d
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 24 deletions.
diff --git a/contracts/src/v0.8/keystone/KeystoneForwarder.sol b/contracts/src/v0.8/keystone/KeystoneForwarder.sol
@@ -224,7 +224,7 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
   function _getMetadata(
     bytes memory rawReport
   ) internal pure returns (bytes32 workflowExecutionId, uint32 donId, uint32 donConfigVersion, bytes2 reportId) {
-    // (first 32 bytes contain length of the report)
+    // (first 32 bytes of memory contain length of the report)
     // version                  // offset  32, size  1
     // workflow_execution_id    // offset  33, size 32
     // timestamp                // offset  65, size  4
@@ -234,17 +234,16 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
     // workflow_name            // offset 109, size 10
     // workflow_owner           // offset 119, size 20
     // report_name              // offset 139, size  2
-    if (uint8(rawReport[32]) != 1) {
-      revert InvalidVersion(uint8(rawReport[32]));
+    if (uint8(rawReport[0]) != 1) {
+      revert InvalidVersion(uint8(rawReport[0]));
     }
     assembly {
       workflowExecutionId := mload(add(rawReport, 33))
       // shift right by 28 bytes to get the actual value
       donId := shr(mul(28, 8), mload(add(rawReport, 69)))
       // shift right by 28 bytes to get the actual value
       donConfigVersion := shr(mul(28, 8), mload(add(rawReport, 73)))
-      // shift right by 30 bytes to get the actual value
-      reportId := shr(mul(30, 8), mload(add(rawReport, 139)))
+      reportId := mload(add(rawReport, 139))
     }
   }
 

diff --git a/contracts/src/v0.8/keystone/test/KeystoneForwarderBaseTest.t.sol b/contracts/src/v0.8/keystone/test/KeystoneForwarderBaseTest.t.sol
@@ -11,6 +11,7 @@ contract BaseTest is Test {
   uint256 internal constant MAX_ORACLES = 31;
   uint32 internal DON_ID = 0x01020304;
   uint8 internal F = 1;
+  uint32 internal CONFIG_VERSION = 1;
 
   struct Signer {
     uint256 mockPrivateKey;

diff --git a/contracts/src/v0.8/keystone/test/KeystoneForwarder_ReportTest.t.sol b/contracts/src/v0.8/keystone/test/KeystoneForwarder_ReportTest.t.sol
@@ -5,19 +5,20 @@ import {BaseTest} from "./KeystoneForwarderBaseTest.t.sol";
 import {KeystoneForwarder} from "../KeystoneForwarder.sol";
 
 contract KeystoneForwarder_ReportTest is BaseTest {
-  event MessageReceived(bytes32 indexed workflowId, address indexed workflowOwner, bytes[] mercuryReports);
-  event ReportProcessed(
-    address indexed receiver,
-    address indexed workflowOwner,
-    bytes32 indexed workflowExecutionId,
-    bool result
-  );
+  event MessageReceived(bytes metadata, bytes[] mercuryReports);
+  event ReportProcessed(address indexed receiver, bytes32 indexed workflowExecutionId, bool result);
 
+  uint8 internal version = 1;
+  uint32 internal timestamp = 0;
   bytes32 internal workflowId = hex"6d795f6964000000000000000000000000000000000000000000000000000000";
+  bytes10 internal workflowName = hex"000000000000DEADBEEF";
   address internal workflowOwner = address(51);
   bytes32 internal executionId = hex"6d795f657865637574696f6e5f69640000000000000000000000000000000000";
+  bytes2 internal reportId = hex"0001";
   bytes[] internal mercuryReports = new bytes[](2);
   bytes internal rawReports;
+  bytes internal header;
+  bytes internal metadata;
   bytes internal report;
   bytes internal reportContext = new bytes(96);
   uint256 internal requiredSignaturesNum = F + 1;
@@ -32,7 +33,9 @@ contract KeystoneForwarder_ReportTest is BaseTest {
     mercuryReports[1] = hex"aabbccdd";
 
     rawReports = abi.encode(mercuryReports);
-    report = abi.encodePacked(workflowId, DON_ID, executionId, workflowOwner, rawReports);
+    metadata = abi.encodePacked(workflowId, workflowName, workflowOwner, reportId);
+    header = abi.encodePacked(version, executionId, timestamp, DON_ID, CONFIG_VERSION, metadata);
+    report = abi.encodePacked(header, rawReports);
 
     for (uint256 i = 0; i < requiredSignaturesNum; i++) {
       (uint8 v, bytes32 r, bytes32 s) = vm.sign(
@@ -48,10 +51,15 @@ contract KeystoneForwarder_ReportTest is BaseTest {
   function test_RevertWhen_ReportHasIncorrectDON() public {
     uint32 invalidDONId = 111;
     bytes memory reportWithInvalidDONId = abi.encodePacked(
-      workflowId,
-      invalidDONId,
+      version,
       executionId,
+      timestamp,
+      invalidDONId,
+      CONFIG_VERSION,
+      workflowId,
+      workflowName,
       workflowOwner,
+      reportId,
       rawReports
     );
 
@@ -112,11 +120,11 @@ contract KeystoneForwarder_ReportTest is BaseTest {
     s_forwarder.report(address(s_receiver), report, reportContext, signatures);
   }
 
-  function test_RevertWhen_ReportAlreadyProcessed() public {
+  function test_RevertWhen_AlreadyProcessed() public {
     s_forwarder.report(address(s_receiver), report, reportContext, signatures);
-    bytes32 reportId = keccak256(bytes.concat(bytes20(uint160(address(s_receiver))), executionId));
+    bytes32 combinedId = keccak256(bytes.concat(bytes20(uint160(address(s_receiver))), executionId, reportId));
 
-    vm.expectRevert(abi.encodeWithSelector(KeystoneForwarder.ReportAlreadyProcessed.selector, reportId));
+    vm.expectRevert(abi.encodeWithSelector(KeystoneForwarder.AlreadyProcessed.selector, combinedId));
     s_forwarder.report(address(s_receiver), report, reportContext, signatures);
   }
 
@@ -125,15 +133,15 @@ contract KeystoneForwarder_ReportTest is BaseTest {
     // bytes memory report = hex"6d795f6964000000000000000000000000000000000000000000000000000000010203046d795f657865637574696f6e5f696400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000301020300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004aabbccdd00000000000000000000000000000000000000000000000000000000";
 
     vm.expectEmit(address(s_receiver));
-    emit MessageReceived(workflowId, workflowOwner, mercuryReports);
+    emit MessageReceived(metadata, mercuryReports);
 
     vm.expectEmit(address(s_forwarder));
-    emit ReportProcessed(address(s_receiver), workflowOwner, executionId, true);
+    emit ReportProcessed(address(s_receiver), executionId, true);
 
     s_forwarder.report(address(s_receiver), report, reportContext, signatures);
 
     // validate transmitter was recorded
-    address transmitter = s_forwarder.getTransmitter(address(s_receiver), executionId);
+    address transmitter = s_forwarder.getTransmitter(address(s_receiver), executionId, reportId);
     assertEq(transmitter, TRANSMITTER, "transmitter mismatch");
   }
 }
diff --git a/contracts/src/v0.8/keystone/test/mocks/Receiver.sol b/contracts/src/v0.8/keystone/test/mocks/Receiver.sol
@@ -4,13 +4,13 @@ pragma solidity ^0.8.19;
 import {IReceiver} from "../../interfaces/IReceiver.sol";
 
 contract Receiver is IReceiver {
-  event MessageReceived(bytes32 indexed workflowId, address indexed workflowOwner, bytes[] mercuryReports);
+  event MessageReceived(bytes metadata, bytes[] mercuryReports);
 
   constructor() {}
 
-  function onReport(bytes32 workflowId, address workflowOwner, bytes calldata rawReport) external {
+  function onReport(bytes calldata metadata, bytes calldata rawReport) external {
     // parse actual report
     bytes[] memory mercuryReports = abi.decode(rawReport, (bytes[]));
-    emit MessageReceived(workflowId, workflowOwner, mercuryReports);
+    emit MessageReceived(metadata, mercuryReports);
   }
 }