sharding: address review comments and add new subpackage

Former-commit-id: 9dfe0415a7809738b7683090d08486d64ca2e4c0 [formerly c8f386658475a6d6a069ab44f63a3d6e39a80ea7]
Former-commit-id: 767719430eda75648b408b4f480b25bb68870421

sharding/collation.go

@@ -49,7 +49,7 @@ func (h *CollationHeader) Hash() (hash common.Hash) {
 	return hash
 }
 
-// ShardID is the identifier for a shard.
+// ShardID the collation corresponds to.
 func (h *CollationHeader) ShardID() *big.Int { return h.data.ShardID }
 
 // Period the collation corresponds to.
@@ -69,8 +69,7 @@ func (h *CollationHeader) EncodeRLP() ([]byte, error) {
 
 // DecodeRLP uses an RLP Stream to populate the data field of a collation header.
 func (h *CollationHeader) DecodeRLP(s *rlp.Stream) error {
-	err := s.Decode(&h.data)
-	return err
+	return s.Decode(&h.data)
 }
 
 // Header returns the collation's header.
@@ -96,6 +95,8 @@ func (c *Collation) AddTransaction(tx *types.Transaction) {
 // CalculateChunkRoot updates the collation header's chunk root based on the body.
 func (c *Collation) CalculateChunkRoot() {
 	// TODO: this needs to be based on blob serialization.
+	// For proof of custody we need to split chunks (body) into chunk + salt and
+	// take the merkle root of that.
 	chunkRoot := common.BytesToHash(c.body)
 	c.header.data.ChunkRoot = &chunkRoot
 }

sharding/database/inmemory.go

@@ -0,0 +1,45 @@
+// Package database provides several constructs including a simple in-memory database.
+// This should not be used for production, but would be a helpful interim
+// solution for development.
+package database
+
+import (
+	"fmt"
+
+	"github.com/ethereum/go-ethereum/common"
+)
+
+// ShardKV is an in-memory mapping of hashes to RLP encoded values.
+type ShardKV struct {
+	kv map[common.Hash][]byte
+}
+
+// MakeShardKV initializes a keyval store in memory.
+func MakeShardKV() *ShardKV {
+	return &ShardKV{kv: make(map[common.Hash][]byte)}
+}
+
+// Get fetches a val from the mappping by key.
+func (sb *ShardKV) Get(k common.Hash) ([]byte, error) {
+	v, ok := sb.kv[k]
+	if !ok {
+		return nil, fmt.Errorf("key not found: %v", k)
+	}
+	return v, nil
+}
+
+// Has checks if the key exists in the mapping.
+func (sb *ShardKV) Has(k common.Hash) bool {
+	v := sb.kv[k]
+	return v != nil
+}
+
+// Put updates a key's value in the mapping.
+func (sb *ShardKV) Put(k common.Hash, v []byte) {
+	sb.kv[k] = v
+}
+
+// Delete removes the key and value from the mapping.
+func (sb *ShardKV) Delete(k common.Hash) {
+	delete(sb.kv, k)
+}

sharding/database/inmemory_test.go

@@ -0,0 +1,21 @@
+package database
+
+import (
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+)
+
+func Test_ShardKVGet(t *testing.T) {
+	kv := MakeShardKV()
+	hash := common.StringToHash("ralph merkle")
+	kv.Put(hash, []byte{1, 2, 3})
+
+	val, err := kv.Get(hash)
+	if err != nil {
+		t.Errorf("get failed: %v", err)
+	}
+	if val == nil {
+		t.Errorf("no value stored for key")
+	}
+}

sharding/db.go

@@ -1,37 +0,0 @@
-package sharding
-
-import (
-	"fmt"
-
-	"github.com/ethereum/go-ethereum/common"
-)
-
-type shardKV struct {
-	// Shard state storage is a mapping of hashes to RLP encoded values.
-	kv map[common.Hash][]byte
-}
-
-func makeShardKV() *shardKV {
-	return &shardKV{kv: make(map[common.Hash][]byte)}
-}
-
-func (sb *shardKV) Get(k common.Hash) ([]byte, error) {
-	v, ok := sb.kv[k]
-	if !ok {
-		return nil, fmt.Errorf("key not found: %v", k)
-	}
-	return v, nil
-}
-
-func (sb *shardKV) Has(k common.Hash) bool {
-	v := sb.kv[k]
-	return v != nil
-}
-
-func (sb *shardKV) Put(k common.Hash, v []byte) {
-	sb.kv[k] = v
-}
-
-func (sb *shardKV) Delete(k common.Hash) {
-	delete(sb.kv, k)
-}

sharding/shard.go

@@ -43,7 +43,7 @@ func (s *Shard) ValidateShardID(h *CollationHeader) error {
 	return nil
 }
 
-// HeaderByHash of collation.
+// HeaderByHash looks up a collation header from the shardDB using the header's hash.
 func (s *Shard) HeaderByHash(hash *common.Hash) (*CollationHeader, error) {
 	encoded, err := s.shardDB.Get(*hash)
 	if err != nil {
@@ -66,6 +66,9 @@ func (s *Shard) CollationByHash(headerHash *common.Hash) (*Collation, error) {
 	if err != nil {
 		return nil, err
 	}
+	if header == nil {
+		return nil, fmt.Errorf("header not found")
+	}
 
 	body, err := s.BodyByChunkRoot(header.ChunkRoot())
 	if err != nil {
@@ -80,7 +83,7 @@ func (s *Shard) CanonicalCollationHash(shardID *big.Int, period *big.Int) (*comm
 	key := canonicalCollationLookupKey(shardID, period)
 	hash := common.BytesToHash(key.Bytes())
 	collationHashBytes, err := s.shardDB.Get(hash)
-	if err != nil {
+	if err != nil || len(collationHashBytes) == 0 {
 		return nil, fmt.Errorf("no canonical collation set for period, shardID pair: %v", err)
 	}
 	collationHash := common.BytesToHash(collationHashBytes)
@@ -153,8 +156,7 @@ func (s *Shard) SaveHeader(header *CollationHeader) error {
 	}
 
 	// Uses the hash of the header as the key.
-	hash := header.Hash()
-	s.shardDB.Put(hash, encoded)
+	s.shardDB.Put(header.Hash(), encoded)
 	return nil
 }
 
@@ -209,7 +211,7 @@ func dataAvailabilityLookupKey(chunkRoot *common.Hash) common.Hash {
 	return common.BytesToHash([]byte(key))
 }
 
-// dataAvailabilityLookupKey formats a string that will become a lookup key
+// canonicalCollationLookupKey formats a string that will become a lookup key
 // in the shardDB that takes into account the shardID and the period
 // of the shard for ease of use.
 func canonicalCollationLookupKey(shardID *big.Int, period *big.Int) common.Hash {

sharding/shard_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto/sha3"
 	"github.com/ethereum/go-ethereum/rlp"
+	"github.com/ethereum/go-ethereum/sharding/database"
 )
 
 // Hash returns the hash of a collation's entire contents. Useful for comparison tests.
@@ -20,7 +21,7 @@ func TestShard_ValidateShardID(t *testing.T) {
 	emptyHash := common.StringToHash("")
 	emptyAddr := common.StringToAddress("")
 	header := NewCollationHeader(big.NewInt(1), &emptyHash, big.NewInt(1), &emptyAddr, []byte{})
-	shardDB := makeShardKV()
+	shardDB := database.MakeShardKV()
 	shard := MakeShard(big.NewInt(3), shardDB)
 
 	if err := shard.ValidateShardID(header); err == nil {
@@ -39,7 +40,7 @@ func TestShard_HeaderByHash(t *testing.T) {
 	emptyHash := common.StringToHash("")
 	emptyAddr := common.StringToAddress("")
 	header := NewCollationHeader(big.NewInt(1), &emptyHash, big.NewInt(1), &emptyAddr, []byte{})
-	shardDB := makeShardKV()
+	shardDB := database.MakeShardKV()
 	shard := MakeShard(big.NewInt(1), shardDB)
 
 	if err := shard.SaveHeader(header); err != nil {
@@ -71,7 +72,7 @@ func TestShard_CollationByHash(t *testing.T) {
 	// We set the chunk root.
 	collation.CalculateChunkRoot()
 
-	shardDB := makeShardKV()
+	shardDB := database.MakeShardKV()
 	shard := MakeShard(big.NewInt(1), shardDB)
 
 	if err := shard.SaveCollation(collation); err != nil {