Make TransactionExtension tuple of tuple transparent for implication
#7028
Conversation
This file contains 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
|
/cmd prdoc --audience runtime_dev runtime_user |
gui1117
added
the
T17-primitives
gui1117
force-pushed
the
gui-pipeline-v2
branch
from
d40106a to
080a19d
Compare
gui1117
changed the title
TransactionExtension tuple of tuple transparent for implicationTransactionExtension tuple of tuple transparent for implication
|
All GitHub workflows were cancelled due to failure one of the required jobs. |
ggwpez
reviewed
Jan 2, 2025
| &ImplicationParts { | ||
| base: implication_parts.base, | ||
| explicit: (&following_explicit_implications, implication_parts.explicit), | ||
| implicit: (&following_implicit_implications, implication_parts.implicit), |
There was a problem hiding this comment.
This keeps the order correct as
((Base1, Base2..), (Explicit1, Explicit2..), (Implicit1, Implicit2..)) instead of
((Base1, Explicit1, Implicit1), (Base2, Explicit2, Implicit2)...)?
There was a problem hiding this comment.
On master the order is:
- For the pipeline
(A, B, C, D)the implication in the validation of A is(Base, ExpB, ExpC, ExpD, ImpB, ImpC, ImpD). - For the pipeline
((A, B), C, D)the implication in the validation of A is((Base, ExpC, ExpD, ImpC, ImpD), ExpB, ImpB). Because when calling the validation of the inner tuple, the outer give the whole(Base, ExpC, ExpD, ImpC, ImpD)as inherited implication, and the inner tuple can't destructure and restructure it.
With this PR the order is in both case (Base, ExpB, ExpC, ExpD, ImpB, ImpC, ImpD).
bkchr
approved these changes
Jan 3, 2025
substrate/primitives/runtime/src/traits/transaction_extension/mod.rs
Outdated
Show resolved
Hide resolved
substrate/primitives/runtime/src/traits/transaction_extension/mod.rs
Outdated
Show resolved
Hide resolved
substrate/primitives/runtime/src/traits/transaction_extension/mod.rs
Outdated
Show resolved
Hide resolved
bkchr
reviewed
Jan 3, 2025
substrate/primitives/runtime/src/traits/transaction_extension/mod.rs
Outdated
Show resolved
Hide resolved
ggwpez
approved these changes
Jan 3, 2025
Co-authored-by: Bastian Köcher <[email protected]>
dudo50
pushed a commit
to paraspell-research/polkadot-sdk
that referenced
this pull request
Jan 4, 2025
paritytech#7028) Currently `(A, B, C)` and `((A, B), C)` change the order of implications in the transaction extension pipeline. This order is not accessible in the metadata, because the metadata is just a vector of transaction extension, the nested structure is not visible. This PR make the implementation for tuple of `TransactionExtension` better for tuple of tuple. `(A, B, C)` and `((A, B), C)` don't change the implication for the validation A. This is a breaking change but only when using the trait `TransactionExtension` the code implementing the trait is not breaking (surprising rust behavior but fine). --------- Co-authored-by: command-bot <> Co-authored-by: Bastian Köcher <[email protected]>
gui1117
added
the
A4-needs-backport
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-7028-to-stable2407
git worktree add --checkout .worktree/backport-7028-to-stable2407 backport-7028-to-stable2407
cd .worktree/backport-7028-to-stable2407
git reset --hard HEAD^
git cherry-pick -x b5a5ac4487890046d226bedb0238eaccb423ae42
git push --force-with-lease |
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-7028-to-stable2409
git worktree add --checkout .worktree/backport-7028-to-stable2409 backport-7028-to-stable2409
cd .worktree/backport-7028-to-stable2409
git reset --hard HEAD^
git cherry-pick -x b5a5ac4487890046d226bedb0238eaccb423ae42
git push --force-with-lease |
github-actions bot
pushed a commit
that referenced
this pull request
Jan 7, 2025
#7028) Currently `(A, B, C)` and `((A, B), C)` change the order of implications in the transaction extension pipeline. This order is not accessible in the metadata, because the metadata is just a vector of transaction extension, the nested structure is not visible. This PR make the implementation for tuple of `TransactionExtension` better for tuple of tuple. `(A, B, C)` and `((A, B), C)` don't change the implication for the validation A. This is a breaking change but only when using the trait `TransactionExtension` the code implementing the trait is not breaking (surprising rust behavior but fine). --------- Co-authored-by: command-bot <> Co-authored-by: Bastian Köcher <[email protected]> (cherry picked from commit b5a5ac4)
|
Successfully created backport PR for |
gui1117
had a problem deploying
to
subsystem-benchmarks
GitHub Actions
Failure
ordian
added a commit
that referenced
this pull request
Jan 7, 2025
* master: (256 commits) fix chunk fetching network compatibility zombienet test (#6988) chore: delete repeat words (#7034) Print taplo version in CI (#7041) Implement cumulus StorageWeightReclaim as wrapping transaction extension + frame system ReclaimWeight (#6140) Make `TransactionExtension` tuple of tuple transparent for implication (#7028) Replace duplicated whitelist with whitelisted_storage_keys (#7024) [WIP] Fix networking-benchmarks (#7036) [docs] Fix release naming (#7032) migrate pallet-mixnet to umbrella crate (#6986) Improve remote externalities logging (#7021) Fix polkadot sdk doc. (#7022) Remove warning log from frame-omni-bencher CLI (#7020) [pallet-revive] fix file case (#6981) Add workflow for networking benchmarks (#7029) [CI] Skip SemVer on R0-silent and update docs (#6285) correct path in cumulus README (#7001) sync: Send already connected peers to new subscribers (#7011) Excluding chainlink domain for link checker CI (#6524) pallet-bounties: Fix benchmarks for 0 ED (#7013) Log peerset set ID -> protocol name mapping (#7005) ...
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.
Currently
(A, B, C)and((A, B), C)change the order of implications in the transaction extension pipeline. This order is not accessible in the metadata, because the metadata is just a vector of transaction extension, the nested structure is not visible.This PR make the implementation for tuple of
TransactionExtensionbetter for tuple of tuple.(A, B, C)and((A, B), C)don't change the implication for the validation A.This is a breaking change but only when using the trait
TransactionExtensionthe code implementing the trait is not breaking (surprising rust behavior but fine).