libwally-core icon indicating copy to clipboard operation
libwally-core copied to clipboard

Infer relative derivation when parent key level > 0

Open Sjors opened this issue 2 years ago • 5 comments

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.

Sjors avatar Apr 14 '22 13:04 Sjors

@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?

jgriffiths avatar Apr 14 '22 21:04 jgriffiths

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.

Sjors avatar Apr 15 '22 08:04 Sjors

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.

jgriffiths avatar Apr 15 '22 09:04 jgriffiths

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?

Sjors avatar Apr 15 '22 10:04 Sjors

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.

jgriffiths avatar Apr 15 '22 11:04 jgriffiths