Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions darepod/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,36 @@ For field-level detail, use `go doc github.com/lightninglabs/darepo-client/darep
OOR actor (`initOORActor`). The VTXO manager is constructed with a
`vtxo.LazyChainResolver` placeholder that `initUnrollSubsystem` fills in
later; anything needing that seam must run after `initUnrollSubsystem`.
- `initUnrollSubsystem` boot ordering is policy-preserving.
`recoverySvc.RestoreNonTerminal` (in-flight vHTLC recovery jobs, each
carrying its durable exit policy) runs **before** the chain resolver is
`Set()`; the force-exit admissions it drives through the VTXO manager are
buffered by the `LazyChainResolver` and replayed to the unroll registry the
instant the resolver is wired. The registry is first-writer-wins on exit
policy, so the generic orphan-job scan (`recoverOrphanedUnrollJobs`) runs
**after** `Set()` and is itself policy-carrying: it is handed a per-outpoint
exit-policy map (`recoveryExitPolicies`, built from the recovery store) and
re-admits each orphaned recovery target under its own vHTLC exit policy
rather than mislabeling it as a standard timeout.
- The chain-resolver→unroll bridge (`ensureUnrollFromExpiring`) maps a VTXO
`ExpiringNotification`'s trigger and optional exit policy into the registry's
`EnsureUnrollRequest`. `unrollStartTrigger` converts the string-typed
`actormsg.UnrollTrigger` (kept string-typed to avoid a `vtxo → unroll` import
cycle) into `unroll.StartTrigger`; an empty or unknown trigger admits as
critical expiry. A `None` exit policy leaves the registry on its standard
VTXO timeout policy.
- The fraud watcher (`initFraudWatcher`) is wired with `VTXOManagerRef`, so
fraud spends drive exits through the VTXO manager — the same admission path
as manual, critical-expiry, and vHTLC recovery exits — rather than talking to
the unroll registry directly.
- The vHTLC recovery service is wired with an `Exiter: managerExitAdmitter`, a
`ForceExit` seam that `Ask`s the VTXO manager to force a materialized
recovery target into unilateral exit. The target materializer
(`EnsureRecoveryTarget`) persists the descriptor directly into
`VTXOStatusUnilateralExit` (not `VTXOStatusSpending`) so the exiting coin is
excluded from the live/coin-selection query and cannot leak back into a
cooperative round as a forfeit; the boot-time orphan scan re-admits it on
restart.
- Boarding-sweep transaction construction, fee estimation, spend watching,
and startup resumption live inside the **wallet actor**
(`wallet.Ark.handleSweepBoardingUTXOs` / `handleResumeBoardingSweeps` in
Expand Down
30 changes: 30 additions & 0 deletions darepod/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,36 @@ For field-level detail, use `go doc github.com/lightninglabs/darepo-client/darep
OOR actor (`initOORActor`). The VTXO manager is constructed with a
`vtxo.LazyChainResolver` placeholder that `initUnrollSubsystem` fills in
later; anything needing that seam must run after `initUnrollSubsystem`.
- `initUnrollSubsystem` boot ordering is policy-preserving.
`recoverySvc.RestoreNonTerminal` (in-flight vHTLC recovery jobs, each
carrying its durable exit policy) runs **before** the chain resolver is
`Set()`; the force-exit admissions it drives through the VTXO manager are
buffered by the `LazyChainResolver` and replayed to the unroll registry the
instant the resolver is wired. The registry is first-writer-wins on exit
policy, so the generic orphan-job scan (`recoverOrphanedUnrollJobs`) runs
**after** `Set()` and is itself policy-carrying: it is handed a per-outpoint
exit-policy map (`recoveryExitPolicies`, built from the recovery store) and
re-admits each orphaned recovery target under its own vHTLC exit policy
rather than mislabeling it as a standard timeout.
- The chain-resolver→unroll bridge (`ensureUnrollFromExpiring`) maps a VTXO
`ExpiringNotification`'s trigger and optional exit policy into the registry's
`EnsureUnrollRequest`. `unrollStartTrigger` converts the string-typed
`actormsg.UnrollTrigger` (kept string-typed to avoid a `vtxo → unroll` import
cycle) into `unroll.StartTrigger`; an empty or unknown trigger admits as
critical expiry. A `None` exit policy leaves the registry on its standard
VTXO timeout policy.
- The fraud watcher (`initFraudWatcher`) is wired with `VTXOManagerRef`, so
fraud spends drive exits through the VTXO manager — the same admission path
as manual, critical-expiry, and vHTLC recovery exits — rather than talking to
the unroll registry directly.
- The vHTLC recovery service is wired with an `Exiter: managerExitAdmitter`, a
`ForceExit` seam that `Ask`s the VTXO manager to force a materialized
recovery target into unilateral exit. The target materializer
(`EnsureRecoveryTarget`) persists the descriptor directly into
`VTXOStatusUnilateralExit` (not `VTXOStatusSpending`) so the exiting coin is
excluded from the live/coin-selection query and cannot leak back into a
cooperative round as a forfeit; the boot-time orphan scan re-admits it on
restart.
- Boarding-sweep transaction construction, fee estimation, spend watching,
and startup resumption live inside the **wallet actor**
(`wallet.Ark.handleSweepBoardingUTXOs` / `handleResumeBoardingSweeps` in
Expand Down
8 changes: 5 additions & 3 deletions darepod/rpc_vhtlc_recovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/btcsuite/btcd/wire/v2"
"github.com/lightninglabs/darepo-client/daemonrpc"
"github.com/lightninglabs/darepo-client/db"
"github.com/lightninglabs/darepo-client/lib/actormsg"
"github.com/lightninglabs/darepo-client/unroll"
"github.com/lightninglabs/darepo-client/vhtlcrecovery"
"github.com/lightninglabs/darepo-client/vhtlcrecovery/coordinator"
Expand Down Expand Up @@ -52,6 +53,7 @@ func TestCancelVHTLCRecoveryMissingIsIdempotent(t *testing.T) {
coordinator.ServiceConfig{
Store: missingRecoveryStore{},
Unroll: noopUnrollRegistry{},
Exiter: noopUnrollRegistry{},
},
)
require.NoError(t, err)
Expand Down Expand Up @@ -157,10 +159,10 @@ func (missingRecoveryStore) FailRecovery(context.Context, string, error) error {

type noopUnrollRegistry struct{}

func (noopUnrollRegistry) EnsureUnroll(context.Context,
unroll.EnsureUnrollRequest) (*unroll.EnsureUnrollResp, error) {
func (noopUnrollRegistry) ForceExit(context.Context,
actormsg.ForceUnrollRequest) error {

return nil, errors.New("unexpected ensure unroll")
return errors.New("unexpected force exit")
}

func (noopUnrollRegistry) GetStatus(context.Context, wire.OutPoint) (
Expand Down
207 changes: 172 additions & 35 deletions darepod/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -4194,12 +4194,18 @@ func resolveExitOutcome(ctx context.Context,
return fn.Some(vtxo.ExitOutcomeResolution{
Outcome: vtxo.ExitOutcomeConfirmed,
Reason: job.LastError,
ExitPolicyKind: actormsg.ExitPolicyKind(
job.ExitPolicyKind,
),
}), nil

case db.UnilateralExitJobStatusFailedRecoverable:
return fn.Some(vtxo.ExitOutcomeResolution{
Outcome: vtxo.ExitOutcomeRecoverable,
Reason: job.LastError,
ExitPolicyKind: actormsg.ExitPolicyKind(
job.ExitPolicyKind,
),
}), nil

default:
Expand Down Expand Up @@ -5317,10 +5323,17 @@ func (s *Server) initUnrollSubsystem(ctx context.Context,
s.unrollRegistry = registry
s.unrollRegistryRef = fn.Some(registry.Ref())

if !s.vtxoMgrRef.IsSome() {
return fmt.Errorf("VTXO manager not initialized for vhtlc " +
"recovery")
}
recoverySvc, err := coordinator.NewService(coordinator.ServiceConfig{
Store: recoveryStore,
Unroll: coordinator.NewActorUnrollRegistry(registry.Ref()),
Log: fn.Some(s.subLogger(VHTLCRecoverySubsystem)),
Exiter: managerExitAdmitter{
mgr: s.vtxoMgrRef.UnsafeFromSome(),
},
Log: fn.Some(s.subLogger(VHTLCRecoverySubsystem)),
TargetMaterializer: newVHTLCRecoveryTargetMaterializer(
vtxoStore, oorStore,
s.subLogger(VHTLCRecoverySubsystem),
Expand Down Expand Up @@ -5349,27 +5362,19 @@ func (s *Server) initUnrollSubsystem(ctx context.Context,
return fmt.Errorf("restore non-terminal unroll jobs: %w", err)
}

// 3a. Convergent boot-time recovery for VTXOs that are already in
// VTXOStatusUnilateralExit in the VTXO store but have no matching
// unroll registry record. The two writes are not atomic: the VTXO
// actor flips status in its own DB tx and then Tells the chain
// resolver, which eventually triggers a separate registry
// UpsertRecord. A crash, full mailbox, or context cancel between
// those steps leaves the VTXO terminal-from-the-manager's
// perspective (it will not respawn a child actor) while the
// registry has nothing to drive forward. Without this scan such a
// VTXO stays stranded until the next manual EnsureUnroll. The
// scan is convergent: EnsureUnrollRequest dedups against
// r.active / r.pending / store.GetRecord, so a target that
// already has a record (e.g. just restored above) is a benign
// no-op. Per-target failures are collected and returned after the
// scan so startup fails closed instead of serving traffic with a
// known-stranded VTXO.
if err := s.recoverOrphanedUnrollJobs(
ctx, vtxoStore, registry,
); err != nil {
return fmt.Errorf("recover orphaned unroll jobs: %w", err)
}
// 3a. Restore in-flight vHTLC recovery jobs BEFORE the generic orphan
// scan below, and before the chain resolver is wired. Restore drives
// each job through the VTXO manager's force-exit, which emits the
// admission notification carrying the job's exit policy (e.g. a vHTLC
// refund). While the chain resolver target is still unset, those
// notifications are buffered by the LazyChainResolver and replayed to
// the registry the instant it is wired (step 4). That replay is what
// makes the policy-bearing admission reach the registry first: the
// registry is first-writer-wins on exit policy, so the generic orphan
// scan (step 5, no policy) must not create the record before the
// refund-policy admission lands, or the target would silently exit
// under the standard timeout policy. Restore failures are non-fatal;
// the job is retried on the next escalation or restart.
if err := recoverySvc.RestoreNonTerminal(ctx); err != nil {
s.log.WarnS(ctx, "Failed to restore vhtlc recovery jobs",
err)
Expand All @@ -5380,10 +5385,7 @@ func (s *Server) initUnrollSubsystem(ctx context.Context,
chainResolverRef := actor.NewMapInputRef(
registry.Ref(),
func(msg vtxo.ExpiringNotification) unroll.RegistryMsg {
return &unroll.EnsureUnrollRequest{
Outpoint: msg.VTXO.Outpoint,
Trigger: unroll.TriggerCriticalExpiry,
}
return ensureUnrollFromExpiring(msg)
},
)

Expand All @@ -5395,11 +5397,122 @@ func (s *Server) initUnrollSubsystem(ctx context.Context,
s.lazyChainResolver.Set(chainResolverRef)
}

// 5. Convergent boot-time recovery for VTXOs that are already in
// VTXOStatusUnilateralExit in the VTXO store but have no matching
// unroll registry record. The two writes are not atomic: the VTXO
// actor flips status in its own DB tx and then Tells the chain
// resolver, which eventually triggers a separate registry UpsertRecord.
// A crash, full mailbox, or context cancel between those steps leaves
// the VTXO terminal-from-the-manager's perspective (it will not respawn
// a child actor) while the registry has nothing to drive forward.
// Without this scan such a VTXO stays stranded until the next manual
// EnsureUnroll. The scan is convergent: EnsureUnrollRequest dedups
// against r.active / r.pending / store.GetRecord, so a target that
// already has a record (e.g. a vHTLC recovery whose refund-policy
// admission was just replayed by the Set above) is a benign no-op that
// preserves the existing policy.
//
// The registry is first-writer-wins on exit policy, so a naive
// no-policy scan could permanently claim a vHTLC target under the
// standard timeout policy if RestoreNonTerminal failed for it (a
// transient error leaves it UnilateralExit on disk with no record).
// To close that, we hand the scan the durable exit policy of every
// non-terminal recovery target so it re-admits under the RIGHT policy
// even when it does create the record. The ordering above stays as
// belt-and-suspenders. Per-target failures are collected and returned
// after the scan so startup fails closed instead of serving traffic
// with a known-stranded VTXO.
recoveryPolicies, err := recoveryExitPolicies(ctx, recoveryStore)
if err != nil {
return fmt.Errorf("load recovery exit policies: %w", err)
}
if err := s.recoverOrphanedUnrollJobs(
ctx, vtxoStore, registry, recoveryPolicies,
); err != nil {
return fmt.Errorf("recover orphaned unroll jobs: %w", err)
}

s.log.InfoS(ctx, "Unroll subsystem initialized")

return nil
}

// ensureUnrollFromExpiring maps a VTXO manager ExpiringNotification into the
// unroll registry's EnsureUnrollRequest. It is the seam that converts the
// string-typed trigger and optional exit policy carried on the notification
// (kept string-typed to avoid a vtxo->unroll import cycle) back into the
// unroll package's own types. A None ExitPolicy leaves the registry on its
// standard VTXO timeout policy.
func ensureUnrollFromExpiring(
msg vtxo.ExpiringNotification) *unroll.EnsureUnrollRequest {

req := &unroll.EnsureUnrollRequest{
Outpoint: msg.VTXO.Outpoint,
Trigger: unrollStartTrigger(msg.Trigger),
}

msg.ExitPolicy.WhenSome(func(p actormsg.ExitPolicy) {
req.ExitPolicyKind = unroll.ExitPolicyKind(p.Kind)
req.ExitPolicyRef = string(p.Ref)
})

return req
}

// unrollStartTrigger converts the string-typed UnrollTrigger that rides the
// ForceUnroll path back into the unroll package's StartTrigger. The trigger is
// carried as a string on the vtxo/actormsg side to avoid an import cycle
// (unroll already imports vtxo); this bridge is the seam where both packages
// are in scope. An empty/unknown trigger admits as critical expiry, preserving
// the auto-expiry default and the historical behavior of manual exits, which
// carried no explicit trigger.
func unrollStartTrigger(t actormsg.UnrollTrigger) unroll.StartTrigger {
switch t {
case actormsg.UnrollTriggerManual:
return unroll.TriggerManual

case actormsg.UnrollTriggerFraudSpend:
return unroll.TriggerFraudSpend

case actormsg.UnrollTriggerCriticalExpiry:
return unroll.TriggerCriticalExpiry

default:
return unroll.TriggerCriticalExpiry
}
}

// recoveryExitPolicy is the durable exit-policy identity of one vHTLC recovery
// target, keyed by its VTXO outpoint in recoveryExitPolicies.
type recoveryExitPolicy struct {
kind unroll.ExitPolicyKind
ref string
}

// recoveryExitPolicies indexes the exit policy of every non-terminal vHTLC
// recovery target by outpoint, so the orphan-recovery scan can re-admit a
// recovery target under its own policy instead of the standard timeout. The
// registry is first-writer-wins on exit policy, so a no-policy re-admission
// would otherwise permanently mislabel a refund target as a standard exit.
func recoveryExitPolicies(ctx context.Context,
store coordinator.Store) (map[wire.OutPoint]recoveryExitPolicy, error) {

jobs, err := store.ListNonTerminalRecoveries(ctx)
if err != nil {
return nil, err
}

policies := make(map[wire.OutPoint]recoveryExitPolicy, len(jobs))
for _, job := range jobs {
policies[job.VTXOOutpoint] = recoveryExitPolicy{
kind: unroll.ExitPolicyKind(job.ExitPolicyKind),
ref: job.ID,
}
}

return policies, nil
}

// recoverOrphanedUnrollJobs closes the atomicity gap between the VTXO
// store's status flip to VTXOStatusUnilateralExit and the unroll
// registry's UpsertRecord (#400). It lists every VTXO that the store
Expand All @@ -5415,7 +5528,8 @@ func (s *Server) initUnrollSubsystem(ctx context.Context,
// fails closed instead of serving traffic while known unilateral-exit VTXOs
// remain stranded.
func (s *Server) recoverOrphanedUnrollJobs(ctx context.Context,
vtxoStore vtxo.VTXOStore, registry *unroll.UnrollRegistryActor) error {
vtxoStore vtxo.VTXOStore, registry *unroll.UnrollRegistryActor,
recoveryPolicies map[wire.OutPoint]recoveryExitPolicy) error {

descs, err := vtxoStore.ListVTXOsByStatus(
ctx, vtxo.VTXOStatusUnilateralExit,
Expand All @@ -5434,10 +5548,36 @@ func (s *Server) recoverOrphanedUnrollJobs(ctx context.Context,
for _, desc := range descs {
op := desc.Outpoint

resp, askErr := ref.Ask(ctx, &unroll.EnsureUnrollRequest{
// A vHTLC recovery target carries a non-standard exit policy.
// Re-admit it under that policy so the first-writer-wins
// registry never locks it to the standard timeout: a standard
// witness against a vHTLC taproot tree would never sweep.
//
// The trigger is not recovered the way the exit policy is. A
// target that was force-exited under TriggerFraudSpend but
// crashed in the gap between the VTXO status flip and the
// registry admission has no registry record, so it re-admits
// here as TriggerRestart. The only effect is that its ready
// checkpoints are broadcast immediately instead of deferred to
// the recipient's fraud backstop window (see
// unroll.shouldSubmitReadyFrontier): earlier fees, same funds
// outcome, no missed deadline. The exit policy is recoverable
// because it lives in the recovery store; the fraud trigger has
// no such durable home. A faithful fix would stamp the trigger
// onto the VTXO row in the same transaction that flips it to
// UnilateralExit and read it back off the listed descriptors
// here, which is a schema change left as separable follow-up
// tracked in darepo-client#914.
ensureReq := &unroll.EnsureUnrollRequest{
Outpoint: op,
Trigger: unroll.TriggerRestart,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve fraud trigger in orphan recovery

When a fraud spend is forced through the VTXO manager, there is now a crash/drop window after the VTXO status is committed to UnilateralExit but before the best-effort chain-resolver Tell creates the registry row. On restart this orphan scan re-admits every such target as TriggerRestart, so a fraud-triggered target loses TriggerFraudSpend; the unroll FSM only applies the fraud checkpoint deferral/backstop logic for TriggerFraudSpend (see shouldSubmitReadyFrontier), so recipient fraud recovery can resume with the wrong behavior. Persist/recover the original fraud trigger or avoid routing fraud orphans through this generic restart admission.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Confirmed the window is real: routing fraud through the manager commits UnilateralExit before the best-effort chain-resolver Tell, so a crash in that gap leaves an orphan the restart scan re-admits as TriggerRestart, dropping TriggerFraudSpend.

Two things worth pinning down on impact before deciding the fix:

  1. It's a lost timing optimization, not a fund/liveness bug. Under a non-fraud trigger shouldSubmitReadyFrontier returns true, so the FSM submits the ready checkpoint immediately instead of deferring it to the fraud backstop deadline. That's a premature but safe broadcast: the exit still completes and the recipient still recovers the output; what's lost is the deferral (hold the checkpoint until near the CSV deadline). There's no path where the earlier broadcast helps an attacker.

  2. This is a byproduct of fixing the larger bug. Before this PR a fraud target never left the live set at all (it stayed Live), so it was never a UnilateralExit orphan-scan candidate. The window only exists because fraud now correctly goes out of the live set through the manager.

On the fix: a faithful "recover the trigger" needs new durable fraud state. Unlike the vHTLC recovery policy (which the orphan scan already recovers from the recovery store via recoveryExitPolicies), the fraud watcher keeps its watch set in memory and has no store the scan can consult, so preserving the trigger means persisting fraud-forced exits durably and joining them in the scan.

Given the impact is a lost deferral optimization in a narrow crash window with no fund risk, my inclination is to document this as a known limitation consistent with the async-admission trade-off rather than add a durable fraud-trigger store in this PR. I'm getting a second opinion on that call and will follow up here either way.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Second opinion is in and concurs: keep it documented, don't fix in this PR. The key confirmations from tracing the FSM: losing TriggerFraudSpend degrades to the same immediate-checkpoint-submission that manual and critical-expiry (the deadline-sensitive path) already use; the CSV delay is relative to checkpoint confirmation so earlier confirmation only starts the sweep clock earlier (never misses a deadline); checkpoints are the recipient's own pre-signed exit lineage, so publishing them reveals nothing and enables no attacker spend. And the fraud watcher can't paper over it on restart — it rebuilds watches from live descriptors, and an orphaned target is UnilateralExit, so it's never re-watched.

Documented the degraded behavior and the faithful follow-up at the re-admission site in e22f731 (recoverOrphanedUnrollJobs): the minimal correct fix is to stamp the trigger onto the VTXO row in the same tx that flips it to UnilateralExit and read it back off the descriptors the scan already lists — atomic, no new store, but a schema change and cleanly separable.

}).Await(ctx).Unpack()
}
if policy, ok := recoveryPolicies[op]; ok {
ensureReq.ExitPolicyKind = policy.kind
ensureReq.ExitPolicyRef = policy.ref
}

resp, askErr := ref.Ask(ctx, ensureReq).Await(ctx).Unpack()
if askErr != nil {
s.log.WarnS(ctx, "Failed to recover orphaned "+
"unroll job; VTXO remains stranded until "+
Expand Down Expand Up @@ -5581,18 +5721,15 @@ func (s *Server) initFraudWatcher(ctx context.Context,
],
) error {

if !s.unrollRegistryRef.IsSome() {
return fmt.Errorf("unroll registry not initialized")
}
if !s.vtxoMgrRef.IsSome() {
return fmt.Errorf("VTXO manager not initialized")
}

//nolint:contextcheck // watcher owns its own root context lifecycle
watcher := fraud.NewWatcherActor(fraud.WatcherConfig{
ChainSource: chainSourceRef,
UnrollRef: s.unrollRegistryRef.UnsafeFromSome(),
Log: fn.Some(s.subLogger(fraud.Subsystem)),
ChainSource: chainSourceRef,
VTXOManagerRef: s.vtxoMgrRef.UnsafeFromSome(),
Log: fn.Some(s.subLogger(fraud.Subsystem)),
})
s.fraudWatcher = watcher
s.fraudWatcherRef = fn.Some(watcher.Ref())
Expand Down
Loading
Loading