Fix RpcFrontend consensus and history publication

[Amaury Chamayou (achamayou)]

Summary

• publish consensus and history explicitly from NodeState to each RPC frontend
• store the pointers atomically in both RpcFrontend and EndpointRegistry
• load each pointer once per request attempt or endpoint operation, preserving a stable snapshot without adding locks
• handle unpublished consensus safely during role-based redirect resolution
• exercise an EndpointRegistry consensus reader through /tx during concurrent publication
• remove the obsolete RpcFrontend TSAN suppression

Issue #8123 coverage

This PR resolves Still Applicable item 2, RpcFrontend races on consensus and history, and the corresponding Phase 3 frontend publication work from #8123:

• NodeState explicitly publishes the initialized consensus and history pointers to every RPC frontend.
• RpcFrontend and EndpointRegistry use synchronized atomic pointer publication instead of racing raw-pointer reads and writes.
• Request attempts and endpoint operations load each pointer once and use that stable value throughout the operation.
• Redirect resolution handles unpublished (nullptr) consensus without dereferencing it, addressing the related draft-only hazard called out in the issue.
• The focused concurrent test covers an endpoint-registry consensus reader, and the now-obsolete node/rpc/frontend.h TSAN race suppression is removed.

Pointer flow before and after

Before

NodeState installed owning pointers in the KV store, but did not publish them directly to the frontends. Request and tick paths repeatedly fetched raw pointers from the store and wrote them into frontend and registry state while other threads could read the active fields.

flowchart TD
  NS["NodeState setup_history / setup_consensus"] -->|"Store::set_history / set_consensus<br/>shared_ptr ownership"| Store["KV Store"]

  Request["RpcFrontend::process / process_forwarded"] --> UpdateConsensus["RpcFrontend::update_consensus"]
  Tick["RpcFrontend::tick"] --> UpdateConsensus
  Attempt["Each request attempt"] --> UpdateConsensus
  Attempt --> UpdateHistory["RpcFrontend::update_history"]

  Store -->|"get_consensus().get()"| UpdateConsensus
  Store -->|"get_history().get()"| UpdateHistory

  UpdateConsensus -->|"unsynchronised raw write"| RFConsensus[("RpcFrontend::consensus")]
  UpdateHistory -->|"unsynchronised raw write"| RFHistory[("RpcFrontend::history")]
  UpdateConsensus -->|"EndpointRegistry::set_consensus"| ERConsensus[("EndpointRegistry::consensus")]
  UpdateHistory -->|"EndpointRegistry::set_history"| ERHistory[("EndpointRegistry::history")]

  RFConsensus -->|"direct raw reads"| RFLogic["forwarding, redirects,<br/>session checks, backpressure"]
  RFHistory -->|"direct raw reads"| SignatureLogic["proposal roots and<br/>signature emission"]
  ERConsensus -->|"direct raw reads"| Endpoints["Base, Common, Node,<br/>JS and app endpoint handlers"]

  OtherThreads["Concurrent request / tick threads"] -. "could read while another thread wrote" .-> RFConsensus
  OtherThreads -. "could read while another thread wrote" .-> RFHistory
  OtherThreads -. "could read while another thread wrote" .-> ERConsensus

Loading

After

NodeState publishes the initialized pointers once to every frontend. The setter release-stores them into both frontend and registry atomics before store readiness is published. Request attempts and endpoint operations acquire-load their own stable local snapshot; request and tick paths no longer poll the KV store or rewrite pointer state.

flowchart TD
  SetupHistory["NodeState::setup_history"] -->|"Store::set_history<br/>shared_ptr ownership"| Store["KV Store"]
  SetupHistory --> History["NodeState history shared_ptr"]
  SetupConsensus["NodeState::setup_consensus"] -->|"Store::set_consensus<br/>shared_ptr ownership"| Store
  SetupConsensus -->|"RPCMap::frontends"| Publish["RpcFrontend::set_consensus_and_history"]
  History -->|"history.get()"| Publish

  Publish -->|"release store once"| RFConsensus[("atomic RpcFrontend::consensus")]
  Publish -->|"release store once"| RFHistory[("atomic RpcFrontend::history")]
  Publish -->|"EndpointRegistry::set_consensus<br/>release store once"| ERConsensus[("atomic EndpointRegistry::consensus")]
  Publish -->|"EndpointRegistry::set_history<br/>release store once"| ERHistory[("atomic EndpointRegistry::history")]

  Publish -->|"happens before"| Ready["Store::set_readiness Ready"]
  Request["RpcFrontend request attempt"] -->|"Store::is_ready acquire"| ReadyCheck["readiness observed"]
  ReadyCheck --> Snapshot["acquire-load consensus and history once"]
  RFConsensus --> Snapshot
  RFHistory --> Snapshot
  Snapshot --> RFLogic["forwarding, redirects, session checks,<br/>backpressure, proposal roots, signatures"]

  Handler["Endpoint operation"] --> Getter["EndpointRegistry::get_consensus<br/>acquire-load once"]
  ERConsensus --> Getter
  Getter --> Endpoints["Base, Common, Node,<br/>JS and app endpoint handlers"]

  Tick["RpcFrontend::tick"] --> NoRefresh["endpoint tick only;<br/>no pointer refresh"]

Loading

Related items already resolved elsewhere

#8123 was written against main before these focused fixes merged:

• LedgerSecrets lock ordering (item 1) was fixed by Fix LedgerSecrets lock ordering #8125, which moved all three KV dependency reads before LedgerSecrets::lock and added a focused TSAN test that runs without the repository-wide deadlock suppressions.
• The snapshot-fetch underflow was fixed by Fix snapshot download since parameter to use closed/inclusive semantics #7775.

This PR does not resolve the remaining work in #8123:

• certificate locking and NodeClient certificate ownership (item 3)
• deferring member/user frontend opening from KV global hooks (item 4)
• removing the broad lock from transition_service_to_open() (item 5)
• dedicated synchronization for backup_snapshot_fetch_task (item 6)
• the partially superseded AFT startup lock-order question

Testing

• ninja -C build
• cd build && ./tests.sh --output-on-failure -R '^(frontend_test|node_frontend_test)$'
• cd build-tsan && TSAN_OPTIONS="suppressions=$PWD/../tsan_env_suppressions:halt_on_error=1" ./tests.sh --output-on-failure -R '^frontend_test$'
• scripts/cpp-format-checks.sh
• scripts/includes-checks.sh
• scripts/copyright-checks.sh
• scripts/ascii-checks.sh

Partially addresses #8123

[Copilot]

Pull request overview

Publishes consensus/history from NodeState to RPC frontends and synchronizes request-side access.

Changes:

• Adds atomic frontend pointer publication and null-safe redirects.
• Adds concurrency and unpublished-consensus tests.
• Removes the obsolete TSAN suppression.

Custom instructions used:

• .github/copilot-instructions.md
• .github/instructions/reviewing.instructions.md

Reviewed changes

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

Show a summary per file

File	Description
tsan_env_suppressions	Removes the frontend race suppression.
src/node/rpc/test/frontend_test.cpp	Adds publication and redirect regression tests.
src/node/rpc/frontend.h	Implements atomic publication and request-local pointer loads.
src/node/node_state.h	Publishes consensus and history to frontends.
src/enclave/rpc_handler.h	Adds the publication interface.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (1)

include/ccf/endpoint_registry.h:177

• EndpointRegistry is a public application API, and changing its protected consensus/history members to atomics changes how downstream subclasses can use them (the logging sample in this PR already has to switch to get_consensus()). Repository policy requires user-facing API changes to be recorded in CHANGELOG.md; please add a migration note directing subclasses to the new accessors.

    std::atomic<ccf::kv::Consensus*> consensus{nullptr};
    std::atomic<ccf::kv::TxHistory*> history{nullptr};