Match Core generateblock txid and raw-tx parse errors - #604
Conversation
64-character hex is a mempool txid looked up at parse time; a miss is Core -5 with the caller's string. Anything else is DecodeHexTx: invalid hex or an incomplete transaction is Core -22 with Core's decode text. Co-authored-by: metaphorics <metaphorics@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e17f4581-aa16-4ccc-b7da-d9da53b5ad16) |
PR Summary by QodoMatch Core generateblock transaction parse errors
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Qodo Fixer🍒 Ready to be cherry-picked — ✅ Merged (0) · ☑ Fixed (2) 🔗 Fix PR: #606 This fix PR was closed automatically. Its branch is preserved so you can cherry pick the changes into the original PR. Prompt for coding agent Process — 2 fixed
|
… a (#606) ## Fixed Findings - Preserve resolved mempool transactions - Reference API-24 transaction semantics Automated fix from agentic review of #604 <a href="https://www.qodo.ai"><img src="https://www.qodo.ai/wp-content/uploads/2025/03/qodo-logo.svg" width="80" alt="Qodo Logo"></a> --------- Co-authored-by: qodo-code-review[bot] <151058649+qodo-code-review[bot]@users.noreply.github.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_5eb4f36b-591b-406d-b3c6-4870d493d4cb) |
| /// Include this transaction resolved from the mempool at parse time. | ||
| ResolvedMempool(SnapshotEntry), |
There was a problem hiding this comment.
1. resolvedmempool breaks compatibility tests 📘 Rule violation ≡ Correctness
GenerateTx derives Eq and PartialEq, but the new ResolvedMempool(SnapshotEntry) payload implements neither trait, causing trait-bound compilation failures; after restoring comparability, generateblock_keeps_raw_transactions must also be updated because it still expects the obsolete GenerateTx::Mempool variant instead of the newly emitted ResolvedMempool variant.
Agent Prompt
## Issue description
The new `ResolvedMempool(SnapshotEntry)` variant is incompatible with `GenerateTx`'s derived `Eq` and `PartialEq` traits, and the existing request assertion still expects the old `GenerateTx::Mempool` variant. Restore compilation by making the payload comparable or changing the surrounding equality implementation without weakening required test behavior, then update the test for the resolved mempool entry produced by parsing.
## Issue Context
API-24 compatibility must be verified by executable tests. `SnapshotEntry` contains equality-compatible scalar fields, a transaction `Arc`, and an ancestor vector, so deriving the missing traits is likely sufficient; once `GenerateTx` remains comparable, the observable-behavior test must assert the newly emitted `ResolvedMempool` variant.
## Fix Focus Areas
- crates/mining/src/control.rs[215-223]
- crates/mempool/src/pool.rs[189-230]
- crates/rpc/src/handlers/mining.rs[2108-2116]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| let snapshot = ctx.mempool.read().mining_snapshot(); | ||
| let Some(entry) = snapshot.entries.into_iter().find(|entry| entry.txid == txid) else { |
There was a problem hiding this comment.
2. Snapshot rebuilt per txid 🐞 Bug ➹ Performance
Each txid-shaped array entry rebuilds and scans a complete mempool mining snapshot, making parsing O(requested txids × mempool size) with repeated allocations and ancestor-topology reconstruction. Large valid generateblock requests can therefore hold the mempool read path and consume substantially more CPU than necessary.
Agent Prompt
## Issue description
`parse_generateblock_transactions` calls `mining_snapshot()` inside the transactions loop and linearly scans every resulting snapshot. Capture one snapshot for the request and index its entries by txid, preferably lazily so raw-only requests do not copy the mempool.
## Issue Context
`mining_snapshot()` copies every mempool entry and reconstructs priority ordering, a position map, and ancestor vectors. Reusing one snapshot also gives all txid resolutions a coherent view.
## Fix Focus Areas
- crates/rpc/src/handlers/mining.rs[379-392]
- crates/mempool/src/pool.rs[715-767]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 1fdd20f |
2 participants
Stacked on #602. Core v31
generateblock(src/rpc/mining.cpp) classifies each transactions-array entry before assembling:Txid::FromHex. A txid missing from the mempool is-5Transaction {str} not in mempool.using the caller’s string.DecodeHexTx. Invalid hex or an incomplete transaction is-22Transaction decode failed for {str}. Make sure the tx has at least one input.This node previously treated unknown 64-hex entries as
GenerateTx::Mempooland only failed later in the coordinator (RPC mapped that to-32603), and decode/hex failures were JSON-RPC-32602. Parse now looks the txid up at the RPC boundary and emits Core’s codes and text. Assembly still receivesGenerateTx::MempoolvsRaw; extra positionals stay rejected (Core 31 has nomaxtriesongenerateblock).Contract:
API-24. Proven bygenerateblock_rejects_unknown_mempool_txid_like_core,generateblock_rejects_undecodable_raw_tx_like_core, and the updatedgenerateblock_keeps_raw_transactionspool insert.Does not complete #151. Remaining mining work includes the live Core differential suite (#78), numeric p95 budgets (#158),
-acceptnonstdtxnas a real option, and pruneBLOCK_HAVE_DATAvs scripts-valid.