-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
KS-204: Add config count to DON struct #13193
KS-204: Add config count to DON struct #13193
Conversation
847cea4
to
fe863be
Compare
/// @notice Mapping from hashed capability IDs to configs | ||
mapping(bytes32 capabilityId => bytes config) capabilityConfigs; | ||
/// @notice Mapping of config counts to configurations | ||
mapping(uint32 configCount => DONNodeCapabilityConfig donConfig) nodeCapabilityConfigs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking that it may also make sense to have separate configCount
s for the different configs so that we don't have to repopulate everything when updating.
if (s_dons[id].nodeCapabilityConfigs[configCount].nodes.contains(nodeP2PId)) | ||
revert DuplicateDONNode(id, nodeP2PId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably do this validation in memory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can load this into memory as we have a nested mapping
bd56ef1
to
1e2ca8d
Compare
/// @notice Mapping from hashed capability IDs to configs | ||
mapping(bytes32 capabilityId => bytes config) capabilityConfigs; | ||
/// @notice Mapping of config counts to configurations | ||
mapping(uint32 configCount => DONCapabilityConfig donConfig) capabilityConfigs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mapping(uint32 configCount => DONCapabilityConfig donConfig) capabilityConfigs; | |
mapping(uint32 configCount => DONCapabilityConfig donConfig) config; |
WDYT, given that it's not just capability config now but also includes nodes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me
/// @notice DON (Decentralized Oracle Network) is a grouping of nodes that support | ||
// the same capabilities. | ||
struct DON { | ||
/// @notice Computed. Auto-increment. | ||
uint32 id; | ||
/// @notice The number of times the DON's capability was configured |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// @notice The number of times the DON's capability was configured | |
/// @notice The number of times the DON was configured |
@@ -516,11 +523,15 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface { | |||
s_dons[id].id = id; | |||
s_dons[id].isPublic = isPublic; | |||
|
|||
uint32 configCount = 1; | |||
|
|||
DONCapabilityConfig storage donCapabilityConfig = s_dons[id].config[configCount]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DONCapabilityConfig storage donCapabilityConfig = s_dons[id].config[configCount]; | |
DONCapabilityConfig storage donConfig = s_dons[id].config[configCount]; |
Quality Gate passedIssues Measures |
97b84d0
into
use-encode-instead-of-encode-packed-capability-id
PR to add a
configCount
in theDON
struct. This does not change anything functionally but is only used so that we can easily replace an existing config by incrementing theconfigCount
and populating a newEnumerableSet
.