chore: bound node transaction-event ledger growth#1406
Conversation
Time Submission Status
Submit or update total time with: Add time on top of previous submission with: See available commands to help comply with our Guidelines. |
📝 WalkthroughWalkthroughAdds a ChangesTransaction event retention trimming
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant DigestScheduler
participant EngineOperations
participant trim_transaction_events
participant transaction_events
DigestScheduler->>EngineOperations: Request bounded trim
EngineOperations->>EngineOperations: Read fresh nonce, build, and sign transaction
EngineOperations->>trim_transaction_events: Broadcast preserve_blocks and delete_cap
trim_transaction_events->>transaction_events: Delete capped rows below cutoff
trim_transaction_events-->>EngineOperations: Return NOTICE summary
EngineOperations-->>DigestScheduler: Return deletion result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@internal/migrations/032-order-book-actions.prod.sql`:
- Around line 335-337: Restore the missing place_sell_order action in the
production migration alongside the existing buy and split-limit paths, matching
the implementation from 032-order-book-actions.sql. Ensure it includes the
explicit method-10 transaction ledger entry via record_transaction_event,
preserving the zero protocol-fee values and sell-order behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84448196-846c-4c8f-af10-21f20609f04e
📒 Files selected for processing (3)
internal/migrations/030-order-book-schema.sqlinternal/migrations/032-order-book-actions.prod.sqlinternal/migrations/032-order-book-actions.sql
7b36160 to
15c2cb8
Compare
|
@holdex pr submit-time 4h |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
extensions/tn_digest/internal/engine_ops.go (1)
615-761: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNear-duplicate of the order-events trim plumbing. The transaction-events broadcast/retry/parse functions and the scheduler drain loop mirror the order-events equivalents almost verbatim; extracting a shared generic helper (parameterized by action name, arg encoders, and NOTICE parser) would avoid this duplication growing with each future
trim_X_eventsaddition.
extensions/tn_digest/internal/engine_ops.go#L615-L761: factorBroadcastTrimTransactionEventsWithRetry,broadcastTrimTransactionEventsOnce, andparseTxEventsTrimResultFromTxLoginto a shared generic/parameterized implementation with the order-events equivalents.extensions/tn_digest/scheduler/scheduler.go#L276-L333: factortrimTransactionEvents's drain loop into a shared helper withtrimOrderEvents(227-274), parameterized by the trim function and constants.🤖 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 `@extensions/tn_digest/internal/engine_ops.go` around lines 615 - 761, The trim transaction-events implementation duplicates the order-events plumbing and scheduler drain logic. In extensions/tn_digest/internal/engine_ops.go:615-761, refactor BroadcastTrimTransactionEventsWithRetry, broadcastTrimTransactionEventsOnce, and parseTxEventsTrimResultFromTxLog into shared parameterized helpers reused by the order-events equivalents, covering action names, argument encoding, and NOTICE parsing. In extensions/tn_digest/scheduler/scheduler.go:276-333, extract the trimTransactionEvents drain loop into a shared helper with trimOrderEvents, parameterized by the trim function and relevant constants.
🤖 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 `@internal/migrations/052-transaction-events-retention.sql`:
- Around line 47-57: Update the cutoff<=0 branch in the trim_transaction_events
migration to emit the parser-compatible NOTICE format, using deleted=0,
remaining=0, and has_more=false rather than the cutoff message. Preserve the
existing early return so parseTxEventsTrimResultFromTxLog and
broadcastTrimTransactionEventsOnce recognize the successful no-op without
retries.
---
Nitpick comments:
In `@extensions/tn_digest/internal/engine_ops.go`:
- Around line 615-761: The trim transaction-events implementation duplicates the
order-events plumbing and scheduler drain logic. In
extensions/tn_digest/internal/engine_ops.go:615-761, refactor
BroadcastTrimTransactionEventsWithRetry, broadcastTrimTransactionEventsOnce, and
parseTxEventsTrimResultFromTxLog into shared parameterized helpers reused by the
order-events equivalents, covering action names, argument encoding, and NOTICE
parsing. In extensions/tn_digest/scheduler/scheduler.go:276-333, extract the
trimTransactionEvents drain loop into a shared helper with trimOrderEvents,
parameterized by the trim function and relevant constants.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16261731-eef7-438e-8743-e52beea258a1
📒 Files selected for processing (7)
extensions/tn_digest/engine_ops_integration_test.goextensions/tn_digest/internal/engine_ops.goextensions/tn_digest/internal/engine_ops_test.goextensions/tn_digest/scheduler/constants.goextensions/tn_digest/scheduler/scheduler.gointernal/migrations/052-transaction-events-retention.sqltests/streams/transaction_events_ledger_test.go
resolves: https://github.com/trufnetwork/trufscan/issues/184
The node's
main.transaction_eventsfee ledger — and its CASCADE childtransaction_event_distributions— accumulates a row per fee event forever, in consensus state replicated to every validator and captured in snapshots/statesync. There is no retention today. The dominant writer isinsert_records(flat 1 TRUF per write, method 2), so the table grows unbounded.Change
Adds a leader-scheduled retention trim, mirroring
trim_order_events/ thetn_digestscheduler:trim_transaction_events($preserve_blocks, $delete_cap)(migration052) — a deterministic (block-height cutoff, never wall-clock), delete-capped prune of the high-volume method-2 write-fee rows only. Low-volume, semantically-rich classes (deployStream, setTaxonomies, requestAttestation, setMetadata, createMarket) are left untouched so their fee/distribution history is preserved. The CASCADE removes each pruned row's distribution child.tn_digestscheduler drain loop (aftertrim_order_events), gated byTrimTxEventsEnabled— which shipsfalseso a binary rollout is decoupled from actually pruning.Why method-2 only
insert_recordscharges a flat 1 TRUF, so its rows are uniform and the transaction stays permanently resolvable off-chain via the Trufscan chain indexer. Keeping the low-volume classes preserves the only node-computed fee/distribution data the indexer does not have.Verification
kwil-cli utils parseon052.go build ./...,go vet ./extensions/tn_digest/...,gofmtclean.TestTrimTransactionEvents(selectivity, cascade, delete-cap drain, young-chain no-op) passes; existingTestTransactionEventsLedger/TestTransactionIDTrackingandtn_digestengine-op tests still pass.Rollout
052applied and the binary rolled out in lockstep.TrimTxEventsEnabled = true) only after the Trufscan indexer fallback is live in prod, so a pruned tx still resolves on the explorer /tx page.Related
Summary by CodeRabbit
New Features
trim_transaction_eventsretention action to prune eligible high-volume write-fee transaction-event records while keeping recent activity.Tests