wallet2: exclude outputs from an unconfirmed send in reserve proofs
What the developer wrote
Authored by Cole Munz
wallet2: exclude outputs from an unconfirmed send in reserve proofs
get_reserve_proof picks outputs with is_spent(td, true). That only
counts an output spent once its spending tx has confirmed. commit_tx
marks an output spent as soon as the tx goes to the daemon, well before
it confirms. spent_height stays 0 until then. So an output already used
as input to a pending send still passes as unspent here.
A wallet can prove reserve over outputs it already committed to a
pending send. check_reserve_proof reports them unspent, but they are
gone as soon as that send confirms. Reported with a stagenet repro in
#6595.
Switched to is_spent(td, false), which the wallet's default balance
already uses for this reason. Both the zero-balance guard and the
account_minreserve check used to go through balance_all()/balance(),
which fold in change and self-transfer amounts from our own
unconfirmed txs (m_unconfirmed_txs, m_unconfirmed_payments). Those
amounts have no matching row in m_transfers yet, so once an output is
tied up in a pending send, is_spent(td, false) drops it from
selected_transfers while the pending change keeps the balance call
non-zero, and the guard passes over an empty or thinner selection than
it should. Now both guards sum selected_transfers directly, computed
once, so they match exactly what the proof is built over.
What changed, and why it matters
This fix corrects a Monero wallet bug where a user could generate a cryptographic 'reserve proof' claiming they still owned money that they had already committed to spending. Before the patch, the wallet only excluded outputs from the proof once the spending transaction was confirmed on the blockchain. Because the wallet marks outputs as spent earlier—when the transaction is first submitted to the network—a user could honestly but incorrectly prove reserve over outputs that would disappear as soon as the pending send confirmed. The patch makes the wallet treat pending-spent outputs as unavailable and checks the exact outputs selected for the proof rather than a broader balance figure that could include pending change.
Reviewers should verify that is_spent(td, false) correctly reflects the wallet's intended behavior for unconfirmed outgoing spends and that no other proof-generation paths still rely on balance_all(true)/balance(..., true) in a similar way. Consider adding regression tests covering reserve proofs with unconfirmed spends and account_minreserve edge cases.
Security signals we found
Incorrect spent-state check allows reserve proof over outputs committed to a pending transaction
Balance-based guard mismatched against actual selected outputs due to unconfirmed change/self-transfer amounts
Fix aligns reserve proof semantics with wallet's default balance behavior
Reported with a stagenet reproduction in issue #6595
Evidence from the diff
In wallet2::get_reserve_proof, output selection previously used is_spent(td, true), which returns true only when td.spent_height is non-zero (i.e., the spending tx is confirmed). commit_tx marks an output spent via set_spent before confirmation, leaving spent_height at 0, so outputs already used as inputs to an unconfirmed send were still included in selected_transfers. The zero-balance and account_minreserve guards used balance_all()/balance(), which incorporate m_unconfirmed_txs/m_unconfirmed_payments change/self-transfer amounts that lack m_transfers rows; this could keep the balance non-zero even after all backing outputs were excluded. The patch switches to is_spent(td, false) and sums selected_transfers directly for the guards, aligning the proved set with the wallet’s default balance semantics.
Changed components
src/wallet/wallet2.cppwallet2::get_reserve_proofInspect captured patch +14 / −4
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index 86eba93..cb2a5e6 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -12424,22 +12424,32 @@ bool wallet2::check_tx_proof(const cryptonote::transaction &tx, const cryptonote
std::string wallet2::get_reserve_proof(const boost::optional<std::pair<uint32_t, uint64_t>> &account_minreserve, const std::string &message)
{
THROW_WALLET_EXCEPTION_IF(m_watch_only || m_multisig, error::wallet_internal_error, "Reserve proof can only be generated by a full wallet");
- THROW_WALLET_EXCEPTION_IF(balance_all(true) == 0, error::wallet_internal_error, "Zero balance");
- THROW_WALLET_EXCEPTION_IF(account_minreserve && balance(account_minreserve->first, true) < account_minreserve->second, error::wallet_internal_error,
- "Not enough balance in this account for the requested minimum reserve amount");
// determine which outputs to include in the proof
+ // is_spent(td, false) excludes an output already used as input to a pending, unconfirmed
+ // send too, not just a confirmed one, so we don't claim reserve we no longer have
std::vector<size_t> selected_transfers;
for (size_t i = 0; i < m_transfers.size(); ++i)
{
const transfer_details &td = m_transfers[i];
- if (!is_spent(td, true) && !td.m_frozen && (!account_minreserve || account_minreserve->first == td.m_subaddr_index.major))
+ if (!is_spent(td, false) && !td.m_frozen && (!account_minreserve || account_minreserve->first == td.m_subaddr_index.major))
selected_transfers.push_back(i);
}
+ // sum what's actually selected instead of calling balance_all()/balance(): those also fold
+ // in pending change and self-transfer amounts from our own unconfirmed txs, which have no
+ // matching entry in m_transfers yet and can leave a balance-based guard non-zero even once
+ // every output backing it has been excluded above
+ uint64_t available = 0;
+ for (size_t idx : selected_transfers)
+ available += m_transfers[idx].amount();
+ THROW_WALLET_EXCEPTION_IF(available == 0, error::wallet_internal_error, "Zero balance");
+
if (account_minreserve)
{
THROW_WALLET_EXCEPTION_IF(account_minreserve->second == 0, error::wallet_internal_error, "Proved amount must be greater than 0");
+ THROW_WALLET_EXCEPTION_IF(available < account_minreserve->second, error::wallet_internal_error,
+ "Not enough balance in this account for the requested minimum reserve amount");
// minimize the number of outputs included in the proof, by only picking the N largest outputs that can cover the requested min reserve amount
std::sort(selected_transfers.begin(), selected_transfers.end(), [&](const size_t a, const size_t b)
{ return m_transfers[a].amount() > m_transfers[b].amount(); });
Why this scored 60/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.