-
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
no_std
compatibility for subxt-signer
#1477
Conversation
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.
Looks good; just some minor comments :)
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, nice one!
@@ -115,14 +117,14 @@ impl std::str::FromStr for SecretUri { | |||
} | |||
|
|||
/// This is returned if `FromStr` cannot parse a string into a `SecretUri`. | |||
#[derive(Debug, Copy, Clone, PartialEq, thiserror::Error)] | |||
#[derive(Debug, Copy, Clone, PartialEq, Display)] |
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.
impl std::error::Error for this one? I saw that James commented on it but I can't find the impl :)
testing/no-std-tests/Cargo.toml
Outdated
@@ -7,6 +7,7 @@ resolver = "2" | |||
|
|||
[dependencies] | |||
subxt-metadata = { path = "../../metadata", default-features = false } | |||
subxt-signer = { path = "../../signer", default-features = false, features = ["sr25519"] } # Note: feature "ecdsa" does not compile to `no-std` right now |
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.
It's probably a good idea to document it somewhere or raise an issue for it
let _signature = keypair.sign(message); | ||
let _public_key = keypair.public_key(); | ||
|
||
// Note: `ecdsa` is not compiling for the `thumbv7em-none-eabi` target. |
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 would be nice to have an issue for the ecdsa no-std stuff
This PR makes subxt-signer no-std compatible. This was factored out into a seperate PR because the
subxt-core
PR got a bit big. For this reason, this PR might already make more changes than necessary to the dependency structure, but it is all to make the integration with the mainsubxt-core
PR a smooth experience.