-
Notifications
You must be signed in to change notification settings - Fork 24
Implement types and pallet storage for Intents and IntentGroups #2560
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
Implement types and pallet storage for Intents and IntentGroups #2560
Conversation
aramikm
left a comment
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.
Looks good and some tests will need to get fixed.
|
|
||
| /// Maximum number of Intents that can be registered | ||
| #[pallet::constant] | ||
| type MaxIntentRegistrations: Get<IntentId>; |
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.
nit: This is not used anywhere.
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.
It will be once the extrinsics are implemented.
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.
Q for the group: I implemented MaxIntentRegistrations because there was already a corresponding MaxSchemaRegistrations. However, given that both ID types are u16, and the constant is currently set to 65_000 (close enough to max u16), I wonder if it would make sense to allow 65_536 registrations and just check for an add overflow when adding a new schema? Would eliminate the need for both constants. Not a runtime or storage efficiency change, but just less design complexity...
...or, if we want to keep the constant, should we just use the same constant for both?
Thoughts?
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.
Some updates to the design doc based on a Slack conversation with @wesbiggs. Mainly renaming DelegationGroup => IntentGroup
saraswatpuneet
left a comment
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.
lgtm
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
88d6d93
into
feat/schemas-permissions-development
# Goal The goal of this PR is to implement *only* the *types* and *pallet storage* for the following new on-chain entities: * Intents * IntentGroups Closes #2561 **NOTE:** No tests or benchmarks added in this PR, as it only contains types and storage, no executable runtime code (coming in a separate PR) # Checklist - [x] Updated Pallet Readme? - [ ] Updated js/api-augment for Custom RPC APIs? - [x] Design doc(s) updated? - [ ] Unit Tests added? - [ ] e2e Tests added? - [ ] Benchmarks added? - [ ] Spec version incremented? # Conflicts: # runtime/frequency/src/lib.rs
# Goal The goal of this PR is to implement *only* the *types* and *pallet storage* for the following new on-chain entities: * Intents * IntentGroups Closes #2561 **NOTE:** No tests or benchmarks added in this PR, as it only contains types and storage, no executable runtime code (coming in a separate PR) # Checklist - [x] Updated Pallet Readme? - [ ] Updated js/api-augment for Custom RPC APIs? - [x] Design doc(s) updated? - [ ] Unit Tests added? - [ ] e2e Tests added? - [ ] Benchmarks added? - [ ] Spec version incremented? # Conflicts: # pallets/schemas/src/lib.rs # pallets/schemas/src/tests/other_tests.rs # pallets/schemas/src/types.rs # runtime/frequency/src/lib.rs
Goal
The goal of this PR is to implement only the types and pallet storage for the following new on-chain entities:
Closes #2561
NOTE: No tests or benchmarks added in this PR, as it only contains types and storage, no executable runtime code (coming in a separate PR)
Checklist