Keystone: add router and other refactors

contracts/scripts/native_solc_compile_all_keystone

@@ -30,4 +30,5 @@ compileContract () {
 
 compileContract keystone/CapabilityRegistry.sol
 compileContract keystone/KeystoneForwarder.sol
+compileContract keystone/KeystoneRouter.sol
 compileContract keystone/OCR3Capability.sol

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

@@ -2,17 +2,15 @@
 pragma solidity ^0.8.19;
 
 import {IReceiver} from "./interfaces/IReceiver.sol";
-import {ConfirmedOwner} from "../shared/access/ConfirmedOwner.sol";
+import {OwnerIsCreator} from "../shared/access/OwnerIsCreator.sol";
 
-contract KeystoneFeedsConsumer is IReceiver, ConfirmedOwner {
+contract KeystoneFeedsConsumer is IReceiver, OwnerIsCreator {
   event FeedReceived(bytes32 indexed feedId, int192 price, uint32 timestamp);
 
   error UnauthorizedSender(address sender);
   error UnauthorizedWorkflowOwner(address workflowOwner);
   error UnauthorizedWorkflowName(bytes10 workflowName);
 
-  constructor() ConfirmedOwner(msg.sender) {}
-
   struct ReceivedFeedReport {
     bytes32 FeedId;
     int192 Price;
@@ -26,55 +24,55 @@ contract KeystoneFeedsConsumer is IReceiver, ConfirmedOwner {
 
   mapping(bytes32 feedId => StoredFeedReport feedReport) internal s_feedReports;
   address[] internal s_allowedSendersList;
-  mapping(address => bool) internal s_allowedSenders;
+  mapping(address sender => bool) internal s_allowedSenders;
   address[] internal s_allowedWorkflowOwnersList;
-  mapping(address => bool) internal s_allowedWorkflowOwners;
+  mapping(address owner => bool) internal s_allowedWorkflowOwners;
   bytes10[] internal s_allowedWorkflowNamesList;
-  mapping(bytes10 => bool) internal s_allowedWorkflowNames;
+  mapping(bytes10 workflowName => 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++) {
+    for (uint32 i = 0; i < s_allowedSendersList.length; ++i) {
       s_allowedSenders[s_allowedSendersList[i]] = false;
     }
-    for (uint32 i = 0; i < _allowedSendersList.length; i++) {
+    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++) {
+    for (uint32 i = 0; i < s_allowedWorkflowOwnersList.length; ++i) {
       s_allowedWorkflowOwners[s_allowedWorkflowOwnersList[i]] = false;
     }
-    for (uint32 i = 0; i < _allowedWorkflowOwnersList.length; i++) {
+    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++) {
+    for (uint32 i = 0; i < s_allowedWorkflowNamesList.length; ++i) {
       s_allowedWorkflowNames[s_allowedWorkflowNamesList[i]] = false;
     }
-    for (uint32 i = 0; i < _allowedWorkflowNamesList.length; i++) {
+    for (uint32 i = 0; i < _allowedWorkflowNamesList.length; ++i) {
       s_allowedWorkflowNames[_allowedWorkflowNamesList[i]] = true;
     }
     s_allowedWorkflowNamesList = _allowedWorkflowNamesList;
   }
 
   function onReport(bytes calldata metadata, bytes calldata rawReport) external {
-    if (s_allowedSenders[msg.sender] == false) {
+    if (!s_allowedSenders[msg.sender]) {
       revert UnauthorizedSender(msg.sender);
     }
 
     (bytes10 workflowName, address workflowOwner) = _getInfo(metadata);
-    if (s_allowedWorkflowNames[workflowName] == false) {
+    if (!s_allowedWorkflowNames[workflowName]) {
       revert UnauthorizedWorkflowName(workflowName);
     }
-    if (s_allowedWorkflowOwners[workflowOwner] == false) {
+    if (!s_allowedWorkflowOwners[workflowOwner]) {
       revert UnauthorizedWorkflowOwner(workflowOwner);
     }
 
     ReceivedFeedReport[] memory feeds = abi.decode(rawReport, (ReceivedFeedReport[]));
-    for (uint32 i = 0; i < feeds.length; i++) {
+    for (uint256 i = 0; i < feeds.length; ++i) {
       s_feedReports[feeds[i].FeedId] = StoredFeedReport(feeds[i].Price, feeds[i].Timestamp);
       emit FeedReceived(feeds[i].FeedId, feeds[i].Price, feeds[i].Timestamp);
     }

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

@@ -1,16 +1,17 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.19;
 
+import {IRouter} from "./interfaces/IRouter.sol";
 import {IForwarder} from "./interfaces/IForwarder.sol";
-import {IReceiver} from "./interfaces/IReceiver.sol";
-import {ConfirmedOwner} from "../shared/access/ConfirmedOwner.sol";
-import {TypeAndVersionInterface} from "../interfaces/TypeAndVersionInterface.sol";
+
+import {OwnerIsCreator} from "../shared/access/OwnerIsCreator.sol";
+import {ITypeAndVersion} from "../shared/interfaces/ITypeAndVersion.sol";
 
 /// @notice This is an entry point for `write_${chain}` Target capability. It
 /// allows nodes to determine if reports have been processed (successfully or
 /// not) in a decentralized and product-agnostic way by recording processed
 /// reports.
-contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterface {
+contract KeystoneForwarder is IForwarder, OwnerIsCreator, ITypeAndVersion {
   error ReentrantCall();
 
   /// @notice This error is returned when the report is shorter than
@@ -66,33 +67,32 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
   /// @param messageId The ID of the message that was already processed
   error AlreadyProcessed(bytes32 messageId);
 
-  bool internal s_reentrancyGuard; // guard against reentrancy
-
   /// @notice Contains the signing address of each oracle
   struct OracleSet {
     uint8 f; // Number of faulty nodes allowed
     address[] signers;
     mapping(address => uint256) _positions; // 1-indexed to detect unset values
   }
 
+  address internal s_router;
+
   /// @notice Contains the configuration for each DON ID
   // @param configId keccak256(donId, donConfigVersion)
   mapping(bytes32 configId => OracleSet) internal s_configs;
 
-  struct DeliveryStatus {
-    address transmitter;
-    bool success;
-  }
-
-  mapping(bytes32 reportId => DeliveryStatus status) internal s_reports;
+  event ConfigSet(uint32 indexed donId, uint32 indexed configVersion, uint8 f, address[] signers);
 
   /// @notice Emitted when a report is processed
   /// @param receiver The address of the receiver contract
   /// @param workflowExecutionId The ID of the workflow execution
   /// @param result The result of the attempted delivery. True if successful.
   event ReportProcessed(address indexed receiver, bytes32 indexed workflowExecutionId, bool result);
 
-  constructor() ConfirmedOwner(msg.sender) {}
+  string public constant override typeAndVersion = "KeystoneForwarder 1.0.0";
+
+  constructor(address router) OwnerIsCreator() {
+    s_router = router;
+  }
 
   uint256 internal constant MAX_ORACLES = 31;
   uint256 internal constant METADATA_LENGTH = 109;
@@ -108,32 +108,33 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
 
     // remove any old signer addresses
     for (uint256 i; i < s_configs[configId].signers.length; ++i) {
-      address signer = s_configs[configId].signers[i];
-      delete s_configs[configId]._positions[signer];
+      delete s_configs[configId]._positions[s_configs[configId].signers[i]];
     }
 
     // add new signer addresses
     s_configs[configId].signers = signers;
-    for (uint256 i; i < signers.length; ++i) {
+    for (uint256 i = 0; i < signers.length; ++i) {
       // assign indices, detect duplicates
       address signer = signers[i];
       if (s_configs[configId]._positions[signer] != 0) revert DuplicateSigner(signer);
       s_configs[configId]._positions[signer] = uint8(i) + 1;
-      s_configs[configId].signers.push(signer);
     }
     s_configs[configId].f = f;
+
+    emit ConfigSet(donId, configVersion, f, signers);
   }
 
   function clearConfig(uint32 donId, uint32 configVersion) external onlyOwner {
     bytes32 configId = keccak256(abi.encode(donId, configVersion));
 
     // remove any old signer addresses
-    for (uint256 i; i < s_configs[configId].signers.length; ++i) {
-      address signer = s_configs[configId].signers[i];
-      delete s_configs[configId]._positions[signer];
+    for (uint256 i = 0; i < s_configs[configId].signers.length; ++i) {
+      delete s_configs[configId]._positions[s_configs[configId].signers[i]];
     }
 
     s_configs[configId].f = 0;
+
+    emit ConfigSet(donId, configVersion, 0, new address[](0));
   }
 
   // send a report to receiver
@@ -142,101 +143,82 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
     bytes calldata rawReport,
     bytes calldata reportContext,
     bytes[] calldata signatures
-  ) external nonReentrant {
+  ) external {
     if (rawReport.length < METADATA_LENGTH) {
       revert InvalidReport();
     }
 
     bytes32 workflowExecutionId;
-    bytes2 reportId;
-    bytes32 configId;
+    bytes32 combinedId;
     {
-      uint32 donId;
-      uint32 configVersion;
-      (workflowExecutionId, donId, configVersion, reportId) = _getMetadata(rawReport);
+      bytes32 configId;
+      OracleSet storage config;
+      {
+        uint32 donId;
+        uint32 configVersion;
+        bytes2 reportId;
+        (workflowExecutionId, donId, configVersion, reportId) = _getMetadata(rawReport);
 
-      configId = keccak256(abi.encode(donId, configVersion));
+        configId = keccak256(abi.encode(donId, configVersion));
 
-      uint8 f = s_configs[configId].f;
-      // f can never be 0, so this means the config doesn't actually exist
-      if (f == 0) revert InvalidConfig(donId, configVersion);
-      if (f + 1 != signatures.length) revert InvalidSignatureCount(f + 1, signatures.length);
-    }
+        config = s_configs[configId];
 
-    bytes32 combinedId = _combinedId(receiverAddress, workflowExecutionId, reportId);
-    if (s_reports[combinedId].transmitter != address(0)) revert AlreadyProcessed(combinedId);
+        uint8 f = config.f;
+        // f can never be 0, so this means the config doesn't actually exist
+        if (f == 0) revert InvalidConfig(donId, configVersion);
+        if (f + 1 != signatures.length) revert InvalidSignatureCount(f + 1, signatures.length);
 
-    // validate signatures
-    {
-      bytes32 completeHash = keccak256(abi.encodePacked(keccak256(rawReport), reportContext));
-
-      address[MAX_ORACLES] memory signed;
-      uint8 index;
-      for (uint256 i; i < signatures.length; ++i) {
-        (bytes32 r, bytes32 s, uint8 v) = _splitSignature(signatures[i]);
-        address signer = ecrecover(completeHash, v + 27, r, s);
-
-        // validate signer is trusted and signature is unique
-        index = uint8(s_configs[configId]._positions[signer]);
-        if (index == 0) revert InvalidSigner(signer); // index is 1-indexed so we can detect unset signers
-        index -= 1;
-        if (signed[index] != address(0)) revert DuplicateSigner(signer);
-        signed[index] = signer;
+        combinedId = _combinedId(receiverAddress, workflowExecutionId, reportId);
       }
-    }
 
-    bool success;
-    try
-      IReceiver(receiverAddress).onReport(
-        rawReport[FORWARDER_METADATA_LENGTH:METADATA_LENGTH],
-        rawReport[METADATA_LENGTH:]
-      )
-    {
-      success = true;
-    } catch {
-      // Do nothing, success is already false
+      // validate signatures
+      {
+        uint256 numSignatures = signatures.length;
+        bytes32 completeHash = keccak256(abi.encodePacked(keccak256(rawReport), reportContext));
+
+        address[MAX_ORACLES] memory signed;
+        uint8 index;
+        bytes calldata signature;
+        for (uint256 i; i < numSignatures; ++i) {
+          signature = signatures[i];
+          if (signature.length != SIGNATURE_LENGTH) revert InvalidSignature(signature);
+          bytes32 r = bytes32(signature[0:32]);
+          bytes32 s = bytes32(signature[32:64]);
+          uint8 v = uint8(signature[64]);
+          address signer = ecrecover(completeHash, v + 27, r, s);
+
+          // validate signer is trusted and signature is unique
+          index = uint8(config._positions[signer]);
+          if (index == 0) revert InvalidSigner(signer); // index is 1-indexed so we can detect unset signers
+          index -= 1;
+          if (signed[index] != address(0)) revert DuplicateSigner(signer);
+          signed[index] = signer;
+        }
+      }
     }
 
-    s_reports[combinedId] = DeliveryStatus(msg.sender, success);
+    bool success = IRouter(s_router).route(
+      combinedId,
+      msg.sender,
+      receiverAddress,
+      rawReport[FORWARDER_METADATA_LENGTH:METADATA_LENGTH],
+      rawReport[METADATA_LENGTH:]
+    );
+
     emit ReportProcessed(receiverAddress, workflowExecutionId, success);
   }
 
   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, reportId));
   }
 
-  // get transmitter of a given report or 0x0 if it wasn't transmitted yet
   function getTransmitter(
-    address receiver,
+    address receiverAddress,
     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);
-
-    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)))
-    }
+    bytes32 combinedId = _combinedId(receiverAddress, workflowExecutionId, reportId);
+    return IRouter(s_router).getTransmitter(combinedId);
   }
 
   // solhint-disable-next-line chainlink-solidity/explicit-returns
@@ -265,19 +247,4 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
       reportId := mload(add(rawReport, 139))
     }
   }
-
-  /// @inheritdoc TypeAndVersionInterface
-  function typeAndVersion() external pure override returns (string memory) {
-    return "KeystoneForwarder 1.0.0";
-  }
-
-  /**
-   * @dev replicates Open Zeppelin's ReentrancyGuard but optimized to fit our storage
-   */
-  modifier nonReentrant() {
-    if (s_reentrancyGuard) revert ReentrantCall();
-    s_reentrancyGuard = true;
-    _;
-    s_reentrancyGuard = false;
-  }
 }