darepod: expose stored vtxos for harness inspection - #239
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the GetStoredVTXO method to the Server struct in darepod/server.go, allowing test harnesses to retrieve persisted VTXO descriptors. Feedback indicates a potential data race when accessing s.vtxoStore during server startup, suggesting the use of a mutex or atomic pointer to ensure thread-safe access.
Comment on lines
+320
to
+328
| func (s *Server) GetStoredVTXO(ctx context.Context, | ||
| outpoint wire.OutPoint) (*vtxo.Descriptor, error) { | ||
|
|
||
| if s.vtxoStore == nil { | ||
| return nil, fmt.Errorf("client daemon VTXO store not initialized") | ||
| } | ||
|
|
||
| return s.vtxoStore.GetVTXO(ctx, outpoint) | ||
| } |
There was a problem hiding this comment.
The access to s.vtxoStore in this method is not thread-safe. Since vtxoStore is initialized within the run method (line 581), which is typically executed in a separate goroutine during integration tests, calling GetStoredVTXO concurrently with the server startup will result in a data race.
To maintain consistency with how other fields like rpcAddr are handled (see RPCAddr() at line 310), consider protecting vtxoStore with a mutex or using an atomic.Pointer for its initialization and access. This ensures that test harnesses can safely call this accessor even if the daemon is still in its startup phase.
Member
There was a problem hiding this comment.
I'd say given this is a test only function this is more of a non-issue.
Member
Author
There was a problem hiding this comment.
yeah - included in comment :)
Add a narrow accessor for persisted VTXO descriptors so server-side integration helpers can inspect a daemon's stored tree path without reaching into private daemon fields. This keeps the harness-facing contract explicit and avoids coupling tests to internal struct layout.
ellemouton
force-pushed
the
partial-unroll-harness-accessor
branch
from
April 8, 2026 11:17
d36cd3f to
b432942
Compare
bhandras
approved these changes
Apr 8, 2026
Comment on lines
+320
to
+328
| func (s *Server) GetStoredVTXO(ctx context.Context, | ||
| outpoint wire.OutPoint) (*vtxo.Descriptor, error) { | ||
|
|
||
| if s.vtxoStore == nil { | ||
| return nil, fmt.Errorf("client daemon VTXO store not initialized") | ||
| } | ||
|
|
||
| return s.vtxoStore.GetVTXO(ctx, outpoint) | ||
| } |
Member
There was a problem hiding this comment.
I'd say given this is a test only function this is more of a non-issue.
Roasbeef
added a commit
that referenced
this pull request
Apr 10, 2026
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.
3 tasks
Roasbeef
added a commit
that referenced
this pull request
Apr 17, 2026
Restore the harness-only accessor that was introduced in PR #239 and then inadvertently removed during the policy-migration refactor in commit 35dc614 (darepod: wire policy through daemon and CLI). The docs in darepod/CLAUDE.md and darepod/AGENTS.md still reference this method, so the removal left the API surface doc-inconsistent. The server-side darepo integration harness (harness/arkharness.go over in the darepo repo) calls this method to inspect a client daemon's persisted partial-unroll state without reaching into the unexported vtxoStore field across package+repo boundaries. Reflection or widening the field visibility would both violate the "harness does not poke private state" invariant the client CLAUDE.md calls out, so a narrow purpose-scoped accessor is the right shape. No behavior change beyond the accessor itself: it delegates straight to s.vtxoStore.GetVTXO.
2 tasks
bhandras
pushed a commit
that referenced
this pull request
Apr 21, 2026
Restore the harness-only accessor that was introduced in PR #239 and then inadvertently removed during the policy-migration refactor in commit 35dc614 (darepod: wire policy through daemon and CLI). The docs in darepod/CLAUDE.md and darepod/AGENTS.md still reference this method, so the removal left the API surface doc-inconsistent. The server-side darepo integration harness (harness/arkharness.go over in the darepo repo) calls this method to inspect a client daemon's persisted partial-unroll state without reaching into the unexported vtxoStore field across package+repo boundaries. Reflection or widening the field visibility would both violate the "harness does not poke private state" invariant the client CLAUDE.md calls out, so a narrow purpose-scoped accessor is the right shape. No behavior change beyond the accessor itself: it delegates straight to s.vtxoStore.GetVTXO.
bhandras
pushed a commit
that referenced
this pull request
Apr 21, 2026
Restore the harness-only accessor that was introduced in PR #239 and then inadvertently removed during the policy-migration refactor in commit 35dc614 (darepod: wire policy through daemon and CLI). The docs in darepod/CLAUDE.md and darepod/AGENTS.md still reference this method, so the removal left the API surface doc-inconsistent. The server-side darepo integration harness (harness/arkharness.go over in the darepo repo) calls this method to inspect a client daemon's persisted partial-unroll state without reaching into the unexported vtxoStore field across package+repo boundaries. Reflection or widening the field visibility would both violate the "harness does not poke private state" invariant the client CLAUDE.md calls out, so a narrow purpose-scoped accessor is the right shape. No behavior change beyond the accessor itself: it delegates straight to s.vtxoStore.GetVTXO.
ellemouton
pushed a commit
that referenced
this pull request
May 22, 2026
Points at #274 which restores the harness-only GetStoredVTXO accessor on darepod.Server. The method was introduced in client PR #239 and inadvertently removed in the policy-migration refactor (client 35dc614); the server-side harness at harness/arkharness.go:712 still depends on it for partial-unroll state inspection during itests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GetStoredVTXOaccessor on the daemon server so integrationharnesses can inspect a client's persisted VTXO descriptor (including its
TreePath) without reaching into private daemon fields.client's stored tree path to drive on-chain branch spends and assert watcher
state.
Test plan
go build ./darepod/...TestPartialUnrollIntegrationRatchetsWatcherForward)exercises this accessor end-to-end via the harness.
🤖 Generated with Claude Code