multi: Add apis for ledger transactions. #28
Conversation
JoeGruffins
force-pushed
the
addcreateaddr
branch
5 times, most recently
from
d0787a4 to
8be7971
Compare
|
I plan on adding tests if everything looks alright so far. |
dcr/addresses.go
Outdated
| if isHardened { | ||
| n += hdkeychain.HardenedKeyStart | ||
| } | ||
| extKey, err = extKey.ChildBIP32Std(uint32(n)) |
There was a problem hiding this comment.
this is likely the correct method for ledger compatibility but i believe you will hit compatibility issues with dcrwallet. later in the pr i saw it was looking up known addresses in the wallet, but these would be derived differently than how ledger would be deriving them
There was a problem hiding this comment.
Oh no. Sounds catastrophic.
There was a problem hiding this comment.
Looking closely we are currently only verifying addresses that we get back from the verbose tx decode, and the reason for that is to discern which output is change. This address is never sent to ledger. So, I will change this to the decred way (Child) and leave notes everywhere.
On a related note, is there another way to discern if an output is a change output rather than the address branch or is it fine to keep using that? We check if internal (1) and assume that is change currently.
Ledger wants to know that change path when signing.
There was a problem hiding this comment.
Add the ability to choose which derivation technique to use.
There was a problem hiding this comment.
1/internal is correct for identifying which address is change
is ledger compatible with decred's bip0032 variation?
i suspect the only way to properly fix this and make the two work properly together would be to have an option on dcrwallet to use the standard bip0032 derivation instead of our own silly thing.
There was a problem hiding this comment.
Currently this method is not used to derive hardened/private keys which have the decred tweak. I don't think there is a problem unless one were to try to restore the trezor wallet using the bip32 tool and dcrwallet.
JoeGruffins
force-pushed
the
addcreateaddr
branch
from
8be7971 to
c762d72
Compare
|
Solve a few of the review concerns like the extended key child method https://github.com/decred/libwallet/compare/8be7971885f60602e31b5b6a61ceb6a2774476bb..c762d727b5fc3847e98839886f0e2f2ee71aa4f7 |
JoeGruffins
force-pushed
the
addcreateaddr
branch
from
c762d72 to
9e605f3
Compare
|
Add a couple tests, will add more https://github.com/decred/libwallet/compare/c762d727b5fc3847e98839886f0e2f2ee71aa4f7..9e605f36c9e598430b80d65767db33be111c529b |
JoeGruffins
force-pushed
the
addcreateaddr
branch
from
9e605f3 to
aa6639c
Compare
|
Maybe will get more tests in while solving #29 but not in this pr I guess. |
2 participants
Adding several apis used with ledger to sign transactions.