feat(node): reorg returned-txs + peer attribution (workstream C) + shadow triage doc - #194
Conversation
…ion, shadow triage doc Workstream C completion (operator letter item 3): the reorg event now names which txs the rollback returned to the mempool and which peer first delivered the winning tip. - Enrichment is captured on the ONE path where the facts exist — the tip-change diff in handle_mempool_tick (TxDiff.demoted + the first-deliverer ring) — and matched to the differ's reorg event BY TIP ID: a stale record attaches nothing (differ test covers both). - returned_tx_ids capped at 128 with an uncapped returned_txs_total (no silent truncation); delivered_by is best-effort (ring eviction). - Carried uniformly: FeedEventKind::Reorg → ApiNodeEvent (GET /api/v1/events) → WS reorg payload (new ReorgPayload builder arg-struct) → ReorgRecord → GET /api/v1/diagnostics/reorgs. OpenAPI goldens regenerated. docs/operating.md gains the Shadow validation section: config, the two signals mapped to consensus-bug classes, metrics/alerts, and the "divergence fired — who is wrong?" triage checklist (workstream D's final deliverable). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BUh2DBnAPqThdYFZW5D8wx
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughChangesThe node captures returned-transaction and tip-deliverer metadata during rollbacks, matches it to reorgs by winning tip ID, and exposes it through API, websocket, realtime, history, and OpenAPI payloads. Operating documentation adds a shadow-validation runbook. Reorg diagnostics enrichment
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Mempool
participant NodeState
participant EventFeed
participant API
Mempool->>NodeState: record rollback enrichment
NodeState->>EventFeed: provide latest enrichment
EventFeed->>EventFeed: match winning tip ID
EventFeed->>API: project enriched reorg event
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Caught during the live soak: the checklist's own first step 404'd as written (/status is not the JSON status route). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BUh2DBnAPqThdYFZW5D8wx
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ergo-api/tests/fixtures/openapi_v1.yaml (1)
5510-5549: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMark the non-optional reorg fields as required.
returnedTxIdsandreturnedTxsTotalare always serialized byApiReorgRecord, but the schema omits them fromrequired. Add both so generated clients receive the accurate contract; keepdeliveredByoptional.Proposed schema fix
required: - unixMs - height - headerId - depth - droppedHeaderIds - orphansTruncated + - returnedTxIds + - returnedTxsTotal🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ergo-api/tests/fixtures/openapi_v1.yaml` around lines 5510 - 5549, Add returnedTxIds and returnedTxsTotal to the required field list for the ApiReorgRecord schema, while leaving deliveredBy optional. Keep their existing property definitions unchanged so generated clients reflect the serialization contract.ergo-api/tests/fixtures/openapi_native.yaml (1)
1952-1998: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the ApiNodeEvent schema description.
The enclosing description still lists only
depthanddropped_header_idsas reorg-only fields, omittingdeliveredBy,returnedTxIds, andreturnedTxsTotal. Generated API documentation will be incomplete unless it is updated.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ergo-api/tests/fixtures/openapi_native.yaml` around lines 1952 - 1998, The enclosing ApiNodeEvent schema description should identify deliveredBy, returnedTxIds, and returnedTxsTotal as reorg-only fields alongside depth and dropped_header_ids. Update only that description so generated API documentation lists all reorg-specific properties.
🧹 Nitpick comments (2)
ergo-api/src/v1/realtime/ws.rs (1)
358-369: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a positive regression test for enriched reorg payloads.
project_tick_maps_reorg_eventsonly suppliesNonefor the new fields and asserts legacy fields, so regressions inreturned_tx_ids,returned_txs_total, ordelivered_bywould pass unnoticed. Populate the fixture and assert all three emitted values.Also applies to: 458-469
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ergo-api/src/v1/realtime/ws.rs` around lines 358 - 369, Update the reorg event regression fixture in project_tick_maps_reorg_events to provide non-empty returned_tx_ids, returned_txs_total, and delivered_by values, then assert the emitted ReorgPayload preserves all three values alongside the existing fields.ergo-api/tests/events_endpoint.rs (1)
22-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise enriched reorg fields in the REST endpoint test.
The helper always sets the new fields to
None, and the current feed has no reorg event. A REST serialization regression could therefore pass unnoticed. Add a reorg fixture with representative values and assertreturnedTxIds,returnedTxsTotal, anddeliveredBy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ergo-api/tests/events_endpoint.rs` around lines 22 - 32, Add a reorg event fixture to the REST endpoint test feed using representative values for returned_tx_ids, returned_txs_total, and delivered_by, while keeping those fields absent for other event kinds as appropriate. Extend the endpoint response assertions to verify the serialized returnedTxIds, returnedTxsTotal, and deliveredBy values for that reorg event, exercising the helper event and REST serialization path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/operating.md`:
- Around line 380-393: Update the curl commands in “Divergence fired — who is
wrong?” to use the documented operator API endpoint 127.0.0.1:9099 instead of
:9063, including both node status checks. Preserve the existing jq filters and
troubleshooting guidance.
---
Outside diff comments:
In `@ergo-api/tests/fixtures/openapi_native.yaml`:
- Around line 1952-1998: The enclosing ApiNodeEvent schema description should
identify deliveredBy, returnedTxIds, and returnedTxsTotal as reorg-only fields
alongside depth and dropped_header_ids. Update only that description so
generated API documentation lists all reorg-specific properties.
In `@ergo-api/tests/fixtures/openapi_v1.yaml`:
- Around line 5510-5549: Add returnedTxIds and returnedTxsTotal to the required
field list for the ApiReorgRecord schema, while leaving deliveredBy optional.
Keep their existing property definitions unchanged so generated clients reflect
the serialization contract.
---
Nitpick comments:
In `@ergo-api/src/v1/realtime/ws.rs`:
- Around line 358-369: Update the reorg event regression fixture in
project_tick_maps_reorg_events to provide non-empty returned_tx_ids,
returned_txs_total, and delivered_by values, then assert the emitted
ReorgPayload preserves all three values alongside the existing fields.
In `@ergo-api/tests/events_endpoint.rs`:
- Around line 22-32: Add a reorg event fixture to the REST endpoint test feed
using representative values for returned_tx_ids, returned_txs_total, and
delivered_by, while keeping those fields absent for other event kinds as
appropriate. Extend the endpoint response assertions to verify the serialized
returnedTxIds, returnedTxsTotal, and deliveredBy values for that reorg event,
exercising the helper event and REST serialization path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a548353f-044a-486f-aed2-fadf5057be76
📒 Files selected for processing (14)
docs/operating.mdergo-api/src/types.rsergo-api/src/v1/realtime/model.rsergo-api/src/v1/realtime/ws.rsergo-api/tests/events_endpoint.rsergo-api/tests/fixtures/openapi_native.yamlergo-api/tests/fixtures/openapi_v1.yamlergo-node/src/node/action_loop.rsergo-node/src/node/boot.rsergo-node/src/node/event_feed.rsergo-node/src/node/reorg_history.rsergo-node/src/node/snapshot_emit.rsergo-node/src/node/state.rsergo-node/src/node/tests.rs
…dge/REST enrichment coverage - operating.md triage commands use the documented default API bind (127.0.0.1:9099), not this operator's local 9063 habit. - ApiNodeEvent doc (the OpenAPI description source) now names ALL reorg-only fields and the full kind list — it was stale twice over, missing syncWedged and shadowDivergence too. Goldens regenerated. - ApiReorgRecord.returned_tx_ids/returned_txs_total drop serde(default) so the v1 schema marks them required — they are always serialized; delivered_by stays optional. - ws bridge test asserts the enrichment triple survives project_tick; events REST test exercises a reorg fixture end-to-end (camelCase returnedTxIds/returnedTxsTotal/deliveredBy) and confirms non-reorg kinds omit the optionals. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BUh2DBnAPqThdYFZW5D8wx
Bump workspace version to 0.5.2 and promote the changelog: the complete v1 product API (#168-#185, #188), shadow validation as a production mode (#193-#195), the operator observability wave (#187, #190, #192, #194), two live accept-invalid consensus fixes (#176, #179), ErgoScript compiler byte-parity completion (#165-#167, #175), and the #160-#163 sync/recovery fixes. Full workspace gate run on the merge result. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BUh2DBnAPqThdYFZW5D8wx
What
Two operator-workload completions in one focused PR:
1. Workstream C tail — reorg introspection is now complete. The
reorgevent (RESTGET /api/v1/events, WSblockschannel, and theGET /api/v1/diagnostics/reorgspostmortem ring) now carries:returned_tx_ids— the rolled-back txs the reorg returned to the mempool (first 128, plus an uncappedreturned_txs_totalso truncation is never silent),delivered_by— the peer that FIRST delivered the winning tip header (best-effort from the first-deliverer ring).Capture happens on the single path where those facts exist — the tip-change diff (
TxDiff.demoted+ deliverer ring) on the action loop — and is matched to the event differ's reorg BY TIP ID, so a stale enrichment from an earlier rollback attaches nothing.2. Workstream D final deliverable — the operator triage doc.
docs/operating.mdgains a Shadow validation section: config, the two signals mapped to consensus-bug classes (accept-invalid / reject-valid), Prometheus alert lines, and the "divergence fired — who is wrong?" checklist (transient check → own apply path → third-node arbitration → reorg context → reference health), ending with the preserve-the-data-dir rule.Test plan
cargo fmt --check/ workspaceclippy -D warnings/cargo test --workspaceall green.🤖 Generated with Claude Code
https://claude.ai/code/session_01BUh2DBnAPqThdYFZW5D8wx
Summary by CodeRabbit