Skip to content

Commit ec700f0

Browse files
committed
Merge bitcoin/bitcoin#30076: test: fix MiniWallet script-path spend (missing parity bit in leaf version)
e4b0dab test: add functional test for tagged MiniWallet instances (Sebastian Falbesoner) 3162c91 test: fix MiniWallet internal key derivation for tagged instances (Sebastian Falbesoner) c9f7364 test: fix MiniWallet script-path spend (missing parity bit in leaf version) (Sebastian Falbesoner) 7774c31 test: refactor: return TaprootInfo from P2TR address creation routine (Sebastian Falbesoner) Pull request description: This PR fixes a dormant bug in MiniWallet that exists since support for P2TR was initially added in #23371 (see commit 041abfe). In the course of spending the output, the leaf version byte of the control block in the witness stack doesn't set the parity bit, i.e. we were so far just lucky that the used combinations of relevant data (internal pubkey, leaf script / version) didn't result in a tweaked pubkey with odd y-parity. If that was the case, we'd get the following validation error: `mandatory-script-verify-flag-failed (Witness program hash mismatch) (-26)` Since MiniWallets can now optionally be tagged (#29939), resulting in different internal pubkeys, the issue is more prevalent now. Fix it by passing the parity bit, as specified in BIP341. Can be tested with the following patch (fails on master, succeeds on PR): ```diff diff --git a/test/functional/test_framework/mempool_util.py b/test/functional/test_framework/mempool_util.py index 148cc93..7ebe858681 100644 --- a/test/functional/test_framework/mempool_util.py +++ b/test/functional/test_framework/mempool_util.py @@ -42,7 +42,7 @@ def fill_mempool(test_framework, node): # Generate UTXOs to flood the mempool # 1 to create a tx initially that will be evicted from the mempool later # 75 transactions each with a fee rate higher than the previous one - ephemeral_miniwallet = MiniWallet(node, tag_name="fill_mempool_ephemeral_wallet") + ephemeral_miniwallet = MiniWallet(node, tag_name="fill_mempool_ephemeral_wallet3") test_framework.generate(ephemeral_miniwallet, 1 + num_of_batches * tx_batch_size) # Mine enough blocks so that the UTXOs are allowed to be spent ``` In addition to that, another bug is fixed where the internal key derivation failed, as not every pseudorandom hash results in a valid x-only pubkey. Fix this by treating the hash result as private key and calculate the x-only public key out of that, to be used then as internal key. Fixes #30528. ACKs for top commit: glozow: ACK e4b0dab rkrux: reACK [e4b0dab](bitcoin/bitcoin@e4b0dab) hodlinator: ACK e4b0dab Tree-SHA512: a16f33f76bcb1012857cc3129438a9f6badf28aa2b1d25696da0d385ba5866b46de0f1f93ba777ed9263fe6952f98d7d9c44ea0c0170a2bcc86cbef90bf6ac58
2 parents 1e8d689 + e4b0dab commit ec700f0

File tree

3 files changed

+30
-7
lines changed

3 files changed

+30
-7
lines changed

test/functional/feature_framework_miniwallet.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
# Distributed under the MIT software license, see the accompanying
44
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
55
"""Test MiniWallet."""
6+
import random
7+
import string
8+
69
from test_framework.blocktools import COINBASE_MATURITY
710
from test_framework.test_framework import BitcoinTestFramework
811
from test_framework.util import (
@@ -31,6 +34,20 @@ def test_tx_padding(self):
3134
assert_greater_than_or_equal(tx.get_weight(), target_weight)
3235
assert_greater_than_or_equal(target_weight + 3, tx.get_weight())
3336

37+
def test_wallet_tagging(self):
38+
"""Verify that tagged wallet instances are able to send funds."""
39+
self.log.info(f"Test tagged wallet instances...")
40+
node = self.nodes[0]
41+
untagged_wallet = self.wallets[0][1]
42+
for i in range(10):
43+
tag = ''.join(random.choice(string.ascii_letters) for _ in range(20))
44+
self.log.debug(f"-> ({i}) tag name: {tag}")
45+
tagged_wallet = MiniWallet(node, tag_name=tag)
46+
untagged_wallet.send_to(from_node=node, scriptPubKey=tagged_wallet.get_scriptPubKey(), amount=100000)
47+
tagged_wallet.rescan_utxos()
48+
tagged_wallet.send_self_transfer(from_node=node)
49+
self.generate(node, 1) # clear mempool
50+
3451
def run_test(self):
3552
node = self.nodes[0]
3653
self.wallets = [
@@ -43,6 +60,7 @@ def run_test(self):
4360
self.generate(wallet, COINBASE_MATURITY)
4461

4562
self.test_tx_padding()
63+
self.test_wallet_tagging()
4664

4765

4866
if __name__ == '__main__':

test/functional/test_framework/address.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,14 @@ def create_deterministic_address_bcrt1_p2tr_op_true(explicit_internal_key=None):
5353
can be spent with a witness stack of OP_TRUE and the control block
5454
with internal public key (script-path spending).
5555
56-
Returns a tuple with the generated address and the internal key.
56+
Returns a tuple with the generated address and the TaprootInfo object.
5757
"""
5858
internal_key = explicit_internal_key or (1).to_bytes(32, 'big')
59-
address = output_key_to_p2tr(taproot_construct(internal_key, [(None, CScript([OP_TRUE]))]).output_pubkey)
59+
taproot_info = taproot_construct(internal_key, [("only-path", CScript([OP_TRUE]))])
60+
address = output_key_to_p2tr(taproot_info.output_pubkey)
6061
if explicit_internal_key is None:
6162
assert_equal(address, 'bcrt1p9yfmy5h72durp7zrhlw9lf7jpwjgvwdg0jr0lqmmjtgg83266lqsekaqka')
62-
return (address, internal_key)
63+
return (address, taproot_info)
6364

6465

6566
def byte_to_base58(b, version):

test/functional/test_framework/wallet.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
)
4040
from test_framework.script import (
4141
CScript,
42-
LEAF_VERSION_TAPSCRIPT,
4342
OP_1,
4443
OP_NOP,
4544
OP_RETURN,
@@ -106,8 +105,8 @@ def __init__(self, test_node, *, mode=MiniWalletMode.ADDRESS_OP_TRUE, tag_name=N
106105
pub_key = self._priv_key.get_pubkey()
107106
self._scriptPubKey = key_to_p2pk_script(pub_key.get_bytes())
108107
elif mode == MiniWalletMode.ADDRESS_OP_TRUE:
109-
internal_key = None if tag_name is None else hash256(tag_name.encode())
110-
self._address, self._internal_key = create_deterministic_address_bcrt1_p2tr_op_true(internal_key)
108+
internal_key = None if tag_name is None else compute_xonly_pubkey(hash256(tag_name.encode()))[0]
109+
self._address, self._taproot_info = create_deterministic_address_bcrt1_p2tr_op_true(internal_key)
111110
self._scriptPubKey = address_to_scriptpubkey(self._address)
112111

113112
# When the pre-mined test framework chain is used, it contains coinbase
@@ -195,7 +194,12 @@ def sign_tx(self, tx, fixed_length=True):
195194
elif self._mode == MiniWalletMode.ADDRESS_OP_TRUE:
196195
tx.wit.vtxinwit = [CTxInWitness()] * len(tx.vin)
197196
for i in tx.wit.vtxinwit:
198-
i.scriptWitness.stack = [CScript([OP_TRUE]), bytes([LEAF_VERSION_TAPSCRIPT]) + self._internal_key]
197+
assert_equal(len(self._taproot_info.leaves), 1)
198+
leaf_info = list(self._taproot_info.leaves.values())[0]
199+
i.scriptWitness.stack = [
200+
leaf_info.script,
201+
bytes([leaf_info.version | self._taproot_info.negflag]) + self._taproot_info.internal_pubkey,
202+
]
199203
else:
200204
assert False
201205

0 commit comments

Comments
 (0)