shachain: grammer fixes, gofmt conformance, slight interface changes

This commit contains a series of post-merge fixes which include:
  * ToBytes -> Encode
    * As the former version doesn’t _require_ allocation of new slices *
  * Store -> AddNextEntry
    * To communicate to the caller that entries MUST be added
      in order
  * Several grammar, spacing, spelling and gofmt fixes.

shachain/element.go

@@ -3,18 +3,20 @@ package shachain
 import (
 	"crypto/sha256"
 	"errors"
+
 	"github.com/roasbeef/btcd/chaincfg/chainhash"
 )
 
-// element represent the entity which contains the hash and
+// element represents the entity which contains the hash and index
-// corresponding to it index. By comparing two indexes we may change hash in
+// corresponding to it. An element is the output of the shachain PRF. By
-// such way to derive another element.
+// comparing two indexes we're able to mutate the hash in such way to derive
+// another element.
 type element struct {
 	index index
 	hash  chainhash.Hash
 }
 
-// newElementFromStr creates new element by the given hash string.
+// newElementFromStr creates new element from the given hash string.
 func newElementFromStr(s string, index index) (*element, error) {
 	hash, err := hashFromString(s)
 	if err != nil {
@@ -27,8 +29,8 @@ func newElementFromStr(s string, index index) (*element, error) {
 	}, nil
 }
 
-// derive used to get one shachain element from another by applying a series of
+// derive computes one shachain element from another by applying a series of
-// bit flipping and hashing operations based on an index.
+// bit flips and hasing operations based on the starting and ending index.
 func (e *element) derive(toIndex index) (*element, error) {
 	fromIndex := e.index
 
@@ -60,44 +62,44 @@ func (e *element) derive(toIndex index) (*element, error) {
 	}, nil
 }
 
-// isEqual checks elements equality.
+// isEqual returns true if two elements are identical and false otherwise.
 func (first *element) isEqual(second *element) bool {
 	return (first.index == second.index) &&
 		(&first.hash).IsEqual(&second.hash)
 }
 
 const (
-	// maxHeight is used to determine the the maximum allowable index and
+	// maxHeight is used to determine the maximum allowable index and the
-	// the length of array which should be stored in order to derive all
+	// length of the array required to order to derive all previous hashes
-	// previous hashes by index, this array also known as buckets.
+	// by index. The entries of this array as also knowns as buckets.
 	maxHeight uint8 = 48
 
 	// rootIndex is an index which corresponds to the root hash.
 	rootIndex index = 0
 )
 
-// startIndex is an index of first element.
+// startIndex is the index of first element in the shachain PRF.
 var startIndex index = (1 << maxHeight) - 1
 
-// index is an number which identifies the hash number and serve as the way to
+// index is a number which identifies the hash number and serves as a way to
-// determine which operation under hash we should made in order to derive one
+// determine the hashing operation required  to derive one hash from another.
-// hash from another. index initialized with start index value and than
+// index is initialized with the startIndex and decreases down to zero with
-// decreases down to zero.
+// successive derivations.
 type index uint64
 
-// newIndex is used to create index instance. The inner operations with
+// newIndex is used to create index instance. The inner operations with index
-// index implies that index decreasing from some max number to zero, but for
+// implies that index decreasing from some max number to zero, but for
 // simplicity and backward compatibility with previous logic it was transformed
 // to work in opposite way.
 func newIndex(v uint64) index {
 	return startIndex - index(v)
 }
 
-// deriveBitTransformations function checks that 'to' index is derivable from
+// deriveBitTransformations function checks that the 'to' index is derivable
-// 'from' index by checking the indexes prefixes and then returns the bit
+// from the 'from' index by checking the indexes are prefixes of another. The
-// positions where the zeroes should be changed to ones in order for the indexes
+// bit positions where the zeroes should be changed to ones in order for the
-// to become the same. This set of bits is needed in order to derive one hash
+// indexes to become the same are returned. This set of bits is needed in order
-// from another.
+// to derive one hash from another.
 //
 // NOTE: The index 'to' is derivable from index 'from' iff index 'from' lies
 // left and above index 'to' on graph below, for example:

shachain/element_test.go

@@ -1,9 +1,10 @@
 package shachain
 
 import (
-	"github.com/go-errors/errors"
 	"reflect"
 	"testing"
+
+	"github.com/go-errors/errors"
 )
 
 // bitsToIndex is a helper function which takes 'n' last bits as input and

shachain/producer.go

@@ -1,25 +1,34 @@
 package shachain
 
 import (
+	"io"
+
 	"github.com/roasbeef/btcd/chaincfg/chainhash"
 )
 
-// Producer is an interface which serves as an abstraction over data
+// Producer is an interface which serves as an abstraction over the data
-// structure responsible for efficient generating the secrets by given index.
+// structure responsible for efficiently generating the secrets for a
-// The generation of secrets should be made in such way that secret store
+// particular index based on a root seed.  The generation of secrets should be
-// might efficiently store and retrieve the secrets.
+// made in such way that secret store might efficiently store and retrieve the
+// secrets. This is typically implemented as a tree-based PRF.
 type Producer interface {
-	// AtIndex produce secret by given index.
+	// AtIndex produces a secret by evaluating using the initial seed and a
+	// particular index.
 	AtIndex(uint64) (*chainhash.Hash, error)
 
-	// ToBytes convert producer to the binary representation.
+	// Encode writes a binary serialization of the Producer implementation
-	ToBytes() ([]byte, error)
+	// to the passed io.Writer.
+	Encode(io.Writer) error
 }
 
-// RevocationProducer implementation of Producer. This version of shachain
+// RevocationProducer is an implementation of Producer interface using the
-// slightly changed in terms of method naming. Initial concept might be found
+// shachain PRF construct. Starting with a single 32-byte element generated
-// here:
+// from a CSPRNG, shachain is able to efficiently generate a nearly unbounded
+// number of secrets while maintaining a constant amount of storage. The
+// original description of shachain can be found here:
 // https://github.com/rustyrussell/ccan/blob/master/ccan/crypto/shachain/design.txt
+// with supplementary material here:
+// https://github.com/lightningnetwork/lightning-rfc/blob/master/03-transactions.md#per-commitment-secret-requirements
 type RevocationProducer struct {
 	// root is the element from which we may generate all hashes which
 	// corresponds to the index domain [281474976710655,0].
@@ -30,18 +39,18 @@ type RevocationProducer struct {
 // interface.
 var _ Producer = (*RevocationProducer)(nil)
 
-// NewRevocationProducer create new instance of shachain producer.
+// NewRevocationProducer creates new instance of shachain producer.
-func NewRevocationProducer(root *chainhash.Hash) *RevocationProducer {
+func NewRevocationProducer(root chainhash.Hash) *RevocationProducer {
 	return &RevocationProducer{
 		root: &element{
 			index: rootIndex,
-			hash:  *root,
+			hash:  root,
 		}}
 }
 
-// NewRevocationProducerFromBytes deserialize an instance of a RevocationProducer
+// NewRevocationProducerFromBytes deserializes an instance of a
-// encoded in the passed byte slice, returning a fully initialize instance of a
+// RevocationProducer encoded in the passed byte slice, returning a fully
-// RevocationProducer.
+// initialized instance of a RevocationProducer.
 func NewRevocationProducerFromBytes(data []byte) (*RevocationProducer, error) {
 	root, err := chainhash.NewHash(data)
 	if err != nil {
@@ -56,7 +65,9 @@ func NewRevocationProducerFromBytes(data []byte) (*RevocationProducer, error) {
 	}, nil
 }
 
-// AtIndex produce secret by given index.
+// AtIndex produces a secret by evaluating using the initial seed and a
+// particular index.
+//
 // NOTE: Part of the Producer interface.
 func (p *RevocationProducer) AtIndex(v uint64) (*chainhash.Hash, error) {
 	ind := newIndex(v)
@@ -69,8 +80,14 @@ func (p *RevocationProducer) AtIndex(v uint64) (*chainhash.Hash, error) {
 	return &element.hash, nil
 }
 
-// ToBytes convert producer to the binary representation.
+// Encode writes a binary serialization of the Producer implementation to the
+// passed io.Writer.
+//
 // NOTE: Part of the Producer interface.
-func (p *RevocationProducer) ToBytes() ([]byte, error) {
+func (p *RevocationProducer) Encode(w io.Writer) error {
-	return p.root.hash.CloneBytes(), nil
+	if _, err := w.Write(p.root.hash[:]); err != nil {
+		return err
+	}
+
+	return nil
 }

shachain/producer_test.go

@@ -1,7 +1,9 @@
 package shachain
 
 import (
+	"bytes"
 	"testing"
+
 	"github.com/roasbeef/btcd/chaincfg/chainhash"
 )
 
@@ -10,8 +12,7 @@ import (
 func TestShaChainProducerRestore(t *testing.T) {
 	var err error
 
-	hash := chainhash.DoubleHashH([]byte("shachaintest"))
+	seed := chainhash.DoubleHashH([]byte("shachaintest"))
-	seed := &hash
 	sender := NewRevocationProducer(seed)
 
 	s1, err := sender.AtIndex(0)
@@ -19,12 +20,12 @@ func TestShaChainProducerRestore(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	data, err := sender.ToBytes()
+	var b bytes.Buffer
-	if err != nil {
+	if err := sender.Encode(&b); err != nil {
 		t.Fatal(err)
 	}
 
-	sender, err = NewRevocationProducerFromBytes(data)
+	sender, err = NewRevocationProducerFromBytes(b.Bytes())
 	if err != nil {
 		t.Fatal(err)
 	}

shachain/store.go

@@ -1,43 +1,51 @@
 package shachain
 
 import (
-	"bytes"
 	"encoding/binary"
+	"io"
+
 	"github.com/go-errors/errors"
 	"github.com/roasbeef/btcd/chaincfg/chainhash"
 )
 
 // Store is an interface which serves as an abstraction over data structure
-// responsible for efficient storing and restoring of hash secrets by given
+// responsible for efficiently storing and restoring of hash secrets by given
 // indexes.
 //
 // Description: The Lightning Network wants a chain of (say 1 million)
-// unguessable 256 bit values; we generate them and send them one at a time
+// unguessable 256 bit values; we generate them and send them one at a time to
-// to a remote node.  We don't want the remote node to have to store all the
+// a remote node.  We don't want the remote node to have to store all the
 // values, so it's better if they can derive them once they see them.
 type Store interface {
 	// LookUp function is used to restore/lookup/fetch the previous secret
 	// by its index.
 	LookUp(uint64) (*chainhash.Hash, error)
 
-	// Store is used to store the given sha hash in efficient manner.
+	// AddNextEntry attempts to store the given hash within its internal
-	Store(*chainhash.Hash) error
+	// storage in an efficient manner.
+	//
+	// NOTE: The hashes derived from the shachain MUST be inserted in the
+	// order they're produced by a shachain.Producer.
+	AddNextEntry(*chainhash.Hash) error
 
-	// ToBytes convert store to the binary representation.
+	// Encode writes a binary serialization of the shachain elements
-	ToBytes() ([]byte, error)
+	// currently saved by implementation of shachain.Store to the passed
+	// io.Writer.
+	Encode(io.Writer) error
 }
 
-// RevocationStore implementation of SecretStore. This version of shachain store
+// RevocationStore is a concrete implementation of the Store interface. The
-// slightly changed in terms of method naming. Initial concept might be found
+// revocation store is able to efficiently store N derived shahain elements in
-// here:
+// a space efficient manner with a space complexity of O(log N). The original
-// https://github.com/rustyrussell/ccan/blob/master/ccan/crypto/shachain/design.txt
+// description of the storage methodology can be found here:
+// https://github.com/lightningnetwork/lightning-rfc/blob/master/03-transactions.md#efficient-per-commitment-secret-storage
 type RevocationStore struct {
 	// lenBuckets stores the number of currently active buckets.
 	lenBuckets uint8
 
-	// buckets is an array of elements from which we may derive all previous
+	// buckets is an array of elements from which we may derive all
-	// elements, each bucket corresponds to the element index number of
+	// previous elements, each bucket corresponds to the element with the
-	// trailing zeros.
+	// particular number of trailing zeros.
 	buckets [maxHeight]element
 
 	// index is an available index which will be assigned to the new
@@ -59,36 +67,32 @@ func NewRevocationStore() *RevocationStore {
 
 // NewRevocationStoreFromBytes recreates the initial store state from the given
 // binary shachain store representation.
-func NewRevocationStoreFromBytes(data []byte) (*RevocationStore, error) {
+func NewRevocationStoreFromBytes(r io.Reader) (*RevocationStore, error) {
-	var err error
-
 	store := &RevocationStore{}
-	buf := bytes.NewBuffer(data)
 
-	err = binary.Read(buf, binary.BigEndian, &store.lenBuckets)
+	if err := binary.Read(r, binary.BigEndian, &store.lenBuckets); err != nil {
+		return nil, err
+	}
+
+	for i := uint8(0); i < store.lenBuckets; i++ {
+		var hashIndex index
+		err := binary.Read(r, binary.BigEndian, &hashIndex)
 		if err != nil {
 			return nil, err
 		}
 
-	i := uint8(0)
+		var nextHash chainhash.Hash
-	for ; i < store.lenBuckets; i++ {
+		if _, err := io.ReadFull(r, nextHash[:]); err != nil {
-		e := &element{}
-
-		err = binary.Read(buf, binary.BigEndian, &e.index)
-		if err != nil {
 			return nil, err
 		}
 
-		hash, err := chainhash.NewHash(buf.Next(chainhash.HashSize))
+		store.buckets[i] = element{
-		if err != nil {
+			index: hashIndex,
-			return nil, err
+			hash:  nextHash,
 		}
-		e.hash = *hash
-		store.buckets[i] = *e
 	}
 
-	err = binary.Read(buf, binary.BigEndian, &store.index)
+	if err := binary.Read(r, binary.BigEndian, &store.index); err != nil {
-	if err != nil {
 		return nil, err
 	}
 
@@ -116,12 +120,14 @@ func (store *RevocationStore) LookUp(v uint64) (*chainhash.Hash, error) {
 	return nil, errors.Errorf("unable to derive hash #%v", ind)
 }
 
-// Store is used to store the given sha hash in efficient manner. Given hash
+// AddNextEntry attempts to store the given hash within its internal storage in
-// should be computable with previous ones, and derived from the previous index
+// an efficient manner.
-// otherwise the function will return the error.
+//
+// NOTE: The hashes derived from the shachain MUST be inserted in the order
+// they're produced by a shachain.Producer.
 //
 // NOTE: This function is part of the Store interface.
-func (store *RevocationStore) Store(hash *chainhash.Hash) error {
+func (store *RevocationStore) AddNextEntry(hash *chainhash.Hash) error {
 	newElement := &element{
 		index: store.index,
 		hash:  *hash,
@@ -150,37 +156,29 @@ func (store *RevocationStore) Store(hash *chainhash.Hash) error {
 	return nil
 }
 
-// ToBytes convert store to the binary representation.
+// Encode writes a binary serialization of the shachain elements currently
+// saved by implementation of shachain.Store to the passed io.Writer.
+//
 // NOTE: This function is part of the Store interface.
-func (store *RevocationStore) ToBytes() ([]byte, error) {
+func (store *RevocationStore) Encode(w io.Writer) error {
-	var buf bytes.Buffer
+	err := binary.Write(w, binary.BigEndian, store.lenBuckets)
-	var err error
-
-	err = binary.Write(&buf, binary.BigEndian, store.lenBuckets)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	i := uint8(0)
+	for i := uint8(0); i < store.lenBuckets; i++ {
-	for ; i < store.lenBuckets; i++ {
 		element := store.buckets[i]
 
-		err = binary.Write(&buf, binary.BigEndian, element.index)
+		err := binary.Write(w, binary.BigEndian, element.index)
 		if err != nil {
-			return nil, err
+			return err
 		}
 
-		_, err = buf.Write(element.hash.CloneBytes())
+		if _, err = w.Write(element.hash[:]); err != nil {
-		if err != nil {
+			return err
-			return nil, err
 		}
 
 	}
 
-	err = binary.Write(&buf, binary.BigEndian, store.index)
+	return binary.Write(w, binary.BigEndian, store.index)
-	if err != nil {
-		return nil, err
-	}
-
-	return buf.Bytes(), nil
 }

shachain/store_test.go

@@ -1,8 +1,10 @@
 package shachain
 
 import (
-	"github.com/roasbeef/btcd/chaincfg/chainhash"
+	"bytes"
 	"testing"
+
+	"github.com/roasbeef/btcd/chaincfg/chainhash"
 )
 
 type testInsert struct {
@@ -398,7 +400,7 @@ var tests = []struct {
 	},
 }
 
-// TestSpecificationShaChainInsert is used to check the consistence with
+// TestSpecificationShaChainInsert is used to check the consistency with
 // specification hash insert function.
 func TestSpecificationShaChainInsert(t *testing.T) {
 	for _, test := range tests {
@@ -410,7 +412,7 @@ func TestSpecificationShaChainInsert(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			if err := receiver.Store(secret); err != nil {
+			if err := receiver.AddNextEntry(secret); err != nil {
 				if insert.successful {
 					t.Fatalf("Failed (%v): error was "+
 						"received but it shouldn't: "+
@@ -428,11 +430,10 @@ func TestSpecificationShaChainInsert(t *testing.T) {
 	}
 }
 
-// TestShaChainStore checks the ability of shachain store to hold the produces
+// TestShaChainStore checks the ability of shachain store to hold the produced
 // secrets after recovering from bytes data.
 func TestShaChainStore(t *testing.T) {
-	hash := chainhash.DoubleHashH([]byte("shachaintest"))
+	seed := chainhash.DoubleHashH([]byte("shachaintest"))
-	seed := &hash
 
 	sender := NewRevocationProducer(seed)
 	receiver := NewRevocationStore()
@@ -443,23 +444,23 @@ func TestShaChainStore(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		if err = receiver.Store(sha); err != nil {
+		if err = receiver.AddNextEntry(sha); err != nil {
 			t.Fatal(err)
 		}
 	}
 
-	data, err := receiver.ToBytes()
+	var b bytes.Buffer
-	if err != nil {
+	if err := receiver.Encode(&b); err != nil {
 		t.Fatal(err)
 	}
 
-	receiver, err = NewRevocationStoreFromBytes(data)
+	newReceiver, err := NewRevocationStoreFromBytes(&b)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	for n := uint64(0); n < 10000; n++ {
-		if _, err := receiver.LookUp(n); err != nil {
+		if _, err := newReceiver.LookUp(n); err != nil {
 			t.Fatal(err)
 		}
 	}

shachain/utils.go

@@ -2,19 +2,21 @@ package shachain
 
 import (
 	"encoding/hex"
+
 	"github.com/roasbeef/btcd/chaincfg/chainhash"
 )
 
-// changeBit function produce all necessary hash bits flips. You should be
+// changeBit is a functio that function that flips a bit of the hash at a
-// aware that the bit flipping in this function a bit strange, example:
+// particluar bit-index. You should be aware that the bit flipping in this
+// function a bit strange, example:
 // hash: [0b00000000, 0b00000000,  ... 0b00000000]
 //	    0		   1       ...      31
 //
 // byte: 0 0 0 0 0 0 0 0
 // 	 7 6 5 4 3 2 1 0
 //
-// By flipping the bit at 7 position you will flip the first bit in hash and
+// By flipping the bit at 7 position you will flip the first bit in hash and by
-// by flipping the bit at 8 position you will flip the 16 bit in hash.
+// flipping the bit at 8 position you will flip the 16 bit in hash.
 func changeBit(hash []byte, position uint8) []byte {
 	byteNumber := position / 8
 	bitNumber := position % 8
@@ -62,11 +64,11 @@ func countTrailingZeros(index index) uint8 {
 	return zeros
 }
 
-// hashFromString takes the string as input an create the wire shahash, the
+// hashFromString takes a hex-encoded string as input and creates an instane of
-// initial wire.ShaHash NewShaHashFromStr function not suitable because it
+// chainhash.Hash. The chainhash.NewHashFromStr function not suitable because
-// reverse the given hash.
+// it reverse the given hash.
 func hashFromString(s string) (*chainhash.Hash, error) {
-	// Return error if hash string is too long.
+	// Return an error if hash string is too long.
 	if len(s) > chainhash.MaxHashStringSize {
 		return nil, chainhash.ErrHashStrSize
 	}