Skip to content
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

Infer relative derivation when parent key level > 0 #326

Open
Sjors opened this issue Apr 14, 2022 · 5 comments
Open

Infer relative derivation when parent key level > 0 #326

Sjors opened this issue Apr 14, 2022 · 5 comments

Comments

@Sjors
Copy link
Contributor

Sjors commented Apr 14, 2022

The new bip32_key_from_parent_path_str function takes a path_str argument. When this starts with m/ it should strip the first depth levels from the path string. Currently it just treats everything as a relative path.

This would help me avoid a string manipulation workaround like this:

// Convert absolute path to relative:
var tmpPath = path
if path.split(separator: "/").first == "m" {
  tmpPath = path.split(separator: "/").dropFirst(1 + Int(self.wally_ext_key.depth)).joined(separator: "/")
}

It's especially useful in de context of PSBT processing, where the PSBT will have absolute paths, but the wallet may want to derive from a (cached) xpub to see if it can sign something, as well as for change detection.

Perhaps a flag could be added to (dis)allow the use of absolute paths when deriving from depth > 0 since that can lead to mistakes.

@jgriffiths
Copy link
Contributor

@Sjors just to be absolutely clear here, what you are looking for is the ability to pass a full path string, but a key that represents not the root of that path but instead a key somewhere along it? And you are proposing that the depth of the key passed in be used to skip that number of elements?

Its not certain that the depth of the key passed in will always be correct with respect to the path I think, so adding a flag to use the depth may not be sufficiently robust or general. How would you feel about a _skip variant that allowed you to explicitly pass in the number of path elements to skip before starting to derive? For your use case you could pass in key->depth, but if the key depth is suspect/created in a non standard way then an explicit value could be used instead?

A _skip variant would also have the advantage of a simpler internal implementation, since the existing derivation functions would just call it with skip = 0. Thoughts?

@Sjors
Copy link
Contributor Author

Sjors commented Apr 15, 2022

And you are proposing that the depth of the key passed in be used to skip that number of elements?

Yes. But only if the path starts with m/.

How would you feel about a _skip variant that allowed you to explicitly pass in the number of path elements to skip before starting to derive?

This would help slightly, but it still requires me to parse the path string to see if there's an m/ in it.

but if the key depth is suspect/created in a non standard way then an explicit value could be used instead

BIP32 seems to suggest that the master key (m) should be the first derivation level from the entropy, but it's not explicitly demanding that. So it's possible some wallets do something different.

Let's say a wallet uses an account level xpub, but marks it as depth 0. That's not a problem, assuming that in their absolute path strings the m/ also represents the account level.

If there are wallets that do this inconsistently, I'm inclined to think those wallets should use relative paths. A _skip could be useful there, but potentially very confusing. Maybe an offset parameter would make more sense: number of levels to offset an absolute path by.

@jgriffiths
Copy link
Contributor

jgriffiths commented Apr 15, 2022

If there are wallets that do this inconsistently, I'm inclined to think those wallets should use relative paths.

The problem is that there is no concept of full vs relative paths in bip32, let alone a standardisation of how they should be represented. You can't know that a path is relative just because it lacks a leading m since many libs don't use it. Wally cannot impose that as a standard on users of the library.

Maybe an offset parameter would make more sense: number of levels to offset an absolute path by.

This is just naming, skip and offset meaning the same thing (skip n elements from the path before deriving).

This would help slightly, but it still requires me to parse the path string to see if there's an m/ in it.

I think this is unavoidable if you want to ascribe meaning to that prefix.

@Sjors
Copy link
Contributor Author

Sjors commented Apr 15, 2022

You can't know that a path is relative just because it lacks a leading m since many libs don't use it.

I hadn't thought about it in that direction. But when m is present we can assume it's absolute? Or not?

@jgriffiths
Copy link
Contributor

But when m is present we can assume it's absolute? Or not?

Unfortunately not, see e.g https://github.com/rust-bitcoin/rust-bitcoin/blob/master/src/util/bip32.rs#L414 where a derived key of depth 1 is further derived using an m-path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants