From 08aa4db35d1ce2a52bada76bceaaf0b6dde71cf2 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Thu, 30 May 2024 21:03:04 +0200 Subject: [PATCH 1/5] loopout: correctly account for the prepay amount when calculating costs Previously we'd not account for the prepay amount which resulted in the total swap cost reported being lower than what it actually is. --- loopout.go | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/loopout.go b/loopout.go index c13d78f6..6be118c1 100644 --- a/loopout.go +++ b/loopout.go @@ -77,6 +77,10 @@ type loopOutSwap struct { swapInvoicePaymentAddr [32]byte + // prepayAmount holds the amount of the prepay invoice. We use this + // to calculate the total cost of the swap. + prepayAmount btcutil.Amount + swapPaymentChan chan paymentResult prePaymentChan chan paymentResult @@ -466,16 +470,20 @@ func (s *loopOutSwap) handlePaymentResult(result paymentResult, if swapPayment { // The client pays for the swap with the swap invoice, // so we can calculate the total cost of the swap by - // subtracting the amount requested from the amount we - // actually paid. - s.cost.Server += result.status.Value.ToSatoshis() - + // subtracting the amount requested from the total + // amount that we actually paid (which is the sum of + // the swap invoice amount and the prepay invoice + // amount). + s.cost.Server += s.prepayAmount + + result.status.Value.ToSatoshis() - s.AmountRequested - - // On top of the swap cost we also pay for routing which - // is reflected in the fee. - s.cost.Offchain += result.status.Fee.ToSatoshis() } + // On top of the swap cost we also pay for routing which + // is reflected in the fee. We add the off-chain fee for both + // the swap payment and the prepay. + s.cost.Offchain += result.status.Fee.ToSatoshis() + return nil case result.status.State == lnrpc.Payment_FAILED: @@ -489,6 +497,16 @@ func (s *loopOutSwap) handlePaymentResult(result paymentResult, // executeSwap executes the swap, but returns as soon as the swap outcome is // final. At that point, there may still be pending off-chain payment(s). func (s *loopOutSwap) executeSwap(globalCtx context.Context) error { + // Decode the prepay invoice so we can ensure that we account for the + // prepay amount when calculating the final costs of the swap. + _, _, _, amt, err := swap.DecodeInvoice( + s.lnd.ChainParams, s.PrepayInvoice, + ) + if err != nil { + return err + } + s.prepayAmount = amt + // We always pay both invoices (again). This is currently the only way // to sort of resume payments. // From 4f5c806ba5e8b915cbb7485fb76099de42aa2a7d Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Thu, 30 May 2024 22:37:45 +0200 Subject: [PATCH 2/5] loopdb: add helper methods to update swap costs This commit adds the necessary sqlc code and SwapStore function to update swap costs for all swaps in one transaction. --- loopdb/interface.go | 5 ++ loopdb/sql_store.go | 32 +++++++++ loopdb/sql_test.go | 119 ++++++++++++++++++++++++++++++++++ loopdb/sqlc/querier.go | 2 + loopdb/sqlc/queries/swaps.sql | 16 +++++ loopdb/sqlc/swaps.sql.go | 41 ++++++++++++ loopdb/store.go | 8 +++ loopdb/store_mock.go | 22 +++++++ 8 files changed, 245 insertions(+) diff --git a/loopdb/interface.go b/loopdb/interface.go index 295c5364..ce8a1b97 100644 --- a/loopdb/interface.go +++ b/loopdb/interface.go @@ -65,6 +65,11 @@ type SwapStore interface { // it's decoding using the proto package's `Unmarshal` method. FetchLiquidityParams(ctx context.Context) ([]byte, error) + // BatchUpdateLoopOutSwapCosts updates the swap costs for a batch of + // loop out swaps. + BatchUpdateLoopOutSwapCosts(ctx context.Context, + swaps map[lntypes.Hash]SwapCost) error + // Close closes the underlying database. Close() error } diff --git a/loopdb/sql_store.go b/loopdb/sql_store.go index 9158e033..60e9d7a0 100644 --- a/loopdb/sql_store.go +++ b/loopdb/sql_store.go @@ -407,6 +407,38 @@ func (s *BaseDB) BatchInsertUpdate(ctx context.Context, }) } +// BatchUpdateLoopOutSwapCosts updates the swap costs for a batch of loop out +// swaps. +func (b *BaseDB) BatchUpdateLoopOutSwapCosts(ctx context.Context, + costs map[lntypes.Hash]SwapCost) error { + + writeOpts := &SqliteTxOptions{} + return b.ExecTx(ctx, writeOpts, func(tx *sqlc.Queries) error { + for swapHash, cost := range costs { + lastUpdateID, err := tx.GetLastUpdateID( + ctx, swapHash[:], + ) + if err != nil { + return err + } + + err = tx.OverrideSwapCosts( + ctx, sqlc.OverrideSwapCostsParams{ + ID: lastUpdateID, + ServerCost: int64(cost.Server), + OnchainCost: int64(cost.Onchain), + OffchainCost: int64(cost.Offchain), + }, + ) + if err != nil { + return err + } + } + + return nil + }) +} + // loopToInsertArgs converts a SwapContract struct to the arguments needed to // insert it into the database. func loopToInsertArgs(hash lntypes.Hash, diff --git a/loopdb/sql_test.go b/loopdb/sql_test.go index b0082069..76ecf91f 100644 --- a/loopdb/sql_test.go +++ b/loopdb/sql_test.go @@ -13,6 +13,7 @@ import ( "github.com/lightninglabs/loop/loopdb/sqlc" "github.com/lightninglabs/loop/test" "github.com/lightningnetwork/lnd/keychain" + "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/routing/route" "github.com/stretchr/testify/require" ) @@ -396,6 +397,124 @@ func TestIssue615(t *testing.T) { require.NoError(t, err) } +// TestBatchUpdateCost tests that we can batch update the cost of multiple swaps +// at once. +func TestBatchUpdateCost(t *testing.T) { + // Create a new sqlite store for testing. + store := NewTestDB(t) + + destAddr := test.GetDestAddr(t, 0) + initiationTime := time.Date(2018, 11, 1, 0, 0, 0, 0, time.UTC) + + testContract := LoopOutContract{ + SwapContract: SwapContract{ + AmountRequested: 100, + CltvExpiry: 144, + HtlcKeys: HtlcKeys{ + SenderScriptKey: senderKey, + ReceiverScriptKey: receiverKey, + SenderInternalPubKey: senderInternalKey, + ReceiverInternalPubKey: receiverInternalKey, + ClientScriptKeyLocator: keychain.KeyLocator{ + Family: 1, + Index: 2, + }, + }, + MaxMinerFee: 10, + MaxSwapFee: 20, + + InitiationHeight: 99, + + InitiationTime: initiationTime, + ProtocolVersion: ProtocolVersionMuSig2, + }, + MaxPrepayRoutingFee: 40, + PrepayInvoice: "prepayinvoice", + DestAddr: destAddr, + SwapInvoice: "swapinvoice", + MaxSwapRoutingFee: 30, + SweepConfTarget: 2, + HtlcConfirmations: 2, + SwapPublicationDeadline: initiationTime, + PaymentTimeout: time.Second * 11, + } + + makeSwap := func(preimage lntypes.Preimage) *LoopOutContract { + contract := testContract + contract.Preimage = preimage + + return &contract + } + + // Next, we'll add two swaps to the database. + preimage1 := testPreimage + preimage2 := lntypes.Preimage{4, 4, 4} + + ctxb := context.Background() + swap1 := makeSwap(preimage1) + swap2 := makeSwap(preimage2) + + hash1 := swap1.Preimage.Hash() + err := store.CreateLoopOut(ctxb, hash1, swap1) + require.NoError(t, err) + + hash2 := swap2.Preimage.Hash() + err = store.CreateLoopOut(ctxb, hash2, swap2) + require.NoError(t, err) + + // Add an update to both swaps containing the cost. + err = store.UpdateLoopOut( + ctxb, hash1, testTime, + SwapStateData{ + State: StateSuccess, + Cost: SwapCost{ + Server: 1, + Onchain: 2, + Offchain: 3, + }, + }, + ) + require.NoError(t, err) + + err = store.UpdateLoopOut( + ctxb, hash2, testTime, + SwapStateData{ + State: StateSuccess, + Cost: SwapCost{ + Server: 4, + Onchain: 5, + Offchain: 6, + }, + }, + ) + require.NoError(t, err) + + updateMap := map[lntypes.Hash]SwapCost{ + hash1: { + Server: 2, + Onchain: 3, + Offchain: 4, + }, + hash2: { + Server: 6, + Onchain: 7, + Offchain: 8, + }, + } + require.NoError(t, store.BatchUpdateLoopOutSwapCosts(ctxb, updateMap)) + + swaps, err := store.FetchLoopOutSwaps(ctxb) + require.NoError(t, err) + require.Len(t, swaps, 2) + + swapsMap := make(map[lntypes.Hash]*LoopOut) + swapsMap[swaps[0].Hash] = swaps[0] + swapsMap[swaps[1].Hash] = swaps[1] + + require.Equal(t, updateMap[hash1], swapsMap[hash1].State().Cost) + require.Equal(t, updateMap[hash2], swapsMap[hash2].State().Cost) +} + const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" func randomString(length int) string { diff --git a/loopdb/sqlc/querier.go b/loopdb/sqlc/querier.go index d12c6ed7..c930b8b7 100644 --- a/loopdb/sqlc/querier.go +++ b/loopdb/sqlc/querier.go @@ -18,6 +18,7 @@ type Querier interface { GetInstantOutSwap(ctx context.Context, swapHash []byte) (GetInstantOutSwapRow, error) GetInstantOutSwapUpdates(ctx context.Context, swapHash []byte) ([]InstantoutUpdate, error) GetInstantOutSwaps(ctx context.Context) ([]GetInstantOutSwapsRow, error) + GetLastUpdateID(ctx context.Context, swapHash []byte) (int32, error) GetLoopInSwap(ctx context.Context, swapHash []byte) (GetLoopInSwapRow, error) GetLoopInSwaps(ctx context.Context) ([]GetLoopInSwapsRow, error) GetLoopOutSwap(ctx context.Context, swapHash []byte) (GetLoopOutSwapRow, error) @@ -38,6 +39,7 @@ type Querier interface { InsertReservationUpdate(ctx context.Context, arg InsertReservationUpdateParams) error InsertSwap(ctx context.Context, arg InsertSwapParams) error InsertSwapUpdate(ctx context.Context, arg InsertSwapUpdateParams) error + OverrideSwapCosts(ctx context.Context, arg OverrideSwapCostsParams) error UpdateBatch(ctx context.Context, arg UpdateBatchParams) error UpdateInstantOut(ctx context.Context, arg UpdateInstantOutParams) error UpdateReservation(ctx context.Context, arg UpdateReservationParams) error diff --git a/loopdb/sqlc/queries/swaps.sql b/loopdb/sqlc/queries/swaps.sql index 8d8dd66e..f44c4e34 100644 --- a/loopdb/sqlc/queries/swaps.sql +++ b/loopdb/sqlc/queries/swaps.sql @@ -133,3 +133,19 @@ INSERT INTO htlc_keys( ) VALUES ( $1, $2, $3, $4, $5, $6, $7 ); + +-- name: GetLastUpdateID :one +SELECT id +FROM swap_updates +WHERE swap_hash = $1 +ORDER BY update_timestamp DESC +LIMIT 1; + +-- name: OverrideSwapCosts :exec +UPDATE swap_updates +SET + server_cost = $2, + onchain_cost = $3, + offchain_cost = $4 +WHERE id = $1; + diff --git a/loopdb/sqlc/swaps.sql.go b/loopdb/sqlc/swaps.sql.go index cda5dec5..c5404d90 100644 --- a/loopdb/sqlc/swaps.sql.go +++ b/loopdb/sqlc/swaps.sql.go @@ -10,6 +10,21 @@ import ( "time" ) +const getLastUpdateID = `-- name: GetLastUpdateID :one +SELECT id +FROM swap_updates +WHERE swap_hash = $1 +ORDER BY update_timestamp DESC +LIMIT 1 +` + +func (q *Queries) GetLastUpdateID(ctx context.Context, swapHash []byte) (int32, error) { + row := q.db.QueryRowContext(ctx, getLastUpdateID, swapHash) + var id int32 + err := row.Scan(&id) + return id, err +} + const getLoopInSwap = `-- name: GetLoopInSwap :one SELECT swaps.id, swaps.swap_hash, swaps.preimage, swaps.initiation_time, swaps.amount_requested, swaps.cltv_expiry, swaps.max_miner_fee, swaps.max_swap_fee, swaps.initiation_height, swaps.protocol_version, swaps.label, @@ -596,3 +611,29 @@ func (q *Queries) InsertSwapUpdate(ctx context.Context, arg InsertSwapUpdatePara ) return err } + +const overrideSwapCosts = `-- name: OverrideSwapCosts :exec +UPDATE swap_updates +SET + server_cost = $2, + onchain_cost = $3, + offchain_cost = $4 +WHERE id = $1 +` + +type OverrideSwapCostsParams struct { + ID int32 + ServerCost int64 + OnchainCost int64 + OffchainCost int64 +} + +func (q *Queries) OverrideSwapCosts(ctx context.Context, arg OverrideSwapCostsParams) error { + _, err := q.db.ExecContext(ctx, overrideSwapCosts, + arg.ID, + arg.ServerCost, + arg.OnchainCost, + arg.OffchainCost, + ) + return err +} diff --git a/loopdb/store.go b/loopdb/store.go index 3726ea2d..dd6b6510 100644 --- a/loopdb/store.go +++ b/loopdb/store.go @@ -1009,3 +1009,11 @@ func (b *boltSwapStore) BatchInsertUpdate(ctx context.Context, return errUnimplemented } + +// BatchUpdateLoopOutSwapCosts updates the swap costs for a batch of loop out +// swaps. +func (b *boltSwapStore) BatchUpdateLoopOutSwapCosts(ctx context.Context, + costs map[lntypes.Hash]SwapCost) error { + + return errUnimplemented +} diff --git a/loopdb/store_mock.go b/loopdb/store_mock.go index 6057e2a4..c64b0c06 100644 --- a/loopdb/store_mock.go +++ b/loopdb/store_mock.go @@ -3,6 +3,7 @@ package loopdb import ( "context" "errors" + "fmt" "testing" "time" @@ -337,3 +338,24 @@ func (b *StoreMock) BatchInsertUpdate(ctx context.Context, return errors.New("not implemented") } + +// BatchUpdateLoopOutSwapCosts updates the swap costs for a batch of loop out +// swaps. +func (s *StoreMock) BatchUpdateLoopOutSwapCosts(ctx context.Context, + costs map[lntypes.Hash]SwapCost) error { + + for hash, cost := range costs { + if _, ok := s.LoopOutUpdates[hash]; !ok { + return fmt.Errorf("swap has no updates: %v", hash) + } + + updates, ok := s.LoopOutUpdates[hash] + if !ok { + return fmt.Errorf("swap has no updates: %v", hash) + } + + updates[len(updates)-1].Cost = cost + } + + return nil +} From c650cdc8a841df4ee00a11404f0fba80a0b5e534 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Fri, 31 May 2024 08:45:34 +0200 Subject: [PATCH 3/5] loopdb: add ability to track manual migrations --- loopdb/interface.go | 7 +++ loopdb/sql_store.go | 23 ++++++++++ loopdb/sql_test.go | 16 +++++++ loopdb/sqlc/migration_tracker.sql.go | 45 +++++++++++++++++++ .../000008_migration_tracker.down.sql | 2 + .../000008_migration_tracker.up.sql | 9 ++++ loopdb/sqlc/models.go | 5 +++ loopdb/sqlc/querier.go | 2 + loopdb/sqlc/queries/migration_tracker.sql | 14 ++++++ loopdb/store.go | 14 ++++++ loopdb/store_mock.go | 14 ++++++ 11 files changed, 151 insertions(+) create mode 100644 loopdb/sqlc/migration_tracker.sql.go create mode 100644 loopdb/sqlc/migrations/000008_migration_tracker.down.sql create mode 100644 loopdb/sqlc/migrations/000008_migration_tracker.up.sql create mode 100644 loopdb/sqlc/queries/migration_tracker.sql diff --git a/loopdb/interface.go b/loopdb/interface.go index ce8a1b97..c3c049ca 100644 --- a/loopdb/interface.go +++ b/loopdb/interface.go @@ -70,6 +70,13 @@ type SwapStore interface { BatchUpdateLoopOutSwapCosts(ctx context.Context, swaps map[lntypes.Hash]SwapCost) error + // HasMigration returns true if the migration with the given ID has + // been done. + HasMigration(ctx context.Context, migrationID string) (bool, error) + + // SetMigration marks the migration with the given ID as done. + SetMigration(ctx context.Context, migrationID string) error + // Close closes the underlying database. Close() error } diff --git a/loopdb/sql_store.go b/loopdb/sql_store.go index 60e9d7a0..6d9a49bc 100644 --- a/loopdb/sql_store.go +++ b/loopdb/sql_store.go @@ -439,6 +439,29 @@ func (b *BaseDB) BatchUpdateLoopOutSwapCosts(ctx context.Context, }) } +// HasMigration returns true if the migration with the given ID has been done. +func (b *BaseDB) HasMigration(ctx context.Context, migrationID string) ( + bool, error) { + + migration, err := b.GetMigration(ctx, migrationID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return false, err + } + + return migration.MigrationTs.Valid, nil +} + +// SetMigration marks the migration with the given ID as done. +func (b *BaseDB) SetMigration(ctx context.Context, migrationID string) error { + return b.InsertMigration(ctx, sqlc.InsertMigrationParams{ + MigrationID: migrationID, + MigrationTs: sql.NullTime{ + Time: time.Now().UTC(), + Valid: true, + }, + }) +} + // loopToInsertArgs converts a SwapContract struct to the arguments needed to // insert it into the database. func loopToInsertArgs(hash lntypes.Hash, diff --git a/loopdb/sql_test.go b/loopdb/sql_test.go index 76ecf91f..5919236e 100644 --- a/loopdb/sql_test.go +++ b/loopdb/sql_test.go @@ -515,6 +515,22 @@ func TestBatchUpdateCost(t *testing.T) { require.Equal(t, updateMap[hash2], swapsMap[hash2].State().Cost) } +// TestMigrationTracker tests the migration tracker functionality. +func TestMigrationTracker(t *testing.T) { + ctxb := context.Background() + + // Create a new sqlite store for testing. + sqlDB := NewTestDB(t) + hasMigration, err := sqlDB.HasMigration(ctxb, "test") + require.NoError(t, err) + require.False(t, hasMigration) + + require.NoError(t, sqlDB.SetMigration(ctxb, "test")) + hasMigration, err = sqlDB.HasMigration(ctxb, "test") + require.NoError(t, err) + require.True(t, hasMigration) +} + const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" func randomString(length int) string { diff --git a/loopdb/sqlc/migration_tracker.sql.go b/loopdb/sqlc/migration_tracker.sql.go new file mode 100644 index 00000000..e68c56d5 --- /dev/null +++ b/loopdb/sqlc/migration_tracker.sql.go @@ -0,0 +1,45 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.25.0 +// source: migration_tracker.sql + +package sqlc + +import ( + "context" + "database/sql" +) + +const getMigration = `-- name: GetMigration :one +SELECT + migration_id, + migration_ts +FROM + migration_tracker +WHERE + migration_id = $1 +` + +func (q *Queries) GetMigration(ctx context.Context, migrationID string) (MigrationTracker, error) { + row := q.db.QueryRowContext(ctx, getMigration, migrationID) + var i MigrationTracker + err := row.Scan(&i.MigrationID, &i.MigrationTs) + return i, err +} + +const insertMigration = `-- name: InsertMigration :exec +INSERT INTO migration_tracker ( + migration_id, + migration_ts +) VALUES ($1, $2) +` + +type InsertMigrationParams struct { + MigrationID string + MigrationTs sql.NullTime +} + +func (q *Queries) InsertMigration(ctx context.Context, arg InsertMigrationParams) error { + _, err := q.db.ExecContext(ctx, insertMigration, arg.MigrationID, arg.MigrationTs) + return err +} diff --git a/loopdb/sqlc/migrations/000008_migration_tracker.down.sql b/loopdb/sqlc/migrations/000008_migration_tracker.down.sql new file mode 100644 index 00000000..6423aa6e --- /dev/null +++ b/loopdb/sqlc/migrations/000008_migration_tracker.down.sql @@ -0,0 +1,2 @@ +DROP TABLE migration_tracker; + diff --git a/loopdb/sqlc/migrations/000008_migration_tracker.up.sql b/loopdb/sqlc/migrations/000008_migration_tracker.up.sql new file mode 100644 index 00000000..01b8a72f --- /dev/null +++ b/loopdb/sqlc/migrations/000008_migration_tracker.up.sql @@ -0,0 +1,9 @@ +CREATE TABLE migration_tracker ( + -- migration_id is the id of the migration. + migration_id TEXT NOT NULL, + + -- migration_ts is the timestamp at which the migration was run. + migration_ts TIMESTAMP, + + PRIMARY KEY (migration_id) +); diff --git a/loopdb/sqlc/models.go b/loopdb/sqlc/models.go index 45185d30..03c5c676 100644 --- a/loopdb/sqlc/models.go +++ b/loopdb/sqlc/models.go @@ -67,6 +67,11 @@ type LoopoutSwap struct { PaymentTimeout int32 } +type MigrationTracker struct { + MigrationID string + MigrationTs sql.NullTime +} + type Reservation struct { ID int32 ReservationID []byte diff --git a/loopdb/sqlc/querier.go b/loopdb/sqlc/querier.go index c930b8b7..4c94d146 100644 --- a/loopdb/sqlc/querier.go +++ b/loopdb/sqlc/querier.go @@ -23,6 +23,7 @@ type Querier interface { GetLoopInSwaps(ctx context.Context) ([]GetLoopInSwapsRow, error) GetLoopOutSwap(ctx context.Context, swapHash []byte) (GetLoopOutSwapRow, error) GetLoopOutSwaps(ctx context.Context) ([]GetLoopOutSwapsRow, error) + GetMigration(ctx context.Context, migrationID string) (MigrationTracker, error) GetParentBatch(ctx context.Context, swapHash []byte) (SweepBatch, error) GetReservation(ctx context.Context, reservationID []byte) (Reservation, error) GetReservationUpdates(ctx context.Context, reservationID []byte) ([]ReservationUpdate, error) @@ -36,6 +37,7 @@ type Querier interface { InsertInstantOutUpdate(ctx context.Context, arg InsertInstantOutUpdateParams) error InsertLoopIn(ctx context.Context, arg InsertLoopInParams) error InsertLoopOut(ctx context.Context, arg InsertLoopOutParams) error + InsertMigration(ctx context.Context, arg InsertMigrationParams) error InsertReservationUpdate(ctx context.Context, arg InsertReservationUpdateParams) error InsertSwap(ctx context.Context, arg InsertSwapParams) error InsertSwapUpdate(ctx context.Context, arg InsertSwapUpdateParams) error diff --git a/loopdb/sqlc/queries/migration_tracker.sql b/loopdb/sqlc/queries/migration_tracker.sql new file mode 100644 index 00000000..da1fd178 --- /dev/null +++ b/loopdb/sqlc/queries/migration_tracker.sql @@ -0,0 +1,14 @@ +-- name: InsertMigration :exec +INSERT INTO migration_tracker ( + migration_id, + migration_ts +) VALUES ($1, $2); + +-- name: GetMigration :one +SELECT + migration_id, + migration_ts +FROM + migration_tracker +WHERE + migration_id = $1; diff --git a/loopdb/store.go b/loopdb/store.go index dd6b6510..a43db544 100644 --- a/loopdb/store.go +++ b/loopdb/store.go @@ -1017,3 +1017,17 @@ func (b *boltSwapStore) BatchUpdateLoopOutSwapCosts(ctx context.Context, return errUnimplemented } + +// HasMigration returns true if the migration with the given ID has been done. +func (b *boltSwapStore) HasMigration(ctx context.Context, migrationID string) ( + bool, error) { + + return false, errUnimplemented +} + +// SetMigration marks the migration with the given ID as done. +func (b *boltSwapStore) SetMigration(ctx context.Context, + migrationID string) error { + + return errUnimplemented +} diff --git a/loopdb/store_mock.go b/loopdb/store_mock.go index c64b0c06..268bba6b 100644 --- a/loopdb/store_mock.go +++ b/loopdb/store_mock.go @@ -359,3 +359,17 @@ func (s *StoreMock) BatchUpdateLoopOutSwapCosts(ctx context.Context, return nil } + +// HasMigration returns true if the migration with the given ID has been done. +func (s *StoreMock) HasMigration(ctx context.Context, migrationID string) ( + bool, error) { + + return false, errUnimplemented +} + +// SetMigration marks the migration with the given ID as done. +func (s *StoreMock) SetMigration(ctx context.Context, + migrationID string) error { + + return errUnimplemented +} From f40ef193e906360a6a5e1eb89b4f78107cf5632f Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Thu, 30 May 2024 22:38:56 +0200 Subject: [PATCH 4/5] loop: add migration for loopout swaps to fix negative stored costs Previously we may have stored negative costs for some loop out swaps which this commit attempts to correct by fetching all completed swap, calculating the correct costs and overriding them in the database. --- cost_migration.go | 177 +++++++++++++++++++++++++++++++++++++++ cost_migration_test.go | 184 +++++++++++++++++++++++++++++++++++++++++ loopdb/store_mock.go | 15 +++- server_mock_test.go | 7 +- 4 files changed, 380 insertions(+), 3 deletions(-) create mode 100644 cost_migration.go create mode 100644 cost_migration_test.go diff --git a/cost_migration.go b/cost_migration.go new file mode 100644 index 00000000..247f63fa --- /dev/null +++ b/cost_migration.go @@ -0,0 +1,177 @@ +package loop + +import ( + "context" + "fmt" + "time" + + "github.com/btcsuite/btcd/chaincfg" + "github.com/lightninglabs/lndclient" + "github.com/lightninglabs/loop/loopdb" + "github.com/lightninglabs/loop/swap" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwire" +) + +const ( + costMigrationID = "cost_migration" +) + +// CalculateLoopOutCost calculates the total cost of a loop out swap. It will +// correctly account for the on-chain and off-chain fees that were paid and +// make sure that all costs are positive. +func CalculateLoopOutCost(params *chaincfg.Params, loopOutSwap *loopdb.LoopOut, + paymentFees map[lntypes.Hash]lnwire.MilliSatoshi) (loopdb.SwapCost, + error) { + + // First make sure that this swap is actually finished. + if loopOutSwap.State().State.IsPending() { + return loopdb.SwapCost{}, fmt.Errorf("swap is not yet finished") + } + + // We first need to decode the prepay invoice to get the prepay hash and + // the prepay amount. + _, _, hash, prepayAmount, err := swap.DecodeInvoice( + params, loopOutSwap.Contract.PrepayInvoice, + ) + if err != nil { + return loopdb.SwapCost{}, fmt.Errorf("unable to decode the "+ + "prepay invoice: %v", err) + } + + // The swap hash is given and we don't need to get it from the + // swap invoice, however we'll decode it anyway to get the invoice amount + // that was paid in case we don't have the payment anymore. + _, _, swapHash, swapPaymentAmount, err := swap.DecodeInvoice( + params, loopOutSwap.Contract.SwapInvoice, + ) + if err != nil { + return loopdb.SwapCost{}, fmt.Errorf("unable to decode the "+ + "swap invoice: %v", err) + } + + var ( + cost loopdb.SwapCost + swapPaid, prepayPaid bool + ) + + // Now that we have the prepay and swap amount, we can calculate the + // total cost of the swap. Note that we only need to account for the + // server cost in case the swap was successful or if the sweep timed + // out. Otherwise the server didn't pull the off-chain htlc nor the + // prepay. + switch loopOutSwap.State().State { + case loopdb.StateSuccess: + cost.Server = swapPaymentAmount + prepayAmount - + loopOutSwap.Contract.AmountRequested + + swapPaid = true + prepayPaid = true + + case loopdb.StateFailSweepTimeout: + cost.Server = prepayAmount + + prepayPaid = true + + default: + cost.Server = 0 + } + + // Now attempt to look up the actual payments so we can calculate the + // total routing costs. + prepayPaymentFee, ok := paymentFees[hash] + if prepayPaid && ok { + cost.Offchain += prepayPaymentFee.ToSatoshis() + } else { + log.Debugf("Prepay payment %s is missing, won't account for "+ + "routing fees", hash) + } + + swapPaymentFee, ok := paymentFees[swapHash] + if swapPaid && ok { + cost.Offchain += swapPaymentFee.ToSatoshis() + } else { + log.Debugf("Swap payment %s is missing, won't account for "+ + "routing fees", swapHash) + } + + // For the on-chain cost, just make sure that the cost is positive. + cost.Onchain = loopOutSwap.State().Cost.Onchain + if cost.Onchain < 0 { + cost.Onchain *= -1 + } + + return cost, nil +} + +// MigrateLoopOutCosts will calculate the correct cost for all loop out swaps +// and override the cost values of the last update in the database. +func MigrateLoopOutCosts(ctx context.Context, lnd lndclient.LndServices, + db loopdb.SwapStore) error { + + migrationDone, err := db.HasMigration(ctx, costMigrationID) + if err != nil { + return err + } + if migrationDone { + log.Infof("Cost cleanup migration already done, skipping") + + return nil + } + + log.Infof("Starting cost cleanup migration") + startTs := time.Now() + defer func() { + log.Infof("Finished cost cleanup migration in %v", + time.Since(startTs)) + }() + + // First we'll fetch all loop out swaps from the database. + loopOutSwaps, err := db.FetchLoopOutSwaps(ctx) + if err != nil { + return err + } + + // Next we fetch all payments from LND. + payments, err := lnd.Client.ListPayments( + ctx, lndclient.ListPaymentsRequest{}, + ) + if err != nil { + return err + } + + // Gather payment fees to a map for easier lookup. + paymentFees := make(map[lntypes.Hash]lnwire.MilliSatoshi) + for _, payment := range payments.Payments { + paymentFees[payment.Hash] = payment.Fee + } + + // Now we'll calculate the cost for each swap and finally update the + // costs in the database. + updatedCosts := make(map[lntypes.Hash]loopdb.SwapCost) + for _, loopOutSwap := range loopOutSwaps { + cost, err := CalculateLoopOutCost( + lnd.ChainParams, loopOutSwap, paymentFees, + ) + if err != nil { + return err + } + + _, ok := updatedCosts[loopOutSwap.Hash] + if ok { + return fmt.Errorf("found a duplicate swap %v while "+ + "updating costs", loopOutSwap.Hash) + } + + updatedCosts[loopOutSwap.Hash] = cost + } + + log.Infof("Updating costs for %d loop out swaps", len(updatedCosts)) + err = db.BatchUpdateLoopOutSwapCosts(ctx, updatedCosts) + if err != nil { + return err + } + + // Finally mark the migration as done. + return db.SetMigration(ctx, costMigrationID) +} diff --git a/cost_migration_test.go b/cost_migration_test.go new file mode 100644 index 00000000..ee5525da --- /dev/null +++ b/cost_migration_test.go @@ -0,0 +1,184 @@ +package loop + +import ( + "context" + "testing" + "time" + + "github.com/btcsuite/btcd/btcutil" + "github.com/lightninglabs/lndclient" + "github.com/lightninglabs/loop/loopdb" + "github.com/lightninglabs/loop/test" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwire" + "github.com/stretchr/testify/require" +) + +// TestCalculateLoopOutCost tests the CalculateLoopOutCost function. +func TestCalculateLoopOutCost(t *testing.T) { + // Set up test context objects. + lnd := test.NewMockLnd() + server := newServerMock(lnd) + store := loopdb.NewStoreMock(t) + + cfg := &swapConfig{ + lnd: &lnd.LndServices, + store: store, + server: server, + } + + height := int32(600) + req := *testRequest + initResult, err := newLoopOutSwap( + context.Background(), cfg, height, &req, + ) + require.NoError(t, err) + swap, err := store.FetchLoopOutSwap( + context.Background(), initResult.swap.hash, + ) + require.NoError(t, err) + + // Override the chain cost so it's negative. + const expectedChainCost = btcutil.Amount(1000) + + // Now we have the swap and prepay invoices so let's calculate the + // costs without providing the payments first, so we don't account for + // any routing fees. + paymentFees := make(map[lntypes.Hash]lnwire.MilliSatoshi) + _, err = CalculateLoopOutCost(lnd.ChainParams, swap, paymentFees) + + // We expect that the call fails as the swap isn't finished yet. + require.Error(t, err) + + // Override the swap state to make it look like the swap is finished + // and make the chain cost negative too, so we can test that it'll be + // corrected to be positive in the cost calculation. + swap.Events = append( + swap.Events, &loopdb.LoopEvent{ + SwapStateData: loopdb.SwapStateData{ + State: loopdb.StateSuccess, + Cost: loopdb.SwapCost{ + Onchain: -expectedChainCost, + }, + }, + }, + ) + costs, err := CalculateLoopOutCost(lnd.ChainParams, swap, paymentFees) + require.NoError(t, err) + + expectedServerCost := server.swapInvoiceAmt + server.prepayInvoiceAmt - + swap.Contract.AmountRequested + require.Equal(t, expectedServerCost, costs.Server) + require.Equal(t, btcutil.Amount(0), costs.Offchain) + require.Equal(t, expectedChainCost, costs.Onchain) + + // Now add the two payments to the payments map and calculate the costs + // again. We expect that the routng fees are now accounted for. + paymentFees[server.swapHash] = lnwire.NewMSatFromSatoshis(44) + paymentFees[server.prepayHash] = lnwire.NewMSatFromSatoshis(11) + + costs, err = CalculateLoopOutCost(lnd.ChainParams, swap, paymentFees) + require.NoError(t, err) + + expectedOffchainCost := btcutil.Amount(44 + 11) + require.Equal(t, expectedServerCost, costs.Server) + require.Equal(t, expectedOffchainCost, costs.Offchain) + require.Equal(t, expectedChainCost, costs.Onchain) + + // Now override the last update to make the swap timed out at the HTLC + // sweep. We expect that the chain cost won't change, and only the + // prepay will be accounted for. + swap.Events[0] = &loopdb.LoopEvent{ + SwapStateData: loopdb.SwapStateData{ + State: loopdb.StateFailSweepTimeout, + Cost: loopdb.SwapCost{ + Onchain: 0, + }, + }, + } + + costs, err = CalculateLoopOutCost(lnd.ChainParams, swap, paymentFees) + require.NoError(t, err) + + expectedServerCost = server.prepayInvoiceAmt + expectedOffchainCost = btcutil.Amount(11) + require.Equal(t, expectedServerCost, costs.Server) + require.Equal(t, expectedOffchainCost, costs.Offchain) + require.Equal(t, btcutil.Amount(0), costs.Onchain) +} + +// TestCostMigration tests the cost migration for loop out swaps. +func TestCostMigration(t *testing.T) { + // Set up test context objects. + lnd := test.NewMockLnd() + server := newServerMock(lnd) + store := loopdb.NewStoreMock(t) + + cfg := &swapConfig{ + lnd: &lnd.LndServices, + store: store, + server: server, + } + + height := int32(600) + req := *testRequest + initResult, err := newLoopOutSwap( + context.Background(), cfg, height, &req, + ) + require.NoError(t, err) + + // Override the chain cost so it's negative. + const expectedChainCost = btcutil.Amount(1000) + + // Override the swap state to make it look like the swap is finished + // and make the chain cost negative too, so we can test that it'll be + // corrected to be positive in the cost calculation. + err = store.UpdateLoopOut( + context.Background(), initResult.swap.hash, time.Now(), + loopdb.SwapStateData{ + State: loopdb.StateSuccess, + Cost: loopdb.SwapCost{ + Onchain: -expectedChainCost, + }, + }, + ) + require.NoError(t, err) + + // Add the two mocked payment to LND. Note that we only care about the + // fees here, so we don't need to provide the full payment details. + lnd.Payments = []lndclient.Payment{ + { + Hash: server.swapHash, + Fee: lnwire.NewMSatFromSatoshis(44), + }, + { + Hash: server.prepayHash, + Fee: lnwire.NewMSatFromSatoshis(11), + }, + } + + // Now we can run the migration. + err = MigrateLoopOutCosts(context.Background(), lnd.LndServices, store) + require.NoError(t, err) + + // Finally check that the swap cost has been updated correctly. + swap, err := store.FetchLoopOutSwap( + context.Background(), initResult.swap.hash, + ) + require.NoError(t, err) + + expectedServerCost := server.swapInvoiceAmt + server.prepayInvoiceAmt - + swap.Contract.AmountRequested + + costs := swap.Events[0].Cost + expectedOffchainCost := btcutil.Amount(44 + 11) + require.Equal(t, expectedServerCost, costs.Server) + require.Equal(t, expectedOffchainCost, costs.Offchain) + require.Equal(t, expectedChainCost, costs.Onchain) + + // Now run the migration again to make sure it doesn't fail. This also + // indicates that the migration did not run the second time as + // otherwise the store mocks SetMigration function would fail. + err = MigrateLoopOutCosts(context.Background(), lnd.LndServices, store) + require.NoError(t, err) +} diff --git a/loopdb/store_mock.go b/loopdb/store_mock.go index 268bba6b..955ae5c4 100644 --- a/loopdb/store_mock.go +++ b/loopdb/store_mock.go @@ -24,6 +24,8 @@ type StoreMock struct { loopInStoreChan chan LoopInContract loopInUpdateChan chan SwapStateData + migrations map[string]struct{} + t *testing.T } @@ -39,6 +41,7 @@ func NewStoreMock(t *testing.T) *StoreMock { loopInUpdateChan: make(chan SwapStateData, 1), LoopInSwaps: make(map[lntypes.Hash]*LoopInContract), LoopInUpdates: make(map[lntypes.Hash][]SwapStateData), + migrations: make(map[string]struct{}), t: t, } } @@ -364,12 +367,20 @@ func (s *StoreMock) BatchUpdateLoopOutSwapCosts(ctx context.Context, func (s *StoreMock) HasMigration(ctx context.Context, migrationID string) ( bool, error) { - return false, errUnimplemented + _, ok := s.migrations[migrationID] + + return ok, nil } // SetMigration marks the migration with the given ID as done. func (s *StoreMock) SetMigration(ctx context.Context, migrationID string) error { - return errUnimplemented + if _, ok := s.migrations[migrationID]; ok { + return errors.New("migration already done") + } + + s.migrations[migrationID] = struct{}{} + + return nil } diff --git a/server_mock_test.go b/server_mock_test.go index 9f52a2c0..f46a4977 100644 --- a/server_mock_test.go +++ b/server_mock_test.go @@ -42,6 +42,7 @@ type serverMock struct { swapInvoice string swapHash lntypes.Hash + prepayHash lntypes.Hash // preimagePush is a channel that preimage pushes are sent into. preimagePush chan lntypes.Preimage @@ -81,13 +82,17 @@ func (s *serverMock) NewLoopOutSwap(_ context.Context, swapHash lntypes.Hash, return nil, errors.New("unexpected test swap amount") } + s.swapHash = swapHash swapPayReqString, err := getInvoice(swapHash, s.swapInvoiceAmt, swapInvoiceDesc) if err != nil { return nil, err } - prePayReqString, err := getInvoice(swapHash, s.prepayInvoiceAmt, + // Set the prepay hash to be different from the swap hash. + s.prepayHash = swapHash + s.prepayHash[0] ^= 1 + prePayReqString, err := getInvoice(s.prepayHash, s.prepayInvoiceAmt, prepayInvoiceDesc) if err != nil { return nil, err From c7fa1e41094b8b4995cb18f3ee4697270b0e3df8 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Fri, 31 May 2024 08:55:26 +0200 Subject: [PATCH 5/5] loopd: run the cost migration on start --- loopd/daemon.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/loopd/daemon.go b/loopd/daemon.go index c6496be3..bbc456df 100644 --- a/loopd/daemon.go +++ b/loopd/daemon.go @@ -409,6 +409,12 @@ func (d *Daemon) initialize(withMacaroonService bool) error { return err } + // Run the costs migration. + err = loop.MigrateLoopOutCosts(d.mainCtx, d.lnd.LndServices, swapDb) + if err != nil { + return err + } + sweeperDb := sweepbatcher.NewSQLStore(baseDb, chainParams) // Create an instance of the loop client library.