Skip to content

sweepbatcher: notify caller about confirmations #919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
58d4969
sweepbatcher: mark channels receive- or send-only
starius Apr 8, 2025
8860f7b
sweepbatcher: unblock sending if notifier quits
starius Apr 8, 2025
f4dfda5
sweepbatcher: send spend error to notifier
starius Apr 8, 2025
e22bc9e
test: fix error messages
starius Apr 8, 2025
b6381a9
sweepbatcher: fix mock TotalSweptAmount
starius Apr 8, 2025
a8995c6
sweepbatcher: fix mistake in batch reading from DB
starius Apr 8, 2025
af80791
test/chainnotifier_mock: support errors
starius Apr 8, 2025
b9f958b
sweepbatcher: cancel spendCtx after processing
starius Apr 8, 2025
ebd972e
sweepbatcher: remove unneded for loops
starius Apr 8, 2025
7647526
sweepbatcher: store batch status before monitoring
starius Apr 8, 2025
14a171d
sweepbatcher: align dbBatch type with DB schema
starius Apr 8, 2025
2de4c8a
sweepbatcher: test spending notification and error
starius Apr 8, 2025
8bb5401
sweepbatcher: notify caller about confirmations
starius Apr 9, 2025
cd29f43
test/chainnotifier: send to specific spend reg
starius Apr 28, 2025
12d3cc0
sweepbatcher: re-add sweeps after fully confirmed
starius Apr 26, 2025
63fd680
sweepbatcher: fix OnChainFeePortion values
starius Apr 27, 2025
5ab9a4b
loopout: close sweepbatcher quitChan
starius Apr 30, 2025
655df5c
sweepbatcher: pass utxo to fee provider
starius May 6, 2025
d87ef03
sweepbatcher: make sure dest pkscript is filled
starius May 6, 2025
63ad638
sweepbatcher: simplify presigned/purging test
starius May 6, 2025
62aae85
sweepbatcher: make sure HTLC.PkScript is filled
starius May 6, 2025
ea312b8
sweepbatcher/presigned: minRelayFee edge cases
starius May 8, 2025
b339904
sweepbatcher: remove method Presign
starius May 15, 2025
846246e
sweepbatcher: format pkscript as hex
starius May 15, 2025
32cc090
sweepbatcher: more logging in PresignSweepsGroup
starius May 15, 2025
381ab93
sweepbatcher: fix a bug in dest address selection
starius May 15, 2025
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
2 changes: 1 addition & 1 deletion loopout.go
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,7 @@ func (s *loopOutSwap) waitForHtlcSpendConfirmedV2(globalCtx context.Context,
quitChan := make(chan bool, 1)

defer func() {
quitChan <- true
close(quitChan)
}()

notifier := sweepbatcher.SpendNotifier{
Expand Down
4 changes: 3 additions & 1 deletion loopout_feerate.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/btcsuite/btcd/btcutil"
"github.com/btcsuite/btcd/chaincfg"
"github.com/btcsuite/btcd/txscript"
"github.com/btcsuite/btcd/wire"
"github.com/lightninglabs/loop/loopdb"
"github.com/lightninglabs/loop/swap"
"github.com/lightninglabs/loop/utils"
Expand Down Expand Up @@ -71,7 +72,8 @@ func newLoopOutSweepFeerateProvider(sweeper sweeper,

// GetMinFeeRate returns minimum required feerate for a sweep by swap hash.
func (p *loopOutSweepFeerateProvider) GetMinFeeRate(ctx context.Context,
swapHash lntypes.Hash) (chainfee.SatPerKWeight, error) {
swapHash lntypes.Hash,
_ wire.OutPoint) (chainfee.SatPerKWeight, error) {

_, feeRate, err := p.GetConfTargetAndFeeRate(ctx, swapHash)

Expand Down
55 changes: 31 additions & 24 deletions sweepbatcher/presigned.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package sweepbatcher
import (
"bytes"
"context"
"encoding/hex"
"fmt"

"github.com/btcsuite/btcd/blockchain"
Expand Down Expand Up @@ -36,13 +37,7 @@ func (b *batch) ensurePresigned(ctx context.Context, newSweeps []*sweep,
// presignedTxChecker has methods to check if the inputs are presigned.
type presignedTxChecker interface {
destPkScripter

// SignTx signs an unsigned transaction or returns a pre-signed tx.
// It is only called with loadOnly=true by ensurePresigned.
SignTx(ctx context.Context, primarySweepID wire.OutPoint,
tx *wire.MsgTx, inputAmt btcutil.Amount,
minRelayFee, feeRate chainfee.SatPerKWeight,
loadOnly bool) (*wire.MsgTx, error)
presigner
}

// ensurePresigned checks that there is a presigned transaction spending the
Expand Down Expand Up @@ -251,7 +246,7 @@ func (b *batch) presign(ctx context.Context, newSweeps []*sweep) error {

// Cache the destination address.
destAddr, err := getPresignedSweepsDestAddr(
ctx, b.cfg.presignedHelper, b.primarySweepID,
ctx, b.cfg.presignedHelper, primarySweepID,
b.cfg.chainParams,
)
if err != nil {
Expand Down Expand Up @@ -287,11 +282,12 @@ func (b *batch) presign(ctx context.Context, newSweeps []*sweep) error {

// presigner tries to presign a batch transaction.
type presigner interface {
// Presign tries to presign a batch transaction. If the method returns
// nil, it is guaranteed that future calls to SignTx on this set of
// sweeps return valid signed transactions.
Presign(ctx context.Context, primarySweepID wire.OutPoint,
tx *wire.MsgTx, inputAmt btcutil.Amount) error
// SignTx signs an unsigned transaction or returns a pre-signed tx.
// It is only called with loadOnly=true by ensurePresigned.
SignTx(ctx context.Context, primarySweepID wire.OutPoint,
tx *wire.MsgTx, inputAmt btcutil.Amount,
minRelayFee, feeRate chainfee.SatPerKWeight,
loadOnly bool) (*wire.MsgTx, error)
}

// presign tries to presign batch sweep transactions of the sweeps. It signs
Expand Down Expand Up @@ -370,7 +366,14 @@ func presign(ctx context.Context, presigner presigner, destAddr btcutil.Address,
}

// Try to presign this transaction.
err = presigner.Presign(ctx, primarySweepID, tx, batchAmt)
const (
loadOnly = false
minRelayFee = chainfee.AbsoluteFeePerKwFloor
)
_, err = presigner.SignTx(
ctx, primarySweepID, tx, batchAmt, minRelayFee, fr,
loadOnly,
)
if err != nil {
return fmt.Errorf("failed to presign unsigned tx %v "+
"for feeRate %v: %w", tx.TxHash(), fr, err)
Expand Down Expand Up @@ -405,9 +408,16 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error,
}
}

// Determine the current minimum relay fee based on our chain backend.
minRelayFee, err := b.wallet.MinRelayFee(ctx)
if err != nil {
return 0, fmt.Errorf("failed to get minRelayFee: %w", err),
false
}

// Cache current height and desired feerate of the batch.
currentHeight := b.currentHeight
feeRate := b.rbfCache.FeeRate
feeRate := max(b.rbfCache.FeeRate, minRelayFee)

// Append this sweep to an array of sweeps. This is needed to keep the
// order of sweeps stored, as iterating the sweeps map does not
Expand Down Expand Up @@ -445,13 +455,6 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error,
batchAmt += sweep.value
}

// Determine the current minimum relay fee based on our chain backend.
minRelayFee, err := b.wallet.MinRelayFee(ctx)
if err != nil {
return 0, fmt.Errorf("failed to get minRelayFee: %w", err),
false
}

// Get a pre-signed transaction.
const loadOnly = false
signedTx, err := b.cfg.presignedHelper.SignTx(
Expand Down Expand Up @@ -506,6 +509,9 @@ func (b *batch) publishPresigned(ctx context.Context) (btcutil.Amount, error,
b.batchTxid = &txHash
b.batchPkScript = tx.TxOut[0].PkScript

// Update cached FeeRate not to broadcast a tx with lower feeRate.
b.rbfCache.FeeRate = max(b.rbfCache.FeeRate, signedFeeRate)

return fee, nil, true
}

Expand Down Expand Up @@ -597,8 +603,9 @@ func CheckSignedTx(unsignedTx, signedTx *wire.MsgTx, inputAmt btcutil.Amount,
unsignedOut := unsignedTx.TxOut[0]
signedOut := signedTx.TxOut[0]
if !bytes.Equal(unsignedOut.PkScript, signedOut.PkScript) {
return fmt.Errorf("mismatch of output pkScript: %v, %v",
unsignedOut.PkScript, signedOut.PkScript)
return fmt.Errorf("mismatch of output pkScript: %s, %s",
hex.EncodeToString(unsignedOut.PkScript),
hex.EncodeToString(signedOut.PkScript))
}

// Find the feerate of signedTx.
Expand Down
20 changes: 13 additions & 7 deletions sweepbatcher/presigned_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,24 +553,30 @@ type mockPresigner struct {
failAt int
}

// Presign memorizes the value of the output and fails if the number of
// SignTx memorizes the value of the output and fails if the number of
// calls previously made is failAt.
func (p *mockPresigner) Presign(ctx context.Context,
primarySweepID wire.OutPoint, tx *wire.MsgTx,
inputAmt btcutil.Amount) error {
func (p *mockPresigner) SignTx(ctx context.Context,
primarySweepID wire.OutPoint, tx *wire.MsgTx, inputAmt btcutil.Amount,
minRelayFee, feeRate chainfee.SatPerKWeight,
loadOnly bool) (*wire.MsgTx, error) {

if ctx.Err() != nil {
return nil, ctx.Err()
}

if !hasInput(tx, primarySweepID) {
return fmt.Errorf("primarySweepID %v not in tx", primarySweepID)
return nil, fmt.Errorf("primarySweepID %v not in tx",
primarySweepID)
}

if len(p.outputs)+1 == p.failAt {
return fmt.Errorf("test error in Presign")
return nil, fmt.Errorf("test error in SignTx")
}

p.outputs = append(p.outputs, btcutil.Amount(tx.TxOut[0].Value))
p.lockTimes = append(p.lockTimes, tx.LockTime)

return nil
return tx, nil
}

// TestPresign checks that function presign presigns correct set of transactions
Expand Down
25 changes: 7 additions & 18 deletions sweepbatcher/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ type dbBatch struct {
// ID is the unique identifier of the batch.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe stupid question, but missing lots of context. This commit changes more than the mock, and feels a bit complicated, can you maybe elaborate what the changes to the other files are?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated the commit message:

sweepbatcher: align dbBatch type with DB schema

Previously, dbBatch had a State field (enum: Open, Closed, Confirmed), but in
the database it is represented as a boolean Confirmed. The Closed state was
stored the same way as Open. This wasn't an issue in practice, since an Open
batch is quickly transitioned to Closed after startup.

However, the in-memory mock stores plain dbBatch instances, leading to
inconsistent behavior between the mock and the real DB-backed store. This
commit updates dbBatch to match the database representation by replacing

Also added a note to the description of Closed const:

// Closed is the state in which the batch is no longer able to accept
// new sweeps. NOTE: this state exists only in-memory. In the database
// it is stored as Open and converted to Closed after a spend
// notification arrives (quickly after start of Batch.Run).
Closed batchState = 1

ID int32

// State is the current state of the batch.
State string
// Confirmed is set when the batch is fully confirmed.
Confirmed bool

// BatchTxid is the txid of the batch transaction.
BatchTxid chainhash.Hash
Expand Down Expand Up @@ -248,18 +248,15 @@ type dbSweep struct {
// Amount is the amount of the sweep.
Amount btcutil.Amount

// Completed indicates whether this sweep is completed.
// Completed indicates whether this sweep is fully-confirmed.
Completed bool
}

// convertBatchRow converts a batch row from db to a sweepbatcher.Batch struct.
func convertBatchRow(row sqlc.SweepBatch) *dbBatch {
batch := dbBatch{
ID: row.ID,
}

if row.Confirmed {
batch.State = batchOpen
ID: row.ID,
Confirmed: row.Confirmed,
}

if row.BatchTxID.Valid {
Expand Down Expand Up @@ -288,7 +285,7 @@ func convertBatchRow(row sqlc.SweepBatch) *dbBatch {
// it into the database.
func batchToInsertArgs(batch dbBatch) sqlc.InsertBatchParams {
args := sqlc.InsertBatchParams{
Confirmed: false,
Confirmed: batch.Confirmed,
BatchTxID: sql.NullString{
Valid: true,
String: batch.BatchTxid.String(),
Expand All @@ -305,10 +302,6 @@ func batchToInsertArgs(batch dbBatch) sqlc.InsertBatchParams {
MaxTimeoutDistance: batch.MaxTimeoutDistance,
}

if batch.State == batchConfirmed {
args.Confirmed = true
}

return args
}

Expand All @@ -317,7 +310,7 @@ func batchToInsertArgs(batch dbBatch) sqlc.InsertBatchParams {
func batchToUpdateArgs(batch dbBatch) sqlc.UpdateBatchParams {
args := sqlc.UpdateBatchParams{
ID: batch.ID,
Confirmed: false,
Confirmed: batch.Confirmed,
BatchTxID: sql.NullString{
Valid: true,
String: batch.BatchTxid.String(),
Expand All @@ -333,10 +326,6 @@ func batchToUpdateArgs(batch dbBatch) sqlc.UpdateBatchParams {
},
}

if batch.State == batchConfirmed {
args.Confirmed = true
}

return args
}

Expand Down
8 changes: 4 additions & 4 deletions sweepbatcher/store_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (s *StoreMock) FetchUnconfirmedSweepBatches(ctx context.Context) (

result := []*dbBatch{}
for _, batch := range s.batches {
if batch.State != "confirmed" {
if !batch.Confirmed {
result = append(result, &batch)
}
}
Expand Down Expand Up @@ -91,7 +91,7 @@ func (s *StoreMock) ConfirmBatch(ctx context.Context, id int32) error {
return errors.New("batch not found")
}

batch.State = "confirmed"
batch.Confirmed = true
s.batches[batch.ID] = batch

return nil
Expand Down Expand Up @@ -201,7 +201,7 @@ func (s *StoreMock) TotalSweptAmount(ctx context.Context, batchID int32) (
return 0, errors.New("batch not found")
}

if batch.State != batchConfirmed && batch.State != batchClosed {
if !batch.Confirmed {
return 0, nil
}

Expand All @@ -212,5 +212,5 @@ func (s *StoreMock) TotalSweptAmount(ctx context.Context, batchID int32) (
}
}

return 0, nil
return total, nil
}
Loading