From 2565478c813fb7278153b113de4b9338fc186872 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 4 Jun 2021 17:28:46 -0400 Subject: [PATCH 1/2] wallet test refactor: add CreateSyncedWallet function No change in behavior. This just moves some code from the ListCoins test setup to a reusable util function, so it can be reused in a new test in the next commit. --- build_msvc/test_bitcoin/test_bitcoin.vcxproj | 1 + src/Makefile.test.include | 2 ++ src/wallet/test/util.cpp | 38 ++++++++++++++++++++ src/wallet/test/util.h | 19 ++++++++++ src/wallet/test/wallet_tests.cpp | 16 ++------- 5 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 src/wallet/test/util.cpp create mode 100644 src/wallet/test/util.h diff --git a/build_msvc/test_bitcoin/test_bitcoin.vcxproj b/build_msvc/test_bitcoin/test_bitcoin.vcxproj index 5c4b777d5164b..bb1a780bfab12 100644 --- a/build_msvc/test_bitcoin/test_bitcoin.vcxproj +++ b/build_msvc/test_bitcoin/test_bitcoin.vcxproj @@ -16,6 +16,7 @@ + diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 105d09f7301d5..1374f3892ee0a 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -170,6 +170,8 @@ endif BITCOIN_TEST_SUITE += \ + wallet/test/util.cpp \ + wallet/test/util.h \ wallet/test/wallet_test_fixture.cpp \ wallet/test/wallet_test_fixture.h \ wallet/test/init_test_fixture.cpp \ diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp new file mode 100644 index 0000000000000..c3061b93c061c --- /dev/null +++ b/src/wallet/test/util.cpp @@ -0,0 +1,38 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include +#include + +#include + +#include + +std::unique_ptr CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, const CKey& key) +{ + auto wallet = std::make_unique(&chain, "", CreateMockWalletDatabase()); + { + LOCK2(wallet->cs_wallet, ::cs_main); + wallet->SetLastBlockProcessed(cchain.Height(), cchain.Tip()->GetBlockHash()); + } + wallet->LoadWallet(); + { + auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); + LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore); + spk_man->AddKeyPubKey(key, key.GetPubKey()); + } + WalletRescanReserver reserver(*wallet); + reserver.reserve(); + CWallet::ScanResult result = wallet->ScanForWalletTransactions(cchain.Genesis()->GetBlockHash(), 0 /* start_height */, {} /* max_height */, reserver, false /* update */); + BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); + BOOST_CHECK_EQUAL(result.last_scanned_block, cchain.Tip()->GetBlockHash()); + BOOST_CHECK_EQUAL(*result.last_scanned_height, cchain.Height()); + BOOST_CHECK(result.last_failed_block.IsNull()); + return wallet; +} diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h new file mode 100644 index 0000000000000..288c111571c4d --- /dev/null +++ b/src/wallet/test/util.h @@ -0,0 +1,19 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_WALLET_TEST_UTIL_H +#define BITCOIN_WALLET_TEST_UTIL_H + +#include + +class CChain; +class CKey; +class CWallet; +namespace interfaces { +class Chain; +} // namespace interfaces + +std::unique_ptr CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, const CKey& key); + +#endif // BITCOIN_WALLET_TEST_UTIL_H diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index a0070b8dd321f..75a08b6f74ca6 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -480,20 +481,7 @@ class ListCoinsTestingSetup : public TestChain100Setup ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - wallet = std::make_unique(m_node.chain.get(), "", CreateMockWalletDatabase()); - { - LOCK2(wallet->cs_wallet, ::cs_main); - wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); - } - wallet->LoadWallet(); - AddKey(*wallet, coinbaseKey); - WalletRescanReserver reserver(*wallet); - reserver.reserve(); - CWallet::ScanResult result = wallet->ScanForWalletTransactions(m_node.chainman->ActiveChain().Genesis()->GetBlockHash(), 0 /* start_height */, {} /* max_height */, reserver, false /* update */); - BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); - BOOST_CHECK_EQUAL(result.last_scanned_block, m_node.chainman->ActiveChain().Tip()->GetBlockHash()); - BOOST_CHECK_EQUAL(*result.last_scanned_height, m_node.chainman->ActiveChain().Height()); - BOOST_CHECK(result.last_failed_block.IsNull()); + wallet = CreateSyncedWallet(*m_node.chain, m_node.chainman->ActiveChain(), coinbaseKey); } ~ListCoinsTestingSetup() From fe6dc76b7c9c5405f37464a3b19fcf82aaf22861 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 4 Jun 2021 18:38:13 -0400 Subject: [PATCH 2/2] wallet test: Add test for subtract fee from recipient behavior Behavior might have recently changed in #17331 (it is not clear) but not noticed because there is no test coverage. This adds test coverage for current subtract from recipient behavior without changing it. Co-authored-by: Andrew Chow --- src/Makefile.test.include | 1 + src/wallet/test/spend_tests.cpp | 61 +++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 src/wallet/test/spend_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 1374f3892ee0a..344590fa136b7 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -151,6 +151,7 @@ BITCOIN_TESTS =\ if ENABLE_WALLET BITCOIN_TESTS += \ wallet/test/psbt_wallet_tests.cpp \ + wallet/test/spend_tests.cpp \ wallet/test/wallet_tests.cpp \ wallet/test/walletdb_tests.cpp \ wallet/test/wallet_crypto_tests.cpp \ diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp new file mode 100644 index 0000000000000..66e7de4273c92 --- /dev/null +++ b/src/wallet/test/spend_tests.cpp @@ -0,0 +1,61 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include + +#include + +BOOST_FIXTURE_TEST_SUITE(spend_tests, WalletTestingSetup) + +BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) +{ + CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); + auto wallet = CreateSyncedWallet(*m_node.chain, m_node.chainman->ActiveChain(), coinbaseKey); + + // Check that a subtract-from-recipient transaction slightly less than the + // coinbase input amount does not create a change output (because it would + // be uneconomical to add and spend the output), and make sure it pays the + // leftover input amount which would have been change to the recipient + // instead of the miner. + auto check_tx = [&wallet](CAmount leftover_input_amount) { + CRecipient recipient{GetScriptForRawPubKey({}), 50 * COIN - leftover_input_amount, true /* subtract fee */}; + CTransactionRef tx; + CAmount fee; + int change_pos = -1; + bilingual_str error; + CCoinControl coin_control; + coin_control.m_feerate.emplace(10000); + coin_control.fOverrideFeeRate = true; + FeeCalculation fee_calc; + BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, fee, change_pos, error, coin_control, fee_calc)); + BOOST_CHECK_EQUAL(tx->vout.size(), 1); + BOOST_CHECK_EQUAL(tx->vout[0].nValue, recipient.nAmount + leftover_input_amount - fee); + BOOST_CHECK_GT(fee, 0); + return fee; + }; + + // Send full input amount to recipient, check that only nonzero fee is + // subtracted (to_reduce == fee). + const CAmount fee{check_tx(0)}; + + // Send slightly less than full input amount to recipient, check leftover + // input amount is paid to recipient not the miner (to_reduce == fee - 123) + BOOST_CHECK_EQUAL(fee, check_tx(123)); + + // Send full input minus fee amount to recipient, check leftover input + // amount is paid to recipient not the miner (to_reduce == 0) + BOOST_CHECK_EQUAL(fee, check_tx(fee)); + + // Send full input minus more than the fee amount to recipient, check + // leftover input amount is paid to recipient not the miner (to_reduce == + // -123). This overpays the recipient instead of overpaying the miner more + // than double the neccesary fee. + BOOST_CHECK_EQUAL(fee, check_tx(fee + 123)); +} + +BOOST_AUTO_TEST_SUITE_END()