From 58fbe2230ead23358c5ba41400b1c3f4738e1982 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Wed, 1 Jul 2026 18:33:31 +0200 Subject: [PATCH] staticaddr/deposit: guard confirmation height access Document deposit lock ownership for mutable confirmation state and route production reads through deposit accessors. Keep store persistence on no-lock helpers while callers hold the deposit lock, preserving the existing transition behavior without leaving direct field reads in user-facing paths. --- loopd/swapclient_server.go | 9 +++++---- staticaddr/deposit/actions.go | 2 +- staticaddr/deposit/deposit.go | 25 +++++++++++++++++++++++-- staticaddr/deposit/fsm.go | 15 ++++++--------- staticaddr/deposit/manager.go | 14 +++++++++----- staticaddr/deposit/sql_store.go | 10 +++++++--- staticaddr/loopin/autoloop_dp.go | 6 +++--- staticaddr/loopin/manager.go | 7 ++++--- staticaddr/withdraw/manager.go | 2 +- 9 files changed, 59 insertions(+), 31 deletions(-) diff --git a/loopd/swapclient_server.go b/loopd/swapclient_server.go index 86d44a52..7bf5ab53 100644 --- a/loopd/swapclient_server.go +++ b/loopd/swapclient_server.go @@ -1896,7 +1896,7 @@ func (s *swapClientServer) ListStaticAddressWithdrawals(ctx context.Context, Id: d.ID[:], Outpoint: d.OutPoint.String(), Value: int64(d.Value), - ConfirmationHeight: d.ConfirmationHeight, + ConfirmationHeight: d.GetConfirmationHeight(), State: toClientDepositState( d.GetState(), ), @@ -1987,7 +1987,8 @@ func (s *swapClientServer) ListStaticAddressSwaps(ctx context.Context, protoDeposits = make([]*looprpc.Deposit, 0, len(ds)) for _, d := range ds { state := toClientDepositState(d.GetState()) - blocksUntilExpiry := d.ConfirmationHeight + + confirmationHeight := d.GetConfirmationHeight() + blocksUntilExpiry := confirmationHeight + int64(addrParams.Expiry) - int64(lndInfo.BlockHeight) @@ -1996,7 +1997,7 @@ func (s *swapClientServer) ListStaticAddressSwaps(ctx context.Context, State: state, Outpoint: d.OutPoint.String(), Value: int64(d.Value), - ConfirmationHeight: d.ConfirmationHeight, + ConfirmationHeight: confirmationHeight, SwapHash: d.SwapHash[:], BlocksUntilExpiry: blocksUntilExpiry, } @@ -2304,7 +2305,7 @@ func filter(deposits []*deposit.Deposit, f filterFunc) []*looprpc.Deposit { ), Outpoint: outpoint, Value: int64(d.Value), - ConfirmationHeight: d.ConfirmationHeight, + ConfirmationHeight: d.GetConfirmationHeight(), SwapHash: swapHash, } diff --git a/staticaddr/deposit/actions.go b/staticaddr/deposit/actions.go index 437c3ac2..77560eb2 100644 --- a/staticaddr/deposit/actions.go +++ b/staticaddr/deposit/actions.go @@ -139,7 +139,7 @@ func (f *FSM) WaitForExpirySweepAction(ctx context.Context, spendChan, errSpendChan, err := f.cfg.ChainNotifier.RegisterConfirmationsNtfn( //nolint:lll ctx, txID, f.deposit.TimeOutSweepPkScript, DefaultConfTarget, - int32(f.deposit.ConfirmationHeight), + int32(f.deposit.GetConfirmationHeight()), ) if err != nil { return f.HandleError(err) diff --git a/staticaddr/deposit/deposit.go b/staticaddr/deposit/deposit.go index cb9723e1..19c5c2b0 100644 --- a/staticaddr/deposit/deposit.go +++ b/staticaddr/deposit/deposit.go @@ -33,6 +33,9 @@ func (r *ID) FromByteSlice(b []byte) error { // Lock order: if both Manager.mu and a Deposit lock are needed, acquire // Manager.mu before Deposit.Lock. Never acquire Manager.mu while holding a // Deposit lock. +// +// The state and ConfirmationHeight fields are mutable and protected by the +// deposit lock. type Deposit struct { sync.Mutex @@ -98,6 +101,10 @@ func (d *Deposit) GetState() fsm.StateType { return d.state } +func (d *Deposit) getStateNoLock() fsm.StateType { + return d.state +} + func (d *Deposit) SetState(state fsm.StateType) { d.Lock() defer d.Unlock() @@ -105,7 +112,7 @@ func (d *Deposit) SetState(state fsm.StateType) { d.state = state } -func (d *Deposit) SetStateNoLock(state fsm.StateType) { +func (d *Deposit) setStateNoLock(state fsm.StateType) { d.state = state } @@ -116,10 +123,24 @@ func (d *Deposit) IsInState(state fsm.StateType) bool { return d.state == state } -func (d *Deposit) IsInStateNoLock(state fsm.StateType) bool { +func (d *Deposit) isInStateNoLock(state fsm.StateType) bool { return d.state == state } +// GetConfirmationHeight returns the deposit confirmation height. +func (d *Deposit) GetConfirmationHeight() int64 { + d.Lock() + defer d.Unlock() + + return d.ConfirmationHeight +} + +// GetConfirmationHeightNoLock returns the deposit confirmation height without +// acquiring the deposit lock. +func (d *Deposit) GetConfirmationHeightNoLock() int64 { + return d.ConfirmationHeight +} + // GetRandomDepositID generates a random deposit ID. func GetRandomDepositID() (ID, error) { var id ID diff --git a/staticaddr/deposit/fsm.go b/staticaddr/deposit/fsm.go index 0833be92..7dd8715d 100644 --- a/staticaddr/deposit/fsm.go +++ b/staticaddr/deposit/fsm.go @@ -452,17 +452,14 @@ func (f *FSM) updateDeposit(ctx context.Context, return } - type checkStateFunc func(state fsm.StateType) bool - type setStateFunc func(state fsm.StateType) - checkFunc := checkStateFunc(f.deposit.IsInState) - setFunc := setStateFunc(f.deposit.SetState) - if _, ok := lockedEvents[notification.Event]; ok { - checkFunc = f.deposit.IsInStateNoLock - setFunc = f.deposit.SetStateNoLock + _, alreadyLocked := lockedEvents[notification.Event] + if !alreadyLocked { + f.deposit.Lock() + defer f.deposit.Unlock() } - setFunc(notification.NextState) - if isUpdateSkipped(notification, checkFunc) { + f.deposit.setStateNoLock(notification.NextState) + if isUpdateSkipped(notification, f.deposit.isInStateNoLock) { return } diff --git a/staticaddr/deposit/manager.go b/staticaddr/deposit/manager.go index b4ea2779..bd98860c 100644 --- a/staticaddr/deposit/manager.go +++ b/staticaddr/deposit/manager.go @@ -438,7 +438,7 @@ func (m *Manager) GetActiveDepositsInState(stateFilter fsm.StateType) ( filteredDeposits := make([]*Deposit, 0, len(deposits)) for _, d := range deposits { - if !d.IsInStateNoLock(stateFilter) { + if !d.isInStateNoLock(stateFilter) { continue } @@ -446,8 +446,8 @@ func (m *Manager) GetActiveDepositsInState(stateFilter fsm.StateType) ( } sort.Slice(filteredDeposits, func(i, j int) bool { - return filteredDeposits[i].ConfirmationHeight < - filteredDeposits[j].ConfirmationHeight + return filteredDeposits[i].GetConfirmationHeightNoLock() < + filteredDeposits[j].GetConfirmationHeightNoLock() }) return filteredDeposits, nil @@ -481,7 +481,7 @@ func (m *Manager) AllOutpointsActiveDeposits(outpoints []wire.OutPoint, lockDeposits(deposits) defer unlockDeposits(deposits) for _, d := range deposits { - if !d.IsInStateNoLock(targetState) { + if !d.isInStateNoLock(targetState) { return nil, false } } @@ -543,7 +543,8 @@ func (m *Manager) TransitionDeposits(ctx context.Context, deposits []*Deposit, for _, deposit := range deposits { if deposit.isInFinalStateNoLock() { return fmt.Errorf("deposit %v is no longer active in "+ - "state %v", deposit.OutPoint, deposit.state) + "state %v", deposit.OutPoint, + deposit.getStateNoLock()) } } @@ -597,6 +598,9 @@ func (m *Manager) GetAllDeposits(ctx context.Context) ([]*Deposit, error) { // UpdateDeposit overrides all fields of the deposit with given ID in the store. func (m *Manager) UpdateDeposit(ctx context.Context, d *Deposit) error { + d.Lock() + defer d.Unlock() + return m.cfg.Store.UpdateDeposit(ctx, d) } diff --git a/staticaddr/deposit/sql_store.go b/staticaddr/deposit/sql_store.go index d9f4249f..a49550e5 100644 --- a/staticaddr/deposit/sql_store.go +++ b/staticaddr/deposit/sql_store.go @@ -47,7 +47,7 @@ func (s *SqlStore) CreateDeposit(ctx context.Context, deposit *Deposit) error { TxHash: deposit.Hash[:], OutIndex: int32(deposit.Index), Amount: int64(deposit.Value), - ConfirmationHeight: deposit.ConfirmationHeight, + ConfirmationHeight: deposit.GetConfirmationHeight(), TimeoutSweepPkScript: deposit.TimeOutSweepPkScript, } @@ -69,11 +69,15 @@ func (s *SqlStore) CreateDeposit(ctx context.Context, deposit *Deposit) error { } // UpdateDeposit updates the deposit in the database. +// +// Callers that pass a live deposit must hold the deposit lock while calling +// this method. The deposit FSM already does this for state transitions, and +// Manager.UpdateDeposit wraps external callers with the same lock. func (s *SqlStore) UpdateDeposit(ctx context.Context, deposit *Deposit) error { insertUpdateArgs := sqlc.InsertDepositUpdateParams{ DepositID: deposit.ID[:], UpdateTimestamp: s.clock.Now().UTC(), - UpdateState: string(deposit.state), + UpdateState: string(deposit.getStateNoLock()), } var ( @@ -83,7 +87,7 @@ func (s *SqlStore) UpdateDeposit(ctx context.Context, deposit *Deposit) error { Valid: true, } confirmationHeight = sql.NullInt64{ - Int64: deposit.ConfirmationHeight, + Int64: deposit.GetConfirmationHeightNoLock(), } ) diff --git a/staticaddr/loopin/autoloop_dp.go b/staticaddr/loopin/autoloop_dp.go index c605c43e..44dd12f4 100644 --- a/staticaddr/loopin/autoloop_dp.go +++ b/staticaddr/loopin/autoloop_dp.go @@ -234,9 +234,9 @@ func filterAutoloopCandidateDeposits(maxAmount btcutil.Amount, continue } + confirmationHeight := candidateDeposit.GetConfirmationHeight() swappable := IsSwappable( - uint32(candidateDeposit.ConfirmationHeight), - blockHeight, csvExpiry, + uint32(confirmationHeight), blockHeight, csvExpiry, ) if !swappable { continue @@ -246,7 +246,7 @@ func filterAutoloopCandidateDeposits(maxAmount btcutil.Amount, continue } - residualLife := candidateDeposit.ConfirmationHeight + + residualLife := confirmationHeight + int64(csvExpiry) - int64(blockHeight) eligibleDeposits = append( diff --git a/staticaddr/loopin/manager.go b/staticaddr/loopin/manager.go index 06ba43d6..b1074195 100644 --- a/staticaddr/loopin/manager.go +++ b/staticaddr/loopin/manager.go @@ -861,8 +861,9 @@ func SelectDeposits(targetAmount btcutil.Amount, // Filter out deposits that are too close to expiry to be swapped. var deposits []*deposit.Deposit for _, d := range unfilteredDeposits { + confirmationHeight := d.GetConfirmationHeight() if !IsSwappable( - uint32(d.ConfirmationHeight), blockHeight, csvExpiry, + uint32(confirmationHeight), blockHeight, csvExpiry, ) { log.Debugf("Skipping deposit %s as it expires before "+ @@ -878,9 +879,9 @@ func SelectDeposits(targetAmount btcutil.Amount, // blocks-until-expiry in ascending order. sort.Slice(deposits, func(i, j int) bool { if deposits[i].Value == deposits[j].Value { - iExp := uint32(deposits[i].ConfirmationHeight) + + iExp := uint32(deposits[i].GetConfirmationHeight()) + csvExpiry - blockHeight - jExp := uint32(deposits[j].ConfirmationHeight) + + jExp := uint32(deposits[j].GetConfirmationHeight()) + csvExpiry - blockHeight return iExp < jExp diff --git a/staticaddr/withdraw/manager.go b/staticaddr/withdraw/manager.go index 71967623..22acb79e 100644 --- a/staticaddr/withdraw/manager.go +++ b/staticaddr/withdraw/manager.go @@ -669,7 +669,7 @@ func (m *Manager) handleWithdrawal(ctx context.Context, d := deposits[0] spentChan, errChan, err := m.cfg.ChainNotifier.RegisterSpendNtfn( ctx, &d.OutPoint, addrParams.PkScript, - int32(d.ConfirmationHeight), + int32(d.GetConfirmationHeight()), ) if err != nil { return fmt.Errorf("unable to register spend ntfn: %w", err)