From 01cda37af25029961383752f44e91aeafe1e19f6 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 4 Nov 2026 16:51:03 +0000 Subject: [PATCH] fix(qt): reserve the same shared masternode contribution again after a release A coordinator who had sent the invitation, released its coins, absorbed a participant's reply and then reserved again got a new change address. The participant's copy still listed the contribution the invitation carried, so it refused the locked terms with "The locked terms do not contain the details you sent", while the coordinator's board showed every reply in and no reason to ask for another. Before the released contribution was kept out of absorbed replies, the reply restored it and the flow happened to work. Reserving again now puts back the most recently released contribution for this share when Reserve built it in this dialog from this wallet's own coin selection, its coins are all still ones Reserve could select, and they still fund exactly what the share needs, or exceed it by dust when there is no change. Otherwise it selects coins as before, and a changed contribution is a real change to the draft. A contribution absorbed from a reply under this share's name is never put back, whatever its inputs, sequences or change. The coins card describes the contribution Reserve will put back, not a fresh selection. Co-Authored-By: Claude Opus 5.5 --- src/qt/mnsharesession.cpp | 26 ++-- src/qt/mnsharesession.h | 3 + src/qt/sharedmncreatedialog.cpp | 126 ++++++++++++---- src/qt/sharedmncreatedialog.h | 14 ++ src/qt/test/sharedmnwizardtests.cpp | 223 ++++++++++++++++++++++++++++ src/qt/test/sharedmnwizardtests.h | 1 + 6 files changed, 349 insertions(+), 44 deletions(-) diff --git a/src/qt/mnsharesession.cpp b/src/qt/mnsharesession.cpp index cf96952a193d..5f6bcad18b66 100644 --- a/src/qt/mnsharesession.cpp +++ b/src/qt/mnsharesession.cpp @@ -104,19 +104,6 @@ bool SameShare(const MnShareSession::Share& a, const MnShareSession::Share& b) a.rewardAddress == b.rewardAddress; } -bool SameContribution(const MnShareSession::Contribution& a, const MnShareSession::Contribution& b) -{ - if (a.inputs.size() != b.inputs.size() || a.hasChange != b.hasChange) return false; - if (a.hasChange && (a.changeAddress != b.changeAddress || a.changeAmount != b.changeAmount)) return false; - for (size_t i = 0; i < a.inputs.size(); ++i) { - if (a.inputs[i].txid.compare(b.inputs[i].txid, Qt::CaseInsensitive) != 0 || - a.inputs[i].vout != b.inputs[i].vout || a.inputs[i].sequence != b.inputs[i].sequence) { - return false; - } - } - return true; -} - bool IsTxidHex(const QString& txid) { return txid.size() == 64 && IsHex(txid.toStdString()); @@ -191,6 +178,19 @@ bool DecodeSharedProTx(const QString& protx_hex, CMutableTransaction& tx, CProRe } // anonymous namespace +bool MnShareSession::SameContribution(const Contribution& a, const Contribution& b) +{ + if (a.inputs.size() != b.inputs.size() || a.hasChange != b.hasChange) return false; + if (a.hasChange && (a.changeAddress != b.changeAddress || a.changeAmount != b.changeAmount)) return false; + for (size_t i = 0; i < a.inputs.size(); ++i) { + if (a.inputs[i].txid.compare(b.inputs[i].txid, Qt::CaseInsensitive) != 0 || + a.inputs[i].vout != b.inputs[i].vout || a.inputs[i].sequence != b.inputs[i].sequence) { + return false; + } + } + return true; +} + namespace { //! U+202F NARROW NO-BREAK SPACE. BitcoinUnits groups thousands with a plain //! thin space, which a layout may break a number across. diff --git a/src/qt/mnsharesession.h b/src/qt/mnsharesession.h index 0dc8f0176ce0..fae57afc8662 100644 --- a/src/qt/mnsharesession.h +++ b/src/qt/mnsharesession.h @@ -313,6 +313,9 @@ class MnShareSession //! Whether a row with `share`'s owner address has the same amount and addresses here bool hasShare(const Share& share) const; + //! Same inputs, in order and with their sequences, and the same change. + //! Labels and changeIndex are ignored. + static bool SameContribution(const Contribution& a, const Contribution& b); //! Whether a contribution with `contribution`'s label spends the same coins and pays the same change here bool hasContribution(const Contribution& contribution) const; diff --git a/src/qt/sharedmncreatedialog.cpp b/src/qt/sharedmncreatedialog.cpp index 5588e7236022..e6c85718582c 100644 --- a/src/qt/sharedmncreatedialog.cpp +++ b/src/qt/sharedmncreatedialog.cpp @@ -1986,8 +1986,18 @@ void SharedMnCreateDialog::refreshContributionPage() const CAmount target{contributionTarget()}; CoinSelection selection; QString error; + CAmount again_total{0}; if (target <= 0) { m_coins_label->setText(tr("Choose which participant you are.")); + } else if (const auto* again{reservableAgain(&again_total)}) { + const QString coins{CoinCount(again->inputs.size())}; + m_coins_label->setText( + again->hasChange + ? tr("Uses %1 (%2) as reserved before; change %3 returns to the same address of yours.") + .arg(coins, FormatAmount(m_wallet_model, again_total), + FormatAmount(m_wallet_model, again->changeAmount)) + : tr("Uses %1 (%2) as reserved before, with no change left over.") + .arg(coins, FormatAmount(m_wallet_model, again_total))); } else if (selectCoins(target, selection, error)) { const QString coins{CoinCount(selection.inputs.size())}; m_coins_label->setText( @@ -2358,6 +2368,7 @@ void SharedMnCreateDialog::startSession() unlockAllContributedCoins(); m_coins_released = false; m_released_contributions.clear(); + m_built_contributions.clear(); m_session = MnShareSession(); m_role = Role::Coordinator; m_my_share = 0; @@ -3372,23 +3383,19 @@ CAmount SharedMnCreateDialog::contributionTarget() const return m_role == Role::Coordinator ? share + m_fee_field->value() : share; } -bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, QString& error) const +bool SharedMnCreateDialog::isDust(CAmount value) const { - selection = CoinSelection{}; - error.clear(); - if (m_wallet_model == nullptr) { - error = tr("No wallet is available."); - return false; - } - if (target <= 0) { - error = tr("Select the participant that is you."); - return false; - } + const CTxOut probe{value, GetScriptForDestination(PKHash{CKeyID{}})}; + return IsDust(probe, m_node.getDustRelayFee()); +} +std::vector> SharedMnCreateDialog::fundingCandidates() const +{ + std::vector> candidates; + if (m_wallet_model == nullptr) return candidates; // listCoins() already holds back unconfirmed coins the wallet does not // trust, so an InstantSend-locked coin or this wallet's own unconfirmed // change can fund a share, as it can any other payment - std::vector> candidates; for (const auto& [dest, coins] : m_wallet_model->wallet().listCoins()) { for (const auto& [outpoint, txout] : coins) { if (txout.is_spent) continue; @@ -3397,6 +3404,58 @@ bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, candidates.emplace_back(txout.txout.nValue, outpoint); } } + return candidates; +} + +bool SharedMnCreateDialog::fundsShareAgain(const MnShareSession::Contribution& released, CAmount* total_out) const +{ + if (m_wallet_model == nullptr || released.inputs.empty()) return false; + const auto candidates{fundingCandidates()}; + CAmount total{0}; + for (const auto& input : released.inputs) { + const COutPoint outpoint{uint256S(input.txid.toStdString()), input.vout}; + const auto coin{std::find_if(candidates.begin(), candidates.end(), + [&](const auto& candidate) { return candidate.second == outpoint; })}; + if (coin == candidates.end()) return false; + total += coin->first; + } + if (total_out != nullptr) *total_out = total; + const CAmount target{contributionTarget()}; + if (released.hasChange) return total - released.changeAmount == target; + // Without change, a dust remainder goes to the fee, as selectCoins() leaves it + return total == target || (total > target && isDust(total - target)); +} + +const MnShareSession::Contribution* SharedMnCreateDialog::reservableAgain(CAmount* total_out) const +{ + if (m_my_share < 0 || static_cast(m_my_share) >= m_session.shares().size()) return nullptr; + const QString& label{m_session.shares()[m_my_share].label}; + // Only a contribution this wallet built itself goes back: one absorbed from + // a reply under this share's name can carry anything the sender chose + for (auto released{m_released_contributions.rbegin()}; released != m_released_contributions.rend(); ++released) { + if (released->label != label) continue; + const auto built{std::find_if(m_built_contributions.begin(), m_built_contributions.end(), [&](const auto& c) { + return c.label == label && MnShareSession::SameContribution(c, *released); + })}; + if (built != m_built_contributions.end() && fundsShareAgain(*built, total_out)) return &*built; + } + return nullptr; +} + +bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, QString& error) const +{ + selection = CoinSelection{}; + error.clear(); + if (m_wallet_model == nullptr) { + error = tr("No wallet is available."); + return false; + } + if (target <= 0) { + error = tr("Select the participant that is you."); + return false; + } + + std::vector> candidates{fundingCandidates()}; // Largest first keeps the input count, and with it everyone's signing work // and the transaction fee, as small as possible. std::sort(candidates.begin(), candidates.end(), @@ -3405,13 +3464,9 @@ bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, // A change output below the dust threshold cannot be relayed, so keep // adding coins until the change clears it; if nothing does, drop the // change and let the remainder go to the fee. - const auto is_dust = [this](CAmount value) { - const CTxOut probe{value, GetScriptForDestination(PKHash{CKeyID{}})}; - return IsDust(probe, m_node.getDustRelayFee()); - }; size_t used{0}; for (; used < candidates.size(); ++used) { - if (selection.total >= target && (selection.total == target || !is_dust(selection.total - target))) break; + if (selection.total >= target && (selection.total == target || !isDust(selection.total - target))) break; selection.total += candidates[used].first; MnShareSession::Input input; input.txid = QString::fromStdString(candidates[used].second.hash.ToString()); @@ -3428,7 +3483,7 @@ bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, return false; } const CAmount change{selection.total - target}; - selection.change = (change > 0 && !is_dust(change)) ? change : 0; + selection.change = (change > 0 && !isDust(change)) ? change : 0; return true; } @@ -3475,28 +3530,37 @@ void SharedMnCreateDialog::reserveCoins() return; } - CoinSelection selection; - if (!selectCoins(contributionTarget(), selection, error)) { - showError(error); - return; - } + // Whoever replied before a release holds the contribution as it was then, + // and refuses locked terms with any other. While its coins are still free + // and fund the same amount, reserve exactly that contribution again. MnShareSession::Contribution contribution; - contribution.label = share.label; - contribution.inputs = selection.inputs; - if (selection.change > 0) { - const QString change_address{MasternodeWidgetUtil::freshAddress(m_wallet_model, error)}; - if (change_address.isEmpty()) { + const auto* const again{reservableAgain()}; + if (again != nullptr) { + contribution = *again; + } else { + CoinSelection selection; + if (!selectCoins(contributionTarget(), selection, error)) { showError(error); return; } - contribution.hasChange = true; - contribution.changeAddress = change_address; - contribution.changeAmount = selection.change; + contribution.label = share.label; + contribution.inputs = selection.inputs; + if (selection.change > 0) { + const QString change_address{MasternodeWidgetUtil::freshAddress(m_wallet_model, error)}; + if (change_address.isEmpty()) { + showError(error); + return; + } + contribution.hasChange = true; + contribution.changeAddress = change_address; + contribution.changeAmount = selection.change; + } } if (!m_session.addContribution(contribution, error)) { showError(error); return; } + if (again == nullptr) m_built_contributions.push_back(contribution); setContributionLocked(contribution, /*lock=*/true); m_coins_released = false; // Coins reserved again are no longer released diff --git a/src/qt/sharedmncreatedialog.h b/src/qt/sharedmncreatedialog.h index 9a33083524a1..808ad1088c84 100644 --- a/src/qt/sharedmncreatedialog.h +++ b/src/qt/sharedmncreatedialog.h @@ -21,6 +21,7 @@ #include #include #include +#include #include struct SharedMnOwnFunding; @@ -274,6 +275,16 @@ private Q_SLOTS: void followNewerInvitation(const MnShareSession& imported); // Funding + //! Whether a P2PKH output of `value` would be dust + bool isDust(CAmount value) const; + //! This wallet's unspent, unlocked, spendable coins that may fund a share + std::vector> fundingCandidates() const; + //! Whether `released`'s coins are all still funding candidates, and fund + //! exactly what the share needs now + bool fundsShareAgain(const MnShareSession::Contribution& released, CAmount* total_out = nullptr) const; + //! The most recently released contribution of this wallet's share that + //! Reserve would put back as it was, with its input total, or nullptr + const MnShareSession::Contribution* reservableAgain(CAmount* total_out = nullptr) const; //! Confirmed, unlocked, spendable wallet coins covering `target`, largest //! first, with the change kept above the dust threshold when possible bool selectCoins(CAmount target, CoinSelection& selection, QString& error) const; @@ -409,6 +420,9 @@ private Q_SLOTS: //! less any coins reserved again since. A reply to an earlier invitation //! still carries them, and only Reserve may put them back. std::vector m_released_contributions; + //! Contributions Reserve has reserved, built from this wallet's own coin + //! selection or put back: the only ones it may put back after a release + std::vector m_built_contributions; Role m_role{Role::Undecided}; int m_my_share{-1}; std::optional m_validation_page; diff --git a/src/qt/test/sharedmnwizardtests.cpp b/src/qt/test/sharedmnwizardtests.cpp index 50ae27f77f61..f4967448649c 100644 --- a/src/qt/test/sharedmnwizardtests.cpp +++ b/src/qt/test/sharedmnwizardtests.cpp @@ -75,6 +75,7 @@ using MasternodeTestUtil::FreshOperatorPubKey; using MasternodeTestUtil::FreshP2PKHAddress; using MasternodeTestUtil::DraftReply; using MasternodeTestUtil::MakeSpendingWallet; +using MasternodeTestUtil::MakeTestWallet; using MasternodeTestUtil::PrepareRegistration; using MasternodeTestUtil::PreparedRegistration; using MasternodeTestUtil::WalletGuard; @@ -400,6 +401,228 @@ void SharedMnWizardTests::coordinatorResumesSavedSession() QCOMPARE(int(participant.currentPage()), int(SharedMnCreateDialog::PageContribution)); } +void SharedMnWizardTests::reservingAgainKeepsEarlierReplies() +{ + TestChain100Setup test; + m_node.setContext(&test.m_node); + WalletContext& context{*m_node.walletLoader().context()}; + const auto wallet{MakeTestWallet(m_node, context, "coord")}; + QVERIFY(wallet != nullptr); + WalletGuard guard{context, wallet}; + CTxDestination owner; + COutPoint coin; + COutPoint other_coin; + COutPoint twice_coin; + COutPoint dust_coin; + COutPoint dust_change_coin; + COutPoint sequence_coin; + COutPoint larger_coin; + { + LOCK(wallet->cs_wallet); + const auto destination{wallet->GetNewDestination("")}; + QVERIFY(destination); + owner = *destination; + const CBlockIndex* tip{ + WITH_LOCK(test.m_node.chainman->GetMutex(), return test.m_node.chainman->ActiveChain().Tip())}; + wallet->SetLastBlockProcessed(tip->nHeight, tip->GetBlockHash()); + CMutableTransaction funding; + funding.vin.emplace_back(GetRandHash(), 0); + funding.vout.emplace_back(450 * COIN, GetScriptForDestination(owner)); + QVERIFY(wallet->AddToWallet(MakeTransactionRef(funding), + wallet::TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/0}) != nullptr); + coin = COutPoint{funding.GetHash(), 0}; + CMutableTransaction other_funding; + other_funding.vin.emplace_back(GetRandHash(), 0); + other_funding.vout.emplace_back(420 * COIN, GetScriptForDestination(owner)); + QVERIFY(wallet->AddToWallet(MakeTransactionRef(other_funding), + wallet::TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/1}) != nullptr); + other_coin = COutPoint{other_funding.GetHash(), 0}; + // 410 for a contribution listing its coin twice; one with a dust + // remainder over a share of 400 and the default 0.001 fee + CMutableTransaction more_funding; + more_funding.vin.emplace_back(GetRandHash(), 0); + more_funding.vout.emplace_back(410 * COIN, GetScriptForDestination(owner)); + more_funding.vout.emplace_back(400 * COIN + COIN / 1000 + 100, GetScriptForDestination(owner)); + more_funding.vout.emplace_back(400 * COIN + COIN / 1000 + 100, GetScriptForDestination(owner)); + more_funding.vout.emplace_back(405 * COIN, GetScriptForDestination(owner)); + QVERIFY(wallet->AddToWallet(MakeTransactionRef(more_funding), + wallet::TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/2}) != nullptr); + twice_coin = COutPoint{more_funding.GetHash(), 0}; + dust_coin = COutPoint{more_funding.GetHash(), 1}; + dust_change_coin = COutPoint{more_funding.GetHash(), 2}; + sequence_coin = COutPoint{more_funding.GetHash(), 3}; + // A larger coin a fresh selection would prefer, held back until needed + CMutableTransaction larger_funding; + larger_funding.vin.emplace_back(GetRandHash(), 0); + larger_funding.vout.emplace_back(600 * COIN, GetScriptForDestination(owner)); + QVERIFY(wallet->AddToWallet(MakeTransactionRef(larger_funding), + wallet::TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/3}) != nullptr); + larger_coin = COutPoint{larger_funding.GetHash(), 0}; + } + MasternodeTestUtil::GuiModels models{m_node}; + QVERIFY2(models.ok, qPrintable(QString::fromStdString(models.error.translated))); + WalletModel wallet_model(interfaces::MakeWallet(context, wallet), models.client); + wallet_model.wallet().lockCoin(larger_coin, /*write_to_db=*/false); + + // This wallet coordinates and has answered for alice's row, but has not + // reserved its coins yet + CBLSSecretKey operator_secret; + operator_secret.MakeNewKey(); + MnShareSession draft{InvitationSession()}; + draft.setPrepareWallet(QStringLiteral("coord")); + draft.terms().operatorPubKey = + QString::fromStdString(operator_secret.GetPublicKey().ToString(/*specificLegacyScheme=*/false)); + draft.shares()[0].ownerAddress = QString::fromStdString(EncodeDestination(owner)); + draft.shares()[0].refundAddress = FreshP2PKHAddress(); + + SharedMnCreateDialog dialog(m_node, &wallet_model, /*parent=*/nullptr); + dialog.handleImportedText(draft.toJsonString()); + QCOMPARE(int(dialog.m_role), int(SharedMnCreateDialog::Role::Coordinator)); + QCOMPARE(dialog.myShareIndex(), 0); + dialog.goToPage(SharedMnCreateDialog::PageContribution); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + const MnShareSession::Contribution sent{*dialog.myContribution()}; + QVERIFY(sent.hasChange); + QVERIFY(wallet_model.wallet().isLockedCoin(coin)); + + // bob and carol answer the invitation that carried this contribution + const MnShareSession invitation{dialog.m_session}; + const MnShareSession bob{DraftReply(invitation, 1, FakeTxid('2'))}; + const MnShareSession carol{DraftReply(invitation, 2, FakeTxid('3'))}; + QVERIFY(bob.hasContribution(sent)); + + // The coordinator releases its coins, takes in bob's reply, and reserves + // again: the same coins and change, which bob's copy already lists + dialog.releaseCoins(); + QVERIFY(!wallet_model.wallet().isLockedCoin(coin)); + dialog.handleImportedText(bob.toJsonString()); + QVERIFY2(dialog.m_error_label->text().isEmpty(), qPrintable(dialog.m_error_label->text())); + QVERIFY(dialog.myContribution() == nullptr); + QVERIFY(!wallet_model.wallet().isLockedCoin(coin)); + + // The coins card describes those coins, not the larger one that has + // become available since and a fresh selection would take + const auto amount = [&](CAmount value) { return SharedMnFormatAmount(SharedMnDisplayUnit(&wallet_model), value); }; + wallet_model.wallet().unlockCoin(larger_coin); + dialog.refreshContributionPage(); + const QString preview{dialog.m_coins_label->text()}; + QVERIFY2(preview.contains(amount(sent.changeAmount)) && !preview.contains(amount(600 * COIN)), qPrintable(preview)); + wallet_model.wallet().lockCoin(larger_coin, /*write_to_db=*/false); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + QVERIFY(dialog.m_session.hasContribution(sent)); + QVERIFY(wallet_model.wallet().isLockedCoin(coin)); + + // So the terms locked from it are the ones bob agreed to answer + dialog.handleImportedText(carol.toJsonString()); + QVERIFY2(dialog.m_error_label->text().isEmpty(), qPrintable(dialog.m_error_label->text())); + QVERIFY(dialog.allDetailsCollected()); + MnShareSession locked{dialog.m_session}; + const PreparedRegistration prepared{PrepareRegistration(locked, operator_secret)}; + QString error; + QVERIFY2(locked.freeze(prepared.txHex, prepared.consentHashHex, prepared.collateralIndex, error), + qPrintable(error)); + MnShareSession bob_copy{bob}; + QVERIFY2(bob_copy.adoptLockedTerms(locked, error), qPrintable(error)); + + // A contribution a reply lists under alice's name is never reserved again + // after a release, however well it funds the share: this wallet goes back + // to the one it built. First, one spending another coin of this wallet + // with its change paid elsewhere. + dialog.releaseCoins(); + MnShareSession planting{dialog.m_session}; + MnShareSession::Contribution planted; + planted.label = sent.label; + planted.inputs = {{QString::fromStdString(other_coin.hash.ToString()), other_coin.n}}; + planted.hasChange = true; + planted.changeAddress = FreshP2PKHAddress(); + planted.changeAmount = 420 * COIN - (400 * COIN + dialog.m_fee_field->value()); + QVERIFY2(planting.addContribution(planted, error), qPrintable(error)); + dialog.handleImportedText(planting.toJsonString()); + QVERIFY(dialog.m_session.hasContribution(planted)); + dialog.releaseCoins(); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + QVERIFY(!dialog.m_session.hasContribution(planted)); + QVERIFY(dialog.m_session.hasContribution(sent)); + QVERIFY(wallet_model.wallet().isLockedCoin(coin)); + QVERIFY(!wallet_model.wallet().isLockedCoin(other_coin)); + + // One listing a coin twice, with this wallet's own change address + const auto plant_and_reserve_again = [&](const MnShareSession::Contribution& contribution) { + dialog.releaseCoins(); + MnShareSession reply{dialog.m_session}; + QString plant_error; + QVERIFY2(reply.addContribution(contribution, plant_error), qPrintable(plant_error)); + dialog.handleImportedText(reply.toJsonString()); + QVERIFY(dialog.m_session.hasContribution(contribution)); + dialog.releaseCoins(); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + }; + MnShareSession::Contribution twice{planted}; + const MnShareSession::Input twice_input{QString::fromStdString(twice_coin.hash.ToString()), twice_coin.n}; + twice.inputs = {twice_input, twice_input}; + twice.changeAddress = sent.changeAddress; + twice.changeAmount = 2 * 410 * COIN - (400 * COIN + dialog.m_fee_field->value()); + plant_and_reserve_again(twice); + QVERIFY(!dialog.m_session.hasContribution(twice)); + QVERIFY(dialog.m_session.hasContribution(sent)); + + // One with 100 duffs of change to this wallet, which could never be relayed + MnShareSession::Contribution dust_change{twice}; + dust_change.inputs = {{QString::fromStdString(dust_change_coin.hash.ToString()), dust_change_coin.n}}; + dust_change.changeAmount = 100; + plant_and_reserve_again(dust_change); + QVERIFY(!dialog.m_session.hasContribution(dust_change)); + QVERIFY(dialog.m_session.hasContribution(sent)); + QVERIFY(!wallet_model.wallet().isLockedCoin(dust_change_coin)); + + // One that funds the share exactly, with change to this wallet, but whose + // input has a relative-lock sequence this wallet refuses to sign + MnShareSession::Contribution relative{twice}; + relative.inputs = {{QString::fromStdString(sequence_coin.hash.ToString()), sequence_coin.n, /*sequence=*/1}}; + relative.changeAmount = 405 * COIN - (400 * COIN + dialog.m_fee_field->value()); + plant_and_reserve_again(relative); + QVERIFY(!dialog.m_session.hasContribution(relative)); + QVERIFY(dialog.m_session.hasContribution(sent)); + QVERIFY(!wallet_model.wallet().isLockedCoin(sequence_coin)); + + // Without change, a dust remainder over the share is left to the fee. A + // contribution Reserve built that way is reserved again as it was. + dialog.releaseCoins(); + const std::vector held{coin, other_coin, twice_coin, dust_change_coin, sequence_coin}; + for (const auto& outpoint : held) { + wallet_model.wallet().lockCoin(outpoint, /*write_to_db=*/false); + } + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + const MnShareSession::Contribution no_change{*dialog.myContribution()}; + QVERIFY(!no_change.hasChange); + QCOMPARE(no_change.inputs.size(), size_t{1}); + QCOMPARE(no_change.inputs[0].txid, QString::fromStdString(dust_coin.hash.ToString())); + for (const auto& outpoint : held) { + wallet_model.wallet().unlockCoin(outpoint); + } + dialog.releaseCoins(); + dialog.reserveCoins(); + QVERIFY(dialog.m_session.hasContribution(no_change)); + QVERIFY(wallet_model.wallet().isLockedCoin(dust_coin)); + QVERIFY(!wallet_model.wallet().isLockedCoin(coin)); + + // With a different fee the released coins no longer fund the share as they + // did, so reserving again selects anew and earlier replies are out of date + dialog.releaseCoins(); + dialog.m_fee_field->setValue(dialog.m_fee_field->value() + COIN / 1000); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + QVERIFY(!dialog.m_session.hasContribution(sent)); + QCOMPARE(dialog.myContribution()->changeAmount, sent.changeAmount - COIN / 1000); + QVERIFY(wallet_model.wallet().isLockedCoin(coin)); + wallet_model.wallet().unlockCoin(coin); +} + void SharedMnWizardTests::coordinatorResumesFullySignedSession() { TestingSetup test{CBaseChainParams::REGTEST}; diff --git a/src/qt/test/sharedmnwizardtests.h b/src/qt/test/sharedmnwizardtests.h index 7c96caad531f..24e961845949 100644 --- a/src/qt/test/sharedmnwizardtests.h +++ b/src/qt/test/sharedmnwizardtests.h @@ -29,6 +29,7 @@ private Q_SLOTS: void participantPagesAndRoleInference(); void participantCannotChooseATakenShare(); void coordinatorResumesSavedSession(); + void reservingAgainKeepsEarlierReplies(); void coordinatorResumesFullySignedSession(); void signedSessionReservation(); void unauthorisedSignedInputsAreDetected();