Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keystone: Forwarder contract updates #12962

Merged
merged 3 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/clever-islands-draw.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

#internal
5 changes: 5 additions & 0 deletions contracts/.changeset/quiet-donkeys-hang.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@chainlink/contracts": patch
---

#internal
241 changes: 199 additions & 42 deletions contracts/src/v0.8/keystone/KeystoneForwarder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,75 +2,232 @@
pragma solidity ^0.8.19;

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 {Utils} from "./libraries/Utils.sol";

// solhint-disable gas-custom-errors, no-unused-vars
/// @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 {
error ReentrantCall();

/// @notice This error is returned when the data with report is invalid.
/// This can happen if the data is shorter than SELECTOR_LENGTH + REPORT_LENGTH.
/// @param data the data that was received
error InvalidData(bytes data);
/// @notice This error is returned when the report is shorter than
/// REPORT_METADATA_LENGTH, which is the minimum length of a report.
error InvalidReport();

/// @notice This error is thrown whenever trying to set a config with a fault
/// tolerance of 0.
error FaultToleranceMustBePositive();

/// @notice This error is thrown whenever configuration provides more signers
/// than the maximum allowed number.
/// @param numSigners The number of signers who have signed the report
/// @param maxSigners The maximum number of signers that can sign a report
error ExcessSigners(uint256 numSigners, uint256 maxSigners);

/// @notice This error is thrown whenever a configuration is provided with
/// less than the minimum number of signers.
/// @param numSigners The number of signers provided
/// @param minSigners The minimum number of signers expected
error InsufficientSigners(uint256 numSigners, uint256 minSigners);

/// @notice This error is thrown whenever a duplicate signer address is
/// provided in the configuration.
/// @param signer The signer address that was duplicated.
error DuplicateSigner(address signer);

/// @notice This error is thrown whenever a report has an incorrect number of
/// signatures.
/// @param expected The number of signatures expected, F + 1
/// @param received The number of signatures received
error InvalidSignatureCount(uint256 expected, uint256 received);

/// @notice This error is thrown whenever a report specifies a DON ID that
/// does not have a configuration.
/// @param donId The DON ID that was provided in the report
error InvalidDonId(uint32 donId);

/// @notice This error is thrown whenever a signer address is not in the
/// configuration.
/// @param signer The signer address that was not in the configuration
error InvalidSigner(address signer);

/// @notice This error is thrown whenever a signature is invalid.
/// @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);

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
}

uint256 private constant SELECTOR_LENGTH = 4;
uint256 private constant REPORT_LENGTH = 64;
/// @notice Contains the configuration for each DON ID
mapping(uint32 donId => OracleSet) internal s_configs;
DeividasK marked this conversation as resolved.
Show resolved Hide resolved

struct HotVars {
bool reentrancyGuard; // guard against reentrancy
struct DeliveryStatus {
address transmitter;
bool success;
}

HotVars internal s_hotVars; // Mixture of config and state, commonly accessed
mapping(bytes32 reportId => DeliveryStatus status) internal s_reports;

mapping(bytes32 => address) internal s_reports;
/// @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,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the owner actually an ethereum address? Or is it just some private key? I didn't want to make assumptions here since this also won't be the same address type across all chains

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume 20 bytes gives us enough space to support the "owner" concept more generally. Nothing prevents us from having a different type for other chains as long as it can fit into 20 bytes. We can also pad 🤷‍♂️

I didn't want to add gas costs unnecessarily for the EVM chains (happy path).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd type it as byte20 then to not imply an actual ethereum address -- for me that usually means a wallet or an on chain contact

bytes32 indexed workflowExecutionId,
bool result
);

constructor() ConfirmedOwner(msg.sender) {}

// send a report to targetAddress
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 SIGNATURE_LENGTH = 65;

function setConfig(uint32 donId, uint8 f, address[] calldata signers) external nonReentrant {
if (f == 0) revert FaultToleranceMustBePositive();
if (signers.length > MAX_ORACLES) revert ExcessSigners(signers.length, MAX_ORACLES);
if (signers.length <= 3 * f) revert InsufficientSigners(signers.length, 3 * f + 1);

// TODO: how does setConfig handle expiration? e.g. if the signer set changes
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I typically expect to see a link to the Jira ticket if you leave the TODO in the code. Otherwise, these aren't visible in Jira.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@archseer what do you mean by expiration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In OCR2Aggregator changing the config also expires any past in-flight transmissions since the config digest will change (and needs to match on transmit).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not sending the digest alongside our reports are we? I thought we will only rely purely on signature validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't but the question is do we want something equivalent for expiration?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


// remove any old signer addresses
for (uint256 i; i < s_configs[donId].signers.length; ++i) {
address signer = s_configs[donId].signers[i];
delete s_configs[donId]._positions[signer];
}

// add new signer addresses
s_configs[donId].signers = signers;
for (uint256 i; i < signers.length; ++i) {
// assign indices, detect duplicates
address signer = signers[i];
if (s_configs[donId]._positions[signer] != 0) revert DuplicateSigner(signer);
s_configs[donId]._positions[signer] = uint8(i) + 1;
s_configs[donId].signers.push(signer);
}
s_configs[donId].f = f;
}

// send a report to receiver
function report(
address targetAddress,
bytes calldata data,
address receiverAddress,
bytes calldata rawReport,
bytes[] calldata signatures
) external nonReentrant returns (bool) {
if (data.length < SELECTOR_LENGTH + REPORT_LENGTH) {
revert InvalidData(data);
) external nonReentrant {
if (rawReport.length < REPORT_METADATA_LENGTH) {
revert InvalidReport();
}

// 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));
(bytes32 workflowId, uint32 donId, bytes32 workflowExecutionId, address workflowOwner) = _getMetadata(rawReport);

// TODO: we probably need some type of f value config?
// f can never be 0, so this means the config doesn't actually exist
if (s_configs[donId].f == 0) revert InvalidDonId(donId);

bytes32 hash = keccak256(rawReport);
bytes32 reportId = _reportId(receiverAddress, workflowExecutionId);
if (s_reports[reportId].transmitter != address(0)) revert ReportAlreadyProcessed(reportId);

if (s_configs[donId].f + 1 != signatures.length)
revert InvalidSignatureCount(s_configs[donId].f + 1, signatures.length);

// 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) = 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 hash = keccak256(rawReport);

address[MAX_ORACLES] memory signed;
uint8 index;
for (uint256 i; i < signatures.length; ++i) {
// TODO: is libocr-style multiple bytes32 arrays more optimal, gas-wise?
(bytes32 r, bytes32 s, uint8 v) = _splitSignature(signatures[i]);
address signer = ecrecover(hash, v, r, s);

// validate signer is trusted and signature is unique
index = uint8(s_configs[donId]._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;
}
}

(bytes32 workflowId, bytes32 workflowExecutionId) = Utils._splitReport(rawReport);

// report was already processed
if (s_reports[workflowExecutionId] != address(0)) {
return false;
IReceiver receiver = IReceiver(receiverAddress);
bool success;
try receiver.onReport(workflowId, workflowOwner, rawReport[REPORT_METADATA_LENGTH:]) {
success = true;
} catch {
// Do nothing, success is already false
}

// solhint-disable-next-line avoid-low-level-calls
(bool success, bytes memory result) = targetAddress.call(data);
s_reports[reportId] = DeliveryStatus(msg.sender, success);

s_reports[workflowExecutionId] = msg.sender;
return true;
emit ReportProcessed(receiverAddress, workflowOwner, workflowExecutionId, success);
}

function _reportId(address receiver, bytes32 workflowExecutionId) 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));
}

// get transmitter of a given report or 0x0 if it wasn't transmitted yet
function getTransmitter(bytes32 workflowExecutionId) external view returns (address) {
return s_reports[workflowExecutionId];
function getTransmitter(address receiver, bytes32 workflowExecutionId) external view returns (address) {
bytes32 reportId = _reportId(receiver, workflowExecutionId);
return s_reports[reportId].transmitter;
}

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

function _getMetadata(
bytes memory rawReport
) internal pure returns (bytes32 workflowId, uint32 donId, bytes32 workflowExecutionId, address workflowOwner) {
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)))
}
}

/// @inheritdoc TypeAndVersionInterface
Expand All @@ -82,9 +239,9 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
* @dev replicates Open Zeppelin's ReentrancyGuard but optimized to fit our storage
*/
modifier nonReentrant() {
if (s_hotVars.reentrancyGuard) revert ReentrantCall();
s_hotVars.reentrancyGuard = true;
if (s_reentrancyGuard) revert ReentrantCall();
s_reentrancyGuard = true;
_;
s_hotVars.reentrancyGuard = false;
s_reentrancyGuard = false;
}
}
7 changes: 7 additions & 0 deletions contracts/src/v0.8/keystone/interfaces/IReceiver.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

/// @title IReceiver - receives keystone reports
interface IReceiver {
function onReport(bytes32 workflowId, address workflowOwner, bytes calldata report) external;
}
42 changes: 0 additions & 42 deletions contracts/src/v0.8/keystone/libraries/Utils.sol

This file was deleted.

Loading
Loading