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: add router and other refactors #13426

Merged
merged 51 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
a99dc5f
keystone: Split forwarder into two contracts
archseer Jun 5, 2024
bb12d5f
keystone: Add a ConfigSet event
archseer Jun 7, 2024
b92b116
add events to router, further changes
archseer Jun 7, 2024
98d50f1
forwarder: Use OwnerIsCreator, ITypeAndVersion
archseer Jun 7, 2024
993075f
Remove TODO
archseer Jun 7, 2024
703bd0f
Replace _splitSignature with code
archseer Jun 7, 2024
0dd9de0
minor gas savings by caching config var
archseer Jun 7, 2024
1597c47
Cache array length
archseer Jun 7, 2024
4ecff4c
Remove TODO
archseer Jun 7, 2024
9429f75
Port over reentrancy guard optimization
archseer Jun 7, 2024
2c1557f
Don't assign signers twice
archseer Jun 7, 2024
04a24c2
Merge 2c1557f4b89fdde917f9585aef2bb6b9cff0f226 into 066afc0877a9e953b…
archseer Jun 7, 2024
4fb78db
Update gethwrappers
app-token-issuer-infra-releng[bot] Jun 7, 2024
cb4e203
Address some feedback on KeystoneFeedsConsumer
archseer Jun 7, 2024
524d92e
Inline signer in setConfig/clearConfig
archseer Jun 7, 2024
f88b268
Merge 524d92edceca98f0ddc9ac265186328bb727d485 into 066afc0877a9e953b…
archseer Jun 7, 2024
554a211
Update gethwrappers
app-token-issuer-infra-releng[bot] Jun 7, 2024
c065972
Remove unused import
archseer Jun 7, 2024
534fb23
prettier format
archseer Jun 7, 2024
8788619
Generate bindings for router too
archseer Jun 7, 2024
741aaac
Merge 87886191b955643e886aad4922086cf618dbb43d into 066afc0877a9e953b…
archseer Jun 7, 2024
ee20856
Update gethwrappers
app-token-issuer-infra-releng[bot] Jun 7, 2024
f721969
Fix keystone scripts
archseer Jun 7, 2024
6302a2c
Add KeystoneRouter tests
archseer Jun 7, 2024
562947d
Update snapshot
DeividasK Jun 7, 2024
87c1499
Remove redundant operations
DeividasK Jun 7, 2024
a4bb4fe
Remove more variables
DeividasK Jun 7, 2024
34af340
Remove another unnecessary conversion
DeividasK Jun 7, 2024
cedb25c
Remove unnecessary version check
DeividasK Jun 7, 2024
d3c9c39
ConfigId bytes32 => uint64
DeividasK Jun 7, 2024
9442537
Slight improvement to error path with duplicate signer
DeividasK Jun 7, 2024
4840536
Optimize clearConfig function
DeividasK Jun 7, 2024
7ea86d0
More micro optimizations
DeividasK Jun 7, 2024
8bd2537
Inline functions
DeividasK Jun 7, 2024
e03a7b8
Remove dangling error
DeividasK Jun 7, 2024
385a346
Move things around
DeividasK Jun 7, 2024
13731d1
Add reportId to ReportProcessed
DeividasK Jun 7, 2024
357bd54
Handle empty receiver address case
DeividasK Jun 7, 2024
b4f9481
Merge remote-tracking branch 'origin/develop' into keystone-router-co…
DeividasK Jun 7, 2024
75196ea
Side: update "f" value in CapabilityRegistry
DeividasK Jun 6, 2024
fcffda8
Nit: import order
DeividasK Jun 7, 2024
86bd0a3
Style Guide: uint256 i = 0
DeividasK Jun 7, 2024
fc64eb4
Add forwarder and router getters
DeividasK Jun 7, 2024
9172e6f
Add changesets
DeividasK Jun 7, 2024
4de76c3
Merge branch 'develop' into keystone-router-contract
archseer Jun 10, 2024
c1ffbfc
Remove unused error
archseer Jun 10, 2024
f1f81c0
Merge c1ffbfcc7d55d688c90588d55125850facd4b1a4 into 8c98c80376c3b6d72…
archseer Jun 10, 2024
434ab51
Update gethwrappers
app-token-issuer-infra-releng[bot] Jun 10, 2024
b36fe56
receiverAddress -> receiver
archseer Jun 10, 2024
0857c79
Merge b36fe56d826a39bec616a6d3b504b0b4e7d6ed8f into 8c98c80376c3b6d72…
archseer Jun 10, 2024
2df7de5
Update gethwrappers
app-token-issuer-infra-releng[bot] Jun 10, 2024
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/shy-deers-hope.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/new-bugs-draw.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

@archseer You only need one of these changesets I think :)

'@chainlink/contracts': patch
---

#internal
178 changes: 92 additions & 86 deletions contracts/gas-snapshots/keystone.gas-snapshot

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions contracts/scripts/native_solc_compile_all_keystone
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,5 @@ compileContract () {

compileContract keystone/CapabilityRegistry.sol
compileContract keystone/KeystoneForwarder.sol
compileContract keystone/KeystoneRouter.sol
compileContract keystone/OCR3Capability.sol
12 changes: 6 additions & 6 deletions contracts/src/v0.8/keystone/CapabilityRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
/// @notice The f value for the DON. This is the number of faulty nodes
/// that the DON can tolerate. This can be different from the f value of
/// the OCR instances that capabilities spawn.
uint32 f;
uint8 f;
/// @notice True if the DON is public. A public DON means that it accepts
/// external capability requests
bool isPublic;
Expand All @@ -177,7 +177,7 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
/// @notice The f value for the DON. This is the number of faulty nodes
/// that the DON can tolerate. This can be different from the f value of
/// the OCR instances that capabilities spawn.
uint32 f;
uint8 f;
/// @notice True if the DON is public. A public DON means that it accepts
/// external capability requests
bool isPublic;
Expand All @@ -196,7 +196,7 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
uint32 configCount;
bool isPublic;
bool acceptsWorkflows;
uint32 f;
uint8 f;
}

/// @notice This error is thrown when a caller is not allowed
Expand Down Expand Up @@ -265,7 +265,7 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
/// fault tolerance value.
/// @param f The proposed fault tolerance value
/// @param nodeCount The proposed number of nodes in the DON
error InvalidFaultTolerance(uint32 f, uint256 nodeCount);
error InvalidFaultTolerance(uint8 f, uint256 nodeCount);

/// @notice This error is thrown when a capability with the provided hashed ID is
/// not found.
Expand Down Expand Up @@ -689,7 +689,7 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
CapabilityConfiguration[] calldata capabilityConfigurations,
bool isPublic,
bool acceptsWorkflows,
uint32 f
uint8 f
) external onlyOwner {
uint32 id = s_nextDONId++;
s_dons[id].id = id;
Expand All @@ -715,7 +715,7 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
CapabilityConfiguration[] calldata capabilityConfigurations,
bool isPublic,
bool acceptsWorkflows,
uint32 f
uint8 f
) external onlyOwner {
uint32 configCount = s_dons[donId].configCount;
if (configCount == 0) revert DONDoesNotExist(donId);
Expand Down
32 changes: 15 additions & 17 deletions contracts/src/v0.8/keystone/KeystoneFeedsConsumer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Copy link
Collaborator

@DeividasK DeividasK Jun 7, 2024

Choose a reason for hiding this comment

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

We should drop this contract or merge it with the mock one we have for tests. I'll skip the review of the rest of the parts (the checks aren't reflective of what would happen in practice as they don't optimize storage reads).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not just an example contract and will be used by CCIP, maybe we can drop the mock instead

Copy link
Contributor

@bolekk bolekk Jun 7, 2024

Choose a reason for hiding this comment

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

@archseer it actually won't be used as-is by CCIP in prod, I was wrong about that.
However, I'd rather still keep it here as an example template on how to implement a basic consumer.


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;
Expand All @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect place for enumerableSet

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);
}
Expand Down
Loading
Loading