-
Notifications
You must be signed in to change notification settings - Fork 18
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
Adds LoadKey command #6
Conversation
var data []byte | ||
if isSeed == true { | ||
p1 = P1LoadKeySeed | ||
data = payload |
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 I missed it before, but I see we just send data/payload. but as explained in the docs, if P1 is 1 or 2 the data sent must be encoded in tlv using the 0xA1 tag for the template.
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.
so maybe we can have NewCommandLoadKey(type LoadType, data []byte) where LoadType are constants defining keypair/ext-keypair/seed. and based on that we define the right p1 and data format.
What do you think @alex-miller-0 ?
I can also continue with this if you don't have a lot of time.
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.
My idea was to punt data encoding to another layer, since I didn't see an example of TLV encoding in this codebase. If you want that logic here, I probably shouldn't be doing it since I'm not sure what types of data structures or libraries you want to use.
Feel free to close this PR and use any of the code in another 👍
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.
just added a couple of comments above
Cleaner commit history, second try for #5