-
Notifications
You must be signed in to change notification settings - Fork 125
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
Create no_std example for extrinsic creation #556
Conversation
@haerdib Not finished yet but can you have a look if this is the functionality you imagined?
|
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 definitely goes into the right direction.
Regarding the comment:
Next step would be to extract the extrinsic creation into a separate function
While I wrote that in the issue, that's just a thought on how to make it clear what's std and not. If you have a better idea or if it's too complicated, then feel free to adapt.
It would be nice to actually compile this part in no_std mode but I don't see an easy way to do it.
Neither do I... move it to another crate and everything is an overkill I believe. So let's do it with best effort. That should suffice.
|
||
// Construct extrinsic without using Api (no_std). | ||
let additional_extrinsic_params: GenericAdditionalParams< | ||
AssetTip<AssetBalanceFor<Runtime>>, |
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 won't work, as importing the Runtime
into no_std
is tricky. So I'd use hardcoded Balance type here.
/// Get the balance type from your node runtime and adapt it if necessary.
type Balance = u128;
/// We need AssetTip here, because the kitchensink runtime uses the asset pallet. Change to PlainTip if your node uses the balance pallet only.
type AdditionalParams = GenericAdditionalParams<AssetTip<Balance>, Hash>
additional_extrinsic_params, | ||
); | ||
let call = | ||
RuntimeCall::Balances(BalancesCall::transfer_allow_death { dest: recipient, value: 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.
Same here for the RuntimeCall
: In no_std the runtime compiles to wasm, so tricky to import in no_std environments. Better to go without it. That's the fun part now: To get the Call you need the metadata.
Check out how it's done in the worker:
metadata call: https://github.com/integritee-network/worker/blob/master/core-primitives/node-api/metadata/src/lib.rs#L72-L88
Retrieve indexes for a specific pallet: https://github.com/integritee-network/worker/blob/master/core-primitives/node-api/metadata/src/pallet_teeracle.rs
82c7825
to
b4e4657
Compare
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.
Cool, thanks a lot for the effort!
Two more things:
- Can you integrate this example into CI?
- Any chance of merging the code into compose_extrinsic_offline to avoid redundancy, as you've mentioned?
type KitchensinkExtrinsicSigner = ExtrinsicSigner<SubstrateKitchensinkConfig>; | ||
type ExtrinsicAddressOf<Signer> = <Signer as SignExtrinsic<AccountId32>>::ExtrinsicAddress; | ||
|
||
type Hash = H256; //<Runtime as FrameSystemConfig>::Hash; |
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.
The situation has changed a little since my last comment - sorry about that. But now you could take the type directly from the Config:
type Hash = H256; |
(same for Balance).
// If this is not acceptable, use the Api::new_offline(..) function instead. There are no examples for this, | ||
// because of the constantly changing substrate node. But check out our unit tests - there are Apis created with `new_offline`. | ||
// | ||
// ! Careful: AssetTipExtrinsicParams is used here, because the substrate kitchensink runtime uses assets as tips. But for most |
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.
Outdated comment
// runtimes, the PlainTipExtrinsicParams needs to be used. | ||
let mut api = Api::<SubstrateKitchensinkConfig, _>::new(client).unwrap(); | ||
let extrinsic_signer = ExtrinsicSigner::<_>::new(signer); | ||
// Signer is needed to get the nonce |
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.
// Signer is needed to get the nonce | |
// Signer is needed to set the nonce and sign the extrinsic. |
MultiAddress::Id(AccountKeyring::Bob.to_account_id()); | ||
|
||
// Construct extrinsic without using Api (no_std). | ||
let additional_extrinsic_params: AdditionalParams = GenericAdditionalParams::new() |
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.
Extracting the no_std part in a separate function is not possible? I think that would be rather important, because in actual use cases (just like in Integritee) the std and no_std part will be separate.
.era(Era::mortal(period, header.number.into()), last_finalized_header_hash) | ||
.tip(0); | ||
let extrinsic_params = | ||
GenericExtrinsicParams::<SubstrateKitchensinkConfig, AssetTip<u128>>::new( |
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 think this type could be taken directly from the SubstrateKitchensinkConfig as well:
type ExtrinsicParams = AssetTipExtrinsicParams<Self>; |
let header = api.get_header(Some(last_finalized_header_hash)).unwrap().unwrap(); | ||
let period = 5; | ||
|
||
// Get information out of Api (online). |
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.
// Get information out of Api (online). | |
// Get information out of Api (online). This information could also be set offline in the `no_std`, but that would need to be static and adapted whenever the node changes. You can get the information directly from the node runtime file or the api of https://polkadot.js.org. |
// Signer is needed to get the nonce | ||
api.set_signer(extrinsic_signer.clone()); | ||
|
||
// Information for Era for mortal transactions (online). |
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.
// Information for Era for mortal transactions (online). | |
// Get the last finalized header to retrieve information for Era for mortal transactions (online). |
Is this PR open for Review btw? |
@haerdib Unfortunately still not quite... I merged the examples but I'm still not quite happy how the code looks. Also the CI integration is missing. |
e6d5652
to
c4a570e
Compare
766028c
to
65da9db
Compare
The two examples are now merged into a single one. Both approaches are in the |
println!("Compose extrinsic in no_std environment (No Api instance)"); | ||
{ | ||
let signer_nonce = api.get_nonce().unwrap(); | ||
println!("[+] Alice's Account Nonce is {}", signer_nonce); |
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.
Would it be possible to move the api part outside of this environment? That would make it clearer, that really no api client is needed (given the information is availbale).
E.g.
let spec_version = api.runtime_version().spec_version;
let transaction_version = api.runtime_version().transaction_version;
let genesis_hash = api.genesis_hash
..
let mut xt = ...;
{
let extrinsic_params = <AssetRuntimeConfig as Config>::ExtrinsicParams::new(
spec_version,
transaction_version,
signer_nonce,
genesis_hash,
additional_extrinsic_params,
);
let recipients_extrinsic_address: ExtrinsicAddressOf<AssetExtrinsicSigner> =
recipient.clone().into();
let call = compose_call!(
metadata,
"Balances",
"transfer_allow_death",
recipients_extrinsic_address,
Compact(4u32)
);
xt = compose_extrinsic_offline!(extrinsic_signer, call, extrinsic_params);
}
// Test created extrinsic via api call to the node (online)
let hash = api
.submit_and_watch_extrinsic_until(xt, XtStatus::InBlock)
.unwrap()
.block_hash
.unwrap();
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.
Same for the api (offline) part - move the online part outside?
Does that make sense?
…and compose_extrinsic_no_std.rs)
b9c8a3b
to
62e11a7
Compare
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.
Cool, I think it makes more sense now. Thanks for the adaptions! Few last nitpicks, other than that it's great :)
.tip(0); | ||
|
||
println!("Compose extrinsic in no_std environment (No Api instance)"); | ||
let signer_nonce = api.get_nonce().unwrap(); |
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.
Can you move this below the descriptive comment below? I think this belongs to the same category.
Co-authored-by: Bigna Härdi <[email protected]>
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.
Cool, thanks a lot!
Adjusted copy of
compose_extrinsic_offline
.Closing #543