Updates to the Write Target capability (#14350)

* Use ERC165Checker in the forwarder

* Bump gas for routing + move Transmission struct

* Generate things

* Validate report ID in the inputs matches one in Report bytes

* Match report IDs

* Fix error message

* Fix report ID check

* Change validation

* Convert hex to bytes

* Fix all tests 🤞

* Incorporate Blaz's changes

* Remoe poller expectation

* Remove unused import

.changeset/clever-months-brake.md

@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+#internal

contracts/.changeset/quiet-lamps-walk.md

@@ -0,0 +1,5 @@
+---
+'@chainlink/contracts': patch
+---
+
+#internal

contracts/gas-snapshots/keystone.gas-snapshot

@@ -82,11 +82,11 @@ CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_RemovingCapabilityRequiredB
 CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_SignerAddressEmpty() (gas: 29080)
 CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_UpdatingNodeWithoutCapabilities() (gas: 27609)
 CapabilitiesRegistry_UpdateNodesTest:test_UpdatesNodeParams() (gas: 162264)
-KeystoneForwarder_ReportTest:test_Report_ConfigVersion() (gas: 2003568)
-KeystoneForwarder_ReportTest:test_Report_FailedDeliveryWhenReceiverInterfaceNotSupported() (gas: 124908)
+KeystoneForwarder_ReportTest:test_Report_ConfigVersion() (gas: 2006044)
+KeystoneForwarder_ReportTest:test_Report_FailedDeliveryWhenReceiverInterfaceNotSupported() (gas: 124668)
 KeystoneForwarder_ReportTest:test_Report_FailedDeliveryWhenReceiverNotContract() (gas: 126927)
-KeystoneForwarder_ReportTest:test_Report_SuccessfulDelivery() (gas: 361243)
-KeystoneForwarder_ReportTest:test_Report_SuccessfulRetryWithMoreGas() (gas: 501084)
+KeystoneForwarder_ReportTest:test_Report_SuccessfulDelivery() (gas: 362419)
+KeystoneForwarder_ReportTest:test_Report_SuccessfulRetryWithMoreGas() (gas: 483436)
 KeystoneForwarder_ReportTest:test_RevertWhen_AnySignatureIsInvalid() (gas: 86326)
 KeystoneForwarder_ReportTest:test_RevertWhen_AnySignerIsInvalid() (gas: 118521)
 KeystoneForwarder_ReportTest:test_RevertWhen_AttemptingTransmissionWithInsufficientGas() (gas: 96279)
@@ -95,20 +95,20 @@ KeystoneForwarder_ReportTest:test_RevertWhen_ReportHasIncorrectDON() (gas: 75930
 KeystoneForwarder_ReportTest:test_RevertWhen_ReportHasInexistentConfigVersion() (gas: 76298)
 KeystoneForwarder_ReportTest:test_RevertWhen_ReportIsMalformed() (gas: 45563)
 KeystoneForwarder_ReportTest:test_RevertWhen_RetryingInvalidContractTransmission() (gas: 143591)
-KeystoneForwarder_ReportTest:test_RevertWhen_RetryingSuccessfulTransmission() (gas: 354042)
+KeystoneForwarder_ReportTest:test_RevertWhen_RetryingSuccessfulTransmission() (gas: 355218)
 KeystoneForwarder_ReportTest:test_RevertWhen_TooFewSignatures() (gas: 55292)
 KeystoneForwarder_ReportTest:test_RevertWhen_TooManySignatures() (gas: 56050)
 KeystoneForwarder_SetConfigTest:test_RevertWhen_ExcessSigners() (gas: 20184)
 KeystoneForwarder_SetConfigTest:test_RevertWhen_FaultToleranceIsZero() (gas: 88057)
 KeystoneForwarder_SetConfigTest:test_RevertWhen_InsufficientSigners() (gas: 14533)
 KeystoneForwarder_SetConfigTest:test_RevertWhen_NotOwner() (gas: 88766)
-KeystoneForwarder_SetConfigTest:test_RevertWhen_ProvidingDuplicateSigners() (gas: 114570)
-KeystoneForwarder_SetConfigTest:test_RevertWhen_ProvidingZeroAddressSigner() (gas: 114225)
-KeystoneForwarder_SetConfigTest:test_SetConfig_FirstTime() (gas: 1540541)
-KeystoneForwarder_SetConfigTest:test_SetConfig_WhenSignersAreRemoved() (gas: 1535211)
+KeystoneForwarder_SetConfigTest:test_RevertWhen_ProvidingDuplicateSigners() (gas: 114578)
+KeystoneForwarder_SetConfigTest:test_RevertWhen_ProvidingZeroAddressSigner() (gas: 114233)
+KeystoneForwarder_SetConfigTest:test_SetConfig_FirstTime() (gas: 1540665)
+KeystoneForwarder_SetConfigTest:test_SetConfig_WhenSignersAreRemoved() (gas: 1535361)
 KeystoneForwarder_TypeAndVersionTest:test_TypeAndVersion() (gas: 9641)
-KeystoneRouter_SetConfigTest:test_AddForwarder_RevertWhen_NotOwner() (gas: 10978)
-KeystoneRouter_SetConfigTest:test_RemoveForwarder_RevertWhen_NotOwner() (gas: 10923)
-KeystoneRouter_SetConfigTest:test_RemoveForwarder_Success() (gas: 17599)
+KeystoneRouter_SetConfigTest:test_AddForwarder_RevertWhen_NotOwner() (gas: 10982)
+KeystoneRouter_SetConfigTest:test_RemoveForwarder_RevertWhen_NotOwner() (gas: 10927)
+KeystoneRouter_SetConfigTest:test_RemoveForwarder_Success() (gas: 17603)
 KeystoneRouter_SetConfigTest:test_Route_RevertWhen_UnauthorizedForwarder() (gas: 18552)
-KeystoneRouter_SetConfigTest:test_Route_Success() (gas: 79379)
+KeystoneRouter_SetConfigTest:test_Route_Success() (gas: 80568)

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

@@ -99,7 +99,7 @@ contract KeystoneFeedsConsumer is IReceiver, OwnerIsCreator, IERC165 {
     return (report.Price, report.Timestamp);
   }
 
-  function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
-    return interfaceId == this.onReport.selector;
+  function supportsInterface(bytes4 interfaceId) public pure returns (bool) {
+    return interfaceId == type(IReceiver).interfaceId || interfaceId == type(IERC165).interfaceId;
   }
 }

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

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.24;
 
-import {IERC165} from "../vendor/openzeppelin-solidity/v4.8.3/contracts/interfaces/IERC165.sol";
+import {ERC165Checker} from "../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/introspection/ERC165Checker.sol";
 
 import {ITypeAndVersion} from "../shared/interfaces/ITypeAndVersion.sol";
 import {OwnerIsCreator} from "../shared/access/OwnerIsCreator.sol";
@@ -66,6 +66,22 @@ contract KeystoneForwarder is OwnerIsCreator, ITypeAndVersion, IRouter {
     mapping(address signer => uint256 position) _positions; // 1-indexed to detect unset values
   }
 
+  struct Transmission {
+    address transmitter;
+    // This is true if the receiver is not a contract or does not implement the
+    // `IReceiver` interface.
+    bool invalidReceiver;
+    // Whether the transmission attempt was successful. If `false`, the
+    // transmission can be retried with an increased gas limit.
+    bool success;
+    // The amount of gas allocated for the `IReceiver.onReport` call. uint80
+    // allows storing gas for known EVM block gas limits.
+    // Ensures that the minimum gas requested by the user is available during
+    // the transmission attempt. If the transmission fails (indicated by a
+    // `false` success state), it can be retried with an increased gas limit.
+    uint80 gasLimit;
+  }
+
   /// @notice Contains the configuration for each DON ID
   // @param configId (uint64(donId) << 32) | configVersion
   mapping(uint64 configId => OracleSet oracleSet) internal s_configs;
@@ -94,7 +110,7 @@ contract KeystoneForwarder is OwnerIsCreator, ITypeAndVersion, IRouter {
 
   /// @dev The gas we require to revert in case of a revert in the call to the
   /// receiver. This is more than enough and does not attempt to be exact.
-  uint256 internal constant REQUIRED_GAS_FOR_ROUTING = 40_000;
+  uint256 internal constant REQUIRED_GAS_FOR_ROUTING = 60_000;
 
   // ================================================================
   // │                          Router                              │
@@ -131,29 +147,24 @@ contract KeystoneForwarder is OwnerIsCreator, ITypeAndVersion, IRouter {
     s_transmissions[transmissionId].transmitter = transmitter;
     s_transmissions[transmissionId].gasLimit = uint80(gasLimit);
 
-    if (receiver.code.length == 0) {
+    if (!ERC165Checker.supportsInterface(receiver, type(IReceiver).interfaceId)) {
       s_transmissions[transmissionId].invalidReceiver = true;
       return false;
     }
 
-    try IERC165(receiver).supportsInterface(type(IReceiver).interfaceId) {
-      bool success;
-      bytes memory payload = abi.encodeCall(IReceiver.onReport, (metadata, validatedReport));
+    bool success;
+    bytes memory payload = abi.encodeCall(IReceiver.onReport, (metadata, validatedReport));
 
-      assembly {
-        // call and return whether we succeeded. ignore return data
-        // call(gas,addr,value,argsOffset,argsLength,retOffset,retLength)
-        success := call(gasLimit, receiver, 0, add(payload, 0x20), mload(payload), 0x0, 0x0)
-      }
+    assembly {
+      // call and return whether we succeeded. ignore return data
+      // call(gas,addr,value,argsOffset,argsLength,retOffset,retLength)
+      success := call(gasLimit, receiver, 0, add(payload, 0x20), mload(payload), 0x0, 0x0)
+    }
 
-      if (success) {
-        s_transmissions[transmissionId].success = true;
-      }
-      return success;
-    } catch {
-      s_transmissions[transmissionId].invalidReceiver = true;
-      return false;
+    if (success) {
+      s_transmissions[transmissionId].success = true;
     }
+    return success;
   }
 
   function getTransmissionId(

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

@@ -19,22 +19,6 @@ interface IRouter {
     FAILED
   }
 
-  struct Transmission {
-    address transmitter;
-    // This is true if the receiver is not a contract or does not implement the
-    // `IReceiver` interface.
-    bool invalidReceiver;
-    // Whether the transmission attempt was successful. If `false`, the
-    // transmission can be retried with an increased gas limit.
-    bool success;
-    // The amount of gas allocated for the `IReceiver.onReport` call. uint80
-    // allows storing gas for known EVM block gas limits.
-    // Ensures that the minimum gas requested by the user is available during
-    // the transmission attempt. If the transmission fails (indicated by a
-    // `false` success state), it can be retried with an increased gas limit.
-    uint80 gasLimit;
-  }
-
   struct TransmissionInfo {
     bytes32 transmissionId;
     TransmissionState state;

contracts/src/v0.8/keystone/test/mocks/Receiver.sol

@@ -18,7 +18,7 @@ contract Receiver is IReceiver, IERC165 {
     emit MessageReceived(metadata, mercuryReports);
   }
 
-  function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
-    return interfaceId == this.onReport.selector;
+  function supportsInterface(bytes4 interfaceId) public pure returns (bool) {
+    return interfaceId == type(IReceiver).interfaceId || interfaceId == type(IERC165).interfaceId;
   }
 }

core/capabilities/integration_tests/streams_test.go

@@ -73,7 +73,7 @@ func waitForConsumerReports(ctx context.Context, t *testing.T, consumer *feeds_c
 		feedToReport[report.FeedID] = report
 	}
 
-	ctxWithTimeout, cancel := context.WithTimeout(ctx, 5*time.Minute)
+	ctxWithTimeout, cancel := context.WithTimeout(ctx, 1*time.Minute)
 	defer cancel()
 	feedCount := 0
 	for {

core/capabilities/targets/write_target.go

@@ -165,11 +165,24 @@ func evaluate(rawRequest capabilities.CapabilityRequest) (r Request, err error)
 		return r, fmt.Errorf("unsupported report version: %d", reportMetadata.Version)
 	}
 
-	if hex.EncodeToString(reportMetadata.WorkflowExecutionID[:]) != rawRequest.Metadata.WorkflowExecutionID ||
-		hex.EncodeToString(reportMetadata.WorkflowOwner[:]) != rawRequest.Metadata.WorkflowOwner ||
-		hex.EncodeToString(reportMetadata.WorkflowName[:]) != rawRequest.Metadata.WorkflowName ||
-		hex.EncodeToString(reportMetadata.WorkflowCID[:]) != rawRequest.Metadata.WorkflowID {
-		return r, fmt.Errorf("report metadata does not match request metadata. reportMetadata: %+v, requestMetadata: %+v", reportMetadata, rawRequest.Metadata)
+	if hex.EncodeToString(reportMetadata.WorkflowExecutionID[:]) != rawRequest.Metadata.WorkflowExecutionID {
+		return r, fmt.Errorf("WorkflowExecutionID in the report does not match WorkflowExecutionID in the request metadata. Report WorkflowExecutionID: %+v, request WorkflowExecutionID: %+v", reportMetadata.WorkflowExecutionID, rawRequest.Metadata.WorkflowExecutionID)
+	}
+
+	if hex.EncodeToString(reportMetadata.WorkflowOwner[:]) != rawRequest.Metadata.WorkflowOwner {
+		return r, fmt.Errorf("WorkflowOwner in the report does not match WorkflowOwner in the request metadata. Report WorkflowOwner: %+v, request WorkflowOwner: %+v", reportMetadata.WorkflowOwner, rawRequest.Metadata.WorkflowOwner)
+	}
+
+	if hex.EncodeToString(reportMetadata.WorkflowName[:]) != rawRequest.Metadata.WorkflowName {
+		return r, fmt.Errorf("WorkflowName in the report does not match WorkflowName in the request metadata. Report WorkflowName: %+v, request WorkflowName: %+v", reportMetadata.WorkflowName, rawRequest.Metadata.WorkflowName)
+	}
+
+	if hex.EncodeToString(reportMetadata.WorkflowCID[:]) != rawRequest.Metadata.WorkflowID {
+		return r, fmt.Errorf("WorkflowID in the report does not match WorkflowID in the request metadata. Report WorkflowID: %+v, request WorkflowID: %+v", reportMetadata.WorkflowCID, rawRequest.Metadata.WorkflowID)
+	}
+
+	if !bytes.Equal(reportMetadata.ReportID[:], r.Inputs.SignedReport.ID) {
+		return r, fmt.Errorf("ReportID in the report does not match ReportID in the inputs. reportMetadata.ReportID: %x, Inputs.SignedReport.ID: %x", reportMetadata.ReportID, r.Inputs.SignedReport.ID)
 	}
 
 	return r, nil

core/capabilities/targets/write_target_test.go

@@ -41,6 +41,7 @@ func TestWriteTarget(t *testing.T) {
 	})
 	require.NoError(t, err)
 
+	reportID := [2]byte{0x00, 0x01}
 	reportMetadata := targets.ReportV1Metadata{
 		Version:             1,
 		WorkflowExecutionID: [32]byte{},
@@ -50,7 +51,7 @@ func TestWriteTarget(t *testing.T) {
 		WorkflowCID:         [32]byte{},
 		WorkflowName:        [10]byte{},
 		WorkflowOwner:       [20]byte{},
-		ReportID:            [2]byte{},
+		ReportID:            reportID,
 	}
 
 	reportMetadataBytes, err := reportMetadata.Encode()
@@ -60,6 +61,8 @@ func TestWriteTarget(t *testing.T) {
 		"signed_report": map[string]any{
 			"report":     reportMetadataBytes,
 			"signatures": [][]byte{},
+			"context":    []byte{4, 5},
+			"id":         reportID[:],
 		},
 	})
 	require.NoError(t, err)