aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
diff options
context:
space:
mode:
Diffstat (limited to 'src/wallet')
-rw-r--r--src/wallet/rpc/spend.cpp6
-rw-r--r--src/wallet/rpc/wallet.cpp4
-rw-r--r--src/wallet/scriptpubkeyman.cpp35
-rw-r--r--src/wallet/spend.cpp14
-rw-r--r--src/wallet/spend.h2
-rw-r--r--src/wallet/test/coinselector_tests.cpp40
-rw-r--r--src/wallet/test/spend_tests.cpp45
-rw-r--r--src/wallet/wallet.cpp4
8 files changed, 124 insertions, 26 deletions
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index bc65cbf7bf..7064a50937 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -747,7 +747,7 @@ RPCHelpMan fundrawtransaction()
"If that happens, you will need to fund the transaction with different inputs and republish it."},
{"changeAddress", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The bitcoin address to receive the change"},
{"changePosition", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"},
- {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."},
+ {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", \"bech32\", and \"bech32m\"."},
{"includeWatching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also select inputs which are watch only.\n"
"Only solvable inputs can be used. Watch-only destinations are solvable if the public key and/or output script was imported,\n"
"e.g. with 'importpubkey' or 'importmulti' with the 'pubkeys' or 'desc' field."},
@@ -1144,7 +1144,7 @@ RPCHelpMan send()
{"add_to_wallet", RPCArg::Type::BOOL, RPCArg::Default{true}, "When false, returns a serialized transaction which will not be added to the wallet or broadcast"},
{"change_address", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The bitcoin address to receive the change"},
{"change_position", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"},
- {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if change_address is not specified. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."},
+ {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if change_address is not specified. Options are \"legacy\", \"p2sh-segwit\", \"bech32\" and \"bech32m\"."},
{"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."},
{"include_watching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also select inputs which are watch only.\n"
"Only solvable inputs can be used. Watch-only destinations are solvable if the public key and/or output script was imported,\n"
@@ -1597,7 +1597,7 @@ RPCHelpMan walletcreatefundedpsbt()
"If that happens, you will need to fund the transaction with different inputs and republish it."},
{"changeAddress", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The bitcoin address to receive the change"},
{"changePosition", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"},
- {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."},
+ {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", \"bech32\", and \"bech32m\"."},
{"includeWatching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also select inputs which are watch only"},
{"lockUnspents", RPCArg::Type::BOOL, RPCArg::Default{false}, "Lock selected unspent outputs"},
{"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."},
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index 675c4a759d..971814e9cd 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -730,7 +730,9 @@ static RPCHelpMan migratewallet()
std::shared_ptr<CWallet> wallet = GetWalletForJSONRPCRequest(request);
if (!wallet) return NullUniValue;
- EnsureWalletIsUnlocked(*wallet);
+ if (wallet->IsCrypted()) {
+ throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: migratewallet on encrypted wallets is currently unsupported.");
+ }
WalletContext& context = EnsureWalletContext(request.context);
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index 41654579c6..3bcc6b4bdc 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -2499,14 +2499,23 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction&
keys->Merge(std::move(*script_keys));
} else {
// Maybe there are pubkeys listed that we can sign for
- script_keys = std::make_unique<FlatSigningProvider>();
- for (const auto& pk_pair : input.hd_keypaths) {
- const CPubKey& pubkey = pk_pair.first;
- std::unique_ptr<FlatSigningProvider> pk_keys = GetSigningProvider(pubkey);
- if (pk_keys) {
- keys->Merge(std::move(*pk_keys));
- }
+ std::vector<CPubKey> pubkeys;
+
+ // ECDSA Pubkeys
+ for (const auto& [pk, _] : input.hd_keypaths) {
+ pubkeys.push_back(pk);
+ }
+
+ // Taproot output pubkey
+ std::vector<std::vector<unsigned char>> sols;
+ if (Solver(script, sols) == TxoutType::WITNESS_V1_TAPROOT) {
+ sols[0].insert(sols[0].begin(), 0x02);
+ pubkeys.emplace_back(sols[0]);
+ sols[0][0] = 0x03;
+ pubkeys.emplace_back(sols[0]);
}
+
+ // Taproot pubkeys
for (const auto& pk_pair : input.m_tap_bip32_paths) {
const XOnlyPubKey& pubkey = pk_pair.first;
for (unsigned char prefix : {0x02, 0x03}) {
@@ -2514,10 +2523,14 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction&
std::copy(pubkey.begin(), pubkey.end(), b + 1);
CPubKey fullpubkey;
fullpubkey.Set(b, b + 33);
- std::unique_ptr<FlatSigningProvider> pk_keys = GetSigningProvider(fullpubkey);
- if (pk_keys) {
- keys->Merge(std::move(*pk_keys));
- }
+ pubkeys.push_back(fullpubkey);
+ }
+ }
+
+ for (const auto& pubkey : pubkeys) {
+ std::unique_ptr<FlatSigningProvider> pk_keys = GetSigningProvider(pubkey);
+ if (pk_keys) {
+ keys->Merge(std::move(*pk_keys));
}
}
}
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index ce41a4e954..f534e10799 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -102,15 +102,13 @@ void CoinsResult::Clear() {
coins.clear();
}
-void CoinsResult::Erase(std::set<COutPoint>& preset_coins)
+void CoinsResult::Erase(const std::set<COutPoint>& coins_to_remove)
{
- for (auto& it : coins) {
- auto& vec = it.second;
- auto i = std::find_if(vec.begin(), vec.end(), [&](const COutput &c) { return preset_coins.count(c.outpoint);});
- if (i != vec.end()) {
- vec.erase(i);
- break;
- }
+ for (auto& [type, vec] : coins) {
+ auto remove_it = std::remove_if(vec.begin(), vec.end(), [&](const COutput& coin) {
+ return coins_to_remove.count(coin.outpoint) == 1;
+ });
+ vec.erase(remove_it, vec.end());
}
}
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index c29e5be5c7..009e680627 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -47,7 +47,7 @@ struct CoinsResult {
* i.e., methods can work with individual OutputType vectors or on the entire object */
size_t Size() const;
void Clear();
- void Erase(std::set<COutPoint>& preset_coins);
+ void Erase(const std::set<COutPoint>& coins_to_remove);
void Shuffle(FastRandomContext& rng_fast);
void Add(OutputType type, const COutput& out);
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 23f024247d..9bc6fafae7 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -969,5 +969,45 @@ BOOST_AUTO_TEST_CASE(SelectCoins_effective_value_test)
BOOST_CHECK(!result);
}
+BOOST_FIXTURE_TEST_CASE(wallet_coinsresult_test, BasicTestingSetup)
+{
+ // Test case to verify CoinsResult object sanity.
+ CoinsResult available_coins;
+ {
+ std::unique_ptr<CWallet> dummyWallet = std::make_unique<CWallet>(m_node.chain.get(), "dummy", m_args, CreateMockWalletDatabase());
+ BOOST_CHECK_EQUAL(dummyWallet->LoadWallet(), DBErrors::LOAD_OK);
+ LOCK(dummyWallet->cs_wallet);
+ dummyWallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
+ dummyWallet->SetupDescriptorScriptPubKeyMans();
+
+ // Add some coins to 'available_coins'
+ for (int i=0; i<10; i++) {
+ add_coin(available_coins, *dummyWallet, 1 * COIN);
+ }
+ }
+
+ {
+ // First test case, check that 'CoinsResult::Erase' function works as expected.
+ // By trying to erase two elements from the 'available_coins' object.
+ std::set<COutPoint> outs_to_remove;
+ const auto& coins = available_coins.All();
+ for (int i = 0; i < 2; i++) {
+ outs_to_remove.emplace(coins[i].outpoint);
+ }
+ available_coins.Erase(outs_to_remove);
+
+ // Check that the elements were actually removed.
+ const auto& updated_coins = available_coins.All();
+ for (const auto& out: outs_to_remove) {
+ auto it = std::find_if(updated_coins.begin(), updated_coins.end(), [&out](const COutput &coin) {
+ return coin.outpoint == out;
+ });
+ BOOST_CHECK(it == updated_coins.end());
+ }
+ // And verify that no extra element were removed
+ BOOST_CHECK_EQUAL(available_coins.Size(), 8);
+ }
+}
+
BOOST_AUTO_TEST_SUITE_END()
} // namespace wallet
diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp
index a75b014870..81a8883f85 100644
--- a/src/wallet/test/spend_tests.cpp
+++ b/src/wallet/test/spend_tests.cpp
@@ -112,5 +112,50 @@ BOOST_FIXTURE_TEST_CASE(FillInputToWeightTest, BasicTestingSetup)
// Note: We don't test the next boundary because of memory allocation constraints.
}
+BOOST_FIXTURE_TEST_CASE(wallet_duplicated_preset_inputs_test, TestChain100Setup)
+{
+ // Verify that the wallet's Coin Selection process does not include pre-selected inputs twice in a transaction.
+
+ // Add 4 spendable UTXO, 50 BTC each, to the wallet (total balance 200 BTC)
+ for (int i = 0; i < 4; i++) CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
+ auto wallet = CreateSyncedWallet(*m_node.chain, WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain()), m_args, coinbaseKey);
+
+ LOCK(wallet->cs_wallet);
+ auto available_coins = AvailableCoins(*wallet);
+ std::vector<COutput> coins = available_coins.All();
+ // Preselect the first 3 UTXO (150 BTC total)
+ std::set<COutPoint> preset_inputs = {coins[0].outpoint, coins[1].outpoint, coins[2].outpoint};
+
+ // Try to create a tx that spends more than what preset inputs + wallet selected inputs are covering for.
+ // The wallet can cover up to 200 BTC, and the tx target is 299 BTC.
+ std::vector<CRecipient> recipients = {{GetScriptForDestination(*Assert(wallet->GetNewDestination(OutputType::BECH32, "dummy"))),
+ /*nAmount=*/299 * COIN, /*fSubtractFeeFromAmount=*/true}};
+ CCoinControl coin_control;
+ coin_control.m_allow_other_inputs = true;
+ for (const auto& outpoint : preset_inputs) {
+ coin_control.Select(outpoint);
+ }
+
+ // Attempt to send 299 BTC from a wallet that only has 200 BTC. The wallet should exclude
+ // the preset inputs from the pool of available coins, realize that there is not enough
+ // money to fund the 299 BTC payment, and fail with "Insufficient funds".
+ //
+ // Even with SFFO, the wallet can only afford to send 200 BTC.
+ // If the wallet does not properly exclude preset inputs from the pool of available coins
+ // prior to coin selection, it may create a transaction that does not fund the full payment
+ // amount or, through SFFO, incorrectly reduce the recipient's amount by the difference
+ // between the original target and the wrongly counted inputs (in this case 99 BTC)
+ // so that the recipient's amount is no longer equal to the user's selected target of 299 BTC.
+
+ // First case, use 'subtract_fee_from_outputs=true'
+ util::Result<CreatedTransactionResult> res_tx = CreateTransaction(*wallet, recipients, /*change_pos*/-1, coin_control);
+ BOOST_CHECK(!res_tx.has_value());
+
+ // Second case, don't use 'subtract_fee_from_outputs'.
+ recipients[0].fSubtractFeeFromAmount = false;
+ res_tx = CreateTransaction(*wallet, recipients, /*change_pos*/-1, coin_control);
+ BOOST_CHECK(!res_tx.has_value());
+}
+
BOOST_AUTO_TEST_SUITE_END()
} // namespace wallet
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index ac7bf46a14..36fe32e54d 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -4102,8 +4102,8 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>
// Make list of wallets to cleanup
std::vector<std::shared_ptr<CWallet>> created_wallets;
- created_wallets.push_back(std::move(res.watchonly_wallet));
- created_wallets.push_back(std::move(res.solvables_wallet));
+ if (res.watchonly_wallet) created_wallets.push_back(std::move(res.watchonly_wallet));
+ if (res.solvables_wallet) created_wallets.push_back(std::move(res.solvables_wallet));
// Get the directories to remove after unloading
for (std::shared_ptr<CWallet>& w : created_wallets) {
generated by cgit v1.2.3 (git 2.26.2) at 2024-07-20 07:11:46 +0000