Skip to content

Commit 691c472

Browse files
carbolymercarbolymer
committed
spec
1 parent 8966469 commit 691c472

21 files changed

Lines changed: 3459 additions & 0 deletions

‎cardano-rpc/docs/node-kernel-access/01-node-access-types.md‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
# Piece 1: NodeKernelAccess types and cardano-rpc plumbing
2+
3+
## Status: as built
4+
5+
Piece 1 shipped a deliberately thin `NodeKernelAccess`, not the snapshot record described below.
6+
As built, the module is `Cardano.Rpc.Server.NodeKernelAccess`, and the record itself lives in `Cardano.Rpc.Server.NodeKernelAccess.Type`.
7+
The module exports `mkNodeKernelAccess`, `fetchBlock` and `grabNodeKernelAccess`; there is no `withNodeKernelAccess`, no `LedgerSnapshot` and no `nkaWithSnapshot` / `nkaSubmitTx`.
8+
The record has three fields - `chainDb`, `systemStart` and `readEraHistory` - because direct `chainDb` access was enough to serve FetchBlock (pieces 2 and 3), so the snapshot interface was deferred.
9+
10+
Everything from "## Acceptance criteria" onwards describes the originally-planned snapshot design.
11+
That design is the target that pieces 4-7 grow into as they migrate the query, submit and eval methods off N2C; it is not current code.
12+
13+
## Problem
14+
15+
cardano-rpc currently threads `LocalNodeConnectInfo` through its environment and `MonadRpc` constraint.
16+
Every RPC method grabs the connection info and opens a fresh N2C socket connection per request.
17+
To support direct ledger state access (ADR-019), we need a new abstraction that replaces this pattern with an `IORef (Maybe NodeKernelAccess)` passed in by cardano-node at startup.
18+
19+
## Why
20+
21+
This piece creates the `NodeKernelAccess` abstraction and wires it through the cardano-rpc infrastructure.
22+
After this piece, cardano-rpc compiles with the new types threaded through, but no new RPC methods exist yet (piece 2) and no node-side implementation exists yet (piece 3).
23+
Separating the plumbing from the method rewrites and node-side implementation keeps each piece small and reviewable.
24+
25+
## User value
26+
27+
As a cardano-rpc developer, I want the `NodeKernelAccess` abstraction and environment wiring in place so that I can rewrite individual RPC methods to use node kernel access in subsequent pieces.
28+
29+
## Acceptance criteria
30+
31+
These criteria describe the originally-planned snapshot design (see "Status: as built" above), which is the target for pieces 4-7 rather than what piece 1 shipped.
32+
33+
1. **AC1: NodeKernelAccess module** - A new module `Cardano.Rpc.Server.Internal.NodeKernelAccess` exists at `src/Cardano/Rpc/Server/Internal/NodeKernelAccess.hs`, exporting `NodeKernelAccess(..)`, `LedgerSnapshot(..)`, and `withNodeKernelAccess`.
34+
`NodeKernelAccess` is a record with three fields: `nkaWithSnapshot :: forall a. (LedgerSnapshot -> IO a) -> IO a`, `nkaSubmitTx :: TxInMode -> IO (SubmitResult TxValidationErrorInCardanoMode)`, and `nkaFetchBlock :: SlotNo -> ByteString -> IO (Maybe ByteString)`.
35+
`LedgerSnapshot` is a newtype wrapping `runQuery :: forall result. QueryInMode result -> IO result`.
36+
The module is listed in `exposed-modules` in `cardano-rpc.cabal`.
37+
- Test: unit - compiles and is importable from the test suite
38+
39+
2. **AC2: withNodeKernelAccess unavailable behaviour** - `withNodeKernelAccess` reads the `IORef (Maybe NodeKernelAccess)`; when the value is `Nothing`, it throws a `GrpcException` with `grpcError = GrpcUnavailable` and message containing "not yet initialised".
40+
When the value is `Just na`, it passes `na` to the callback and returns the callback's result.
41+
- Test: unit - `H.propertyOnce`: create `IORef Nothing`, call `withNodeKernelAccess`, assert `GrpcException` with `GrpcUnavailable` is thrown; create `IORef (Just mockNodeKernelAccess)`, call `withNodeKernelAccess`, assert callback receives the value and its return value is propagated
42+
43+
3. **AC3: Server.hs signature change** - `runRpcServer` signature changes from `Tracer IO TraceRpc -> (RpcConfig, NetworkMagic) -> IO ()` to `Tracer IO TraceRpc -> RpcConfig -> NetworkMagic -> IORef (Maybe NodeKernelAccess) -> IO ()`.
44+
The module re-exports `NodeKernelAccess(..)` and `LedgerSnapshot(..)`.
45+
`RpcEnv` construction is updated to include both `rpcNodeKernelAccess` (from the new parameter) and `rpcLocalNodeConnectInfo` (preserved temporarily).
46+
Note: `methodsSyncRpc` is NOT registered in this piece - that happens in piece 2 when the SyncService proto and handler exist.
47+
- Test: unit - compiles (API change verified by build)
48+
49+
4. **AC4: Environment and MonadRpc wiring** - `RpcEnv` in `Env.hs` gains a new field `rpcNodeKernelAccess :: !(IORef (Maybe NodeKernelAccess))`.
50+
A `Has (IORef (Maybe NodeKernelAccess)) RpcEnv` instance is added to `Monad.hs`.
51+
`MonadRpc` constraint includes `Has (IORef (Maybe NodeKernelAccess)) e`.
52+
The old `rpcLocalNodeConnectInfo` field and `Has LocalNodeConnectInfo RpcEnv` instance are kept temporarily so that method files compile unchanged.
53+
- Test: unit - compiles; the new constraint is exercised by `withNodeKernelAccess` usage in the test from AC2
54+
55+
5. **AC5: Tracing for new trace types** - `Tracing.hs` gains a `TraceRpcSync` sum type with constructors: `TraceRpcFetchBlockSpan TraceSpanEvent` (span begin/end), `TraceRpcFetchBlockNotFound SlotNo` (block not on chain).
56+
`TraceRpc` gains a `TraceRpcSync TraceRpcSync` constructor.
57+
`Pretty` instances render the span events as "Started fetch block method" / "Finished fetch block method" and the not-found as "Block not found at slot <n>".
58+
An `Inject TraceRpcSync TraceRpc` instance is provided.
59+
`TraceRpcSubmitN2cConnectionError SomeException` is replaced by `TraceRpcNodeKernelAccessUnavailable` (no payload) and `TraceRpcForkerError String`.
60+
`Pretty TraceRpcSubmit` renders them as `"Ledger access unavailable (node kernel not yet initialised)"` and `"Forker error: <msg>"` respectively.
61+
The corresponding one-line update in `Submit.hs` (replacing `Left $ TraceRpcSubmitN2cConnectionError e` with `Left $ TraceRpcNodeKernelAccessUnavailable`) is included so that the build stays clean.
62+
- Test: unit - `H.propertyOnce` asserting the `Pretty` output of each new constructor contains the expected substrings
63+
64+
## Out of scope
65+
66+
- Populating the `cardano` oneof field in `AnyChainBlock` (requires protobuf block type mapping, a separate piece of work).
67+
- Streaming RPCs from the sync proto (`FollowTip`, `DumpHistory`).
68+
- Rewriting existing RPC methods (Query, Submit, Eval, Node) to use `NodeKernelAccess` (pieces 4-7).
69+
- Removing `rpcLocalNodeConnectInfo` and `Has LocalNodeConnectInfo` from `RpcEnv` / `MonadRpc` (happens when the last N2C method is rewritten in pieces 4-7).
70+
- Removing `mkLocalNodeConnectInfo` (removed alongside `rpcLocalNodeConnectInfo`).
71+
- Removing `nodeSocketPath` from `RpcConfig` (still needed for `nodeSocketPathToRpcSocketPath`).
72+
- Proto definitions and codegen (piece 2).
73+
- FetchBlock handler (piece 2).
74+
- `mkNodeKernelAccess` in cardano-node (piece 3).
75+
- Node startup wiring (piece 3).
76+
- Adding new E2E tests (no runtime behaviour changes in this piece).
77+
78+
## Definition of done
79+
80+
- [ ] All AC tests written (compile, fail on stubs)
81+
- [ ] Implementation complete (all tests pass via `cabal test`)
82+
- [ ] `cabal build cardano-rpc` succeeds from `/work` with no warnings
83+
- [ ] Nix CI checks pass
84+
- [ ] haskell-reviewer agent finds no critical or style issues
85+
- [ ] fourmolu clean (`scripts/devshell/prettify` run on changed files)
86+
- [ ] No build warnings
87+
88+
## Notes
89+
90+
### Design decision: keep both fields temporarily
91+
92+
This piece adds `rpcNodeKernelAccess :: IORef (Maybe NodeKernelAccess)` to `RpcEnv` alongside the existing `rpcLocalNodeConnectInfo :: LocalNodeConnectInfo`.
93+
Removing `rpcLocalNodeConnectInfo` would break every existing method file (`Node.hs`, `Query.hs`, `Submit.hs`, `Eval.hs`) because they all use `nodeConnInfo <- grab` to obtain a `LocalNodeConnectInfo`.
94+
Rewriting those method bodies is the work of pieces 4-7.
95+
96+
Both fields coexist in `RpcEnv` and both `Has` instances exist in `MonadRpc`.
97+
This means:
98+
- Existing method files compile without any changes.
99+
- Runtime behaviour of existing methods is unchanged (they still use N2C).
100+
- Pieces 4-7 each rewrite one method's N2C usage; the last piece to land removes the old field and instance.
101+
102+
### Files affected
103+
104+
| File | Change |
105+
|---|---|
106+
| `src/Cardano/Rpc/Server/Internal/NodeKernelAccess.hs` | **New.** `NodeKernelAccess`, `LedgerSnapshot`, `withNodeKernelAccess`. |
107+
| `src/Cardano/Rpc/Server/Internal/Env.hs` | Add `rpcNodeKernelAccess` field alongside existing `rpcLocalNodeConnectInfo`. |
108+
| `src/Cardano/Rpc/Server/Internal/Monad.hs` | Add `Has (IORef (Maybe NodeKernelAccess)) RpcEnv` instance. Add constraint to `MonadRpc`. |
109+
| `src/Cardano/Rpc/Server/Internal/Tracing.hs` | Add `TraceRpcSync` type and constructors. Replace `TraceRpcSubmitN2cConnectionError` with `TraceRpcNodeKernelAccessUnavailable` and `TraceRpcForkerError`. |
110+
| `src/Cardano/Rpc/Server/Internal/UtxoRpc/Submit.hs` | One-line trace constructor update. |
111+
| `src/Cardano/Rpc/Server.hs` | New signature, re-exports, updated `RpcEnv` construction. |
112+
| `cardano-rpc.cabal` | Add `NodeKernelAccess` module to `exposed-modules`. |
113+
114+
**cardano-node** (must update in lockstep to keep `-Werror` clean):
115+
116+
| File | Change |
117+
|---|---|
118+
| `src/Cardano/Node/Tracing/Tracers/Rpc.hs` | Handle renamed `TraceRpcNodeKernelAccessUnavailable`/`TraceRpcForkerError` and new `TraceRpcSync` constructors in `forMachine`, `asMetrics`, `namespaceFor`, `severityFor`, `documentFor`, `allNamespaces`. |
119+
| `src/Cardano/Node/Run.hs` | Create `nodeKernelAccessRef <- newIORef Nothing`, pass through `rpcServerLoop` to `runRpcServer`. Update `rpcServerLoop` signature. |
120+
121+
### Gotchas for the implementer
122+
123+
- **Import narrowing in `Monad.hs`**: when adding the new `Has` instance, ensure `Inject` (used by `putTrace`) is still available.
124+
Currently it comes from `import Cardano.Api`; if imports are narrowed, import it explicitly from `Cardano.Api.Era`.
125+
126+
- **`RankNTypes` extension.** Both `NodeKernelAccess` and `LedgerSnapshot` use higher-rank fields, requiring the `RankNTypes` extension in `NodeKernelAccess.hs`.
127+
128+
- **`runRpcServer` keeps `NetworkMagic`.** The old `rpcLocalNodeConnectInfo` is still used by existing methods, so `mkLocalNodeConnectInfo` still needs `NetworkMagic`.
129+
It is dropped only when `rpcLocalNodeConnectInfo` is finally removed in a later piece.
130+
131+
- **`Submit.hs` trace constructor.** `Submit.hs` currently references `TraceRpcSubmitN2cConnectionError` in its `submitTx` helper.
132+
The trace constructor rename requires a corresponding one-line update in `Submit.hs`: replace `Left $ TraceRpcSubmitN2cConnectionError e` with `Left $ TraceRpcNodeKernelAccessUnavailable` (dropping the exception payload, since the new constructor carries no payload).
133+
134+
- **`SomeException` import**: `Control.Exception` is still needed in `Tracing.hs` because `TraceRpcError` and `TraceRpcFatalError` use `SomeException`.
135+
136+
- **`GrpcException` import**: `withNodeKernelAccess` throws `GrpcException` from `Network.GRPC.Spec`.
137+
`grpc-spec` is already a dependency of `cardano-rpc`.
138+
139+
- **`RpcConfig.nodeSocketPath` stays**: ADR-019 explicitly notes this.
140+
The config field remains for deriving `rpcSocketPath` via `nodeSocketPathToRpcSocketPath`.
141+
142+
### Dependencies
143+
144+
- **Upstream:** none (this is the first piece).
145+
- **Downstream:** all pieces 2-8 depend on this (for the `NodeKernelAccess` record, environment wiring, and tracing).
146+
147+
### Testing approach
148+
149+
This piece is primarily a wiring/structural change.
150+
Two ACs have genuine Hedgehog property tests:
151+
- AC2 (`withNodeKernelAccess` behaviour): `H.propertyOnce` covering the `Nothing` and `Just` branches.
152+
- AC5 (tracing pretty-print): `H.propertyOnce` asserting rendered output of the new constructors.
153+
154+
AC1, AC3, AC4 are verified by successful compilation.
155+
156+
## Reference docs
157+
158+
- [Consensus protocol and snapshots](analysis-consensus-protocol.md) - snapshot consistency rationale
159+
- [API signatures](prereqs-api-signatures.md) - `NodeKernelAccess` type design context
160+
- [Implementation details](prereqs-implementation-details.md) - subtle gotchas for the interface

0 commit comments

Comments
 (0)