diff --git a/loopdb/swapstate.go b/loopdb/swapstate.go index 87c21f3..c2cddb2 100644 --- a/loopdb/swapstate.go +++ b/loopdb/swapstate.go @@ -60,6 +60,10 @@ const ( // StateInvoiceSettled means that the swap invoice has been paid by the // server. StateInvoiceSettled SwapState = 9 + + // StateFailIncorrectHtlcAmt indicates that the amount of an externally + // published loop in htlc didn't match the swap amount. + StateFailIncorrectHtlcAmt SwapState = 10 ) // SwapStateType defines the types of swap states that exist. Every swap state @@ -127,6 +131,9 @@ func (s SwapState) String() string { case StateInvoiceSettled: return "InvoiceSettled" + case StateFailIncorrectHtlcAmt: + return "IncorrectHtlcAmt" + default: return "Unknown" } diff --git a/loopin.go b/loopin.go index ab82c47..4936501 100644 --- a/loopin.go +++ b/loopin.go @@ -9,17 +9,16 @@ import ( "github.com/btcsuite/btcutil" - "github.com/lightningnetwork/lnd/chainntnfs" - "github.com/lightningnetwork/lnd/channeldb" - "github.com/lightningnetwork/lnd/lnwire" - + "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/wire" - "github.com/lightningnetwork/lnd/lnrpc/invoicesrpc" - "github.com/lightninglabs/lndclient" "github.com/lightninglabs/loop/loopdb" "github.com/lightninglabs/loop/swap" + "github.com/lightningnetwork/lnd/chainntnfs" + "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/lnrpc/invoicesrpc" "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwire" ) var ( @@ -58,6 +57,9 @@ type loopInSwap struct { htlcNP2WSH *swap.Htlc + // htlcTxHash is the confirmed htlc tx id. + htlcTxHash *chainhash.Hash + timeoutAddr btcutil.Address } @@ -209,6 +211,7 @@ func resumeLoopInSwap(reqContext context.Context, cfg *swapConfig, } else { swap.state = lastUpdate.State swap.lastUpdateTime = lastUpdate.Time + swap.htlcTxHash = lastUpdate.HtlcTxHash } return swap, nil @@ -333,7 +336,7 @@ func (s *loopInSwap) executeSwap(globalCtx context.Context) error { // HtlcPublished state directly and wait for // confirmation. s.setState(loopdb.StateHtlcPublished) - err = s.persistState(globalCtx) + err = s.persistAndAnnounceState(globalCtx) if err != nil { return err } @@ -363,6 +366,13 @@ func (s *loopInSwap) executeSwap(globalCtx context.Context) error { return err } + // Verify that the confirmed (external) htlc value matches the swap + // amount. Otherwise fail the swap immediately. + if htlcValue != s.LoopInContract.AmountRequested { + s.setState(loopdb.StateFailIncorrectHtlcAmt) + return s.persistAndAnnounceState(globalCtx) + } + // TODO: Add miner fee of htlc tx to swap cost balance. // The server is expected to see the htlc on-chain and knowing that it @@ -376,7 +386,7 @@ func (s *loopInSwap) executeSwap(globalCtx context.Context) error { } // Persist swap outcome. - if err := s.persistState(globalCtx); err != nil { + if err := s.persistAndAnnounceState(globalCtx); err != nil { return err } @@ -387,39 +397,53 @@ func (s *loopInSwap) executeSwap(globalCtx context.Context) error { func (s *loopInSwap) waitForHtlcConf(globalCtx context.Context) ( *chainntnfs.TxConfirmation, error) { + // Register for confirmation of the htlc. It is essential to specify not + // just the pk script, because an attacker may publish the same htlc + // with a lower value and we don't want to follow through with that tx. + // In the unlikely event that our call to SendOutputs crashes and we + // restart, htlcTxHash will be nil at this point. Then only register + // with PkScript and accept the risk that the call triggers on a + // different htlc outpoint. + s.log.Infof("Register for htlc conf (hh=%v, txid=%v)", + s.InitiationHeight, s.htlcTxHash) + + if s.htlcTxHash == nil { + s.log.Warnf("No htlc tx hash available, registering with " + + "just the pkscript") + } + ctx, cancel := context.WithCancel(globalCtx) defer cancel() notifier := s.lnd.ChainNotifier confChanP2WSH, confErrP2WSH, err := notifier.RegisterConfirmationsNtfn( - ctx, nil, s.htlcP2WSH.PkScript, 1, s.InitiationHeight, + ctx, s.htlcTxHash, s.htlcP2WSH.PkScript, 1, s.InitiationHeight, ) if err != nil { return nil, err } confChanNP2WSH, confErrNP2WSH, err := notifier.RegisterConfirmationsNtfn( - ctx, nil, s.htlcNP2WSH.PkScript, 1, s.InitiationHeight, + ctx, s.htlcTxHash, s.htlcNP2WSH.PkScript, 1, s.InitiationHeight, ) if err != nil { return nil, err } - for { + var conf *chainntnfs.TxConfirmation + for conf == nil { select { // P2WSH htlc confirmed. - case conf := <-confChanP2WSH: + case conf = <-confChanP2WSH: s.htlc = s.htlcP2WSH s.log.Infof("P2WSH htlc confirmed") - return conf, nil // NP2WSH htlc confirmed. - case conf := <-confChanNP2WSH: + case conf = <-confChanNP2WSH: s.htlc = s.htlcNP2WSH s.log.Infof("NP2WSH htlc confirmed") - return conf, nil // Conf ntfn error. case err := <-confErrP2WSH: @@ -438,6 +462,19 @@ func (s *loopInSwap) waitForHtlcConf(globalCtx context.Context) ( return nil, globalCtx.Err() } } + + // Store htlc tx hash for accounting purposes. Usually this call is a + // no-op because the htlc tx hash was already known. Exceptions are: + // + // - Old pending swaps that were initiated before we persisted the htlc + // tx hash directly after publish. + // + // - Swaps that experienced a crash during their call to SendOutputs. In + // that case, we weren't able to record the tx hash. + txHash := conf.Tx.TxHash() + s.htlcTxHash = &txHash + + return conf, nil } // publishOnChainHtlc checks whether there are still enough blocks left and if @@ -451,7 +488,7 @@ func (s *loopInSwap) publishOnChainHtlc(ctx context.Context) (bool, error) { // Verify whether it still makes sense to publish the htlc. if blocksRemaining < MinLoopInPublishDelta { s.setState(loopdb.StateFailTimeout) - return false, s.persistState(ctx) + return false, s.persistAndAnnounceState(ctx) } // Get fee estimate from lnd. @@ -465,7 +502,7 @@ func (s *loopInSwap) publishOnChainHtlc(ctx context.Context) (bool, error) { // Transition to state HtlcPublished before calling SendOutputs to // prevent us from ever paying multiple times after a crash. s.setState(loopdb.StateHtlcPublished) - err = s.persistState(ctx) + err = s.persistAndAnnounceState(ctx) if err != nil { return false, err } @@ -483,7 +520,20 @@ func (s *loopInSwap) publishOnChainHtlc(ctx context.Context) (bool, error) { if err != nil { return false, fmt.Errorf("send outputs: %v", err) } - s.log.Infof("Published on chain HTLC tx %v", tx.TxHash()) + txHash := tx.TxHash() + s.log.Infof("Published on chain HTLC tx %v", txHash) + + // Persist the htlc hash so that after a restart we are still waiting + // for our own htlc. We don't need to announce to clients, because the + // state remains unchanged. + // + // TODO(joostjager): Store tx hash before calling SendOutputs. This is + // not yet possible with the current lnd api. + s.htlcTxHash = &txHash + s.lastUpdateTime = time.Now() + if err := s.persistState(); err != nil { + return false, fmt.Errorf("persist htlc tx: %v", err) + } return true, nil @@ -499,7 +549,7 @@ func (s *loopInSwap) waitForSwapComplete(ctx context.Context, rpcCtx, cancel := context.WithCancel(ctx) defer cancel() spendChan, spendErr, err := s.lnd.ChainNotifier.RegisterSpendNtfn( - rpcCtx, nil, s.htlc.PkScript, s.InitiationHeight, + rpcCtx, htlcOutpoint, s.htlc.PkScript, s.InitiationHeight, ) if err != nil { return fmt.Errorf("register spend ntfn: %v", err) @@ -589,7 +639,7 @@ func (s *loopInSwap) waitForSwapComplete(ctx context.Context, // accounting data. if s.state == loopdb.StateHtlcPublished { s.setState(loopdb.StateInvoiceSettled) - err := s.persistState(ctx) + err := s.persistAndAnnounceState(ctx) if err != nil { return err } @@ -689,17 +739,11 @@ func (s *loopInSwap) publishTimeoutTx(ctx context.Context, return nil } -// persistState updates the swap state and sends out an update notification. -func (s *loopInSwap) persistState(ctx context.Context) error { +// persistAndAnnounceState updates the swap state on disk and sends out an +// update notification. +func (s *loopInSwap) persistAndAnnounceState(ctx context.Context) error { // Update state in store. - err := s.store.UpdateLoopIn( - s.hash, s.lastUpdateTime, - loopdb.SwapStateData{ - State: s.state, - Cost: s.cost, - }, - ) - if err != nil { + if err := s.persistState(); err != nil { return err } @@ -707,6 +751,18 @@ func (s *loopInSwap) persistState(ctx context.Context) error { return s.sendUpdate(ctx) } +// persistState updates the swap state on disk. +func (s *loopInSwap) persistState() error { + return s.store.UpdateLoopIn( + s.hash, s.lastUpdateTime, + loopdb.SwapStateData{ + State: s.state, + Cost: s.cost, + HtlcTxHash: s.htlcTxHash, + }, + ) +} + // setState updates the swap state and last update timestamp. func (s *loopInSwap) setState(state loopdb.SwapState) { s.lastUpdateTime = time.Now() diff --git a/loopin_test.go b/loopin_test.go index 9765d76..ceda3e1 100644 --- a/loopin_test.go +++ b/loopin_test.go @@ -10,6 +10,7 @@ import ( "github.com/lightninglabs/loop/test" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" + "github.com/stretchr/testify/require" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" @@ -61,6 +62,10 @@ func TestLoopInSuccess(t *testing.T) { // Expect htlc to be published. htlcTx := <-ctx.lnd.SendOutputsChannel + // Expect the same state to be written again with the htlc tx hash. + state := ctx.store.assertLoopInState(loopdb.StateHtlcPublished) + require.NotNil(t, state.HtlcTxHash) + // Expect register for htlc conf. <-ctx.lnd.RegisterConfChannel <-ctx.lnd.RegisterConfChannel @@ -182,6 +187,10 @@ func testLoopInTimeout(t *testing.T, if externalValue == 0 { // Expect htlc to be published. htlcTx = <-ctx.lnd.SendOutputsChannel + + // Expect the same state to be written again with the htlc tx hash. + state := ctx.store.assertLoopInState(loopdb.StateHtlcPublished) + require.NotNil(t, state.HtlcTxHash) } else { // Create an external htlc publish tx. var pkScript []byte @@ -209,6 +218,20 @@ func testLoopInTimeout(t *testing.T, Tx: &htlcTx, } + // Assert that the swap is failed in case of an invalid amount. + invalidAmt := externalValue != 0 && externalValue != int64(req.Amount) + if invalidAmt { + ctx.assertState(loopdb.StateFailIncorrectHtlcAmt) + ctx.store.assertLoopInState(loopdb.StateFailIncorrectHtlcAmt) + + err = <-errChan + if err != nil { + t.Fatal(err) + } + + return + } + // Client starts listening for spend of htlc. <-ctx.lnd.RegisterSpendChannel @@ -375,11 +398,17 @@ func testLoopInResume(t *testing.T, state loopdb.SwapState, expired bool) { // Expect htlc to be published. htlcTx = <-ctx.lnd.SendOutputsChannel + + // Expect the same state to be written again with the htlc tx + // hash. + state := ctx.store.assertLoopInState(loopdb.StateHtlcPublished) + require.NotNil(t, state.HtlcTxHash) } else { ctx.assertState(loopdb.StateHtlcPublished) htlcTx.AddTxOut(&wire.TxOut{ PkScript: htlc.PkScript, + Value: int64(contract.AmountRequested), }) } diff --git a/store_mock_test.go b/store_mock_test.go index 9d932a8..77f36c3 100644 --- a/store_mock_test.go +++ b/store_mock_test.go @@ -215,13 +215,19 @@ func (s *storeMock) assertLoopInStored() { <-s.loopInStoreChan } -func (s *storeMock) assertLoopInState(expectedState loopdb.SwapState) { +// assertLoopInState asserts that a specified state transition is persisted to +// disk. +func (s *storeMock) assertLoopInState( + expectedState loopdb.SwapState) loopdb.SwapStateData { + s.t.Helper() state := <-s.loopInUpdateChan if state.State != expectedState { s.t.Fatalf("expected state %v, got %v", expectedState, state) } + + return state } func (s *storeMock) assertStorePreimageReveal() {