2581 update messages pallet to support intents #2616
+971
−238
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
github-actions
bot
added
metadata-changed
JoeCap08055
commented
Sep 30, 2025
There was a problem hiding this comment.
Made the following enhancements for try-runtime-* targets:
- Only build the runtime, not the entire node; speeds up the build
- Consolidate targets using patterns to avoid duplicated targets/commands
- Fetch state for a minimal set of system pallets so that multi-block migrations can be properly tested
- Allow the use of the
PALLETSenv var to specify additional pallets for dumping/restoring state
JoeCap08055
commented
Sep 30, 2025
There was a problem hiding this comment.
Revert types here; need to remain compatible with existing custom RPC
JoeCap08055
commented
Sep 30, 2025
Comment on lines
-82
to
-87
| // Max payload size was picked specifically to be large enough to accommodate | ||
| // a CIDv1 using SHA2-256, but too small to accommodate CIDv1 w/SHA2-512. | ||
| // This is purely so that we can test the error condition. Real world configuration | ||
| // should have this set large enough to accommodate the largest possible CID. | ||
| // Take care when adding new tests for on-chain (not IPFS) messages that the payload | ||
| // is not too big. |
There was a problem hiding this comment.
This was an erroneous assumption. Since the configured value would never be this small on mainnet or testnet, it's better to allow a CIDv1/SHA2-512 by size, and have it fail for CID validation instead of for size reasons.
JoeCap08055
commented
Sep 30, 2025
| /// Helper function to generate message payloads | ||
| /// | ||
| /// # Arguments | ||
| /// * `num_items` - Number of message to include in payload |
There was a problem hiding this comment.
This seemed overly complicated--a payload is just an array of bytes; why all the JSON stuff?
github-actions
bot
removed
metadata-changed
JoeCap08055
commented
Sep 30, 2025
Comment on lines
+393
to
+394
| let cid = Cid::read_bytes(&cid_b[..]).map_err(|_| Error::<T>::InvalidCid)?; | ||
| ensure!([SHA2_256, BLAKE3].contains(&cid.hash().code()), Error::<T>::InvalidCid); |
There was a problem hiding this comment.
When cleaning up the mocks for the tests, noticed that it was attempting to test failure to accept CIDv1 (SHA2-512) by setting the max payload length artificially small.
The more correct approach (and what was missing from the code, eclipsed by that artificial test scenario) is to reject any CIDs with an unsupported hash. DSNP supports ONLY SHA2-256 and Blake3.
github-actions
bot
added
metadata-changed
github-actions
bot
added
metadata-changed
JoeCap08055
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal
The goal of this PR is to migrate the
messagespallet to addschema_idto message storage, and useintent_idas the index instead.Existing index values do not need to be changed, as the
schemaspallet migration ensures that the chain is seeded with Intent IDs that match existing Schema IDsRelated to #2581
Checklist