Feature/sma 106 audit log project events into get v1elections response#211
Conversation
…types-config-serde
…-101-audit-log-trait-noop-factory-composition-root-wiring
…-102 Integrate SMA-101 wiring (enabled flag, audit in ElectionRunner) with JsonlAuditLog implementation: factory returns NoopAuditLog when disabled and starts JsonlAuditLog when enabled. Co-authored-by: Cursor <[email protected]>
Take sma-99 audit writer/log hardening (oneshot shutdown, rotated paths, shutdown timeout, flush/recovery). Adapt to SMA-103 event format: AuditFileHeader, system_audit_events_dropped constructor, payload.source() in record diagnostics, header-aware writer tests. Co-authored-by: Cursor <[email protected]>
… events to CLI - Merge feature/sma-104-audit-log-rest-producers-auditactorbuilder into current branch - Resolve conflicts: AppState gains both audit_ring (SMA-105) and actor_builder (SMA-104) - Fix RestApiAuthLoginSuccess → RestApiAuthLoginSucceeded in event.rs and auth_tests.rs - Add recent elections audit events to GET /v1/elections response (recent_events field, newest-first, from ring buffer) - Add elections events table to `nodectl api elections` CLI output Co-authored-by: Cursor <[email protected]>
…cture' into feature/sma-104-audit-log-rest-producers-auditactorbuilder
- Add `use serde::{Deserialize, Serialize}` to enums.rs (bare derives on
StakeSkipReason/ConfigFieldChange/AuditOutcome broke after SMA-103 merge)
- Add ElectionsTickFailed arm to severity() and source() match blocks
- Restore REST event constructors to event.rs (rest_api_auth_login_success,
rest_api_auth_login_rejected, rest_api_token_rejected, rest_api_config_updated)
removed by SMA-103 merge; required by rest_audit.rs (SMA-104)
- Fix RestApiAuthLoginSuccess -> RestApiAuthLoginSucceeded in auth_tests.rs
Co-authored-by: Cursor <[email protected]>
Co-authored-by: Cursor <[email protected]>
…roducers-auditactorbuilder' into feature/sma-105-audit-log-in-memory-ring-buffer-for-read-path
- Rename ElectionsStakeSubmittedParams.stake_nanotons → stake and elections_stake_recovered param amount_nanotons → amount to match the enum field names introduced by sma-104 (which dropped _nanotons suffixes from all payload fields). - Remove dead duplicate function adaptive_split50_defer_reason that was synthesised by the ort merge strategy from both SMA-103 and SMA-104 versions of adaptive_strategy.rs; the identical logic with node_id is already in adaptive_split50_status. - Update runner.rs and event.rs test fixtures to use the renamed fields. All 310 lib tests pass. Co-authored-by: Cursor <[email protected]>
Remove the intermediate AdaptiveStakeZero enum and move its variants (Defer, NoTopUpNeeded, InsufficientFree) directly into AdaptiveStakeResult. Update all call-sites in runner.rs accordingly, renaming adaptive_zero_to_skip → adaptive_result_to_skip. Co-authored-by: Cursor <[email protected]>
There was a problem hiding this comment.
Pull request overview
Adds an in-memory audit ring buffer and elections-focused projection so recent elections audit events can be surfaced directly inside the existing GET /v1/elections response (enriching our_participants with stake submissions and last_error). In parallel, REST auth/config mutation endpoints now emit structured audit events (with configurable client IP handling), and nodectl is updated to display the projected last_error.
Changes:
- Introduce
AuditEventBufferring + elections projection/merge utilities, and wire the ring into the HTTP server’s/v1/electionshandler. - Emit REST audit events for login/token rejection and config/entity mutations, using a centralized
AuditActorBuilder(PII policy applied consistently). - Update CLI elections table output (adds “Last error”) and adjust load-test tooling to support chunked highload batches / consistent workchains.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/node/tests/test_load_net/scripts/add-nominators-to-pool.ts | Adds env-configurable highload workchain + chunked sendBatch to reduce single-BOC size and avoid cross-workchain batch deploy issues. |
| src/node-control/service/src/service_main_task.rs | Switches audit initialization to return both AuditLog and AuditEventBuffer, passing the ring to the HTTP server. |
| src/node-control/service/src/http/rest_audit.rs | New helpers for emitting REST audit events (login/token/config/entity) and generating field-diff payloads. |
| src/node-control/service/src/http/mod.rs | Exposes the new rest_audit module. |
| src/node-control/service/src/http/http_server_task.rs | Wires ring-buffer projection into GET /v1/elections; adds REST audit emission for login + elections include/exclude; extends AppState with actor_builder and audit_ring; adds handler tests. |
| src/node-control/service/src/http/entity_crud_handlers_tests.rs | Updates test AppState construction to include actor_builder + audit_ring and fixes RuntimeConfig Arc cloning. |
| src/node-control/service/src/http/config_handlers.rs | Adds Claims+headers extraction to mutation endpoints and emits audit “diff” events for config/entity changes. |
| src/node-control/service/src/http/config_handlers_tests.rs | Adds test coverage verifying config updates emit the expected audit diff event. |
| src/node-control/service/src/http/auth_tests.rs | Adds tests verifying login/token rejection audit events are emitted and do not leak passwords. |
| src/node-control/service/src/elections/runner.rs | Renames/refactors adaptive stake result handling and updates audit payload field names. |
| src/node-control/service/src/elections/runner_tests.rs | Updates expectations for renamed elections audit payload fields. |
| src/node-control/service/src/elections/adaptive_strategy.rs | Simplifies adaptive result enums (removes nested “Zero” enum) and updates tests accordingly. |
| src/node-control/service/src/auth/mod.rs | Adds stable token rejection reason mapping for audit emission. |
| src/node-control/service/src/auth/middleware.rs | Emits audit events on token rejection; injects synthetic Claims when auth is disabled so mutation handlers can consistently extract claims. |
| src/node-control/service/src/audit/ring_buffer.rs | Adds fixed-capacity in-memory ring buffer for recent audit events (with tests). |
| src/node-control/service/src/audit/projection.rs | Adds elections event projection + merge into participants, plus unit tests. |
| src/node-control/service/src/audit/mod.rs | Re-exports new audit modules and helpers. |
| src/node-control/service/src/audit/log.rs | Minor async_trait import style change. |
| src/node-control/service/src/audit/jsonl_writer.rs | Qualifies tokio channel types explicitly (no behavior change intended). |
| src/node-control/service/src/audit/jsonl_log.rs | Pushes recorded events into the ring before enqueue; adds dedup-on-ring behavior and tests. |
| src/node-control/service/src/audit/factory.rs | Returns { log, ring } instead of only log; passes JsonlAuditLog’s ring through; updates tests. |
| src/node-control/service/src/audit/event.rs | Renames payload fields (stake_nanotons→stake, etc.), adds REST audit event constructors, and introduces dedup_key(). |
| src/node-control/service/src/audit/enums.rs | Renames serialized payload field names to match new event struct fields. |
| src/node-control/service/src/audit/actor_builder.rs | New builder applying live PII policy for REST actor IPs; adds header IP parsing + anonymization helpers and tests. |
| src/node-control/service/Cargo.toml | Adds parking_lot dependency for the ring buffer lock. |
| src/node-control/common/src/app_config.rs | Changes default audit ring buffer capacity (10_000 → 100). |
| src/node-control/commands/src/commands/nodectl/service_api_cmd.rs | Adds “Last error” column to elections participants table output. |
| src/Cargo.lock | Adds parking_lot to the lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Deduplication: drop events whose key already exists in the ring. | ||
| // Prevents e.g. repeated elections.stake_skipped for the same (node, election, reason). | ||
| if let Some(key) = event.dedup_key() | ||
| && self.ring.contains_dedup_key(&key) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Push into ring first: readers see the event immediately and even | ||
| // queue-dropped events remain accessible on the REST read-path. | ||
| self.ring.push(event.clone()); |
| /// Returns `true` if the buffer already contains an event with the given | ||
| /// deduplication key. Used by [`crate::audit::jsonl_log::JsonlAuditLog`] to | ||
| /// suppress repeated identical `elections.stake_skipped` events within one election. | ||
| pub fn contains_dedup_key(&self, key: &str) -> bool { | ||
| self.inner.read().iter().any(|e| e.dedup_key().as_deref() == Some(key)) | ||
| } |
…a-105 Pull SMA-104 review fixes merged into sma-99 (test_support refactor, middleware without headers clone, AppError::internal in config handlers, rest_audit noop-update comment) while keeping sma-105 audit ring buffer. Co-authored-by: Cursor <[email protected]>
Apply sma-105 conflict resolutions (test_support builders, jsonl_writer cleanup) while keeping sma-106 projection test helpers for custom ring. Co-authored-by: Cursor <[email protected]>
…tion - Atomic dedup: replace two-step contains+push with push_unless_dedup_duplicate that holds a single write lock, eliminating the TOCTOU race on concurrent record() calls for the same elections.stake_skipped key. - Zero-copy dedup: replace dedup_key() -> Option<String> with dedup_identity() -> Option<AuditDedupIdentity<'_>>, removing per-event String allocations on the hot path. - Contract fix in collect_recent_election_ids: early-return when max == 0 and after inserting current_election_id when ids.len() >= max. Co-authored-by: Cursor <[email protected]>
Resolve conflicts preferring sma-99 for audit core (dedup, ring buffer, jsonl) while keeping sma-106 projection into elections response. Co-authored-by: Cursor <[email protected]>
| use std::collections::BTreeMap; | ||
|
|
||
| /// Projected stake submission from audit events. | ||
| #[derive(Debug, Clone, PartialEq, Eq)] |
There was a problem hiding this comment.
The Projected prefix on these structs reads as noise — there are no competing variants, so it carries no disambiguating value. Suggest dropping it: ProjectedStakeSkip → StakeSkip, ProjectedWithdraw → Withdraw, ProjectedStakeFailure → StakeFailure.
ProjectedStakeSubmission is the only one with a real reason to differ: it collides with the imported common::snapshot::StakeSubmission used in this same file. But the projection ultimately produces exactly that snapshot type, so rather than keep a parallel struct we can store snapshot::StakeSubmission directly in NodeElectionProjection.stake_submissions and build it during project_elections. That removes the collision, the projected_to_stake_submission converter, and the fields that are currently written but never read (ts, node_id, policy, and the raw max_factor).
| let recent_ids = collect_recent_election_ids(current_election_id, &elections_events, 3); | ||
| let projection = project_elections(&elections_events, &recent_ids); |
There was a problem hiding this comment.
elections_events is traversed twice in a row here (collect_recent_election_ids, then project_elections). The first pass is actually dead work: merge_projection_into_participants is only ever called with current_election_id, and our_participants is a snapshot of the current cycle — so the two extra election ids collected here never match a participant and their projection is built for nothing.
Suggest dropping the recent_ids layer entirely and projecting the current cycle in a single pass:
let mut our_participants = view.our_participants;
if let Some(election_id) = current_election_id {
let projection = project_elections(&elections_events, &[election_id]);
merge_projection_into_participants(&mut our_participants, &projection, election_id);
}This removes the collect_recent_election_ids call, the function itself (+ its unit tests), and its re-export in mod.rs. As a bonus, when current_election_id is None no projection is built at all.
If we do want to surface several cycles in the future, the better shape is a single fused function rather than two passes — project_recent_elections(events, current_election_id, max) that builds the full per-id projection in one pass over the events and then prunes to the most recent ids afterward. Since election_id is the (monotonic) election start timestamp, "most recent" is just the largest ids, so pruning is retain on the BTreeMap keys (highest max of them, current first) — no second traversal of the event slice, and merge_* would then loop over those ids instead of only the current one.
| fn latest_error_message(node_proj: &NodeElectionProjection) -> Option<String> { | ||
| let mut candidates: Vec<(DateTime<Utc>, String)> = Vec::new(); | ||
|
|
||
| for skip in &node_proj.stake_skips { | ||
| candidates.push((skip.ts, format!("stake skipped: {}", format_skip_reason(skip.reason)))); | ||
| } | ||
| for failure in &node_proj.stake_failures { | ||
| candidates.push((failure.ts, format!("stake failed: {}", failure.reason))); | ||
| } | ||
| for withdraw in &node_proj.withdraws { | ||
| if withdraw.outcome == AuditOutcome::Failure | ||
| && let Some(error) = &withdraw.error | ||
| { | ||
| candidates.push((withdraw.ts, format!("withdraw failed: {error}"))); | ||
| } | ||
| } | ||
|
|
||
| candidates.sort_by_key(|(ts, _)| *ts); | ||
| candidates.last().map(|(_, msg)| msg.clone()) |
There was a problem hiding this comment.
latest_error_message returns the most recent error-class event but never considers whether a successful ElectionsStakeSubmitted happened afterward. So a node that skips/fails at t1 and then submits successfully at t2 > t1 will still surface a last_error (and the CLI Last error column stays populated) even though the latest action succeeded.
If the field is meant as "last error ever seen", this is fine — but operators are likely to read it as "the current cycle is failing". Two options:
- Clear/suppress it when the latest successful submission for that node post-dates the latest error event, or
- Keep the behavior but make the historical semantics explicit (e.g. rename to
last_error_seen/ document it).
Worth a deliberate decision either way.
| } | ||
| println!(); | ||
|
|
||
| print_recent_events_table(&value); |
There was a problem hiding this comment.
Removing the print_recent_events_table call here is good, but two other call sites survive in the early-return branches of print_elections_table (the "No participants in response" and "No controlled participants" cases). Since the server now always sends an empty recent_events (skipped by skip_serializing_if), those two calls are permanent no-ops, and print_recent_events_table together with its helpers (format_event_details, truncate_preview) is effectively dead code.
Either remove the remaining two calls and the now-unused helpers here, or, if the full recent_events retirement is intentionally deferred to SMA-107 (#212 "final cleanup"), drop a note so it is not lost.
Summary
Projects recent elections audit events from the in-memory ring buffer into the existing GET /v1/elections response, so operators see stake activity and errors in the same shape they already use — without a separate raw recent_events field.
The handler takes a single snapshot of elections-sourced events from the ring, builds a projection for up to three recent election cycles, and merges it into our_participants: stake_submissions are enriched from audit records, and last_error is derived from the latest stake skip, stake failure, or withdraw failure. The nodectl CLI elections table gains a Last error column aligned with the API.
Changes
Add project_elections() to fold ring-buffer events into per-election, per-node structures (stake_submissions, stake_skips, stake_failures, withdraws).
Add collect_recent_election_ids() — current cycle first, then other IDs seen in events (cap: 3).
Add merge_projection_into_participants() to merge projected data into live OurElectionParticipant snapshots without duplicate submissions.
Derive last_error from the chronologically latest error-class event (stake skipped / stake failed / withdraw failed).
Unit tests for grouping, filtering, merge behavior, and empty-ring no-op.
Read elections events once via audit_ring.filter_collect(AuditSource::Elections).
Project and merge into our_participants for the current election id.
Remove top-level recent_events from the response.
Handler tests: empty ring passthrough, stake submissions from audit, last_error from skip/failure/withdraw, end-to-end via JsonlAuditLog.
Add Last error column to the elections participants table.
Remove client-side parsing/display of raw recent_events.
Closes SMA-106.