[KS-241] Update metadata passed to Forwarder and Receiver

.changeset/twenty-zoos-do.md

@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+#internal Update metadata passed to Forwarder and Receiver

contracts/.changeset/rich-crabs-smoke.md

@@ -0,0 +1,5 @@
+---
+'@chainlink/contracts': patch
+---
+
+#internal Keystone Forwarder and Feeds Consumer

contracts/src/v0.8/keystone/KeystoneFeedsConsumer.sol

@@ -2,27 +2,101 @@
 pragma solidity ^0.8.19;
 
 import {IReceiver} from "./interfaces/IReceiver.sol";
+import {ConfirmedOwner} from "../shared/access/ConfirmedOwner.sol";
 
-contract KeystoneFeedsConsumer is IReceiver {
-  event MessageReceived(bytes32 indexed workflowId, address indexed workflowOwner, uint256 nReports);
-  event FeedReceived(bytes32 indexed feedId, uint256 price, uint64 timestamp);
+contract KeystoneFeedsConsumer is IReceiver, ConfirmedOwner {
+  event FeedReceived(bytes32 indexed feedId, int192 price, uint32 timestamp);
 
-  constructor() {}
+  error UnauthorizedSender(address sender);
+  error UnauthorizedWorkflowOwner(address workflowOwner);
+  error UnauthorizedWorkflowName(bytes10 workflowName);
 
-  struct FeedReport {
-    bytes32 FeedID;
-    uint256 Price;
-    uint64 Timestamp;
+  constructor() ConfirmedOwner(msg.sender) {}
+
+  struct ReceivedFeedReport {
+    bytes32 FeedId;
+    int192 Price;
+    uint32 Timestamp;
+  }
+
+  struct StoredFeedReport {
+    int192 Price;
+    uint32 Timestamp;
+  }
+
+  mapping(bytes32 feedId => StoredFeedReport feedReport) internal s_feedReports;
+  address[] internal s_allowedSendersList;
+  mapping(address => bool) internal s_allowedSenders;
+  address[] internal s_allowedWorkflowOwnersList;
+  mapping(address => bool) internal s_allowedWorkflowOwners;
+  bytes10[] internal s_allowedWorkflowNamesList;
+  mapping(bytes10 => bool) internal s_allowedWorkflowNames;
+
+  function setConfig(
+    address[] calldata _allowedSendersList,
+    address[] calldata _allowedWorkflowOwnersList,
+    bytes10[] calldata _allowedWorkflowNamesList
+  ) external onlyOwner {
+    for (uint32 i = 0; i < s_allowedSendersList.length; i++) {
+      s_allowedSenders[s_allowedSendersList[i]] = false;
+    }
+    for (uint32 i = 0; i < _allowedSendersList.length; i++) {
+      s_allowedSenders[_allowedSendersList[i]] = true;
+    }
+    s_allowedSendersList = _allowedSendersList;
+    for (uint32 i = 0; i < s_allowedWorkflowOwnersList.length; i++) {
+      s_allowedWorkflowOwners[s_allowedWorkflowOwnersList[i]] = false;
+    }
+    for (uint32 i = 0; i < _allowedWorkflowOwnersList.length; i++) {
+      s_allowedWorkflowOwners[_allowedWorkflowOwnersList[i]] = true;
+    }
+    s_allowedWorkflowOwnersList = _allowedWorkflowOwnersList;
+    for (uint32 i = 0; i < s_allowedWorkflowNamesList.length; i++) {
+      s_allowedWorkflowNames[s_allowedWorkflowNamesList[i]] = false;
+    }
+    for (uint32 i = 0; i < _allowedWorkflowNamesList.length; i++) {
+      s_allowedWorkflowNames[_allowedWorkflowNamesList[i]] = true;
+    }
+    s_allowedWorkflowNamesList = _allowedWorkflowNamesList;
   }
 
-  function onReport(bytes32 workflowId, address workflowOwner, bytes calldata rawReport) external {
-    // TODO: validate sender and workflowOwner
+  function onReport(bytes calldata metadata, bytes calldata rawReport) external {
+    if (s_allowedSenders[msg.sender] == false) {
+      revert UnauthorizedSender(msg.sender);
+    }
+
+    (bytes10 workflowName, address workflowOwner) = _getInfo(metadata);
+    if (s_allowedWorkflowNames[workflowName] == false) {
+      revert UnauthorizedWorkflowName(workflowName);
+    }
+    if (s_allowedWorkflowOwners[workflowOwner] == false) {
+      revert UnauthorizedWorkflowOwner(workflowOwner);
+    }
 
-    FeedReport[] memory feeds = abi.decode(rawReport, (FeedReport[]));
+    ReceivedFeedReport[] memory feeds = abi.decode(rawReport, (ReceivedFeedReport[]));
     for (uint32 i = 0; i < feeds.length; i++) {
-      emit FeedReceived(feeds[i].FeedID, feeds[i].Price, feeds[i].Timestamp);
+      s_feedReports[feeds[i].FeedId] = StoredFeedReport(feeds[i].Price, feeds[i].Timestamp);
+      emit FeedReceived(feeds[i].FeedId, feeds[i].Price, feeds[i].Timestamp);
     }
+  }
+
+  // solhint-disable-next-line chainlink-solidity/explicit-returns
+  function _getInfo(bytes memory metadata) internal pure returns (bytes10 workflowName, address workflowOwner) {
+    // (first 32 bytes contain length of the byte array)
+    // workflow_cid             // offset 32, size 32
+    // workflow_name            // offset 64, size 10
+    // workflow_owner           // offset 74, size 20
+    // report_name              // offset 94, size  2
+    assembly {
+      // shift right by 22 bytes to get the actual value
+      workflowName := shr(mul(22, 8), mload(add(metadata, 64)))
+      // shift right by 12 bytes to get the actual value
+      workflowOwner := shr(mul(12, 8), mload(add(metadata, 74)))
+    }
+  }
 
-    emit MessageReceived(workflowId, workflowOwner, feeds.length);
+  function getPrice(bytes32 feedId) external view returns (int192, uint32) {
+    StoredFeedReport memory report = s_feedReports[feedId];
+    return (report.Price, report.Timestamp);
   }
 }

contracts/src/v0.8/keystone/KeystoneForwarder.sol

@@ -17,6 +17,9 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
   /// REPORT_METADATA_LENGTH, which is the minimum length of a report.
   error InvalidReport();
 
+  /// @notice This error is returned when the metadata version is not supported.
+  error InvalidVersion(uint8 version);
+
   /// @notice This error is thrown whenever trying to set a config with a fault
   /// tolerance of 0.
   error FaultToleranceMustBePositive();
@@ -58,9 +61,9 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
   /// @param signature The signature that was invalid
   error InvalidSignature(bytes signature);
 
-  /// @notice This error is thrown whenever a report has already been processed.
-  /// @param reportId The ID of the report that was already processed
-  error ReportAlreadyProcessed(bytes32 reportId);
+  /// @notice This error is thrown whenever a message has already been processed.
+  /// @param messageId The ID of the message that was already processed
+  error AlreadyProcessed(bytes32 messageId);
 
   bool internal s_reentrancyGuard; // guard against reentrancy
 
@@ -83,22 +86,15 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
 
   /// @notice Emitted when a report is processed
   /// @param receiver The address of the receiver contract
-  /// @param workflowOwner The address of the workflow owner
   /// @param workflowExecutionId The ID of the workflow execution
   /// @param result The result of the attempted delivery. True if successful.
-  event ReportProcessed(
-    address indexed receiver,
-    address indexed workflowOwner,
-    bytes32 indexed workflowExecutionId,
-    bool result
-  );
+  event ReportProcessed(address indexed receiver, bytes32 indexed workflowExecutionId, bool result);
 
   constructor() ConfirmedOwner(msg.sender) {}
 
   uint256 internal constant MAX_ORACLES = 31;
-  // 32 bytes for workflowId, 4 bytes for donId, 32 bytes for
-  // workflowExecutionId, 20 bytes for workflowOwner
-  uint256 internal constant REPORT_METADATA_LENGTH = 88;
+  uint256 internal constant METADATA_LENGTH = 109;
+  uint256 internal constant FORWARDER_METADATA_LENGTH = 45;
   uint256 internal constant SIGNATURE_LENGTH = 65;
 
   function setConfig(uint32 donId, uint8 f, address[] calldata signers) external onlyOwner {
@@ -133,17 +129,19 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
     bytes calldata reportContext,
     bytes[] calldata signatures
   ) external nonReentrant {
-    if (rawReport.length < REPORT_METADATA_LENGTH) {
+    if (rawReport.length < METADATA_LENGTH) {
       revert InvalidReport();
     }
 
-    (bytes32 workflowId, uint32 donId, bytes32 workflowExecutionId, address workflowOwner) = _getMetadata(rawReport);
+    (bytes32 workflowExecutionId, uint32 donId /* uint32 donConfigVersion */, , bytes2 reportId) = _getMetadata(
+      rawReport
+    );
 
     // f can never be 0, so this means the config doesn't actually exist
     if (s_configs[donId].f == 0) revert InvalidDonId(donId);
 
-    bytes32 reportId = _reportId(receiverAddress, workflowExecutionId);
-    if (s_reports[reportId].transmitter != address(0)) revert ReportAlreadyProcessed(reportId);
+    bytes32 combinedId = _combinedId(receiverAddress, workflowExecutionId, reportId);
+    if (s_reports[combinedId].transmitter != address(0)) revert AlreadyProcessed(combinedId);
 
     if (s_configs[donId].f + 1 != signatures.length)
       revert InvalidSignatureCount(s_configs[donId].f + 1, signatures.length);
@@ -169,28 +167,37 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
     }
 
     bool success;
-    try IReceiver(receiverAddress).onReport(workflowId, workflowOwner, rawReport[REPORT_METADATA_LENGTH:]) {
+    try
+      IReceiver(receiverAddress).onReport(
+        rawReport[FORWARDER_METADATA_LENGTH:METADATA_LENGTH],
+        rawReport[METADATA_LENGTH:]
+      )
+    {
       success = true;
     } catch {
       // Do nothing, success is already false
     }
 
-    s_reports[reportId] = DeliveryStatus(msg.sender, success);
-
-    emit ReportProcessed(receiverAddress, workflowOwner, workflowExecutionId, success);
+    s_reports[combinedId] = DeliveryStatus(msg.sender, success);
+    emit ReportProcessed(receiverAddress, workflowExecutionId, success);
   }
 
-  function _reportId(address receiver, bytes32 workflowExecutionId) internal pure returns (bytes32) {
+  function _combinedId(address receiver, bytes32 workflowExecutionId, bytes2 reportId) internal pure returns (bytes32) {
     // TODO: gas savings: could we just use a bytes key and avoid another keccak256 call
-    return keccak256(bytes.concat(bytes20(uint160(receiver)), workflowExecutionId));
+    return keccak256(bytes.concat(bytes20(uint160(receiver)), workflowExecutionId, reportId));
   }
 
   // get transmitter of a given report or 0x0 if it wasn't transmitted yet
-  function getTransmitter(address receiver, bytes32 workflowExecutionId) external view returns (address) {
-    bytes32 reportId = _reportId(receiver, workflowExecutionId);
-    return s_reports[reportId].transmitter;
+  function getTransmitter(
+    address receiver,
+    bytes32 workflowExecutionId,
+    bytes2 reportId
+  ) external view returns (address) {
+    bytes32 combinedId = _combinedId(receiver, workflowExecutionId, reportId);
+    return s_reports[combinedId].transmitter;
   }
 
+  // solhint-disable-next-line chainlink-solidity/explicit-returns
   function _splitSignature(bytes memory sig) internal pure returns (bytes32 r, bytes32 s, uint8 v) {
     if (sig.length != SIGNATURE_LENGTH) revert InvalidSignature(sig);
 
@@ -213,20 +220,30 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
     }
   }
 
+  // solhint-disable-next-line chainlink-solidity/explicit-returns
   function _getMetadata(
     bytes memory rawReport
-  ) internal pure returns (bytes32 workflowId, uint32 donId, bytes32 workflowExecutionId, address workflowOwner) {
+  ) internal pure returns (bytes32 workflowExecutionId, uint32 donId, uint32 donConfigVersion, bytes2 reportId) {
+    // (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
+    // don_id                   // offset  69, size  4
+    // don_config_version,	    // offset  73, size  4
+    // workflow_cid             // offset  77, size 32
+    // workflow_name            // offset 109, size 10
+    // workflow_owner           // offset 119, size 20
+    // report_name              // offset 139, size  2
+    if (uint8(rawReport[0]) != 1) {
+      revert InvalidVersion(uint8(rawReport[0]));
+    }
     assembly {
-      // skip first 32 bytes, contains length of the report
-      // first 32 bytes is the workflowId
-      workflowId := mload(add(rawReport, 32))
-      // next 4 bytes is donId. We shift right by 28 bytes to get the actual value
-      donId := shr(mul(28, 8), mload(add(rawReport, 64)))
-      // next 32 bytes is the workflowExecutionId
-      workflowExecutionId := mload(add(rawReport, 68))
-      // next 20 bytes is the workflowOwner. We shift right by 12 bytes to get
-      // the actual value
-      workflowOwner := shr(mul(12, 8), mload(add(rawReport, 100)))
+      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)))
+      reportId := mload(add(rawReport, 139))
     }
   }
 

contracts/src/v0.8/keystone/interfaces/IReceiver.sol

@@ -3,5 +3,5 @@ pragma solidity ^0.8.19;
 
 /// @title IReceiver - receives keystone reports
 interface IReceiver {
-  function onReport(bytes32 workflowId, address workflowOwner, bytes calldata report) external;
+  function onReport(bytes calldata metadata, bytes calldata report) external;
 }

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;

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");
   }
 }