darepod: recover indexed vHTLC refunds from seed restore - #757
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 vHTLC recovery manifest registration and wallet recovery capabilities, allowing the daemon to persist compact recovery metadata on the indexer and restore pay-side sender refunds during seed recovery. The feedback focuses on making the recovery process more robust by continuing the recovery loop instead of aborting on malformed labels or validation failures. Additionally, the reviewer recommends adding defensive nil checks for s.cfg.VHTLCConfig, terms, script, and indexed to prevent potential nil pointer dereferences.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
If decoding a manifest label fails, returning the error aborts the entire wallet recovery process. Since wallet recovery is a critical operation, a single corrupt or malformed label on the indexer should not prevent the recovery of other valid scripts and VTXOs. Consider logging the error and continuing the loop instead of returning the error.
| if err != nil { | |
| return err | |
| } | |
| if err != nil { | |
| continue | |
| } |
| if len(manifest.PkScript) > 0 && | ||
| !bytes.Equal(manifest.PkScript, pkScript) { | ||
|
|
||
| return fmt.Errorf("vHTLC recovery manifest script " + | ||
| "does not match registered script") | ||
| } | ||
|
|
||
| if err := validateVHTLCManifestScript( | ||
| manifest, pkScript, | ||
| ); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Similar to the decoding error, returning an error here when a manifest script mismatch occurs or validation fails will abort the entire wallet recovery. It is highly recommended to log these validation errors and continue processing other scripts to ensure the recovery process is robust against isolated data corruption.
if len(manifest.PkScript) > 0 &&
!bytes.Equal(manifest.PkScript, pkScript) {
continue
}
if err := validateVHTLCManifestScript(
manifest, pkScript,
); err != nil {
continue
}| func (s *paySession) recoveryManifestLabel() (string, error) { | ||
| if s.cfg == nil { | ||
| return "", fmt.Errorf("pay swap config is required") | ||
| } |
There was a problem hiding this comment.
Add a defensive check to ensure s.cfg.VHTLCConfig is not nil before accessing its fields (such as RefundLocktime, UnilateralClaimDelay, etc.). If VHTLCConfig is nil, this will cause a nil pointer dereference and panic the client/daemon during swap execution.
func (s *paySession) recoveryManifestLabel() (string, error) {
if s.cfg == nil {
return "", fmt.Errorf("pay swap config is required")
}
if s.cfg.VHTLCConfig == nil {
return "", fmt.Errorf("VHTLC config is required")
}| func (r *RPCServer) recoverIndexedVHTLCs(ctx context.Context, | ||
| terms *libtypes.OperatorTerms, result *walletRecoveryResult) error { | ||
|
|
||
| registered, err := r.server.indexer.ListMyReceiveScripts(ctx) |
There was a problem hiding this comment.
Add a nil check for terms at the beginning of recoverIndexedVHTLCs to prevent potential nil pointer dereferences if the operator terms are not available or returned as nil.
| func (r *RPCServer) recoverIndexedVHTLCs(ctx context.Context, | |
| terms *libtypes.OperatorTerms, result *walletRecoveryResult) error { | |
| registered, err := r.server.indexer.ListMyReceiveScripts(ctx) | |
| func (r *RPCServer) recoverIndexedVHTLCs(ctx context.Context, | |
| terms *libtypes.OperatorTerms, result *walletRecoveryResult) error { | |
| if terms == nil { | |
| return fmt.Errorf("operator terms are required") | |
| } | |
| registered, err := r.server.indexer.ListMyReceiveScripts(ctx) |
| for _, script := range registered.GetScripts() { | ||
| manifest, ok, err := vhtlcrecovery.DecodeManifestLabel( |
There was a problem hiding this comment.
Add a nil check for script to prevent a nil pointer dereference when iterating over registered.GetScripts().
for _, script := range registered.GetScripts() {
if script == nil {
continue
}
manifest, ok, err := vhtlcrecovery.DecodeManifestLabel(| for _, indexed := range vtxos { | ||
| live := arkrpc.VTXOStatus_VTXO_STATUS_LIVE | ||
| if indexed.GetStatus() != live { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Add a nil check for indexed to prevent a nil pointer dereference when calling indexed.GetStatus().
for _, indexed := range vtxos {
if indexed == nil {
continue
}
live := arkrpc.VTXOStatus_VTXO_STATUS_LIVE
if indexed.GetStatus() != live {
continue
}
|
@claude review pls |
|
Claude finished @sputn1ck's task in 3m 5s —— View job Review: indexer-backed vHTLC refund recovery
Nice, focused stack. The manifest encode/decode is clean, the idempotency key matches the SDK's A few things worth addressing, and a verdict on the Gemini comments. 1. Robustness: aborting the whole recovery on one bad manifest (valid concern)This is the one substantive point from the Gemini review I agree with. In I'd manifest, ok, err := vhtlcrecovery.DecodeManifestLabel(script.GetLabel())
if err != nil {
r.server.subLogger(Subsystem).WarnS(ctx, "Skipping undecodable "+
"vHTLC recovery manifest", slog.String("err", err.Error()))
continue
}Same treatment for the script-mismatch ( 2. The other Gemini suggestions are not needed
3. Privacy: payment hash + amount published to the indexerThe manifest label stores the 4. Minor
Plumbing (proto fields, walletdk types, swapwallet admin) is consistent end-to-end, and the tests cover the round-trip, the pay-sender filter, script validation, and the idempotency-key shape. No correctness blockers — item #1 is the one I'd land before merge. |
4f75b83 to
c164d0e
Compare
f98b3bb to
16aaed7
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f98b3bb659
ℹ️ 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".
| _, _, err = service.ArmRecovery(ctx, vhtlcrecovery.RecoveryJob{ | ||
| RequestID: requestID, | ||
| SwapID: append([]byte(nil), manifest.PaymentHash...), | ||
| Direction: vhtlcrecovery.DirectionPay, | ||
| Action: vhtlcrecovery.ActionRefundWithoutReceiver, | ||
| VTXOOutpoint: outpoint, | ||
| VTXOAmountSat: int64(indexed.GetValueSat()), |
There was a problem hiding this comment.
When restoring into a fresh data dir, this only creates the vHTLC recovery row from the indexed VTXO. Later EscalateVHTLCRecovery calls the materializer, which loads the funding OOR package from the local package store by target.Hash (darepod/vhtlc_recovery_target.go), but recoverIndexedVHTLCs never fetches or persists that package/checkpoints during seed restore. As a result the recovered refund is counted/armed but escalation fails with a missing recovery target package, leaving the restored pay-side vHTLC unrecoverable through the daemon.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR extends seed-restore recovery to find and re-arm pay-side vHTLC refund recovery using compact metadata embedded in indexer receive-script labels, and surfaces new recovery counters through daemon RPC and walletdk RPC/SDK results.
Changes:
- Add vHTLC recovery manifests encoded into indexer receive-script labels, and emit them when funding pay-side vHTLCs.
- Register labeled OOR recipient scripts with the indexer before OOR submission, and extend seed restore to scan/index those manifests and arm refund recovery.
- Expose recovered vHTLC/vHTLC-refund counters via
daemonrpc.InitWalletResponse,walletdkrpc.CreateResponse, and walletdk SDK results.
Reviewed changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| vhtlcrecovery/manifest.go | Adds manifest encoding/decoding + validation for indexer-stored vHTLC recovery labels. |
| vhtlcrecovery/manifest_test.go | Adds round-trip + negative tests for manifest label parsing. |
| swapwallet/admin.go | Plumbs new recovered vHTLC counters into walletdkrpc create handler response. |
| sdk/walletdk/types.go | Extends CreateWalletResult with recovered vHTLC counters. |
| sdk/walletdk/client.go | Maps new daemon recovery counters into walletdk SDK create result. |
| sdk/swaps/out_swap_test.go | Updates test daemon conn to record the new OOR label parameter. |
| sdk/swaps/in_swap.go | Generates and attaches vHTLC recovery manifest label when sending OOR funding for pay swaps. |
| sdk/swaps/in_swap_test.go | Verifies manifest label content is attached during pay flow; adjusts a timeout to reduce flakes. |
| sdk/swaps/client.go | Extends DaemonConn with SendOORWithPolicyDetailsAndLabel. |
| sdk/ark/client.go | Implements labeled OOR send methods and sets receive_script_label on outputs. |
| rpc/walletdkrpc/wallet.proto | Adds recovered_vhtlcs and recovered_vhtlc_refunds to create response. |
| rpc/walletdkrpc/wallet.pb.go | Regenerated protobuf bindings for walletdkrpc create response fields. |
| darepod/wallet_recovery.go | Implements seed-restore vHTLC manifest discovery and refund recovery arming; returns new counters. |
| darepod/wallet_recovery_test.go | Adds unit tests for manifest filtering, script validation, and idempotency key shape. |
| darepod/rpc_server.go | Registers labeled recipient scripts with the indexer prior to OOR submission; adds label TTL constant. |
| daemonrpc/daemon.proto | Adds vHTLC recovery counters to init response; adds receive_script_label to OOR Output. |
| daemonrpc/daemon.pb.go | Regenerated protobuf bindings for daemonrpc changes. |
Files not reviewed (2)
- daemonrpc/daemon.pb.go: Generated file
- rpc/walletdkrpc/wallet.pb.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case m.UnilateralRefundDelay == 0: | ||
| return fmt.Errorf("vhtlc recovery manifest refund delay is " + | ||
| "required") | ||
|
|
||
| case m.UnilateralRefundWithoutReceiverDelay == 0: | ||
| return fmt.Errorf("vhtlc recovery manifest refund-without-" + | ||
| "receiver delay is required") | ||
| } |
| if r.server.indexer == nil { | ||
| return status.Errorf(codes.Internal, "indexer client "+ | ||
| "not initialized") | ||
| } | ||
|
|
||
| idx := r.server.indexer.WithSigner( | ||
| r.server.proofKeyBackend.ProofSigner( | ||
| r.server.clientKeyDesc, | ||
| ), | ||
| ) | ||
| expiresAt := r.server.clk.Now().Add( | ||
| vhtlcRecoveryManifestRegistrationTTL, | ||
| ) | ||
| _, err := idx.RegisterReceiveScriptTaproot( | ||
| ctx, oorRecipients[i].PkScript, expiresAt, label, | ||
| ) | ||
| if err != nil && status.Code(err) != codes.AlreadyExists { | ||
| return status.Errorf(codes.Internal, "register "+ | ||
| "OOR recipient script %d: %v", i, err) | ||
| } | ||
| } |
| statuses, err := service.ListRecoveryStatuses(ctx) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| for i := range statuses { | ||
| if statuses[i].Job.RequestID == requestID { | ||
| return true, nil | ||
| } | ||
| } | ||
|
|
||
| return false, nil |
| if err := r.registerSendOORRecipientScripts( | ||
| ctx, requestRecipients, oorRecipients, | ||
| ); err != nil { | ||
| return nil, err | ||
| } |
| // vhtlcRecoveryManifestRegistrationTTL keeps swap recovery manifests | ||
| // available across realistic offline restore windows. The signed | ||
| // registration proof itself remains short lived in the indexer client. | ||
| vhtlcRecoveryManifestRegistrationTTL = 365 * 24 * time.Hour |
Add a compact versioned manifest label that can be stored with indexer receive-script registrations and later decoded during seed restore. The manifest records the vHTLC participant keys, hash, locktimes, script, amount, and signer locator needed to rebuild recovery candidates.
Expose the OOR output receive-script label used by indexer-backed recovery and add seed-restore counters for discovered vHTLCs and restored refunds. Generated daemonrpc and walletdkrpc stubs are updated from make rpc.
Attach a vHTLC recovery manifest to pay-side OOR funding outputs and have SendOOR register labelled recipient scripts with the indexer before submitting the transfer. This keeps unswept out-swaps out of the recovery path while making client-funded pay vHTLCs discoverable after local swap DB loss.
Extend seed restore to parse indexed vHTLC recovery manifests, verify each manifest against the registered script, and arm daemon-owned refund recovery for live pay-side candidates. Propagate the recovered vHTLC counters through walletdk and add focused tests for manifest filtering, script validation, and request-id compatibility.
16aaed7 to
6442b22
Compare
|
@sputn1ck, remember to re-request review from reviewers when ready |
Summary
This is intentionally stacked on #711 so seed-restore recovery can merge first.
Tests