diff --git a/htlcswitch/control_tower.go b/channeldb/control_tower.go similarity index 85% rename from htlcswitch/control_tower.go rename to channeldb/control_tower.go index 380fb787..9fe93736 100644 --- a/htlcswitch/control_tower.go +++ b/channeldb/control_tower.go @@ -1,10 +1,9 @@ -package htlcswitch +package channeldb import ( "errors" "github.com/coreos/bbolt" - "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/lnwire" ) @@ -59,7 +58,7 @@ type ControlTower interface { type paymentControl struct { strict bool - db *channeldb.DB + db *DB } // NewPaymentControl creates a new instance of the paymentControl. The strict @@ -70,7 +69,7 @@ type paymentControl struct { // contain such payments. In the meantime, non-strict mode enforces a superset // of the state transitions that prevent additional payments to a given payment // hash from being added. -func NewPaymentControl(strict bool, db *channeldb.DB) ControlTower { +func NewPaymentControl(strict bool, db *DB) ControlTower { return &paymentControl{ strict: strict, db: db, @@ -83,7 +82,7 @@ func (p *paymentControl) ClearForTakeoff(htlc *lnwire.UpdateAddHTLC) error { var takeoffErr error err := p.db.Batch(func(tx *bbolt.Tx) error { // Retrieve current status of payment from local database. - paymentStatus, err := channeldb.FetchPaymentStatusTx( + paymentStatus, err := FetchPaymentStatusTx( tx, htlc.PaymentHash, ) if err != nil { @@ -96,21 +95,21 @@ func (p *paymentControl) ClearForTakeoff(htlc *lnwire.UpdateAddHTLC) error { switch paymentStatus { - case channeldb.StatusGrounded: + case StatusGrounded: // It is safe to reattempt a payment if we know that we // haven't left one in flight. Since this one is // grounded, Transition the payment status to InFlight // to prevent others. - return channeldb.UpdatePaymentStatusTx( - tx, htlc.PaymentHash, channeldb.StatusInFlight, + return UpdatePaymentStatusTx( + tx, htlc.PaymentHash, StatusInFlight, ) - case channeldb.StatusInFlight: + case StatusInFlight: // We already have an InFlight payment on the network. We will // disallow any more payment until a response is received. takeoffErr = ErrPaymentInFlight - case channeldb.StatusCompleted: + case StatusCompleted: // We've already completed a payment to this payment hash, // forbid the switch from sending another. takeoffErr = ErrAlreadyPaid @@ -134,7 +133,7 @@ func (p *paymentControl) ClearForTakeoff(htlc *lnwire.UpdateAddHTLC) error { func (p *paymentControl) Success(paymentHash [32]byte) error { var updateErr error err := p.db.Batch(func(tx *bbolt.Tx) error { - paymentStatus, err := channeldb.FetchPaymentStatusTx( + paymentStatus, err := FetchPaymentStatusTx( tx, paymentHash, ) if err != nil { @@ -147,27 +146,27 @@ func (p *paymentControl) Success(paymentHash [32]byte) error { switch { - case paymentStatus == channeldb.StatusGrounded && p.strict: + case paymentStatus == StatusGrounded && p.strict: // Our records show the payment as still being grounded, // meaning it never should have left the switch. updateErr = ErrPaymentNotInitiated - case paymentStatus == channeldb.StatusGrounded && !p.strict: + case paymentStatus == StatusGrounded && !p.strict: // Though our records show the payment as still being // grounded, meaning it never should have left the // switch, we permit this transition in non-strict mode // to handle inconsistent db states. fallthrough - case paymentStatus == channeldb.StatusInFlight: + case paymentStatus == StatusInFlight: // A successful response was received for an InFlight // payment, mark it as completed to prevent sending to // this payment hash again. - return channeldb.UpdatePaymentStatusTx( - tx, paymentHash, channeldb.StatusCompleted, + return UpdatePaymentStatusTx( + tx, paymentHash, StatusCompleted, ) - case paymentStatus == channeldb.StatusCompleted: + case paymentStatus == StatusCompleted: // The payment was completed previously, alert the // caller that this may be a duplicate call. updateErr = ErrPaymentAlreadyCompleted @@ -191,7 +190,7 @@ func (p *paymentControl) Success(paymentHash [32]byte) error { func (p *paymentControl) Fail(paymentHash [32]byte) error { var updateErr error err := p.db.Batch(func(tx *bbolt.Tx) error { - paymentStatus, err := channeldb.FetchPaymentStatusTx( + paymentStatus, err := FetchPaymentStatusTx( tx, paymentHash, ) if err != nil { @@ -204,27 +203,27 @@ func (p *paymentControl) Fail(paymentHash [32]byte) error { switch { - case paymentStatus == channeldb.StatusGrounded && p.strict: + case paymentStatus == StatusGrounded && p.strict: // Our records show the payment as still being grounded, // meaning it never should have left the switch. updateErr = ErrPaymentNotInitiated - case paymentStatus == channeldb.StatusGrounded && !p.strict: + case paymentStatus == StatusGrounded && !p.strict: // Though our records show the payment as still being // grounded, meaning it never should have left the // switch, we permit this transition in non-strict mode // to handle inconsistent db states. fallthrough - case paymentStatus == channeldb.StatusInFlight: + case paymentStatus == StatusInFlight: // A failed response was received for an InFlight // payment, mark it as Grounded again to allow // subsequent attempts. - return channeldb.UpdatePaymentStatusTx( - tx, paymentHash, channeldb.StatusGrounded, + return UpdatePaymentStatusTx( + tx, paymentHash, StatusGrounded, ) - case paymentStatus == channeldb.StatusCompleted: + case paymentStatus == StatusCompleted: // The payment was completed previously, and we are now // reporting that it has failed. Leave the status as // completed, but alert the user that something is diff --git a/htlcswitch/control_tower_test.go b/channeldb/control_tower_test.go similarity index 86% rename from htlcswitch/control_tower_test.go rename to channeldb/control_tower_test.go index 2728e362..c4d97711 100644 --- a/htlcswitch/control_tower_test.go +++ b/channeldb/control_tower_test.go @@ -1,14 +1,38 @@ -package htlcswitch +package channeldb import ( + "crypto/rand" "fmt" + "io" + "io/ioutil" "testing" "github.com/btcsuite/fastsha256" - "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/lnwire" ) +func initDB() (*DB, error) { + tempPath, err := ioutil.TempDir("", "switchdb") + if err != nil { + return nil, err + } + + db, err := Open(tempPath) + if err != nil { + return nil, err + } + + return db, err +} + +func genPreimage() ([32]byte, error) { + var preimage [32]byte + if _, err := io.ReadFull(rand.Reader, preimage[:]); err != nil { + return preimage, err + } + return preimage, nil +} + func genHtlc() (*lnwire.UpdateAddHTLC, error) { preimage, err := genPreimage() if err != nil { @@ -99,7 +123,7 @@ func testPaymentControlSwitchFail(t *testing.T, strict bool) { t.Fatalf("unable to send htlc message: %v", err) } - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusInFlight) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusInFlight) // Fail the payment, which should moved it to Grounded. if err := pControl.Fail(htlc.PaymentHash); err != nil { @@ -107,7 +131,7 @@ func testPaymentControlSwitchFail(t *testing.T, strict bool) { } // Verify the status is indeed Grounded. - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusGrounded) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusGrounded) // Sends the htlc again, which should succeed since the prior payment // failed. @@ -115,14 +139,14 @@ func testPaymentControlSwitchFail(t *testing.T, strict bool) { t.Fatalf("unable to send htlc message: %v", err) } - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusInFlight) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusInFlight) // Verifies that status was changed to StatusCompleted. if err := pControl.Success(htlc.PaymentHash); err != nil { t.Fatalf("error shouldn't have been received, got: %v", err) } - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusCompleted) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusCompleted) // Attempt a final payment, which should now fail since the prior // payment succeed. @@ -154,7 +178,7 @@ func testPaymentControlSwitchDoubleSend(t *testing.T, strict bool) { t.Fatalf("unable to send htlc message: %v", err) } - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusInFlight) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusInFlight) // Try to initiate double sending of htlc message with the same // payment hash, should result in error indicating that payment has @@ -188,7 +212,7 @@ func testPaymentControlSwitchDoublePay(t *testing.T, strict bool) { } // Verify that payment is InFlight. - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusInFlight) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusInFlight) // Move payment to completed status, second payment should return error. if err := pControl.Success(htlc.PaymentHash); err != nil { @@ -196,7 +220,7 @@ func testPaymentControlSwitchDoublePay(t *testing.T, strict bool) { } // Verify that payment is Completed. - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusCompleted) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusCompleted) if err := pControl.ClearForTakeoff(htlc); err != ErrAlreadyPaid { t.Fatalf("payment control wrong behaviour:" + @@ -228,7 +252,7 @@ func TestPaymentControlNonStrictSuccessesWithoutInFlight(t *testing.T) { t.Fatalf("unable to mark payment hash success: %v", err) } - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusCompleted) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusCompleted) err = pControl.Success(htlc.PaymentHash) if err != ErrPaymentAlreadyCompleted { @@ -260,28 +284,28 @@ func TestPaymentControlNonStrictFailsWithoutInFlight(t *testing.T) { t.Fatalf("unable to mark payment hash failed: %v", err) } - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusGrounded) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusGrounded) err = pControl.Fail(htlc.PaymentHash) if err != nil { t.Fatalf("unable to remark payment hash failed: %v", err) } - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusGrounded) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusGrounded) err = pControl.Success(htlc.PaymentHash) if err != nil { t.Fatalf("unable to remark payment hash success: %v", err) } - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusCompleted) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusCompleted) err = pControl.Fail(htlc.PaymentHash) if err != ErrPaymentAlreadyCompleted { t.Fatalf("unable to remark payment hash failed: %v", err) } - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusCompleted) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusCompleted) } // TestPaymentControlStrictSuccessesWithoutInFlight checks that a strict payment @@ -306,7 +330,7 @@ func TestPaymentControlStrictSuccessesWithoutInFlight(t *testing.T) { t.Fatalf("expected ErrPaymentNotInitiated, got %v", err) } - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusGrounded) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusGrounded) } // TestPaymentControlStrictFailsWithoutInFlight checks that a strict payment @@ -331,11 +355,11 @@ func TestPaymentControlStrictFailsWithoutInFlight(t *testing.T) { t.Fatalf("expected ErrPaymentNotInitiated, got %v", err) } - assertPaymentStatus(t, db, htlc.PaymentHash, channeldb.StatusGrounded) + assertPaymentStatus(t, db, htlc.PaymentHash, StatusGrounded) } -func assertPaymentStatus(t *testing.T, db *channeldb.DB, - hash [32]byte, expStatus channeldb.PaymentStatus) { +func assertPaymentStatus(t *testing.T, db *DB, + hash [32]byte, expStatus PaymentStatus) { t.Helper() diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index fb3cfc56..2af3a36d 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -3889,8 +3889,7 @@ func TestChannelLinkAcceptDuplicatePayment(t *testing.T) { } // With the invoice now added to Carol's registry, we'll send the - // payment. It should succeed w/o any issues as it has been crafted - // properly. + // payment. err = n.aliceServer.htlcSwitch.SendHTLC( n.firstBobChannelLink.ShortChanID(), pid, htlc, ) @@ -3905,6 +3904,16 @@ func TestChannelLinkAcceptDuplicatePayment(t *testing.T) { t.Fatalf("unable to get payment result: %v", err) } + // Now, if we attempt to send the payment *again* it should be rejected + // as it's a duplicate request. + err = n.aliceServer.htlcSwitch.SendHTLC( + n.firstBobChannelLink.ShortChanID(), pid, htlc, + ) + if err != ErrPaymentIDAlreadyExists { + t.Fatalf("ErrPaymentIDAlreadyExists should have been "+ + "received got: %v", err) + } + select { case result, ok := <-resultChan: if !ok { @@ -3917,15 +3926,6 @@ func TestChannelLinkAcceptDuplicatePayment(t *testing.T) { case <-time.After(5 * time.Second): t.Fatalf("payment result did not arrive") } - - // Now, if we attempt to send the payment *again* it should be rejected - // as it's a duplicate request. - err = n.aliceServer.htlcSwitch.SendHTLC( - n.firstBobChannelLink.ShortChanID(), pid, htlc, - ) - if err != ErrAlreadyPaid { - t.Fatalf("ErrAlreadyPaid should have been received got: %v", err) - } } // TestChannelLinkAcceptOverpay tests that if we create an invoice for sender, diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index c850783e..45e2e2db 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -208,9 +208,6 @@ type Switch struct { pendingPayments map[uint64]*pendingPayment pendingMutex sync.RWMutex - // control provides verification of sending htlc mesages - control ControlTower - // circuits is storage for payment circuits which are used to // forward the settle/fail htlc updates back to the add htlc initiator. circuits CircuitMap @@ -290,7 +287,6 @@ func New(cfg Config, currentHeight uint32) (*Switch, error) { bestHeight: currentHeight, cfg: &cfg, circuits: circuitMap, - control: NewPaymentControl(false, cfg.DB), linkIndex: make(map[lnwire.ChannelID]ChannelLink), mailOrchestrator: newMailOrchestrator(), forwardingIndex: make(map[lnwire.ShortChannelID]ChannelLink), @@ -402,13 +398,6 @@ func (s *Switch) GetPaymentResult(paymentID uint64, func (s *Switch) SendHTLC(firstHop lnwire.ShortChannelID, paymentID uint64, htlc *lnwire.UpdateAddHTLC) error { - // Before sending, double check that we don't already have 1) an - // in-flight payment to this payment hash, or 2) a complete payment for - // the same hash. - if err := s.control.ClearForTakeoff(htlc); err != nil { - return err - } - // Create payment and add to the map of payment in order later to be // able to retrieve it and return response to the user. payment := &pendingPayment{ @@ -439,10 +428,6 @@ func (s *Switch) SendHTLC(firstHop lnwire.ShortChannelID, paymentID uint64, if err := s.forward(packet); err != nil { s.removePendingPayment(paymentID) - if err := s.control.Fail(htlc.PaymentHash); err != nil { - return err - } - return err } @@ -939,15 +924,6 @@ func (s *Switch) extractResult(deobfuscator ErrorDecrypter, n *networkResult, // We've received a settle update which means we can finalize the user // payment and return successful response. case *lnwire.UpdateFulfillHTLC: - // Persistently mark that a payment to this payment hash - // succeeded. This will prevent us from ever making another - // payment to this hash. - err := s.control.Success(paymentHash) - if err != nil && err != ErrPaymentAlreadyCompleted { - return nil, fmt.Errorf("Unable to mark completed "+ - "payment %x: %v", paymentHash, err) - } - return &PaymentResult{ Preimage: htlc.PaymentPreimage, }, nil @@ -955,14 +931,6 @@ func (s *Switch) extractResult(deobfuscator ErrorDecrypter, n *networkResult, // We've received a fail update which means we can finalize the // user payment and return fail response. case *lnwire.UpdateFailHTLC: - // Persistently mark that a payment to this payment hash - // failed. This will permit us to make another attempt at a - // successful payment. - err := s.control.Fail(paymentHash) - if err != nil && err != ErrPaymentAlreadyCompleted { - return nil, fmt.Errorf("Unable to ground payment "+ - "%x: %v", paymentHash, err) - } paymentErr := s.parseFailedPayment( deobfuscator, paymentID, paymentHash, n.unencrypted, n.isResolution, htlc,