channeldb: refactor payments code

Go-fmt files. Refactored code according to the guidelines.
Enhanced payment test: add error checking
and individual context for each API call.
Add Timestamp field to payment struct.

channeldb/invoices.go

@@ -116,8 +116,7 @@ func validateInvoice(i *Invoice) error {
 // insertion will be aborted and rejected due to the strict policy banning any
 // duplicate payment hashes.
 func (d *DB) AddInvoice(i *Invoice) error {
-	err := validateInvoice(i)
-	if err != nil {
+	if err := validateInvoice(i); err != nil {
 		return err
 	}
 	return d.Update(func(tx *bolt.Tx) error {

channeldb/payments.go

@@ -1,51 +1,55 @@
 package channeldb
 
 import (
+	"bytes"
+	"encoding/binary"
+	"github.com/boltdb/bolt"
+	"github.com/roasbeef/btcd/wire"
 	"github.com/roasbeef/btcutil"
 	"io"
-	"github.com/roasbeef/btcd/wire"
-	"github.com/boltdb/bolt"
-	"encoding/binary"
-	"bytes"
+	"time"
 )
 
 var (
-	// invoiceBucket is the name of the bucket within the database that
-	// stores all data related to payments.
-	// Within the payments bucket, each invoice is keyed by its invoice ID
+	// invoiceBucket is the name of the bucket within
+	// the database that stores all data related to payments.
+	// Within the payments bucket, each invoice is keyed
+	// by its invoice ID
 	// which is a monotonically increasing uint64.
-	// BoltDB sequence feature is used for generating monotonically increasing
-	// id.
+	// BoltDB sequence feature is used for generating
+	// monotonically increasing id.
 	paymentBucket = []byte("payments")
 )
 
+// OutgoingPayment represents payment from given node.
 type OutgoingPayment struct {
 	Invoice
-	// Total fee paid
-	Fee btcutil.Amount
-	// Path including starting and ending nodes
-	Path [][]byte
-	TimeLockLength uint64
-	// We probably need both RHash and Preimage
-	// because we start knowing only RHash
-	RHash [32]byte
-}
 
-func validatePayment(p *OutgoingPayment) error {
-	err := validateInvoice(&p.Invoice)
-	if err != nil {
-		return err
-	}
-	return nil
+	// Total fee paid.
+	Fee btcutil.Amount
+
+	// Path including starting and ending nodes.
+	Path [][33]byte
+
+	// Timelock length.
+	TimeLockLength uint32
+
+	// RHash value used for payment.
+	// We need RHash because we start payment knowing only RHash
+	RHash [32]byte
+
+	// Timestamp is time when payment was created.
+	Timestamp time.Time
 }
 
 // AddPayment adds payment to DB.
 // There is no checking that payment with the same hash already exist.
 func (db *DB) AddPayment(p *OutgoingPayment) error {
-	err := validatePayment(p)
+	err := validateInvoice(&p.Invoice)
 	if err != nil {
 		return err
 	}
+
 	// We serialize before writing to database
 	// so no db access in the case of serialization errors
 	b := new(bytes.Buffer)
@@ -54,15 +58,17 @@ func (db *DB) AddPayment(p *OutgoingPayment) error {
 		return err
 	}
 	paymentBytes := b.Bytes()
-	return db.Update(func (tx *bolt.Tx) error {
+	return db.Update(func(tx *bolt.Tx) error {
 		payments, err := tx.CreateBucketIfNotExists(paymentBucket)
 		if err != nil {
 			return err
 		}
+
 		paymentId, err := payments.NextSequence()
 		if err != nil {
 			return err
 		}
+
 		// We use BigEndian for keys because
 		// it orders keys in ascending order
 		paymentIdBytes := make([]byte, 8)
@@ -78,12 +84,12 @@ func (db *DB) AddPayment(p *OutgoingPayment) error {
 // FetchAllPayments returns all outgoing payments in DB.
 func (db *DB) FetchAllPayments() ([]*OutgoingPayment, error) {
 	var payments []*OutgoingPayment
-	err := db.View(func (tx *bolt.Tx) error {
+	err := db.View(func(tx *bolt.Tx) error {
 		bucket := tx.Bucket(paymentBucket)
 		if bucket == nil {
-			return nil
+			return ErrNoPaymentsCreated
 		}
-		err := bucket.ForEach(func (k, v []byte) error {
+		err := bucket.ForEach(func(k, v []byte) error {
 			// Value can be nil if it is a sub-backet
 			// so simply ignore it.
 			if v == nil {
@@ -109,11 +115,12 @@ func (db *DB) FetchAllPayments() ([]*OutgoingPayment, error) {
 // If payments bucket does not exist it will create
 // new bucket without error.
 func (db *DB) DeleteAllPayments() error {
-	return db.Update(func (tx *bolt.Tx) error {
+	return db.Update(func(tx *bolt.Tx) error {
 		err := tx.DeleteBucket(paymentBucket)
 		if err != nil && err != bolt.ErrBucketNotFound {
 			return err
 		}
+
 		_, err = tx.CreateBucket(paymentBucket)
 		if err != nil {
 			return err
@@ -127,6 +134,7 @@ func serializeOutgoingPayment(w io.Writer, p *OutgoingPayment) error {
 	if err != nil {
 		return err
 	}
+
 	// Serialize fee.
 	feeBytes := make([]byte, 8)
 	byteOrder.PutUint64(feeBytes, uint64(p.Fee))
@@ -134,43 +142,60 @@ func serializeOutgoingPayment(w io.Writer, p *OutgoingPayment) error {
 	if err != nil {
 		return err
 	}
+
 	// Serialize path.
 	pathLen := uint32(len(p.Path))
 	pathLenBytes := make([]byte, 4)
+	// Write length of the path
 	byteOrder.PutUint32(pathLenBytes, pathLen)
 	_, err = w.Write(pathLenBytes)
 	if err != nil {
 		return err
 	}
+	// Serialize each element of the path
 	for i := uint32(0); i < pathLen; i++ {
-		err := wire.WriteVarBytes(w, 0, p.Path[i])
+		_, err := w.Write(p.Path[i][:])
 		if err != nil {
 			return err
 		}
 	}
+
 	// Serialize TimeLockLength
-	timeLockLengthBytes := make([]byte, 8)
-	byteOrder.PutUint64(timeLockLengthBytes, p.TimeLockLength)
+	timeLockLengthBytes := make([]byte, 4)
+	byteOrder.PutUint32(timeLockLengthBytes, p.TimeLockLength)
 	_, err = w.Write(timeLockLengthBytes)
 	if err != nil {
 		return err
 	}
+
 	// Serialize RHash
 	_, err = w.Write(p.RHash[:])
 	if err != nil {
 		return err
 	}
+
+	// Serialize Timestamp.
+	tBytes, err := p.Timestamp.MarshalBinary()
+	if err != nil {
+		return err
+	}
+	err = wire.WriteVarBytes(w, 0, tBytes)
+	if err != nil {
+		return err
+	}
 	return nil
 }
 
 func deserializeOutgoingPayment(r io.Reader) (*OutgoingPayment, error) {
 	p := &OutgoingPayment{}
+
 	// Deserialize invoice
 	inv, err := deserializeInvoice(r)
 	if err != nil {
 		return nil, err
 	}
 	p.Invoice = *inv
+
 	// Deserialize fee
 	feeBytes := make([]byte, 8)
 	_, err = r.Read(feeBytes)
@@ -178,6 +203,7 @@ func deserializeOutgoingPayment(r io.Reader) (*OutgoingPayment, error) {
 		return nil, err
 	}
 	p.Fee = btcutil.Amount(byteOrder.Uint64(feeBytes))
+
 	// Deserialize path
 	pathLenBytes := make([]byte, 4)
 	_, err = r.Read(pathLenBytes)
@@ -185,27 +211,38 @@ func deserializeOutgoingPayment(r io.Reader) (*OutgoingPayment, error) {
 		return nil, err
 	}
 	pathLen := byteOrder.Uint32(pathLenBytes)
-	path := make([][]byte, pathLen)
-	for i := uint32(0); i<pathLen; i++ {
-		// Each node in path have 33 bytes. It may be changed in future.
-		// So put 100 here.
-		path[i], err = wire.ReadVarBytes(r, 0, 100, "Node id")
+	path := make([][33]byte, pathLen)
+	for i := uint32(0); i < pathLen; i++ {
+		_, err := r.Read(path[i][:])
 		if err != nil {
 			return nil, err
 		}
 	}
 	p.Path = path
+
 	// Deserialize TimeLockLength
-	timeLockLengthBytes := make([]byte, 8)
+	timeLockLengthBytes := make([]byte, 4)
 	_, err = r.Read(timeLockLengthBytes)
 	if err != nil {
 		return nil, err
 	}
-	p.TimeLockLength = byteOrder.Uint64(timeLockLengthBytes)
+	p.TimeLockLength = byteOrder.Uint32(timeLockLengthBytes)
+
 	// Deserialize RHash
 	_, err = r.Read(p.RHash[:])
 	if err != nil {
 		return nil, err
 	}
+
+	// Deserialize Timestamp
+	tBytes, err := wire.ReadVarBytes(r, 0, 100,
+		"OutgoingPayment.Timestamp")
+	if err != nil {
+		return nil, err
+	}
+	err = p.Timestamp.UnmarshalBinary(tBytes)
+	if err != nil {
+		return nil, err
+	}
 	return p, nil
-}
+}

channeldb/payments_test.go

@@ -1,20 +1,20 @@
 package channeldb
 
 import (
-	"testing"
-	"time"
-	"github.com/roasbeef/btcutil"
 	"bytes"
-	"reflect"
-	"github.com/davecgh/go-spew/spew"
-	"math/rand"
 	"fmt"
 	"github.com/btcsuite/fastsha256"
+	"github.com/davecgh/go-spew/spew"
+	"github.com/roasbeef/btcutil"
+	"math/rand"
+	"reflect"
+	"testing"
+	"time"
 )
 
 func makeFakePayment() *OutgoingPayment {
-	// Create a fake invoice which we'll use several times in the tests
-	// below.
+	// Create a fake invoice which
+	// we'll use several times in the tests below.
 	fakeInvoice := &Invoice{
 		CreationDate: time.Now(),
 	}
@@ -23,63 +23,84 @@ func makeFakePayment() *OutgoingPayment {
 	copy(fakeInvoice.Terms.PaymentPreimage[:], rev[:])
 	fakeInvoice.Terms.Value = btcutil.Amount(10000)
 	// Make fake path
-	fakePath := make([][]byte, 3)
-	for i:=0; i<3; i++ {
-		fakePath[i] = make([]byte, 33)
-		for j:=0; j<33; j++ {
+	fakePath := make([][33]byte, 3)
+	for i := 0; i < 3; i++ {
+		for j := 0; j < 33; j++ {
 			fakePath[i][j] = byte(i)
 		}
 	}
 	var rHash [32]byte = fastsha256.Sum256(rev[:])
-	fakePayment := & OutgoingPayment{
-		Invoice: *fakeInvoice,
-		Fee: 101,
-		Path: fakePath,
+	fakePayment := &OutgoingPayment{
+		Invoice:        *fakeInvoice,
+		Fee:            101,
+		Path:           fakePath,
 		TimeLockLength: 1000,
-		RHash: rHash,
+		RHash:          rHash,
+		Timestamp:      time.Unix(100000, 0),
 	}
 	return fakePayment
 }
 
 // randomBytes creates random []byte with length
 // in range [minLen, maxLen)
-func randomBytes(minLen, maxLen int) []byte {
+func randomBytes(minLen, maxLen int) ([]byte, error) {
 	l := minLen + rand.Intn(maxLen-minLen)
 	b := make([]byte, l)
 	_, err := rand.Read(b)
 	if err != nil {
-		panic(fmt.Sprintf("Internal error. Cannot generate random string: %v", err))
+		return nil, fmt.Errorf("Internal error. "+
+			"Cannot generate random string: %v", err)
 	}
-	return b
+	return b, nil
 }
 
-func makeRandomFakePayment() *OutgoingPayment {
-	// Create a fake invoice which we'll use several times in the tests
-	// below.
+func makeRandomFakePayment() (*OutgoingPayment, error) {
+	var err error
 	fakeInvoice := &Invoice{
 		CreationDate: time.Now(),
 	}
-	fakeInvoice.Memo = randomBytes(1, 50)
-	fakeInvoice.Receipt = randomBytes(1, 50)
-	copy(fakeInvoice.Terms.PaymentPreimage[:], randomBytes(32, 33))
+
+	fakeInvoice.Memo, err = randomBytes(1, 50)
+	if err != nil {
+		return nil, err
+	}
+
+	fakeInvoice.Receipt, err = randomBytes(1, 50)
+	if err != nil {
+		return nil, err
+	}
+
+	preImg, err := randomBytes(32, 33)
+	if err != nil {
+		return nil, err
+	}
+	copy(fakeInvoice.Terms.PaymentPreimage[:], preImg)
+
 	fakeInvoice.Terms.Value = btcutil.Amount(rand.Intn(10000))
+
 	// Make fake path
 	fakePathLen := 1 + rand.Intn(5)
-	fakePath := make([][]byte, fakePathLen)
-	for i:=0; i<fakePathLen; i++ {
-		fakePath[i] = randomBytes(33, 34)
+	fakePath := make([][33]byte, fakePathLen)
+	for i := 0; i < fakePathLen; i++ {
+		b, err := randomBytes(33, 34)
+		if err != nil {
+			return nil, err
+		}
+		copy(fakePath[i][:], b)
 	}
+
 	var rHash [32]byte = fastsha256.Sum256(
 		fakeInvoice.Terms.PaymentPreimage[:],
 	)
-	fakePayment := & OutgoingPayment{
-		Invoice: *fakeInvoice,
-		Fee: btcutil.Amount(rand.Intn(1001)),
-		Path: fakePath,
-		TimeLockLength: uint64(rand.Intn(10000)),
-		RHash: rHash,
+	fakePayment := &OutgoingPayment{
+		Invoice:        *fakeInvoice,
+		Fee:            btcutil.Amount(rand.Intn(1001)),
+		Path:           fakePath,
+		TimeLockLength: uint32(rand.Intn(10000)),
+		RHash:          rHash,
+		Timestamp:      time.Unix(rand.Int63n(10000), 0),
 	}
-	return fakePayment
+	return fakePayment, nil
 }
 
 func TestOutgoingPaymentSerialization(t *testing.T) {
@@ -89,12 +110,15 @@ func TestOutgoingPaymentSerialization(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Can't serialize outgoing payment: %v", err)
 	}
+
 	newPayment, err := deserializeOutgoingPayment(b)
 	if err != nil {
 		t.Fatalf("Can't deserialize outgoing payment: %v", err)
 	}
+
 	if !reflect.DeepEqual(fakePayment, newPayment) {
-		t.Fatalf("Payments do not match after serialization/deserialization %v vs %v",
+		t.Fatalf("Payments do not match after "+
+			"serialization/deserialization %v vs %v",
 			spew.Sdump(fakePayment),
 			spew.Sdump(newPayment),
 		)
@@ -128,18 +152,23 @@ func TestOutgoingPaymentWorkflow(t *testing.T) {
 	}
 
 	// Make some random payments
-	for i:=0; i<5; i++ {
-		randomPayment := makeRandomFakePayment()
-		err := db.AddPayment(randomPayment)
+	for i := 0; i < 5; i++ {
+		randomPayment, err := makeRandomFakePayment()
+		if err != nil {
+			t.Fatalf("Internal error in tests: %v", err)
+		}
+		err = db.AddPayment(randomPayment)
 		if err != nil {
 			t.Fatalf("Can't put payment in DB: %v", err)
 		}
 		correctPayments = append(correctPayments, randomPayment)
 	}
+
 	payments, err = db.FetchAllPayments()
 	if err != nil {
 		t.Fatalf("Can't get payments from DB: %v", err)
 	}
+
 	if !reflect.DeepEqual(payments, correctPayments) {
 		t.Fatalf("Wrong payments after reading from DB."+
 			"Got %v, want %v",
@@ -153,11 +182,14 @@ func TestOutgoingPaymentWorkflow(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Can't delete payments from DB: %v", err)
 	}
+
+	// Check that there is no payments after deletion
 	paymentsAfterDeletion, err := db.FetchAllPayments()
 	if err != nil {
 		t.Fatalf("Can't get payments after deletion: %v", err)
 	}
 	if len(paymentsAfterDeletion) != 0 {
-		t.Fatalf("After deletion DB has %v payments, want %v", len(paymentsAfterDeletion), 0)
+		t.Fatalf("After deletion DB has %v payments, want %v",
+			len(paymentsAfterDeletion), 0)
 	}
 }