Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Version-index selection doesn’t validate index bounds, which can produce undefined defs and fail with a non-actionable runtime error on invalid metadata.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes extrinsic signed-extension selection to respect versioned signed extension metadata (v16+), preventing initialization of extensions that are not part of the version-0 (v4 extrinsic) extension set and avoiding runtime signing failures on Asset Hub V16.
Changes:
- Update
ExtraSignedExtensionto select/sequence signed extensions viasignedExtensionsByVersion.get(0)(matchingPortableRegistry.$Extra()for v4 encoding). - Update fallback-extension tests to include a version-0 mapping in mocked metadata.
- Add regression tests covering selection/ordering, payload processing parity, empty sets, older single-set behavior, and failure modes.
File summaries
| File | Description |
|---|---|
| packages/api/src/extrinsic/extensions/ExtraSignedExtension.ts | Selects signed extensions from the version-0 extension set (v4) instead of initializing all metadata entries. |
| packages/api/src/extrinsic/extensions/tests/FallbackSignedExtension.spec.ts | Updates mocks to include signedExtensionsByVersion for version-0 selection. |
| packages/api/src/extrinsic/extensions/tests/ExtraSignedExtension.spec.ts | Adds regression coverage for versioned extension selection, ordering, encoding, and failure behavior. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+52
to
+56
| const { signedExtensions, signedExtensionsByVersion } = this.registry.metadata.extrinsic; | ||
| // Match PortableRegistry.$Extra(): version 4 transactions use extension version 0. | ||
| const indexes = signedExtensionsByVersion.get(0); | ||
| assert(indexes, 'No signed extensions found for extension version 0'); | ||
| return indexes.map((index) => signedExtensions[index]); |
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
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.
Asset Hub metadata V16 includes multiple transaction-extension sets.
ExtraSignedExtensioncurrently initializes every entry in the metadata table, including extensions outside the set used for version 4 transactions. On runtime2005000, this throwsSignedExtension for VerifyMultiSignature requires input but is not implementedbefore a validator payout can be submitted.Select and order extensions through
signedExtensionsByVersion.get(0), matchingPortableRegistry.$Extra()and the existing version 4 transaction encoding. The shared selector also keeps wallet payload processing and additional signed data encoding consistent. Unsupported extensions in the selected set and a missing version-zero mapping still fail.Regression coverage exercises initialization, wallet payload processing, extension ordering and encoded signing data, older metadata's single-set layout, empty sets, and invalid sets. The existing fallback-extension fixture now includes its version-zero mapping.
Validation on Node 24:
yarn workspace @dedot/api test: 445 tests passed.yarn build: all 14 projects passed.