Skip to content

feat(indexer): add /debug/recent_transactions page#3435

Open
pbeza wants to merge 8 commits into
mainfrom
2890-debug-page-for-submitted-transactions
Open

feat(indexer): add /debug/recent_transactions page#3435
pbeza wants to merge 8 commits into
mainfrom
2890-debug-page-for-submitted-transactions

Conversation

@pbeza
Copy link
Copy Markdown
Contributor

@pbeza pbeza commented Jun 1, 2026

Closes #2890

Nodes submit transactions to the chain "fire and forget": a successful submission only means the transaction was routed, not that it was included or had its intended effect. The transaction processor already observes the on-chain effect after a delay, but that outcome is only aggregated into a prometheus counter, so an operator can't see which specific transaction failed.

This adds a /debug/recent_transactions page backed by an in-memory ring buffer. Each entry records the txid, nonce, signer access key, method, reference block height, submission time, and observed outcome. Status is tracked across the full lifecycle (SubmittingExecuted/NotExecuted/Unknown/SubmitFailed), so failures can be correlated with the access key and nonce involved — relevant to the suspected out-of-order nonce rejections.

The buffer is shared between the web server and the transaction processor and read directly by the handler, so the page works regardless of the node's running state.

Nodes submit transactions to the chain "fire and forget": a successful
submission only means the transaction was routed, not that it was
included or had its intended effect. The transaction processor already
observes the on-chain effect after a delay, but that outcome is only
aggregated into a prometheus counter, leaving operators unable to see
which specific transaction failed.

Add an in-memory ring buffer of recently submitted transactions,
surfaced on a new /debug/recent_transactions endpoint. Each entry records
the txid, nonce, signer access key, method, reference block height,
submission time, and observed outcome. The status is tracked across the
full lifecycle: an entry is created at submission and updated in place
once the existing observation completes, so an operator can correlate
failures with the access key and nonce involved (relevant to the
suspected out-of-order nonce rejections).

The buffer is shared between the web server and the transaction
processor and read directly by the handler, so the page works
regardless of the node's running state.
Copilot AI review requested due to automatic review settings June 1, 2026 13:46
@pbeza pbeza linked an issue Jun 1, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds operational debugging support to the node by tracking recently submitted NEAR transactions in-memory and exposing them via a new /debug/recent_transactions endpoint, enabling operators to correlate submission attempts with later observed outcomes (e.g., nonce/order issues).

Changes:

  • Introduces an in-memory bounded ring buffer (RecentTransactions) with status updates across the tx lifecycle.
  • Wires a shared RecentTransactions buffer through node startup so it’s available to both the web server and the transaction processor.
  • Adds a new web debug endpoint /debug/recent_transactions to render the buffer contents.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
crates/node/src/web.rs Adds shared recent-transactions state to the web server and exposes /debug/recent_transactions.
crates/node/src/tests.rs Updates test web server startup to provide a RecentTransactions buffer.
crates/node/src/run.rs Creates and shares the RecentTransactions buffer between web server and indexer.
crates/node/src/indexer/tx_signer.rs Exposes account_id() from TransactionSigner for recording metadata.
crates/node/src/indexer/tx_sender.rs Records submissions/outcomes into the buffer and updates metric labeling accordingly.
crates/node/src/indexer/recent_transactions.rs New module implementing the bounded buffer, statuses, formatting, and unit tests.
crates/node/src/indexer/real.rs Threads the shared buffer into the real indexer transaction processor startup.
crates/node/src/indexer.rs Exposes the new recent_transactions module.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/node/src/indexer/tx_sender.rs
Comment thread crates/node/src/indexer/recent_transactions.rs
@claude
Copy link
Copy Markdown

claude Bot commented Jun 1, 2026

Pull request overview

Adds a /debug/recent_transactions endpoint backed by an in-memory ring buffer that records every transaction the node submits along with its observed on-chain outcome. The buffer is shared between the web server and the transaction processor and read directly by the handler, so the page renders even while the node is not in Running state. Records capture txid, nonce, signer access key, method, reference block height, submission time and current lifecycle status (Submitting -> Executed/NotExecuted/Unknown/SubmitFailed/ObserveError), giving operators enough detail to correlate failures with specific access keys/nonces.

Changes:

  • New indexer::recent_transactions module with RecentTransactions ring buffer, SubmittedTransaction record, and SubmittedTransactionStatus enum.
  • tx_sender::ensure_send_transaction and submit_tx rewired to (a) return the built tx's hash/nonce/block height on success and (b) write a record + later status update through an Arc<Mutex<RecentTransactions>>. Metric labels are unchanged; the new status enum mirrors the metric label arms.
  • tx_signer::TransactionSigner::account_id() accessor added.
  • run.rs instantiates the shared buffer and threads it into both start_web_server and spawn_real_indexer.
  • web.rs adds the route, a state field, and the handler that renders the buffer via Debug.
  • Test wiring in crates/node/src/tests.rs passes a fresh buffer.

Reviewed changes

Per-file summary
File Description
crates/node/src/indexer.rs Declares the new recent_transactions module.
crates/node/src/indexer/real.rs Accepts and forwards the shared buffer into the transaction processor.
crates/node/src/indexer/recent_transactions.rs New module: ring buffer, record type, status enum, Debug rendering, and unit tests.
crates/node/src/indexer/tx_sender.rs submit_tx now returns metadata; ensure_send_transaction records submit + final status into the buffer alongside existing metrics.
crates/node/src/indexer/tx_signer.rs Adds account_id() accessor used by the record writer.
crates/node/src/run.rs Creates the shared Arc<Mutex<RecentTransactions>> and threads it into both the web server and indexer.
crates/node/src/tests.rs Supplies a fresh buffer to start_web_server in the test harness.
crates/node/src/web.rs New /debug/recent_transactions route, handler, and state field.

Findings

Non-blocking (nits, follow-ups, suggestions):

  • crates/node/src/indexer/recent_transactions.rs:101self.next_id += 1 uses raw arithmetic on a u64 counter; per docs/engineering-standards.md ("Use safe arithmetic methods") consider self.next_id = self.next_id.wrapping_add(1) (the standard explicitly calls out wrapping/checked methods for primitives). Overflow is not a practical concern at any realistic submission rate, so this is purely about adhering to the convention.
  • crates/node/src/indexer/recent_transactions.rs:112-119update_status does a linear scan over up to 200 entries. Since entries is sorted by id (IDs are assigned monotonically and the deque is FIFO), iterating in reverse (or binary-searching) would find the most-recently-recorded entry — which is the common case immediately after a submit — much faster. Negligible at n=200, but a one-line change with .iter_mut().rev().
  • crates/node/src/indexer/tx_sender.rs:369,387Clock::real().now_utc() is called directly inside ensure_send_transaction, mixing wall-clock I/O with business logic (engineering standards "Separate business logic from I/O"). Consistent with the rest of this function, which is already heavily I/O-bound, so not blocking — but if you ever want to write a unit test that asserts on submitted_at, you would need to inject a Clock instead.
  • crates/node/src/indexer/tx_sender.rs:359-371,376-389,411-414 — The three recent_transactions.lock().unwrap()... call sites duplicate the construction of SubmittedTransaction { signer_account_id, signer_public_key, method, ... }. Extracting a small helper (or letting RecentTransactions build records from a smaller param struct) would consolidate them and remove the need to clone() signer_account_id and signer_public_key separately for the failure branch.

Approved.

@pbeza pbeza changed the title feat(indexer): add /debug/recent_transactions page feat(indexer): add /debug/recent_transactions page Jun 1, 2026
Address PR review feedback:
- Record the transaction signature alongside each entry (issue #2890's
  acceptance criteria lists txid, nonce, signature). near_crypto::Signature
  is captured in submit_tx before the signed transaction is moved into the
  RPC handler, carried through SubmittedTxMetadata, and rendered via Display
  (ed25519:<base58>) on the debug page. It is None on the SubmitFailed path.
- Consolidate the two record-construction sites behind
  SubmittedTransaction::submitting / submit_failed, taking a SignerContext so
  the shared signer fields are written in one place and no longer cloned per
  branch.
- Use safe arithmetic for the record-id counter and scan update_status from
  the back, where the just-recorded entry lives.
@pbeza
Copy link
Copy Markdown
Contributor Author

pbeza commented Jun 1, 2026

@claude review

@claude
Copy link
Copy Markdown

claude Bot commented Jun 1, 2026

Pull request overview

Adds a /debug/recent_transactions endpoint backed by an in-memory ring buffer that records every transaction the node submits along with its observed on-chain outcome. The buffer is shared between the web server and the transaction processor via Arc<Mutex<RecentTransactions>> and read directly by the handler, so the page renders even while the node is not in Running state. Each record captures txid, nonce, signer access key, signature, method, reference block height, submission time, and current lifecycle status (SubmittingExecuted/NotExecuted/Unknown/SubmitFailed/ObserveError).

Changes:

  • New indexer::recent_transactions module with RecentTransactions ring buffer (capacity 200), SubmittedTransaction record, SubmittedTransactionStatus enum, SignerContext/SubmittedTxMetadata helper structs, and Debug rendering.
  • tx_sender::submit_tx now returns SubmittedTxMetadata (hash, nonce, signature, block height); ensure_send_transaction records submission + later status update through the shared buffer alongside existing metrics. Metric label arms are preserved and now mapped 1:1 to the new status enum.
  • tx_signer::TransactionSigner::account_id() accessor added.
  • run.rs instantiates the shared buffer and threads it into both start_web_server and spawn_real_indexer; test harness in tests.rs supplies a fresh buffer.
  • web.rs adds the route, state field, and handler that renders the buffer via Debug.

Reviewed changes

Per-file summary
File Description
crates/node/src/indexer.rs Declares the new recent_transactions module.
crates/node/src/indexer/real.rs Accepts and forwards the shared buffer into the transaction processor.
crates/node/src/indexer/recent_transactions.rs New module: ring buffer, record type, status enum, signer/tx metadata helpers, Debug rendering, and unit tests.
crates/node/src/indexer/tx_sender.rs submit_tx now returns metadata; ensure_send_transaction writes submit + final status into the buffer; metric arms refactored to a single label-table emission.
crates/node/src/indexer/tx_signer.rs Adds account_id() accessor used by the record writer.
crates/node/src/run.rs Creates the shared Arc<Mutex<RecentTransactions>> and threads it into both the web server and indexer.
crates/node/src/tests.rs Supplies a fresh buffer to start_web_server in the test harness.
crates/node/src/web.rs New /debug/recent_transactions route, handler, and state field.

Findings

Non-blocking (nits, follow-ups, suggestions):

  • crates/node/src/web.rs:198debug_recent_transactions holds the std::sync::Mutex guard across the format!("{:?}", &*recent_transactions) call inside an async handler. With 200 entries this is brief, but it does block tx-sender writes (record_submitted/update_status) for the duration of formatting on the executor thread. A snapshot-then-format pattern (e.g. expose RecentTransactions::snapshot() -> Vec<SubmittedTransaction> that clones under the lock, drop the guard, then format the Vec) would avoid the cross-section contention and is the more idiomatic shape for a sync mutex inside an async fn. Acceptable as-is for a debug endpoint.
  • crates/node/src/indexer/recent_transactions.rs:90,105SubmittedTransaction::submitting and submit_failed call Clock::real().now_utc() directly inside the constructor (engineering standards: "Separate business logic from I/O"). Consequently the unit tests construct SubmittedTransaction field-by-field and bypass the constructors, leaving the submitted_at capture itself untested. Threading a &Clock (or accepting a Utc argument) would let the constructors be exercised in tests; consistent with the existing I/O-heavy shape of ensure_send_transaction so not blocking.
  • crates/node/src/indexer/recent_transactions.rs — The Debug impl on RecentTransactions is not a debug-formatter of fields but a human-readable multi-line rendering for the web page. This works but couples the "wire format" of the page to a derived-style trait — a custom Display impl (or a dedicated render() method) would express intent more accurately and let Debug stay structural.

✅ Approved

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Comment thread crates/node/src/web.rs
Comment thread crates/node/src/indexer/tx_sender.rs Outdated
Comment thread crates/node/src/web.rs
Comment thread crates/node/src/indexer/tx_sender.rs Outdated
Comment thread crates/node/src/web.rs
pbeza added 4 commits June 1, 2026 16:49
Address PR review feedback:
- Add RecentTransactions::snapshot(), which clones the entries under the
  lock so the web handler can format the page after releasing it. Holding
  the std::sync::Mutex across the format! could block the transaction
  processor's record_submitted/update_status writes.
- Move the human-readable per-entry rendering into a Display impl on
  SubmittedTransaction plus a render() helper, so the page's wire format is
  no longer expressed through a Debug impl.
- Reword the signature-clone comment in submit_tx: the move happens when the
  transaction is passed into rpc_handler.submit_tx, not when entering
  submit_tx.
The `no-use-in-fn` ast-grep lint (cargo make check-all-fast) rejects `use`
declarations inside function bodies. Move the `std::fmt::Write` import used by
`render()` up to the module-level imports, matching requests/debug.rs.
…ffer

Replace update_status's reverse linear scan with O(1) arithmetic id-to-index
lookup. Ids come from a monotonic counter and the buffer is strict FIFO, so the
live ids are always a contiguous range and an id maps to a deque position by
subtraction (id - (next_id - len)); no auxiliary index needed. The previous
scan walked past every submission made during the 10s observe timeout, not the
O(1) it claimed.

Also: fold the four built-transaction fields into a single
Option<SubmittedTxMetadata> (enforcing the all-or-nothing invariant in the
type), derive Display on both types instead of a hand-written impl, consolidate
the two constructors through a private new(), bump the retained-entry cap to
2000, and derive Default. Add tests for buried/evicted/never-issued/empty-buffer
lookups.
…g broadcast

Document, in the recent_transactions module doc, why this page reads the shared
buffer directly instead of using the debug-request broadcast like the other
debug pages: the broadcast is only subscribed in the Running state, whereas the
transaction processor runs across all states, so reading the buffer directly
keeps the page available (e.g. while Initializing) when submission failures most
need inspecting. Point the web.rs field comment at that rationale.
@pbeza pbeza requested a review from Copilot June 2, 2026 15:42
@pbeza
Copy link
Copy Markdown
Contributor Author

pbeza commented Jun 2, 2026

@claude review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment on lines +177 to +180
pub fn record_submitted(&mut self, transaction: SubmittedTransaction) -> TransactionRecordId {
let id = TransactionRecordId(self.next_id);
self.next_id = self.next_id.wrapping_add(1);

@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

Pull request overview

Adds a /debug/recent_transactions endpoint backed by an in-memory ring buffer (capacity 2000) that records every transaction the node submits along with its observed on-chain outcome. The buffer is shared between the web server and the transaction processor via Arc<Mutex<RecentTransactions>> and read directly by the handler, so the page renders even while the node is not Running. Each record captures txid, nonce, signer access key, signature, method, reference block height, submission time, and a lifecycle status (SubmittingExecuted/NotExecuted/Unknown/SubmitFailed/ObserveError).

Since the last review pass, this iteration:

  • swaps raw += 1 for wrapping_add(1) on the id counter
  • replaces the linear scan in update_status with an O(1) position-by-id lookup (index_of)
  • uses snapshot() + dedicated render() so the std mutex is no longer held across formatting
  • promotes the page rendering from Debug to a derive_more::Display impl
  • adds the transaction signature to each record (closing Debug page for submitted transactions #2890's signature requirement)
  • raises retention from 200 → 2000

Changes:

  • New indexer::recent_transactions module: RecentTransactions ring buffer, SubmittedTransaction (Display) record, SubmittedTransactionStatus, SignerContext/SubmittedTxMetadata helpers, TransactionRecordId handle with O(1) index_of lookup, and render() helper.
  • tx_sender::submit_tx now returns SubmittedTxMetadata (hash, nonce, signature, block height); ensure_send_transaction records submit + final status into the buffer; metric arms refactored to a single label-table emission keyed on the same enum.
  • tx_signer::TransactionSigner::account_id() accessor.
  • run.rs constructs the shared Arc<Mutex<RecentTransactions>> and threads it into both start_web_server and spawn_real_indexer; test harness in tests.rs supplies a fresh buffer.
  • web.rs adds the route, state field, and snapshot → render handler.

Reviewed changes

Per-file summary
File Description
crates/node/src/indexer.rs Declares the new recent_transactions module.
crates/node/src/indexer/real.rs Accepts and forwards the shared buffer into the transaction processor.
crates/node/src/indexer/recent_transactions.rs New module: ring buffer, record/status types, O(1) lookup, Display impl, render(), and unit tests.
crates/node/src/indexer/tx_sender.rs submit_tx returns metadata; ensure_send_transaction writes submit + final status; metric arms collapsed.
crates/node/src/indexer/tx_signer.rs Adds account_id() accessor.
crates/node/src/run.rs Creates the shared Arc<Mutex<RecentTransactions>> and threads it through.
crates/node/src/tests.rs Supplies a fresh buffer to start_web_server.
crates/node/src/web.rs New /debug/recent_transactions route, state field, and snapshot+render handler.

Findings

Non-blocking (nits, follow-ups, suggestions):

  • crates/node/src/indexer/recent_transactions.rs:249index_of does self.next_id - self.entries.len() as u64 as a raw subtraction at the top of the function. The doc comment ("The subtraction runs only after the lower-bound check, so it cannot underflow") only justifies the second subtraction (id.0 - oldest_id) inside the .then() closure; the first one relies on a different, undocumented invariant — that every record_submitted increments next_id by exactly 1 and pushes exactly one entry (with pop_front before push_back at capacity), so next_id >= entries.len() always holds. With next_id now using wrapping_add, that invariant only fails after 2^64 submissions, which is firmly under the engineering-standards "dead by runtime invariants" exception — but per engineering-standards.md "Use safe arithmetic methods", the reasoning for the first subtraction should be made explicit (split the doc into the two cases, or use checked_sub(...).expect("invariant: ...")).
  • crates/node/src/indexer/tx_sender.rs:382-385,401-404recent_transactions.lock().unwrap().record_submitted(SubmittedTransaction::submitting(signer, metadata)): in Rust the method receiver is evaluated before the argument, so the lock is held while SubmittedTransaction::submitting runs Clock::real().now_utc(). Trivial wall-clock cost in practice, but it also means the std mutex briefly serializes the page handler's snapshot() clones against this clock call. Hoisting the constructor call out of the locked expression (let record = SubmittedTransaction::submitting(signer, metadata); recent_transactions.lock().unwrap().record_submitted(record);) removes that coupling and is consistent with the existing comment in web.rs about minimizing time under the lock.
  • crates/node/src/indexer/recent_transactions.rs:182 — Same point as above at the construction site: submitted_at: Clock::real().now_utc() inside new() keeps the constructors un-unit-testable, which is why test_transaction builds via struct literal and bypasses submitting/submit_failed entirely. Accepting an injected Utc/Clock would let those constructors (and the submitted_at capture) be exercised directly. Consistent with the rest of tx_sender's I/O-heavy shape, so non-blocking — but worth flagging that the submit_failed constructor in particular is currently completely uncovered.

✅ Approved

Copy link
Copy Markdown
Collaborator

@netrome netrome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Started reviewing, will continue next week.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't gone deep yet, but given that this is a data structure for the recent_transactions page it feels weird to have it live under the indexer. It feels like this belongs closer to the web module - potentially as a submodule of it, or somewhere adjacent.

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.

Debug page for submitted transactions

3 participants