Skip to content

Conversation

@j-berman
Copy link
Collaborator

@j-berman j-berman commented Sep 17, 2025

A reorg can end up causing an output's position in the chain to move. Since the wallet doesn't update the RingDB on reorg, it may refer to the output's stale position in the chain, and then throw when attempting to spend that output in the future.

Logging a warning instead of throwing seems a reasonable solution rather than introducing complex logic to update the stale ring member's value on rerog, since RingDB can be deprecated with FCMP++, and the downside to warning here is minimal.

A reorg can end up causing an output's position in the chain to
move. Since the wallet doesn't update the RingDB on reorg, it
may refer to the output's stale position in the chain.

This seems a reasonable solution rather than introducing complex
logic to update the stale ring member's value on rerog, since
RingDB can be deprecated with FCMP++.
@j-berman
Copy link
Collaborator Author

Repeating more context from #10085 (comment):

the wallet saves rings used in prior constructed txs, so that it will attempt to reuse the same ring if spending the same output (thus avoiding revealing which output is the real spend in the ring). That ring saves references to ring members by global output ID.

A reorg can modify global output ID's tied to members of a ring, including the output spent. The logic inside get_outs that reads saved rings does not account for the potential of the spent ring member's output id to be modified by the reorg here, and thus the wallet may be unable to spend an output in that case unless the user explicitly calls unset_ring, which could reveal to the network which output they are spending.

The simplest solution to that is probably to remove this throw, and log a warning in that case instead.

@selsta
Copy link
Collaborator

selsta commented Oct 6, 2025

@j-berman please also open against release branch

@luigi1111 luigi1111 merged commit 44b242d into monero-project:master Oct 7, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants