Skip to content

simplewallet: fix payment ID false positive on Carrot receive#217

Draft
jeffro256 wants to merge 8 commits intoseraphis-migration:fcmp++-stagefrom
jeffro256:cli_wallet_pid_recv_warn
Draft

simplewallet: fix payment ID false positive on Carrot receive#217
jeffro256 wants to merge 8 commits intoseraphis-migration:fcmp++-stagefrom
jeffro256:cli_wallet_pid_recv_warn

Conversation

@jeffro256
Copy link
Collaborator

@jeffro256 jeffro256 commented Nov 5, 2025

Also add payment ID paramater to on_money_received() wallet callback.

Resolves #136

jeffro256 and others added 8 commits October 27, 2025 16:33
This replaces `ver_rct_non_semantics_simple_cached()` with an API that offloads
the responsibility of tracking input verification successes to the caller. The
main caller of this function in the codebase, `cryptonote::Blockchain()` instead
keeps track of the verification results for transaction in the mempool by
storing a "verification ID" in the mempool metadata table (with `txpool_tx_meta_t`).
This has several benefits, including:

* When the mempool is large (>8192 txs), we no longer experience cache misses and unnecessarily re-verify ring signatures. This greatly improves block propagation time for FCMP++ blocks under load
* For the same reason, reorg handling can be sped up by storing verification IDs of transactions popped from the chain
* Speeds up re-validating every mempool transaction on fork change (monerod revalidates the whole tx-pool on HFs monero-project#10142)
* Caches results for every single type of Monero transaction, not just latest RCT type
* Cache persists over a node restart
* Uses 512KiB less RAM (8192*2*32B)
* No additional storage or DB migration required since `txpool_tx_meta_t` already had padding allocated
* Moves more verification logic out of `cryptonote::Blockchain`

Furthermore, this opens the door to future multi-threaded block verification
speed-ups. Right now, transactions' input proof verification is limited to one
transaction at a time. However, one can imagine a scenario with verification IDs
where input proofs are optimistically multi-threaded in advance of block
processing. Then, even though ring member fetching and verification is
single-threaded inside of `cryptonote::Blockchain::check_tx_inputs()`, the
single thread can skip the CPU-intensive cryptographic code if the verification
ID allows it.

Also changes the default log category in `tx_verification_utils.cpp` from "blockchain" to "verify".
Co-authored-by: j-berman <justinberman@protonmail.com>
Co-authored-by: jeffro256 <jeffro256@tutanota.com>
Co-authored-by: Luke Parker <lukeparker5132@gmail.com>
Co-authored-by: SyntheticBird45 <someoneelse.is_on.github.rio7x@simplelogin.com>
Otherwise we can end up double counting txs towards the weight,
which can over-state the pool weight. E.g. relay tx to node in
stem phase, add its weight to pool weight, then receive tx
from another node, then bump the pool weight again. That double
counts the tx towards the pool weight.

If the weight exceeds the max, the node will "prune" txs from the
pool. Thus, over-counting is probably a cause of, but perhaps
not the only cause of:
seraphis-migration#148
Curve Trees: handle get_max_concurrency() == 0
tx pool: only increment m_txpool_weight for newly added pool txs
Also add payment ID paramater to `on_money_received()` wallet callback.
@jeffro256
Copy link
Collaborator Author

Actually, before this gets merged to fcmp++-stage, I might want to change it to make it easier to upstream. But merging the fix to stressnet should be fine as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants