-
Notifications
You must be signed in to change notification settings - Fork 248
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
Browser extension signing example #1067
Browser extension signing example #1067
Conversation
Co-authored-by: Niklas Adolfsson <[email protected]>
* Reduce some repetition when obtaining metadata pallets/runtime_traits * make them pub * fix docs and clippy
Bumps [tokio](https://github.com/tokio-rs/tokio) from 1.28.1 to 1.28.2. - [Release notes](https://github.com/tokio-rs/tokio/releases) - [Commits](tokio-rs/tokio@tokio-1.28.1...tokio-1.28.2) --- updated-dependencies: - dependency-name: tokio dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [regex](https://github.com/rust-lang/regex) from 1.8.2 to 1.8.3. - [Release notes](https://github.com/rust-lang/regex/releases) - [Changelog](https://github.com/rust-lang/regex/blob/master/CHANGELOG.md) - [Commits](rust-lang/regex@1.8.2...1.8.3) --- updated-dependencies: - dependency-name: regex dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [quote](https://github.com/dtolnay/quote) from 1.0.27 to 1.0.28. - [Release notes](https://github.com/dtolnay/quote/releases) - [Commits](dtolnay/quote@1.0.27...1.0.28) --- updated-dependencies: - dependency-name: quote dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [proc-macro2](https://github.com/dtolnay/proc-macro2) from 1.0.58 to 1.0.59. - [Release notes](https://github.com/dtolnay/proc-macro2/releases) - [Commits](dtolnay/proc-macro2@1.0.58...1.0.59) --- updated-dependencies: - dependency-name: proc-macro2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* practice TDD * implement a hashmap 2-phases approach * use nicer types * add test for cache filling * adjust test --------- Co-authored-by: James Wilson <[email protected]>
pub enum Route { | ||
#[at("/fetching")] | ||
Fetching, | ||
#[at("/signing")] |
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.
Yew is quite nice really; I like the built in browser history/routing stuff, and actually I generally find it quite easy to follow overall!
I wonder how the other frameworks differ; I should really try some of them at some point!
examples/wasm-example/Cargo.toml
Outdated
wasm-bindgen-futures = "0.4.36" | ||
anyhow = "1.0.71" | ||
serde = "1.0.163" | ||
serde_json = "1.0.96" |
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.
nit: we could add a new line here and below
examples/wasm-example/index.js
Outdated
@@ -0,0 +1,92 @@ | |||
/// The `@polkadot/extension-dapp` package can be dynamically imported. | |||
/// Usually it is wise to use a package manager like npm or yarn to install it as a dependency. | |||
/// The `getPolkadotJsExtensionMod` |
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'm not super familiar with js docs, but is this line complete here? Or should we remove it?
subxt/src/config/extrinsic_params.rs
Outdated
transaction_version: u32, | ||
genesis_hash: T::Hash, | ||
mortality_checkpoint: T::Hash, | ||
/// era |
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.
/// era | |
/// Era defines how long the transaction will be valid for. |
subxt/src/config/extrinsic_params.rs
Outdated
pub transaction_version: u32, | ||
/// genesis hash of the chain | ||
pub genesis_hash: T::Hash, | ||
/// mortality checkpoint |
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.
/// mortality checkpoint | |
/// The block after which the transaction becomes valid. |
subxt/src/config/extrinsic_params.rs
Outdated
pub era: Era, | ||
/// nonce (account nonce sent along an extrinsic, such that no extrinsic is submitted twice) | ||
pub nonce: u64, | ||
/// tip |
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.
/// tip | |
/// The tip you would like to give to the block author for this transaction. | |
/// Note: Can be zero. |
subxt/src/tx/tx_client.rs
Outdated
@@ -318,7 +318,8 @@ where | |||
pub struct PartialExtrinsic<T: Config, C> { | |||
client: C, | |||
call_data: Vec<u8>, | |||
additional_and_extra_params: T::ExtrinsicParams, | |||
/// Extrinsic params (Additional and Extra) | |||
pub additional_and_extra_params: T::ExtrinsicParams, |
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.
Nit: Similarly to call_data
we could create a getter for these params
/// Returns the additional and extra extrinsic params.
pub fn extra_params(&self) -> &T::ExtrinsicParams { .. }
Looks great to me, and it works well! I'd prefer that we don't expose any of those extra props in Subxt, and instead gather the information up from elsewhere; see my above comment :) |
fn encode_to_hex_reverse<E: Encode>(input: &E) -> String { | ||
let mut bytes = input.encode(); | ||
bytes.reverse(); | ||
format!("0x{}", hex::encode(bytes)) | ||
} |
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.
Yeah, it's really because we aren't scale encoding some of these numbers at all; we're just providing the hex representation of them, which is naturally big endian. So really we should just hex::encode
the numbers when converted to big endian bytes.
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.
Maybe the functions should be:
// SCALE encode and then hex:
fn scale_encoded_hex<E: Encode>(input: &E) -> String { ... }
// Just hex encode the provided bytes:
fn hex_encoded_bytes(input: &[u8]) -> String { ... }
And then .to_be_bytes
or whatever on the numbers we pass to the latter?
/// | ||
/// Some parameters are hard-coded here and not taken from the partial_extrinsic itself (mortality_checkpoint, era, tip). | ||
pub async fn extension_signature_for_partial_extrinsic( | ||
partial_extrinsic: &PartialExtrinsic<PolkadotConfig, OnlineClient<PolkadotConfig>>, |
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.
Nit: you could just pass in the api.tx().call_data(remark_call)
bytes here I think; you don't use any other part of the PartialExtrinsic
:)
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.
A couple of minor nits but assuming it still works when tested again, it looks great to me!
I split the Yew WASM App into routes for the 2 examples.
This example constructs a
System->remark
call with a remark parsed from user input.We then let the user choose an account in the Talisman or PolkadotJS browser extension and sign the extrinsic in the extension. (Currently tested with the Dev version of the polkadotjs extension)
The full
SubmittableExtrinsic
is then sent to a local node where we wait for it to be successful.What is missing?
Right now
era
,tip
andmortality_checkpoint
of BaseExtrinsicParams are not used for the extrinsic and we use hardcoded values instead (no tip and immortal)