From 38624e861279fe3f5e278b2485398cdc4bacdce7 Mon Sep 17 00:00:00 2001 From: carla Date: Wed, 10 Jun 2020 12:34:28 +0200 Subject: [PATCH] channeldb: add fetchPaymentWithSequenceNumber lookup and test With our new index of sequence number to index, it is possible for more than one sequence number to point to the same hash because legacy lnd allowed duplicate payments under the same hash. We now store these payments in a nested bucket within the payments database. To allow lookup of the correct payment from an index, we require matching of the payment hash and sequence number. --- channeldb/payments.go | 97 +++++++++++++++++++++++++++++ channeldb/payments_test.go | 122 +++++++++++++++++++++++++++++++++++++ 2 files changed, 219 insertions(+) diff --git a/channeldb/payments.go b/channeldb/payments.go index 3229c093..73624902 100644 --- a/channeldb/payments.go +++ b/channeldb/payments.go @@ -104,6 +104,26 @@ var ( paymentsIndexBucket = []byte("payments-index-bucket") ) +var ( + // ErrNoSequenceNumber is returned if we lookup a payment which does + // not have a sequence number. + ErrNoSequenceNumber = errors.New("sequence number not found") + + // ErrDuplicateNotFound is returned when we lookup a payment by its + // index and cannot find a payment with a matching sequence number. + ErrDuplicateNotFound = errors.New("duplicate payment not found") + + // ErrNoDuplicateBucket is returned when we expect to find duplicates + // when looking up a payment from its index, but the payment does not + // have any. + ErrNoDuplicateBucket = errors.New("expected duplicate bucket") + + // ErrNoDuplicateNestedBucket is returned if we do not find duplicate + // payments in their own sub-bucket. + ErrNoDuplicateNestedBucket = errors.New("nested duplicate bucket not " + + "found") +) + // FailureReason encodes the reason a payment ultimately failed. type FailureReason byte @@ -568,6 +588,83 @@ func (db *DB) QueryPayments(query PaymentsQuery) (PaymentsResponse, error) { return resp, err } +// fetchPaymentWithSequenceNumber get the payment which matches the payment hash +// *and* sequence number provided from the database. This is required because +// we previously had more than one payment per hash, so we have multiple indexes +// pointing to a single payment; we want to retrieve the correct one. +func fetchPaymentWithSequenceNumber(tx kvdb.RTx, paymentHash lntypes.Hash, + sequenceNumber []byte) (*MPPayment, error) { + + // We can now lookup the payment keyed by its hash in + // the payments root bucket. + bucket, err := fetchPaymentBucket(tx, paymentHash) + if err != nil { + return nil, err + } + + // A single payment hash can have multiple payments associated with it. + // We lookup our sequence number first, to determine whether this is + // the payment we are actually looking for. + seqBytes := bucket.Get(paymentSequenceKey) + if seqBytes == nil { + return nil, ErrNoSequenceNumber + } + + // If this top level payment has the sequence number we are looking for, + // return it. + if bytes.Equal(seqBytes, sequenceNumber) { + return fetchPayment(bucket) + } + + // If we were not looking for the top level payment, we are looking for + // one of our duplicate payments. We need to iterate through the seq + // numbers in this bucket to find the correct payments. If we do not + // find a duplicate payments bucket here, something is wrong. + dup := bucket.NestedReadBucket(duplicatePaymentsBucket) + if dup == nil { + return nil, ErrNoDuplicateBucket + } + + var duplicatePayment *MPPayment + err = dup.ForEach(func(k, v []byte) error { + subBucket := dup.NestedReadBucket(k) + if subBucket == nil { + // We one bucket for each duplicate to be found. + return ErrNoDuplicateNestedBucket + } + + seqBytes := subBucket.Get(duplicatePaymentSequenceKey) + if seqBytes == nil { + return err + } + + // If this duplicate payment is not the sequence number we are + // looking for, we can continue. + if !bytes.Equal(seqBytes, sequenceNumber) { + return nil + } + + duplicatePayment, err = fetchDuplicatePayment(subBucket) + if err != nil { + return err + } + + return nil + }) + if err != nil { + return nil, err + } + + // If none of the duplicate payments matched our sequence number, we + // failed to find the payment with this sequence number; something is + // wrong. + if duplicatePayment == nil { + return nil, ErrDuplicateNotFound + } + + return duplicatePayment, nil +} + // DeletePayments deletes all completed and failed payments from the DB. func (db *DB) DeletePayments() error { return kvdb.Update(db, func(tx kvdb.RwTx) error { diff --git a/channeldb/payments_test.go b/channeldb/payments_test.go index 706060a2..532003bb 100644 --- a/channeldb/payments_test.go +++ b/channeldb/payments_test.go @@ -453,6 +453,128 @@ func TestQueryPayments(t *testing.T) { } } +// TestFetchPaymentWithSequenceNumber tests lookup of payments with their +// sequence number. It sets up one payment with no duplicates, and another with +// two duplicates in its duplicates bucket then uses these payments to test the +// case where a specific duplicate is not found and the duplicates bucket is not +// present when we expect it to be. +func TestFetchPaymentWithSequenceNumber(t *testing.T) { + db, cleanup, err := makeTestDB() + require.NoError(t, err) + + defer cleanup() + + pControl := NewPaymentControl(db) + + // Generate a test payment which does not have duplicates. + noDuplicates, _, _, err := genInfo() + require.NoError(t, err) + + // Create a new payment entry in the database. + err = pControl.InitPayment(noDuplicates.PaymentHash, noDuplicates) + require.NoError(t, err) + + // Fetch the payment so we can get its sequence nr. + noDuplicatesPayment, err := pControl.FetchPayment( + noDuplicates.PaymentHash, + ) + require.NoError(t, err) + + // Generate a test payment which we will add duplicates to. + hasDuplicates, _, _, err := genInfo() + require.NoError(t, err) + + // Create a new payment entry in the database. + err = pControl.InitPayment(hasDuplicates.PaymentHash, hasDuplicates) + require.NoError(t, err) + + // Fetch the payment so we can get its sequence nr. + hasDuplicatesPayment, err := pControl.FetchPayment( + hasDuplicates.PaymentHash, + ) + require.NoError(t, err) + + // We declare the sequence numbers used here so that we can reference + // them in tests. + var ( + duplicateOneSeqNr = hasDuplicatesPayment.SequenceNum + 1 + duplicateTwoSeqNr = hasDuplicatesPayment.SequenceNum + 2 + ) + + // Add two duplicates to our second payment. + appendDuplicatePayment( + t, db, hasDuplicates.PaymentHash, duplicateOneSeqNr, + ) + appendDuplicatePayment( + t, db, hasDuplicates.PaymentHash, duplicateTwoSeqNr, + ) + + tests := []struct { + name string + paymentHash lntypes.Hash + sequenceNumber uint64 + expectedErr error + }{ + { + name: "lookup payment without duplicates", + paymentHash: noDuplicates.PaymentHash, + sequenceNumber: noDuplicatesPayment.SequenceNum, + expectedErr: nil, + }, + { + name: "lookup payment with duplicates", + paymentHash: hasDuplicates.PaymentHash, + sequenceNumber: hasDuplicatesPayment.SequenceNum, + expectedErr: nil, + }, + { + name: "lookup first duplicate", + paymentHash: hasDuplicates.PaymentHash, + sequenceNumber: duplicateOneSeqNr, + expectedErr: nil, + }, + { + name: "lookup second duplicate", + paymentHash: hasDuplicates.PaymentHash, + sequenceNumber: duplicateTwoSeqNr, + expectedErr: nil, + }, + { + name: "lookup non-existent duplicate", + paymentHash: hasDuplicates.PaymentHash, + sequenceNumber: 999999, + expectedErr: ErrDuplicateNotFound, + }, + { + name: "lookup duplicate, no duplicates bucket", + paymentHash: noDuplicates.PaymentHash, + sequenceNumber: duplicateTwoSeqNr, + expectedErr: ErrNoDuplicateBucket, + }, + } + + for _, test := range tests { + test := test + + t.Run(test.name, func(t *testing.T) { + err := kvdb.Update(db, + func(tx walletdb.ReadWriteTx) error { + + var seqNrBytes [8]byte + byteOrder.PutUint64( + seqNrBytes[:], test.sequenceNumber, + ) + + _, err := fetchPaymentWithSequenceNumber( + tx, test.paymentHash, seqNrBytes[:], + ) + return err + }) + require.Equal(t, test.expectedErr, err) + }) + } +} + // appendDuplicatePayment adds a duplicate payment to an existing payment. Note // that this function requires a unique sequence number. //