Skip to content

docs: gardening sweep for OwnedScriptChecker, IncomingVTXOHandler, SendVTXO hardening - #245

Merged
Roasbeef merged 2 commits into
mainfrom
doc-4-9
Apr 10, 2026
Merged

docs: gardening sweep for OwnedScriptChecker, IncomingVTXOHandler, SendVTXO hardening#245
Roasbeef merged 2 commits into
mainfrom
doc-4-9

Conversation

@Roasbeef

Copy link
Copy Markdown
Member

In this PR, we run the doc gardener over the per-package CLAUDE.md/AGENTS.md graph and ARCHITECTURE.md to bring them back in sync with main. The last broad sweep was PR #237, and three PRs have landed on top of it: #225 (send-in-round-followup), #238 (LndClient fee-estimator double-conversion fix, no doc impact), and #239 (partial-unroll harness accessor). The structural changes that needed documenting are the new data-driven pkScript ownership path that replaces the old IsOwner flag, and the new actor that materializes round-produced VTXOs from indexer push notifications.

The sweep itself is mechanical w.r.t the source: read the diffs against the doc-gardening-sweep merge, walk each affected package, update its CLAUDE.md to reflect the new key types/relationships/invariants, mirror to AGENTS.md, then patch ARCHITECTURE.md for cross-cutting bits. make doc-check is clean.

Data-driven script ownership

The round FSM no longer relies on a per-intent IsOwner flag. buildOwnedClientVTXOs now resolves ownership at confirmation time by asking a round.OwnedScriptChecker whether each VTXO's pkScript is recognized by the local wallet, and round.OwnedScriptRegistrar persists locally-owned scripts at intent-build time and inside handleRegisterIntent for incoming intents whose owner key has a non-zero KeyLocator (remote recipients in directed sends carry a zero locator and are intentionally left unregistered). The darepod package exposes both interfaces as thin adapters over the OOR owned-receive-scripts store, so the same table now backs three abstractions: round.OwnedScriptChecker, round.OwnedScriptRegistrar, and vtxo.OwnedScriptLookup. The per-package docs pick up these new interfaces, the adapter types, and the invariants they enforce. ARCHITECTURE.md gains a new "Data-Driven Script Ownership" pattern section so agents reading the code can follow the flow without re-deriving it from commits.

Incoming VTXO handler

The vtxo package now hosts an IncomingVTXOHandler actor that decodes arkrpc.IncomingVTXOEvent push notifications, derives the VTXO tapscript via lib/scripts.VTXOTapScript, persists the descriptor, and tells the VTXO manager via VTXOsMaterializedNotification (the same notification the OOR receive path already uses). darepod registers the actor under vtxo.IncomingVTXOServiceKey and wires a new MethodIncomingVTXO route into the EventRouter. The vtxo CLAUDE.md picks up the handler's inputs, validation rules (only VTXO_CREATED events, bounds-checked values, nil-safe pkScripts), and the fact that CommitmentTxID comes from the event rather than the leaf txid. The darepod doc records the route registration and the service-key bootstrap; ARCHITECTURE.md mentions the new route under the RPC-over-Mailbox pattern.

SendVTXO hardening and smaller updates

wallet/CLAUDE.md describes the hardened handleSendVTXOs path: recipient amounts are now bounded by MaxSatoshi, the running total uses overflow-safe accumulation, and the old releaseAndFail helper is replaced by a defer-based release that uses context.WithoutCancel so cleanup survives caller disconnect. The darepod doc records the matching RPC-side validation cap (maxRecipients = 256, see TODO #241) and notes that s.clk is now a cached clock.Clock instance shared by all sub-stores so tests can inject deterministic time. The new GetStoredVTXO harness accessor used by the partial unroll itests also lands in the darepod doc.

Tools fixup

There's also a small standalone commit that re-syncs tools/AGENTS.md with tools/CLAUDE.md. Commit 6e4c202 ("multi: add make lint-native target for faster local linting") updated CLAUDE.md to recommend make lint-native but left AGENTS.md pointing at the older make lint-local workflow. make doc-check was reporting this divergence; this PR clears it.

See each commit message for the per-area details.

Test plan

  • make doc-check passes
  • All updated CLAUDE.md/AGENTS.md pairs are byte-identical
  • No source files touched, docs only

Roasbeef added 2 commits April 9, 2026 18:07
Commit 6e4c202 ("multi: add make lint-native target for faster local
linting") updated tools/CLAUDE.md to recommend `make lint-native` and
`make install-custom-gcl`, but the sibling AGENTS.md was left pointing
at the older `make lint-local` workflow. Re-mirror the file so the two
copies agree and `make doc-check` reports a clean state.
Three landed PRs since the last broad gardening sweep touch subsystems
whose CLAUDE.md/AGENTS.md did not yet reflect the new shape: PR #225
(send-in-round-followup), PR #238 (fee-estimator fix, no doc impact),
and PR #239 (partial-unroll-harness-accessor). The most significant
structural changes are the introduction of a data-driven pkScript
ownership path and a dedicated actor that materializes round-produced
VTXOs from indexer push notifications.

The round FSM no longer relies on a per-intent IsOwner flag. Instead,
buildOwnedClientVTXOs resolves ownership at round confirmation time by
asking a round.OwnedScriptChecker whether each VTXO's pkScript is
recognized by the local wallet, and round.OwnedScriptRegistrar persists
locally-owned scripts at intent-build time and inside
handleRegisterIntent for incoming intents whose owner key has a
non-zero KeyLocator. The darepod package exposes both interfaces as
thin adapters over the OOR owned-receive-scripts store, which means the
same table now backs three abstractions (OwnedScriptChecker,
OwnedScriptRegistrar, and vtxo.OwnedScriptLookup). The per-package docs
pick up these new interfaces, the adapter types, and the invariants
they enforce; ARCHITECTURE.md gains a new "Data-Driven Script
Ownership" pattern section so agents encountering the code can follow
the flow without re-deriving it from commits.

The vtxo package now hosts an IncomingVTXOHandler actor that decodes
arkrpc.IncomingVTXOEvent push notifications, materializes the VTXO
descriptor via lib/scripts.VTXOTapScript, persists it, and notifies the
VTXO manager via VTXOsMaterializedNotification. darepod registers the
actor under vtxo.IncomingVTXOServiceKey and wires a new
MethodIncomingVTXO route into the EventRouter. The vtxo CLAUDE.md now
documents the handler's inputs, validation rules (only VTXO_CREATED
events, bounds-checked values, nil-safe pkScripts), and the fact that
CommitmentTxID comes from the event rather than the leaf txid. The
darepod doc picks up the route registration and the service-key
bootstrap; ARCHITECTURE.md mentions the new route under the
RPC-over-Mailbox pattern.

Smaller updates round out the sweep. wallet/CLAUDE.md describes the
hardened handleSendVTXOs path: recipient amounts are now bounded by
MaxSatoshi, the running total uses overflow-safe accumulation, and the
old releaseAndFail helper is replaced by a deferred release that uses
context.WithoutCancel so cleanup survives caller disconnect. The
darepod doc records the matching RPC-side validation cap
(maxRecipients = 256) and notes that s.clk is now a cached clock
instance so sub-stores share a single Clock for deterministic tests,
plus the new GetStoredVTXO harness accessor used by partial unroll
itests. Round, darepod, vtxo, and wallet docs all cross-reference the
new ownership interfaces where relevant so navigating the per-package
graph reaches the same explanation from any entry point.

make doc-check is clean after these changes.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e59c3db6fb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread darepod/AGENTS.md
- Auth headers (Schnorr signature) are injected into all outbound envelopes including response envelopes in `handleInboundRPC`.
- TLS client cert generation is skipped in insecure mode.
- Per-subsystem logging: configurable log writer, no global mutable loggers. Each subsystem receives its own logger instance.
- All sub-stores share the single `s.clk` clock instance assigned at `NewServer`. New code must not call `clock.NewDefaultClock()` inside `init*` methods — use `s.clk` so tests can inject deterministic time.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Correct the shared-clock invariant text

This new invariant says all sub-stores use s.clk, but the current implementation still creates at least one store with clock.NewDefaultClock() (darepod/rpc_oor_receive.go, newOORReceiveScriptStore). That makes the documented rule inaccurate and can mislead follow-up changes/tests that rely on deterministic clock injection from Server. Please narrow this statement or update it to reflect the exception.

Useful? React with 👍 / 👎.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements a data-driven VTXO ownership model, replacing the IsOwner flag with OwnedScriptChecker and OwnedScriptRegistrar interfaces to resolve ownership at round confirmation. It also introduces the IncomingVTXOHandler actor for materializing VTXOs from indexer push notifications and adds robust validation and cleanup logic for directed sends. Documentation across multiple subsystems has been updated to reflect these architectural changes, including the addition of shared clock injection for testing. I have no feedback to provide.

@Roasbeef
Roasbeef merged commit bfbd91b into main Apr 10, 2026
18 checks passed
ellemouton pushed a commit that referenced this pull request May 22, 2026
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