From a4dc356604a11b91806e495970b3d0447d52b29c Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 6 May 2020 14:19:26 +0200 Subject: [PATCH] Merge #18853: wallet: Fix typo in assert that is compile-time true fa47cf9d95dc2c2822fc96df16f179176935bf96 wallet: Fix typo in assert that is compile-time true (MarcoFalke) Pull request description: Commit 92bcd70808b9cac56b184903aa6d37baf9641b37 presumably added a check that a `dest` of type `CNoDestination` implies an empty `scriptChange`. However, it accidentally checked for `boost::variant::empty`, which always returns false: https://www.boost.org/doc/libs/1_72_0/doc/html/boost/variant.html#id-1_3_46_5_4_1_1_16_2-bb ACKs for top commit: Sjors: utACK fa47cf9d95dc2c2822fc96df16f179176935bf96 Tree-SHA512: 9626b1e2947039853703932a362c2ee204e002d3344856eb93eef0e0f833401336f2dfa80fd43b83c8ec6eac624e6302aee771fb67aec436ba6483be02b8d615 --- src/test/script_standard_tests.cpp | 6 ++++++ src/wallet/wallet.cpp | 7 +++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index cf0899cdc7d974..2ab7f72c5cef0b 100644 --- a/src/test/script_standard_tests.cpp +++ b/src/test/script_standard_tests.cpp @@ -12,6 +12,12 @@ BOOST_FIXTURE_TEST_SUITE(script_standard_tests, BasicTestingSetup) +BOOST_AUTO_TEST_CASE(dest_default_is_no_dest) +{ + CTxDestination dest; + BOOST_CHECK(!IsValidDestination(dest)); +} + BOOST_AUTO_TEST_CASE(script_standard_Solver_success) { CKey keys[3]; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b6cd2de22a4d86..cd1223727787c2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3474,7 +3474,10 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CTransac error = _("Transaction needs a change address, but we can't generate it. Please call keypoolrefill first."); } scriptChange = GetScriptForDestination(dest); - assert(!dest.empty() || scriptChange.empty()); + // A valid destination implies a change script (and + // vice-versa). An empty change script will abort later, if the + // change keypool ran out, but change is required. + CHECK_NONFATAL(IsValidDestination(dest) != scriptChange.empty()); } nFeeRet = 0; @@ -3725,7 +3728,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CTransac return false; } - // Give up if change keypool ran out and we failed to find a solution without change: + // Give up if change keypool ran out and change is required if (scriptChange.empty() && nChangePosInOut != -1) { return false; }