-
Notifications
You must be signed in to change notification settings - Fork 1
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
WIP - Arculus support #358
base: main
Are you sure you want to change the base?
Conversation
This reverts commit 34777b9.
Ed25519 = 0x0201, | ||
Ed25519Blake2bNano = 0x0202, | ||
Ed25519Curve = 0x0203, | ||
Nist256p1 = 0x0301, |
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 Secp256r1
aka P256, aka nist256p1
/// The curves supported by the arculus card | ||
#[derive(Debug, Copy, Clone, PartialEq, Eq)] | ||
#[repr(u16)] | ||
pub enum CardCurve { |
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.
These Enum variants are probably specified by Arculus? And they make very little sense.
All named 25519
I would probably group together in a nested associated value.
Some of them are not... even curves. Like Ed25519Blake2bNano. That is a term saying "use Ed25519 as curve and before hashing payload perform Blake2b hash on it".
Annoying that Arculus has chosen this poor design.
I prefer it if we only listed the one we need.
Since they are such a mess.
status.as_result() | ||
} | ||
|
||
pub async fn do_chainned_card_io<Response, F>( |
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.
typo chained
pub async fn create_wallet_seed_io( | ||
&self, | ||
wallet: ArculusWalletPointer, | ||
word_count: i64, |
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.
Prefer using our own BIP39WordCount type
} | ||
|
||
impl TryFrom<i32> for ArculusWalletCSDKResponseStatus { | ||
type Error = CommonError; |
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.
Use Strum FromRepr here instead
|
||
impl ArculusCSDKDriver for RustArculusCSDKDriver { | ||
fn wallet_init(&self) -> Option<ArculusWalletPointer> { | ||
todo!() |
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.
All todos? Is this type going to be removed?
Still in progress, just pushed to have the whole picture.