From 083c93e7120defb4f53cdcbadb0f7c817e95d516 Mon Sep 17 00:00:00 2001
From: Raul Jordan <rauljordan@college.harvard.edu>
Date: Tue, 15 May 2018 22:55:02 -0400
Subject: [PATCH] sharding: address review comments

Former-commit-id: 3d5cd49930ff1a07ccb544910e2b4bfeb75f04de [formerly 0275d8ec3ce981aed3ea2f88c660505e809f6b7d]
Former-commit-id: d291b375e7550f8792ece3e982f6b85da862ffed
---
 sharding/collation.go      |  1 +
 sharding/collation_test.go | 36 ------------------------------------
 sharding/shard.go          |  7 ++-----
 sharding/shard_test.go     |  2 +-
 4 files changed, 4 insertions(+), 42 deletions(-)

diff --git a/sharding/collation.go b/sharding/collation.go
index b12225c3b..373a6ba9c 100644
--- a/sharding/collation.go
+++ b/sharding/collation.go
@@ -13,6 +13,7 @@ import (
 type Collation struct {
 	header *CollationHeader
 	// body represents the serialized blob of a collation's transactions.
+	// this is a read-only property.
 	body []byte
 	// transactions serves as a useful slice to store deserialized chunks from the
 	// collation's body. Every time this transactions slice is updated, the serialized
diff --git a/sharding/collation_test.go b/sharding/collation_test.go
index 46bc5521a..2d815f336 100644
--- a/sharding/collation_test.go
+++ b/sharding/collation_test.go
@@ -39,42 +39,6 @@ func TestCollation_ProposerAddress(t *testing.T) {
 	}
 }
 
-// TODO: this test needs to change as we will be serializing tx's into blobs
-// within the collation body instead.
-
-// func TestCollation_AddTransactions(t *testing.T) {
-// 	tests := []struct {
-// 		transactions []*types.Transaction
-// 	}{
-// 		{
-// 			transactions: []*types.Transaction{
-// 				makeTxWithGasLimit(0),
-// 				makeTxWithGasLimit(1),
-// 				makeTxWithGasLimit(2),
-// 				makeTxWithGasLimit(3),
-// 			},
-// 		}, {
-// 			transactions: []*types.Transaction{},
-// 		},
-// 	}
-
-// 	for _, tt := range tests {
-// 		c := &Collation{}
-// 		for _, tx := range tt.transactions {
-// 			c.AddTransaction(tx)
-// 		}
-// 		results := c.Transactions()
-// 		if len(results) != len(tt.transactions) {
-// 			t.Fatalf("Wrong number of transactions. want=%d. got=%d", len(tt.transactions), len(results))
-// 		}
-// 		for i, tx := range tt.transactions {
-// 			if results[i] != tx {
-// 				t.Fatalf("Mismatched transactions. wanted=%+v. got=%+v", tt.transactions, results)
-// 			}
-// 		}
-// 	}
-// }
-
 func makeTxWithGasLimit(gl uint64) *types.Transaction {
 	return types.NewTransaction(0 /*nonce*/, common.HexToAddress("0x0") /*to*/, nil /*amount*/, gl, nil /*gasPrice*/, nil /*data*/)
 }
diff --git a/sharding/shard.go b/sharding/shard.go
index 1d62af965..816ca2c84 100644
--- a/sharding/shard.go
+++ b/sharding/shard.go
@@ -129,7 +129,7 @@ func (s *Shard) BodyByChunkRoot(chunkRoot *common.Hash) ([]byte, error) {
 		return nil, err
 	}
 	if body == nil {
-		return nil, fmt.Errorf("no corresponding body with chunk root found: %v", chunkRoot.String())
+		return nil, fmt.Errorf("no corresponding body with chunk root found: %s", chunkRoot)
 	}
 	return *body, nil
 }
@@ -239,10 +239,7 @@ func (s *Shard) SetCanonical(header *CollationHeader) error {
 	}
 	// sets the key to be the canonical collation lookup key and val as RLP encoded
 	// collation header.
-	if err := s.shardDB.Put(key, encoded); err != nil {
-		return err
-	}
-	return nil
+	return s.shardDB.Put(key, encoded)
 }
 
 // dataAvailabilityLookupKey formats a string that will become a lookup
diff --git a/sharding/shard_test.go b/sharding/shard_test.go
index fc2e73c24..4c174a200 100644
--- a/sharding/shard_test.go
+++ b/sharding/shard_test.go
@@ -192,7 +192,7 @@ func TestShard_CanonicalHeaderHash(t *testing.T) {
 	}
 
 	if canonicalHeaderHash.String() != headerHash.String() {
-		t.Errorf("header hashes do not match. want=%v. got=%v", headerHash.String(), canonicalHeaderHash.String())
+		t.Errorf("header hashes do not match. want=%s. got=%s", headerHash.String(), canonicalHeaderHash.String())
 	}
 }