Skip to content

refactor: ChainStore, RpcError, typed boundaries, errors module#44

Merged
RaoulSchaffranek merged 2 commits into
mainfrom
refactor/architecture-store-types
Jul 23, 2026
Merged

refactor: ChainStore, RpcError, typed boundaries, errors module#44
RaoulSchaffranek merged 2 commits into
mainfrom
refactor/architecture-store-types

Conversation

@RaoulSchaffranek

Copy link
Copy Markdown
Member

A bigger-picture architectural pass, following the contained cleanups in #43. Stacked on #43 (base is that branch, not main) because both heavily touch server.py — retarget to main once #43 merges. No behaviour change: all 90 tests pass and make check is clean.

This grew out of three questions: what does the dependency graph look like, which stateful components can be isolated, and how strong are the types. Four related changes:

1. errors.py — decouple the encoder from the interpreter

transaction.py raised the interpreter's NodeInterpreterError for encoding failures (sub-stroop XLM, bad strkey), which forced a transaction → interpreter import that existed only for that exception type — the one semantically-wrong edge in the module graph. A new errors.py collects the internal exceptions (NodeInterpreterError, TransactionEncodingError, RpcError); the encoder now raises TransactionEncodingError and no longer imports interpreter at all.

2. RpcError — remove union-encoded control flow

The envelope builders returned dict[str, Any] | str | None, where str secretly meant "pre-formatted JSON-RPC error" and None meant "unknown method", disambiguated at the call site with isinstance(read_only_envelope, str). They now return dict[str, Any] and raise RpcError; handle_rpc delegates to a _dispatch helper and catches RpcError once. request_id drops out of the builder signatures (it was only there for error formatting). This matches the exception-based style ledger_entries.py already used.

3. ChainStore (store.py) — isolate the stateful disk layer

StellarRpcServer conflated HTTP transport, io-dir/disk layout, and RPC orchestration, with the disk schema smeared across ~6 *_dir attributes and ~5 scattered inline json.loads((io_dir/'metadata.json')…) reads. ChainStore now owns the layout and is the sole reader/writer of state.kore, metadata.json, and the receipts/ ledgers/ events/ requests/ wasms/ files. The server holds one and asks it for receipts, ledgers, events, and the ledger counter. server.io_dir / server.state_file are preserved for callers and tests.

4. TypedDicts at the boundaries

  • TxRequest / SimulateRequest — the encoder's request envelopes
  • SimulateResult — the K simulate result shape
  • LedgerRecord / EventRecord — the disk records the store reads/writes

interpreter.run widened to Mapping[str, Any] so typed envelopes are assignable. Functional TypedDict(...) syntax is used where keys are camelCase JSON wire names (so pep8-naming stays happy).

Deliberately left as dict[str, Any]

The read-only request envelopes (_read_only_envelope and friends) — they are built by {**base, …} spread and pattern-matched dynamically by node.md, where a TypedDict fights the idiom (spreads widen to dict[str, object], forcing casts) for little gain. Noted rather than forced.

Module graph after this change

server → store, interpreter, transaction, ledger, ledger_entries, result_xdr, scval, errors
transaction → scval, errors        (was: scval, interpreter)
interpreter → utils, errors
store → (stdlib only)

Test changes

test_unit.py now expects TransactionEncodingError from _xlm_to_stroops (was NodeInterpreterError) — a direct consequence of change #1.

… module

A bigger-picture pass following the earlier contained cleanups. Four related
changes, no behaviour change (90 tests pass, make check clean):

1. errors.py: one home for the internal exceptions. transaction.py raised the
   interpreter's NodeInterpreterError for encoding failures, forcing a
   transaction -> interpreter dependency that existed only for that type. It now
   raises TransactionEncodingError and no longer imports interpreter, removing
   the one semantically-wrong edge in the module graph.

2. RpcError replaces the union-encoded control flow. The envelope builders
   returned dict | str | None, where str secretly meant "pre-formatted error"
   and None "unknown method", forced open with isinstance(x, str). They now
   return dict and raise RpcError; handle_rpc delegates to _dispatch and catches
   once. request_id drops out of the builders' signatures.

3. ChainStore (store.py) owns the io-dir layout. StellarRpcServer conflated HTTP
   transport, disk layout, and RPC orchestration, with ~5 scattered inline
   metadata reads and path-joins. The store is now the sole reader/writer of
   state.kore, metadata.json, and the receipts/ ledgers/ events/ requests/ wasms/
   files; the server asks it for records. server.io_dir/state_file are kept for
   callers and tests.

4. TypedDicts at the boundaries: TxRequest/SimulateRequest (encoder outputs),
   SimulateResult (K result), and LedgerRecord/EventRecord (disk records).
   interpreter.run widened to Mapping[str, Any] to accept them. Read-only
   envelopes stay dict[str, Any] on purpose — they are built by {**base} spread
   and pattern-matched dynamically by node.md, where TypedDicts fight the idiom.

test_unit.py updated to expect TransactionEncodingError.
@RaoulSchaffranek
RaoulSchaffranek changed the base branch from refactor/implementation-dedup-dead-code to main July 23, 2026 09:51
@RaoulSchaffranek
RaoulSchaffranek merged commit de55339 into main Jul 23, 2026
1 check failed
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.

1 participant