WIP: Keychain support for Taproot P2TR addresses #3342
Conversation
* Add ScriptType.P2TR to various checks and if/else blocks * Add P2TR and HDPath.BIP86_PARENT to KeyChainGroupStructure * Add OutputScriptType of `P2TR = 3` to wallet.proto * Add BIP-86 paths and address != null tests to WalletAccountPathTest Remaining issues with this PR: 1. ECKey.toAddress(): Needs to call `calcWitnessProgram` (see TODO on line 431) 2. KeyChainGroup: decision/implementation needed for fallbackChain support. Remaining issues for P2TR support: 1. Need Taproot ECC support (for address generation, tx verification, tx signing) 2. Need to implement TX verification support for P2TR in bitcoinj 3. Need to implement TX signing support for P2TR in bitcoinj
msgilligan
added
Module: Crypto
msgilligan
added
the
Module: Wallet
There was a problem hiding this comment.
Thanks for getting the Taproot train going!
I always planned to start with the prerequisites, namely Schnorr.
Honestly I would not try to target this to 0.17 in its current state. It could needlessly defer the release of 0.17, which I'd really like to get out soon.
| @@ -108,6 +108,8 @@ static HDPath purpose(ScriptType scriptType) { | |||
| return HDPath.BIP44_PARENT; | |||
| } else if (scriptType == ScriptType.P2WPKH) { | |||
| return HDPath.BIP84_PARENT; | |||
| } else if (scriptType == ScriptType.P2TR) { | |||
There was a problem hiding this comment.
I think we should at the same time define a path for BIP32 – I'd pick m/2'.
There was a problem hiding this comment.
Yeah, that makes sense. We should also consider not using the ACCOUNT_ZERO, ACCOUNT_ONE constants for these child numbers as they are misleading in the BIP32 context.
| @@ -64,6 +67,10 @@ void walletStructurePathTest2(KeyChainGroupStructure structure, HDPath expectedP | |||
|
|
|||
| // Then the account path is as expected | |||
| assertEquals(expectedPath, wallet.getActiveKeyChain().getAccountPath()); | |||
|
|
|||
There was a problem hiding this comment.
Would it make sense to PR and merge this change right away?
There was a problem hiding this comment.
The change asserting that the first receive address is not null?
|
I feel we won't get this into a releasable state for 0.17. |
P2TR = 3to wallet.protoRemaining issues with this PR:
calcWitnessProgram(see TODO on line 431)Remaining issues for full P2TR support:
I tagged this as for milestone 0.17, because I think we should at least consider the possibility of trying to get P2TR into 0.17. Though I think it make make more sense to wait for 0.18 for this feature.