Skip to content

Commit

Permalink
DEBUG unit test availablecoins_tests fails intermittently
Browse files Browse the repository at this point in the history
this is a debugging commit the previously introduced
availablecoins_tests in the merge of bitcoin/bitcoin#24584

about 30% of the time the test fails with "invalid schnorr signature"
when trying to verify the signature of the transaction it has just
created

this only occurs when both bech32 and bech32m tests are run in the test.
removing either one does not show the issue. even stranger, the issue
only occurs when bech32m is run before bech32 as it is in the upstream
PR. swapping these around so that bech32 is run before bech32m works
around the issue - BUT WHY
  • Loading branch information
delta1 committed Oct 24, 2024
1 parent 03ed031 commit 0739a57
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 17 deletions.
22 changes: 21 additions & 1 deletion src/script/interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,14 @@ static bool EvalChecksigPreTapscript(const valtype& vchSig, const valtype& vchPu
return true;
}

static void print_hex(const unsigned char* data, size_t size) {
size_t i;
for (i = 0; i < size; i++) {
printf("%02x", data[i]);
}
printf("\n");
}

static bool EvalTapScriptCheckSigFromStack(const valtype& sig, const valtype& vchPubKey, ScriptExecutionData& execdata, unsigned int flags, const valtype& msg, SigVersion sigversion, ScriptError* serror, bool& success)
{
// This code follows the behaviour of EvalCheckSigTapscript
Expand Down Expand Up @@ -2836,7 +2844,19 @@ bool GenericTransactionSignatureChecker<T>::VerifyECDSASignature(const std::vect
template <class T>
bool GenericTransactionSignatureChecker<T>::VerifySchnorrSignature(Span<const unsigned char> sig, const XOnlyPubKey& pubkey, const uint256& sighash) const
{
return pubkey.VerifySchnorr(sighash, sig);
std::cout << "sig: ";
print_hex(sig.data(), sig.size());
std::cout << "pubkey: ";
print_hex(pubkey.data(), pubkey.size());
std::cout << "sighash: " << sighash.GetHex() << std::endl;;

auto r = pubkey.VerifySchnorr(sighash, sig);
if (r) {
std::cout << "verify passed\n";
} else {
std::cout << "verify failed\n";
}
return r;
}

template <class T>
Expand Down
10 changes: 8 additions & 2 deletions src/wallet/spend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1812,7 +1812,10 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal(


if (sign) {
if (!wallet.SignTransaction(txNew)) {
auto s = wallet.SignTransaction(txNew);
std::cout << "signed: " << s << std::endl;
if (!s) {
std::cout << "internal return: " << __LINE__ << std::endl;
return _("Signing transaction failed");
}
}
Expand Down Expand Up @@ -1889,7 +1892,10 @@ BResult<CreatedTransactionResult> CreateTransaction(
auto res = CreateTransactionInternal(wallet, vecSend, change_pos, coin_control, sign, blind_details, issuance_details);
TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), res.HasRes(),
res ? res.GetObj().fee : 0, res ? res.GetObj().change_pos : 0);
if (!res) return res;
if (!res) {
std::cout << "returning\n";
return res;
}
const auto& txr_ungrouped = res.GetObj();
// try with avoidpartialspends unless it's enabled already
if (txr_ungrouped.fee > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) {
Expand Down
38 changes: 24 additions & 14 deletions src/wallet/test/availablecoins_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,14 @@ class AvailableCoinsTestingSetup : public TestChain100Setup
{
constexpr int RANDOM_CHANGE_POSITION = -1;
auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, dummy);
BOOST_CHECK(res);
tx = res.GetObj().tx;
// BOOST_CHECK(res);
if (res) {
tx = res.GetObj().tx;
std::cout << "pass\n";
} else {
std::cout << "fail\n";
BOOST_CHECK(res);
}
}
wallet->CommitTransaction(tx, {}, {});
CMutableTransaction blocktx;
Expand Down Expand Up @@ -76,29 +82,33 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup)
// Bech32m
dest = wallet->GetNewDestination(OutputType::BECH32M, "");
BOOST_ASSERT(dest.HasRes());
AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 1 * COIN, CAsset(), CPubKey(), /*fSubtractFeeFromAmount=*/true});
std::cout << "bech32m\n";
AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 1 * COIN, ::policyAsset, CPubKey(), /*fSubtractFeeFromAmount=*/true});
available_coins = AvailableCoins(*wallet);
BOOST_CHECK_EQUAL(available_coins.bech32m.size(), 2U);

// Bech32
dest = wallet->GetNewDestination(OutputType::BECH32, "");
BOOST_ASSERT(dest.HasRes());
AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 2 * COIN, CAsset(), CPubKey(), /*fSubtractFeeFromAmount=*/true});
// std::cout << "bech32\n";
AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 2 * COIN, ::policyAsset, CPubKey(), /*fSubtractFeeFromAmount=*/true});
available_coins = AvailableCoins(*wallet);
BOOST_CHECK_EQUAL(available_coins.bech32.size(), 2U);

// P2SH-SEGWIT
dest = wallet->GetNewDestination(OutputType::P2SH_SEGWIT, "");
AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 3 * COIN, CAsset(), CPubKey(), /*fSubtractFeeFromAmount=*/true});
available_coins = AvailableCoins(*wallet);
BOOST_CHECK_EQUAL(available_coins.P2SH_segwit.size(), 2U);
// //P2SH-SEGWIT
// dest = wallet->GetNewDestination(OutputType::P2SH_SEGWIT, "");
// std::cout << "p2sh\n";
// AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 3 * COIN, ::policyAsset, CPubKey(), /*fSubtractFeeFromAmount=*/true});
// available_coins = AvailableCoins(*wallet);
// BOOST_CHECK_EQUAL(available_coins.P2SH_segwit.size(), 2U);

// Legacy (P2PKH)
dest = wallet->GetNewDestination(OutputType::LEGACY, "");
BOOST_ASSERT(dest.HasRes());
AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 4 * COIN, CAsset(), CPubKey(), /*fSubtractFeeFromAmount=*/true});
available_coins = AvailableCoins(*wallet);
BOOST_CHECK_EQUAL(available_coins.legacy.size(), 2U);
// dest = wallet->GetNewDestination(OutputType::LEGACY, "");
// std::cout << "legacy\n";
// BOOST_ASSERT(dest.HasRes());
// AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 4 * COIN, ::policyAsset, CPubKey(), /*fSubtractFeeFromAmount=*/true});
// available_coins = AvailableCoins(*wallet);
// BOOST_CHECK_EQUAL(available_coins.legacy.size(), 2U);
}

BOOST_AUTO_TEST_SUITE_END()
Expand Down
1 change: 1 addition & 0 deletions src/wallet/test/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cc
FlatSigningProvider provider;
std::string error;
std::unique_ptr<Descriptor> desc = Parse("combo(" + EncodeSecret(key) + ")", provider, error, /* require_checksum=*/ false);
std::cout << "desc: " << desc->ToString() << std::endl;
assert(desc);
WalletDescriptor w_desc(std::move(desc), 0, 0, 1, 1);
if (!wallet->AddWalletDescriptor(w_desc, provider, "", false)) assert(false);
Expand Down
5 changes: 5 additions & 0 deletions test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env bash

# set -euxo pipefail

for N in $(seq 1 100); do src/test/test_bitcoin --run_test=availablecoins_tests; done

0 comments on commit 0739a57

Please sign in to comment.