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