HD wallets with BIP 44

[yaroslavyaroslav]

Is your feature request related to a problem?

Supporting to managing multiple wallet address based on BIP 44 standard.

What solution would you like?

This BIP defines a logical hierarchy for deterministic wallets based on an algorithm described in BIP-0032 (BIP32 from now on) and purpose scheme described in BIP-0043 (BIP43 from now on). Details of implementation could be found here.

Any additional context?

No response

[yaroslavyaroslav]

Ah, what a nice number for enhancement referenced to the bitcoin specs.

[albertopeam]

Hi!
I would like to work on this! :)

I have some knowledge about BIP32/39/44 as I did some investigation by my own in cosmos blockchain(you can check my go repository on github).
I am open to receive feedback on the approach or to collaborate with others designing/analyzing...

[yaroslavyaroslav]

That would be great. But please wait a bit follow 2 weeks would be the time of cleanup, means codebase would change drastically.

But anyway, speaking about our design, there's CodableTransaction which have sender and from properties. Also there's KeystoreManager which as it leads to its name BIP32 and BIP39 compatible, so I guess it would be a good place to fit this extension as well.

Also just to mention that we're follow swift version convention, e.g. within 3.*.* version we could not drop or change any public API, but it definitely could be added with ease any given one with bumping minor version, say 3.2.*.

[albertopeam]

I have took a look at the code and probably I will need support :)
I think I understand BIP44 spec, but not sure how to proceed

[yaroslavyaroslav]

@albertopeam I'm here to help, but could you elaborate the things that you've struggling with?

[albertopeam]

I am preparing some thoughts and doubts, I expect I could send them soon :)

[albertopeam]

I have been reading bip44 doc and I think the goal is to:
WARN if the creation of an account imply that doesn't exists a previous one with at least one transaction(scanned at least 20 indexes before return)

I think must be done on demand, ie:

protocol BIP44 {
     /// Derive an `HDNode` based on the `path` and warns if it won't follow BIP44 standard
     func discoveryDeriveHDNode(for path: String) async throws -> (result: HDNode?, warn: Bool)
}
extension HDNode: BIP44 {
     func discoveryDeriveHDNode(for path: String) async throws -> (result: HDNode?, warn: Bool) {
         // TODO: apply discovery algorythm asynchronously
         // https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#account-discovery
         return (nil, false)
     }
}
/// API usage(to showcase only created a simple test)
func testBIP44DiscoveryDerivation() async throws {
    let seed = try XCTUnwrap(BIP39.seedFromMmemonics(mnemonic, password: ""))
    let rootNode = try XCTUnwrap(HDNode(seed: seed))
    // let result = try await rootNode.discoveryDeriveHDNode(for: "m/44'/60'/1'/0/1") // mistake
    let childNode = try await rootNode.discoveryDeriveHDNode(for: "m/44'/60'/1'/0/1")
    
    //XCTAssertTrue(result.warn)// mistake 
    //XCTAssertNotNil(result.result) // mistake
    XCTAssertTrue(childNode.warn)
    XCTAssertNotNil(childNode.result)
}

Naming is totally temporal, I was only trying to define the general idea.

I see another piece of code that is needed, the storage in the BIP32Keystore of the generated child key but I prefer keep things simple at the beginning.

Well, I hope you can give me feedback as I have been messing with code and I see this one the best option. Feel totally free to share your thoughts. :)

[yaroslavyaroslav]

I myself not into that part of lib actually, so let me clarify and sum up what you've said at first.

1. The BIP 44 is a standard that provides an algorithm to derive additional yet determined public keys aka wallet addresses from a single private key.
2. The standard demands to warn user if they tries to derive an additional address whilst they doesn't use the previous one anyhow.

So for example, it's required to send a warning to user when they tries to let's say derive second address, but only if that method, that derives such second address acknowledged that the first derived address doesn't have a either incoming or outgoing tx. Is this behaviour example is correct in an algorithm meaning?

If it is, I guess we would have to made this given method online in terms of reading chain for a given address to implement such standard demand, is it correct?

[yaroslavyaroslav]

And I wonder where childNode comes from within this code?

    XCTAssertTrue(result.warn)
    XCTAssertNotNil(childNode.result)

[albertopeam]

And I wonder where childNode comes from within this code?

    XCTAssertTrue(result.warn)
    XCTAssertNotNil(childNode.result)

my bad, apologies. I have edited with original code + fix.

[albertopeam]

I myself not into that part of lib actually, so let me clarify and sum up what you've said at first.

1. The BIP 44 is a standard that provides an algorithm to derive additional yet determined public keys aka wallet addresses from a single private key.
2. The standard demands to warn user if they tries to derive an additional address whilst they doesn't use the previous one anyhow.

So for example, it's required to send a warning to user when they tries to let's say derive second address, but only if that method, that derives such second address acknowledged that the first derived address doesn't have a either incoming or outgoing tx. Is this behaviour example is correct in an algorithm meaning?

If it is, I guess we would have to made this given method online in terms of reading chain for a given address to implement such standard demand, is it correct?

Thanks for your feedback, I really appreciate it.

I think I should clarify deeper what I have been checking on the code :)

Regarding point 1:

I was able to derive correctly with current code an BIP44 address, ie:

func testBIP32derivationAccount1Addr1() throws {
    let seed = try XCTUnwrap(BIP39.seedFromMmemonics(mnemonic, password: ""))
    XCTAssertEqual(seed.toHexString(), "aff5f966252803c79dfe35ccd5f7faa3d345c4aa5363f80afd2e7a7bd0774806307f2d7b1ba338aa75cf22c6a62d1e26a299d6175cf2fc71a89eddaf3d4ef429")
    let rootNode = try XCTUnwrap(HDNode(seed: seed))
        
    let childNode = try XCTUnwrap(rootNode.derive(path: "m/44'/60'/1'/0/1", derivePrivateKey: true)) // acc 1, index 1
    let ethAddress = try XCTUnwrap(Utilities.publicToAddress(try XCTUnwrap(childNode.publicKey)))
    XCTAssertEqual(childNode.publicKey.toHexString(), "03fe5fad1740a0c749c3c976c9b37f337204bf3df1d73d78da282dd365a7d47ad9")
    XCTAssertEqual(ethAddress.address, "0x73C13e421eF367c4F55BBC02a8e2a2b12e82f717")
}

You can cp it directly in KeystoresTests and it will pass correctly.
NOTE: The pubKey and address where obtained from this site iancoleman/bip39, feel free to take a look. I haven't derived it manually(will need some learning + time on the topic), so exists certain risk of error, but the site provides matches with some of the tests that you have on KeystoresTests
Conclusion
No new code needed to derive keys according to my investigation, I encourage you or others investigate it and provide diff conclusions if I am wrong :)

Regarding point 2:

The intention of the function that I wrote discoveryDeriveHDNode will:

1. build child node
2. check if that node has a previous account that has transactions(it must be checked online as you have mentioned, correct)
3. if no transactions found warn the user, but still can use the child(as the standard states). This is the dupla property warn: Bool in the function result type

Conclusion
yes to your first question
yes, online to the second question

I hope I have responded all your questions
Thanks for taking your time thinking the expected solution and my idea :)

[yaroslavyaroslav]

Haven't looking into your last answer yet, but will do it this week for sure.

But just meet this thing within the tests: could you please check, whether this is related to this task somehow?

https://github.com/web3swift-team/web3swift/blob/develop/Tests/web3swiftTests/localTests/KeystoresTests.swift#L216

[albertopeam]

Hi!

I should say that BIP44 task is related with the test that you point, in concrete line 251.

The diff that I have seen is that the test you point is not using the entire path, it uses "m/44'/60'/0'/0" which represents:
44' -> purpose; 60' -> coin; 0' -> account; 0 -> change(external/internal),
While the entire path for BIP-32 must include also address_index(screenshot taken from bitcoin BIP-44 website)

If I try to create a private key using the entire path I can do it with the current code.

To sum up,
BIP-44 only warns to the user if the keys should be used or not(not implemented).
BIP-32 handles completely the creation of keys(already implemented).

I hope this clarifies your doubt :)

[yaroslavyaroslav]

Thank you for your elaboration, now the things seems clear to me.

So the BIP-44 seems pretty necessary so far. Time's to get back to its API.
``swift
protocol BIP44 {
/// Derive an HDNode based on the `path` and warns if it won't follow BIP44 standard
func discoveryDeriveHDNode(for path: String) async throws -> (result: HDNode?, warn: Bool)
}

These are too javascriptish at my taste. I mean it's rare and unlikely to return tuple in swift especially in the public APIs. I would suggest this design
```swift
func discoveryDeriveHDNode(for path: String, force: Bool = false) async throws -> throws HDNode {
    guard !force else {
        return rootNode.derive(path: "m/44'/60'/1'/0/1", derivePrivateKey: true)
    }
}

The force parameter is purposed to gives a control to the user whether they wants to be notified about already created HDNodes or not.

So the client code with according to the standard could be look that way:

var derivedAddress: Address
do {
    derivedAddress = try func discoveryDeriveHDNode(for path: "String") 
    // will throw an error in any of those:
    // - Network error appeared and method hasen't reach the chain 
    //      (error would come from within depth of swift standard library)
    // - There's an HDNode already exists
    //      Error would be Web3Error.BIP44Error.
} catch Web3Error.BIP44Error as err {
    derivedAddress = try! func discoveryDeriveHDNode(for path: "String", force: true) 
    // safe to try! because this is syntetic error thrown
    // force attribute also supress any network related error
    // There's no way that new HDNode would not be created, bacuse force trigers ondevice control flow execution.
}