Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
ADR-28 derive address functions #9088
ADR-28 derive address functions #9088
Changes from 5 commits
013ab53
c547f26
89a895b
dd0478d
9d415ad
ac53c6e
7e9bc8a
7ee294b
ba1b14b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 a bit confusing for me that
Derive
doesn't length-prefix derivationKey, andDeriveMulti
length-prefixes.I understand
Derive
doesn't need legnth-prefixing. But isn't it just simpler to always use the same algorithm (i.e. always lenght-prefix), so thatDerive == DeriveMulti with 1 derivationKey
?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.
Indeed, we use length prefixing only if we want to glue a sequence of keys. So, for
Derive
it's unnecessary operation.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.
Yes, but does it create more confusion? We could just:
so that we have 1 algorithm only.
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 think the confusion will be there anyway:
DeriveMulti
doesn't decompose into multiple Derive calls (even with LengthPrfixing) - what's in the function comment:The motivation here was to use a composed derivation path, Normally we would put slashes into the segments. But here, the key can be anything, so we can't have any special separator.
So to summarize, we have 3 options:
DeriveMulti
, and not adding it later in the form described above.Maybe we don't need
DeriveMuli
- it seams that we will always have theparent
key and will never derive into subusub-account without having a sub-account. What do you think?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.
Another option:
The 1st line of your
NOTE
is then an equality (less confusion), and we never use length-prefixing (also less confusion). The 2nd line stays an inequality, but in general compositions are never commutative.True, that also makes sense. Overall I'm actually in favor of
Option 2: remove DeriveMulti
for 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.
Yes, that's why I was more thinking about the
Option 2
. Again, the motivation forDeriveMulti
was not to use composition.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 mean - the motivation was to have a sequence of parameters, which don't compose, but use the whole sequence as a single parameter (as we have in the hardware wallets now).