Skip to content

Commit 1b65d4d

Browse files
psgrecopsgreco
authored
Merge pull request #1284 from jamesdorfman/schnorr-fix-master
pubkey: fix VerifySchnorr bug which is causing testnet validation failure
2 parents df8e572 + 9368fba commit 1b65d4d

File tree

6 files changed

+50
-39
lines changed

6 files changed

+50
-39
lines changed

‎src/pubkey.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ bool XOnlyPubKey::VerifySchnorr(const Span<const unsigned char> msg, Span<const
208208
assert(sigbytes.size() == 64);
209209
secp256k1_xonly_pubkey pubkey;
210210
if (!secp256k1_xonly_pubkey_parse(secp256k1_context_verify, &pubkey, m_keydata.data())) return false;
211-
return secp256k1_schnorrsig_verify(secp256k1_context_verify, sigbytes.data(), msg.begin(), 32, &pubkey);
211+
return secp256k1_schnorrsig_verify(secp256k1_context_verify, sigbytes.data(), msg.data(), msg.size(), &pubkey);
212212
}
213213

214214
// ELEMENTS: this is preserved from an old version of the Taproot code for use in OP_TWEAKVERIFY

‎src/test/script_tests.cpp‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333

3434
#include <univalue.h>
3535

36+
using namespace std;
37+
3638
// Uncomment if you want to output updated JSON tests.
3739
// #define UPDATE_JSON_TESTS
3840

@@ -1756,6 +1758,7 @@ BOOST_AUTO_TEST_CASE(script_assets_test)
17561758
file.close();
17571759
}
17581760

1761+
// ELEMENTS: TODO: Some of these test vectors need updating
17591762
BOOST_AUTO_TEST_CASE(bip341_keypath_test_vectors)
17601763
{
17611764
UniValue tests;
@@ -1777,9 +1780,14 @@ BOOST_AUTO_TEST_CASE(bip341_keypath_test_vectors)
17771780

17781781
PrecomputedTransactionData txdata;
17791782
txdata.Init(tx, std::vector<CTxOut>{utxos}, true);
1783+
// ELEMENTS: add a txout witness for each output
1784+
for (u_int i = 0; i < (u_int)utxos.size(); i++) {
1785+
tx.witness.vtxoutwit.emplace_back(CTxOutWitness());
1786+
}
17801787

17811788
BOOST_CHECK(txdata.m_bip341_taproot_ready);
1782-
// BOOST_CHECK_EQUAL(HexStr(txdata.m_spent_amounts_single_hash), vec["intermediary"]["hashAmounts"].get_str());
1789+
// ELEMENTS: FIXME
1790+
//BOOST_CHECK_EQUAL(HexStr(txdata.m_spent_amounts_single_hash), vec["intermediary"]["hashAmounts"].get_str());
17831791
BOOST_CHECK_EQUAL(HexStr(txdata.m_outputs_single_hash), vec["intermediary"]["hashOutputs"].get_str());
17841792
BOOST_CHECK_EQUAL(HexStr(txdata.m_prevouts_single_hash), vec["intermediary"]["hashPrevouts"].get_str());
17851793
BOOST_CHECK_EQUAL(HexStr(txdata.m_spent_scripts_single_hash), vec["intermediary"]["hashScriptPubkeys"].get_str());
@@ -1809,22 +1817,24 @@ BOOST_AUTO_TEST_CASE(bip341_keypath_test_vectors)
18091817
provider.keys[key.GetPubKey().GetID()] = key;
18101818
MutableTransactionSignatureCreator creator(&tx, txinpos, utxos[txinpos].nValue, &txdata, hashtype);
18111819
std::vector<unsigned char> signature;
1820+
BOOST_CHECK(creator.CreateSchnorrSig(provider, signature, pubkey, nullptr, &merkle_root, SigVersion::TAPROOT));
18121821
// ELEMENTS: FIXME
1813-
// BOOST_CHECK(creator.CreateSchnorrSig(provider, signature, pubkey, nullptr, &merkle_root, SigVersion::TAPROOT));
1814-
// BOOST_CHECK_EQUAL(HexStr(signature), input["expected"]["witness"][0].get_str());
1822+
//BOOST_CHECK_EQUAL(HexStr(signature), input["expected"]["witness"][0].get_str());
18151823

18161824
// We can't observe the tweak used inside the signing logic, so verify by recomputing it.
1817-
// BOOST_CHECK_EQUAL(HexStr(pubkey.ComputeTapTweakHash(merkle_root.IsNull() ? nullptr : &merkle_root)), input["intermediary"]["tweak"].get_str());
1825+
// ELEMENTS: FIXME
1826+
//BOOST_CHECK_EQUAL(HexStr(pubkey.ComputeTapTweakHash(merkle_root.IsNull() ? nullptr : &merkle_root)), input["intermediary"]["tweak"].get_str());
18181827

18191828
// We can't observe the sighash used inside the signing logic, so verify by recomputing it.
18201829
ScriptExecutionData sed;
18211830
sed.m_annex_init = true;
18221831
sed.m_annex_present = false;
1823-
// uint256 sighash;
1824-
// BOOST_CHECK(SignatureHashSchnorr(sighash, sed, tx, txinpos, hashtype, SigVersion::TAPROOT, txdata, MissingDataBehavior::FAIL));
1832+
uint256 sighash;
1833+
BOOST_CHECK(SignatureHashSchnorr(sighash, sed, tx, txinpos, hashtype, SigVersion::TAPROOT, txdata, MissingDataBehavior::FAIL));
18251834
// BOOST_CHECK_EQUAL(HexStr(sighash), input["intermediary"]["sigHash"].get_str());
18261835

18271836
// To verify the sigmsg, hash the expected sigmsg, and compare it with the (expected) sighash.
1837+
// ELEMENTS: FIXME
18281838
//BOOST_CHECK_EQUAL(HexStr((CHashWriter(HASHER_TAPSIGHASH_ELEMENTS) << Span{ParseHex(input["intermediary"]["sigMsg"].get_str())}).GetSHA256()), input["intermediary"]["sigHash"].get_str());
18291839
}
18301840

‎test/functional/feature_taphash_pegins_issuances.py‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ def create_taproot_utxo(self):
8080
self.nodes[0].sendrawtransaction(signed_raw_tx['hex'])
8181
tx = tx_from_hex(signed_raw_tx['hex'])
8282
tx.rehash()
83-
self.nodes[0].generate(1)
83+
self.generate(self.nodes[0], 1)
8484
last_blk = self.nodes[0].getblock(self.nodes[0].getbestblockhash())
8585
assert(tx.hash in last_blk['tx'])
8686

@@ -95,7 +95,7 @@ def pegin_test(self, sighash_ty):
9595
peg_id = self.nodes[0].sendtoaddress(fund_info["mainchain_address"], 1)
9696
raw_peg_tx = self.nodes[0].gettransaction(peg_id)["hex"]
9797
peg_txid = self.nodes[0].sendrawtransaction(raw_peg_tx)
98-
self.nodes[0].generate(101)
98+
self.generate(self.nodes[0], 101)
9999
peg_prf = self.nodes[0].gettxoutproof([peg_txid])
100100
claim_script = fund_info["claim_script"]
101101

@@ -126,7 +126,7 @@ def pegin_test(self, sighash_ty):
126126
assert(verify_schnorr(pub_tweak, sig, msg))
127127
# Since we add in/outputs the min feerate is no longer maintained.
128128
self.nodes[0].sendrawtransaction(hexstring = raw_claim.serialize().hex())
129-
self.nodes[0].generate(1)
129+
self.generate(self.nodes[0], 1)
130130
last_blk = self.nodes[0].getblock(self.nodes[0].getbestblockhash())
131131
raw_claim.rehash()
132132
assert(raw_claim.hash in last_blk['tx'])
@@ -166,15 +166,18 @@ def issuance_test(self, sighash_ty):
166166
assert(verify_schnorr(pub_tweak, sig, msg))
167167
# Since we add in/outputs the min feerate is no longer maintained.
168168
self.nodes[0].sendrawtransaction(hexstring = issued_tx.serialize().hex())
169-
self.nodes[0].generate(1)
169+
self.generate(self.nodes[0], 1)
170170
last_blk = self.nodes[0].getblock(self.nodes[0].getbestblockhash())
171171
issued_tx.rehash()
172172
assert(issued_tx.hash in last_blk['tx'])
173173

174174

175175
def run_test(self):
176-
self.nodes[0].generate(101)
176+
self.generate(self.nodes[0], 101)
177177
self.wait_until(lambda: self.nodes[0].getblockcount() == 101, timeout=5)
178+
# spend the initialfreecoins to node0, to fix bad-txns-inputs-missingorspent error
179+
self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 50)
180+
self.generate(self.nodes[0], 1)
178181
self.log.info("Testing sighash taproot pegins")
179182
# Note that this does not test deposit to taproot pegin addresses
180183
# because there is no support for taproot pegins in rpc. The current rpc assumes

‎test/functional/feature_tapscript_opcodes.py‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ def create_taproot_utxo(self, scripts = None, blind = False):
9696
self.nodes[0].sendrawtransaction(signed_raw_tx['hex'])
9797
tx = tx_from_hex(signed_raw_tx['hex'])
9898
tx.rehash()
99-
self.nodes[0].generate(1)
99+
self.generate(self.nodes[0], 1)
100100
last_blk = self.nodes[0].getblock(self.nodes[0].getbestblockhash())
101101
assert(tx.hash in last_blk['tx'])
102102

@@ -118,7 +118,7 @@ def tapscript_satisfy_test(self, script, inputs = None, add_issuance = False,
118118
peg_id = self.nodes[0].sendtoaddress(fund_info["mainchain_address"], 1)
119119
raw_peg_tx = self.nodes[0].gettransaction(peg_id)["hex"]
120120
peg_txid = self.nodes[0].sendrawtransaction(raw_peg_tx)
121-
self.nodes[0].generate(101)
121+
self.generate(self.nodes[0], 101)
122122
peg_prf = self.nodes[0].gettxoutproof([peg_txid])
123123
claim_script = fund_info["claim_script"]
124124

@@ -249,15 +249,19 @@ def tapscript_satisfy_test(self, script, inputs = None, add_issuance = False,
249249

250250

251251
self.nodes[0].sendrawtransaction(hexstring = tx.serialize().hex())
252-
self.nodes[0].generate(1)
252+
self.generate(self.nodes[0], 1)
253253
last_blk = self.nodes[0].getblock(self.nodes[0].getbestblockhash())
254254
tx.rehash()
255255
assert(tx.hash in last_blk['tx'])
256256

257257

258258
def run_test(self):
259-
self.nodes[0].generate(101)
259+
self.generate(self.nodes[0], 101)
260260
self.wait_until(lambda: self.nodes[0].getblockcount() == 101, timeout=5)
261+
# ELEMENTS: spend the initialfreecoins to node0, to fix bad-txns-inputs-missingorspent error when creating the first taproot utxo
262+
self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 50)
263+
self.generate(self.nodes[0], 1)
264+
261265
# Test whether the above test framework is working
262266
self.log.info("Test simple op_1")
263267
self.tapscript_satisfy_test(CScript([OP_1]))
@@ -582,9 +586,6 @@ def csfs_test(msg, mutute_sig = False, fail=None):
582586
self.tapscript_satisfy_test(CScript([msg, pub, OP_CHECKSIGFROMSTACK]), inputs = [sig], fail=fail)
583587

584588
csfs_test("e168c349d0d2499caf3a6d71734c743d517f94f8571fa52c04285b68deec1936")
585-
# Elements: FIXME this test fails
586-
# with test_framework.authproxy.JSONRPCException: non-mandatory-script-verify-flag (Invalid Schnorr signature) (-26)
587-
# csfs_test("Hello World!".encode('utf-8').hex())
588589
csfs_test("Hello World!".encode('utf-8').hex(), fail="Invalid Schnorr signature", mutute_sig=True)
589590
msg = bytes.fromhex("e168c349d0d2499caf3a6d71734c743d517f94f8571fa52c04285b68deec1936")
590591
pub = bytes.fromhex("3f67e97da0df6931189cfb0072447da22707897bd5de04936a277ed7e00b35b3")

‎test/functional/test_runner.py‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,7 @@
131131
'feature_segwit.py --descriptors',
132132
# vv Tests less than 2m vv
133133
'wallet_basic.py --legacy-wallet',
134-
# ELEMENTS: FIXME
135-
# 'wallet_basic.py --descriptors',
134+
'wallet_basic.py --descriptors',
136135
'wallet_labels.py --legacy-wallet',
137136
'wallet_labels.py --descriptors',
138137
'p2p_segwit.py',
@@ -141,13 +140,11 @@
141140
'mempool_updatefromblock.py',
142141
'wallet_dump.py --legacy-wallet',
143142
'feature_taproot.py --previous_release',
144-
# ELEMENTS: FIXME taproot test needs to be updated
145-
# 'feature_taproot.py',
143+
'feature_taproot.py',
146144
'rpc_signer.py',
147145
'wallet_signer.py --descriptors',
148-
# ELEMENTS: FIXME failing tests
149-
# 'feature_taphash_pegins_issuances.py',
150-
# 'feature_tapscript_opcodes.py',
146+
'feature_taphash_pegins_issuances.py',
147+
'feature_tapscript_opcodes.py',
151148
# vv Tests less than 60s vv
152149
'p2p_sendheaders.py',
153150
'wallet_importmulti.py --legacy-wallet',
@@ -211,9 +208,8 @@
211208
'rpc_signrawtransaction.py --descriptors',
212209
'rpc_rawtransaction.py --legacy-wallet',
213210
'rpc_rawtransaction.py --descriptors',
214-
# ELEMENTS: FIXME failing tests
215-
# 'wallet_groups.py --legacy-wallet',
216-
# 'wallet_groups.py --descriptors',
211+
'wallet_groups.py --legacy-wallet',
212+
'wallet_groups.py --descriptors',
217213
'wallet_transactiontime_rescan.py --descriptors',
218214
'wallet_transactiontime_rescan.py --legacy-wallet',
219215
'p2p_addrv2_relay.py',

‎test/functional/wallet_groups.py‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ def run_test(self):
4949

5050
# For each node, send 0.2 coins back to 0;
5151
# - node[1] should pick one 0.5 UTXO and leave the rest
52+
# ELEMENTS: since SRD was added, node[1] sometimes picks a 1.0 UTXO
5253
# - node[2] should pick one (1.0 + 0.5) UTXO group corresponding to a
5354
# given address, and leave the rest
5455
self.log.info("Test sending transactions picks one UTXO group and leaves the rest")
@@ -60,13 +61,13 @@ def run_test(self):
6061
# one output should be 0.2, the other should be ~0.3
6162
v = [vout["value"] for vout in tx1["vout"] if vout["scriptPubKey"]["type"] != "fee"]
6263
v.sort()
63-
# JAMES/BYRON DELETE ME
64-
#print(self.nodes[1].listunspent())
65-
print("vin amount = {}".format(self.nodes[0].gettxout(tx1["vin"][0]["txid"], tx1["vin"][0]["vout"], True)["value"]))
66-
print("vout amounts = {}".format(v))
67-
# END JAMES?BYRON
6864
assert_approx(v[0], vexp=0.2, vspan=0.0001)
69-
assert_approx(v[1], vexp=0.3, vspan=0.0001)
65+
# ELEMENTS
66+
try:
67+
assert_approx(v[1], vexp=0.3, vspan=0.0001)
68+
except AssertionError:
69+
assert_approx(v[1], vexp=0.8, vspan=0.0001)
70+
7071

7172
txid2 = self.nodes[2].sendtoaddress(self.nodes[0].getnewaddress(), 0.2)
7273
tx2 = self.nodes[2].getrawtransaction(txid2, True)
@@ -114,10 +115,10 @@ def run_test(self):
114115
assert_equal(input_addrs[0], input_addrs[1])
115116
# Node 2 enforces avoidpartialspends so needs no checking here
116117

117-
tx4_ungrouped_fee = 2820
118-
tx4_grouped_fee = 4160
119-
tx5_6_ungrouped_fee = 5520
120-
tx5_6_grouped_fee = 8240
118+
tx4_ungrouped_fee = 5140
119+
tx4_grouped_fee = 6520
120+
tx5_6_ungrouped_fee = 7880
121+
tx5_6_grouped_fee = 10620
121122

122123
self.log.info("Test wallet option maxapsfee")
123124
addr_aps = self.nodes[3].getnewaddress()

0 commit comments

Comments
 (0)