-
Notifications
You must be signed in to change notification settings - Fork 46
Support Extrinsiv V5 #1724
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
Support Extrinsiv V5 #1724
Conversation
|
do we need to upgrade integritee-node to make integration tests work? |
| pub type IntegriteeUncheckedExtrinsic<Call> = | ||
| UncheckedExtrinsicV4<Address, Call, PairSignature, IntegriteeSignedExtra>; | ||
|
|
||
| /// Signature type of the [UncheckedExtrinsicV4]. | ||
| pub type Signature<SignedExtra> = Option<(Address, PairSignature, SignedExtra)>; | ||
| UncheckedExtrinsic<Address, Call, PairSignature, IntegriteeSignedExtra>; | ||
|
|
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.
This was the tricky part as to why the node failed to interpret the extrinsic correctly. The signature used by the api client is now a plain MultiSignature
cli/Cargo.toml
Outdated
| substrate-api-client = { default-features = false, features = ["std", "sync-api"], git = "https://github.com/encointer/substrate-api-client.git", branch = "v0.9.42-tag-v0.14.0-retracted-check-metadata-hash" } | ||
| substrate-client-keystore = { git = "https://github.com/encointer/substrate-api-client.git", branch = "v0.9.42-tag-v0.14.0-retracted-check-metadata-hash" } | ||
| substrate-api-client = { default-features = false, features = ["std", "sync-api"], git = "https://github.com/encointer/substrate-api-client.git", branch = "cl/extrinsic-v5-v0.9.42" } | ||
| substrate-client-keystore = { git = "https://github.com/encointer/substrate-api-client.git", branch = "cl/extrinsic-v5-v0.9.42" } |
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 still have to think of a proper branch name for this.
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.
Added a new branch with the rationale that we have too many patches to name them all in the branch name. I added the listed patches in the Readme of this branch.
v0.9.42-tag-v0.14.0-integritee-patch
However, it lives in the encointer fork. I am not sure if we want to have our own fork of it.
brenzi
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, but I'd like to trigger a -dev release and test this on Paseo before merging
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 tested the following steps with 0.17.0-dev1
- deploy for asset-hub-paseo Incognitee testnet (running against stable2506-2 nodes)
- using try.incognitee.io: shield & unshield works
- using PJS, sending funds to shard vault yields in shielding (meaning the worker understood the extrinsic)
the errors about decoding extrinsics no longer show in the logs
| #[test] | ||
| fn can_parse_asset_hub_transaction_v4() { | ||
| use itp_utils::FromHexPrefixed; | ||
| use substrate_api_client::ac_primitives::extrinsics::UncheckedExtrinsic as XTV5; | ||
|
|
||
| // We only care about the preamble, so we use the `()` to stop decoding after the preamble. | ||
| let ex_v5: XTV5<Address, (), ParentchainSignature, ParentchainTxExtension> = XTV5::from_hex("0x4d0284007c77c5f95a72c19e051233b3b1e01df74ec13cb161b109fc5f16ce1c65316f2401f63f850629722493607fb04dfa64d9bd62c454ea325b96d71938d0a52f513c4b35681b9c323b9a09b9f7227e10a5707624afbeb2232759505119da68fa6de08b2400040000000a0300b1df638e78cd896db34d8e62722fd755360d61dac2f3b048e24cd26a5ace35690b008cb6611e01").unwrap(); | ||
|
|
||
| match &ex_v5.preamble { | ||
| // If the preamble is signed, it means that an XT V4 was sent | ||
| Preamble::Signed(..) => {}, | ||
| Preamble::General(..) => panic!("unexpected preamble belonging to XT V5"), | ||
| other => panic!("unexpected preamble: {:?}", other), | ||
| }; | ||
| } |
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.
This is how you can identify to which tx version an extrinsic belongs.
This PR mostly consists of an api-client update to get the backwards compatible support for the Extrinsic V5. Closes #1723.
Tests