buidl-bitcoin / buidl-python

python3 bitcoin library with no dependencies and extensive test coverage
https://pypi.org/project/buidl/
MIT License
83 stars 26 forks source link

Add multisig testnet matching #146

Closed humanumbrella closed 2 years ago

humanumbrella commented 2 years ago

small bugfix

mflaxman commented 2 years ago

Thanks for the PR! Do you have any documentation you can link to on why m/45'/1... is testnet? I'm not familiar with that and don't see it in the the BIP45 spec. The examples in the spec even include some m/45'/1... that I believe are mainnet (they're not explicitly testnet).

I checked Electrum and Sparrow and they both default to m/45'/0 (not m/45'/1), but you can alter them.

Note that if you're trying to import a Named{HD}PublicKey (perhaps as part of a PSBT) you can set the network (this code is only called when no network is supplied). Perhaps that's the logic you're looking for?

humanumbrella commented 2 years ago

Unfortunately, I don't have a reference to a BIP or spec as we use a combination of them, hehe.

One way to think of Unchained's usage pattern is essentially BIP44 except under m/45' instead of m/44' to both designate it's multisig and to avoid any BIP44 space usage collisions that may be used for singlesig. Here's a link to trezor firmware carveouts that may provide some additional clarity.

You're right that m/45'/1 could be mainnet per the spec - but this PR adds m/45'/1'/* as testnet, which is not specified in the spec. Interestingly that would still not cover all of our use cases, so perhaps I do need to go find the other logic you reference.

What do you think?

mflaxman commented 2 years ago

Gotcha... Can you try passing in the network to the Named{HD}PublicKey constructor?

humanumbrella commented 2 years ago

Going to close this, as I think this is user error. Will reopen if I determine otherwise, ty for the responses.