WIP: Shard Chain Type With Useful Methods for Notary/Proposer Clients #100

rauljordan · 2018-05-02T16:21:48Z

This is a PR referencing #99 to create a shard chain interface/struct that can be used across our various packages.

Context

This was worked on in Py-EVM ethereum/py-evm#570 and would also serve as a useful wrapper class to thin out the code we would otherwise have to write for notaries/proposers.

Requirements for Merge

Create a Shard struct/interface
Define common methods such as fetching collation by hash, checking availability, fetching x number of collations from within the shard
Define an in-memory db for now as the shardDB backend
Finalize all the tests for the methods below. Currently have 3 tests passing.

The design for an actual db backend used in our sharding client is beyond the scope of this PR. This PR just uses a simple k-v store.

Design Rationale

In this PR, we include both a transactions []*types.Transaction field along with a body []byte field in the Collation struct.

// Collation base struct.
type Collation struct {
	header *CollationHeader
	// body represents the serialized blob of a collation's transactions.
	body []byte
	transactions []*types.Transaction
}

The reason for this is that transactions serves as a useful slice to store deserialized chunks from the
collation's body we can manipulate programmatically. Every time this transactions slice is updated, the serialize would need to be recalculated. This will be a useful property for proposers and executors in our sharding implementation down the line.

Functions Being Implemented

func (s *Shard) HeaderByHash(hash *common.Hash) (*CollationHeader, error) {}
func (s *Shard) CollationByHash(headerHash *common.Hash) (*Collation, error) {}
func (s *Shard) CanonicalCollationHash(shardID *big.Int, period *big.Int) (*common.Hash, error) {}
func (s *Shard) CanonicalCollation(shardID *big.Int, period *big.Int) (*Collation, error) {}
func (s *Shard) BodyByChunkRoot(chunkRoot *common.Hash) ([]byte, error) {}
func (s *Shard) CheckAvailability(header *CollationHeader) (bool, error) {}
func (s *Shard) SetAvailability(chunkRoot *common.Hash, availability bool) error {}
func (s *Shard) SaveHeader(header *CollationHeader) error {}
func (s *Shard) SaveBody(body []byte) error {}
func (s *Shard) SaveCollation(collation *Collation) error {}
func (s *Shard) SetCanonical(header *CollationHeader) error {}

prestonvanloon · 2018-05-07T22:40:36Z

sharding/shard.go

+
+// Shard base struct.
+type Shard struct {
+	shardDB *shardKV


Can you change this to a common interface? Right now, it only accepts a database of struct shardKV, so it’s not very swappable.

prestonvanloon · 2018-05-07T22:42:36Z

sharding/shard.go

+// MakeShard creates an instance of a Shard struct given a shardID.
+func MakeShard(shardID *big.Int) *Shard {
+	// Swappable - can be makeShardLevelDB, makeShardSparseTrie, etc.
+	shardDB := makeShardKV()


Shouldn’t this be a function argument? Why does a shard know which database to initialize? It feels a bit out of the responsibilities of a initializing a shard.

I can imagine that a nontrivial database has many arguments so this method will quickly get out of control.

prestonvanloon · 2018-05-07T22:44:46Z

sharding/collation.go


-// ShardID is the identifier for a shard.
-func (c *Collation) ShardID() *big.Int { return c.header.shardID }
+// Hash returns the hash of a collation's entire contents. Useful for tests.


Is this only useful for tests? Maybe move to the testing package

prestonvanloon · 2018-05-07T22:46:29Z

sharding/shard_test.go

+	shard := MakeShard(big.NewInt(3))
+
+	if err := shard.ValidateShardID(header); err == nil {
+		t.Fatalf("Shard ID validation incorrect. Function should throw error when shardID's do not match. want=%d. got=%d", header.ShardID().Int64(), shard.ShardID().Int64())


This should be Errorf. You should only fatal when the test cannot possibly proceed forward.

Please change this everywhere in this test

prestonvanloon · 2018-05-07T22:47:02Z

sharding/db.go

+	fmt.Printf("Map: %v\n", sb.kv)
+	fmt.Printf("Key: %v\n", k)
+	fmt.Printf("Val: %v\n", sb.kv[k])
+	fmt.Printf("Ok: %v\n", ok)


Remove all of this printing. If you feel that you really need it, use ethereum log with the appropriate level (debug).

prestonvanloon · 2018-05-07T22:50:18Z

sharding/collation.go

-// Transactions returns an array of tx's in the collation.
-func (c *Collation) Transactions() []*types.Transaction { return c.transactions }
+// Body returns the collation's byte body.
+func (c *Collation) Body() []byte { return c.body }


Shouldn’t this return a pointer to the byte array?

Why would it? In case it's nil?

prestonvanloon · 2018-05-07T22:52:32Z

sharding/db.go

+	fmt.Printf("Val: %v\n", sb.kv[k])
+	fmt.Printf("Ok: %v\n", ok)
+	if !ok {
+		return nil, fmt.Errorf("Key Not Found")


Change to (“key not found %v”, k)

prestonvanloon · 2018-05-07T22:55:12Z

sharding/shard.go

+}
+
+// GetHeaderByHash of collation.
+func (s *Shard) GetHeaderByHash(hash *common.Hash) (*CollationHeader, error) {


Please remove “Get” here and everywhere https://golang.org/doc/effective_go.html#Getters

prestonvanloon · 2018-05-07T22:58:30Z

sharding/shard_test.go

+// 	if collation.Hash() != dbCollation.Hash() {
+// 		t.Fatalf("Collations do not match. want=%v. got=%v", collation, dbCollation)
+// 	}
+// }


Uncomment or delete this test. No point in checking in dead code

prestonvanloon · 2018-05-07T22:58:53Z

sharding/shard.go

+	return common.BytesToHash([]byte(key))
+}
+
+// dataAvailabilityLookupKey formats a string that will become a lookup key


Wrong method name

Still wrong!

prestonvanloon · 2018-05-10T02:01:49Z

sharding/collation.go

 )

 // Collation base struct.
 type Collation struct {
 	header       *CollationHeader
+	body         []byte
 	transactions []*types.Transaction


+1. How are we handling a body and transactions?

Are we supposed to update the body every time we modify this array?

Wouldnt transactions be called chunks? There's no TransactionRoot in collation header, only ChunkRoot

transactions are actual deserialized tx's that will be helpful for our implementation

terencechain · 2018-05-09T04:45:08Z

sharding/collation.go

-// Period the collation corresponds to.
-func (c *Collation) Period() *big.Int { return c.header.period }
+// Transactions returns an array of tx's in the collation.
+func (c *Collation) Transactions() []*types.Transaction { return c.transactions }


Transactions here too. I thought that's replaced by chunks

We still need them because we'll be deserializing the chunks into transactions. We'll need a place to store this deserialized data and keeping a tx's slice is good here.

prestonvanloon · 2018-05-10T02:04:02Z

sharding/db.go

+
+	"github.com/ethereum/go-ethereum/common"
+)
+


+1, a highlevel comment on how this works would be nice

terencechain · 2018-05-09T05:42:33Z

sharding/shard_test.go

+	shard := MakeShard(big.NewInt(1))
+
+	if err := shard.SaveHeader(header); err != nil {
+		t.Fatal(err)


Should the Fatal be more descriptive here? Like t.Fataf("Save header failed: %v", err)

terencechain · 2018-05-09T05:43:36Z

sharding/shard_test.go

+	// It's being saved, but the .Get func doesn't fetch the value...?
+	dbHeader, err := shard.GetHeaderByHash(&hash)
+	if err != nil {
+		t.Fatal(err)


More description for t.Fatal(err)?

…s pass

prestonvanloon

A lot of questions about design choices.
Could you add some details to the pull request description where necessary?

prestonvanloon · 2018-05-10T02:01:49Z

sharding/collation.go

 )

 // Collation base struct.
 type Collation struct {
 	header       *CollationHeader
+	body         []byte
 	transactions []*types.Transaction


+1. How are we handling a body and transactions?

Are we supposed to update the body every time we modify this array?

prestonvanloon · 2018-05-10T02:02:52Z

sharding/collation.go

+
+// 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 s.Decode(&h.data)

prestonvanloon · 2018-05-10T02:03:22Z

sharding/collation.go

+
+// 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.


Can you create a github issue for this?

+1 There's a lot more to be done here. For proof of custody we need to split chunks (body) into chunk + salt and take the merkle root of that

prestonvanloon · 2018-05-10T02:04:02Z

sharding/db.go

+
+	"github.com/ethereum/go-ethereum/common"
+)
+


+1, a highlevel comment on how this works would be nice

prestonvanloon · 2018-05-10T02:04:13Z

sharding/db.go

@@ -0,0 +1,37 @@
+package sharding


Can/should we move this to its own package?

prestonvanloon · 2018-05-10T02:35:06Z

sharding/shard.go

+	hash := common.BytesToHash(key.Bytes())
+	collationHashBytes, err := s.shardDB.Get(hash)
+	if err != nil {
+		return nil, fmt.Errorf("no canonical collation set for period, shardID pair: %v", err)


This error seems to assume that the error was caused by non-existent data. See my other comment about this.

prestonvanloon · 2018-05-10T02:36:00Z

sharding/shard.go

+	if err != nil {
+		return nil, fmt.Errorf("no canonical collation set for period, shardID pair: %v", err)
+	}
+	collationHash := common.BytesToHash(collationHashBytes)


What happens when the data isn't there but there was no error? i.e. if collationHashBytes == nil

prestonvanloon · 2018-05-10T02:38:00Z

sharding/shard.go

+	if err != nil {
+		return nil, err
+	}
+	return &Collation{header: header, body: body}, nil


return &Collation{header, body}, nil

Getting "too few values in struct initializer"

That doesn't sound right...

prestonvanloon · 2018-05-10T02:39:55Z

sharding/shard.go

+	}
+
+	// Uses the hash of the header as the key.
+	hash := header.Hash()


Do you need to declare this variable? It would be simpler as

s.shardDB.Put(header.Hash(), encoded)

prestonvanloon · 2018-05-10T02:40:49Z

sharding/shard.go

+	if err != nil {
+		return nil, err
+	}
+


Handle case for header == nil?

This is covered by the headerbyhash method. error means that the header was nil.

terencechain · 2018-05-10T05:29:26Z

sharding/collation.go

+// ProposerAddress is the coinbase addr of the creator for the collation.
+func (c *Collation) ProposerAddress() *common.Address {
+	return c.header.data.ProposerAddress
+}

 // AddTransaction adds to the collation's body of tx blobs.
 func (c *Collation) AddTransaction(tx *types.Transaction) {
 	// TODO: Include blob serialization instead.


github issue here too

What exactly should the issue be @terenc3t ?

Use blob serialization instead of appending tx to c.transactions c.transactions = append(c.transactions, tx)

terencechain · 2018-05-10T05:32:24Z

sharding/collation.go

+	rlp.Encode(hw, h)
+	hw.Sum(hash[:0])
+	return hash
+}

 // ShardID is the identifier for a shard.


This comment is not exactly accurate. It should be something like ShardID the collation belongs to

terencechain · 2018-05-10T23:08:58Z

sharding/collation.go

-	period            *big.Int        //the period number in which collation to be included.
-	proposerAddress   *common.Address //address of the collation proposer.
-	proposerSignature []byte          //the proposer's signature for calculating collation hash.
+	data collationHeaderData


why put a nested struct collationHeaderData?

This is so that it acts as a read only property. You can only access the data via getters and cannot modify it

Ah! got it. Thanks for the explanations

terencechain · 2018-05-10T23:27:24Z

sharding/collation.go

+
+// 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.


+1 There's a lot more to be done here. For proof of custody we need to split chunks (body) into chunk + salt and take the merkle root of that

terencechain · 2018-05-10T23:34:00Z

sharding/db.go

+	"github.com/ethereum/go-ethereum/common"
+)
+
+type shardKV struct {


+1 Some basic tests for Get, Has, Put and Delete would be nice

terencechain · 2018-05-11T00:08:39Z

sharding/shard.go

+	return nil
+}
+
+// HeaderByHash of collation.


can we make the function description comment more descriptive?

prestonvanloon

This is looking really solid. Just a few comments for code optimization and to remove commented code.

prestonvanloon · 2018-05-16T02:31:06Z

sharding/collation.go

+	// body represents the serialized blob of a collation's transactions.
+	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


Please update comment as described above.

prestonvanloon · 2018-05-16T02:31:53Z

sharding/collation_test.go

+// 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) {


Delete all of this?

prestonvanloon · 2018-05-16T02:33:35Z

sharding/shard.go

+	if err != nil {
+		return nil, fmt.Errorf("cannot fetch body by chunk root: %v", err)
+	}
+	// TODO: deserializes the body into a txs slice instead of using


Please create github issue for this. It would be a nice place for someone to contribute after serialization is added.

I'll create as soon as we merge

prestonvanloon · 2018-05-16T02:34:43Z

sharding/shard.go

+		return nil, err
+	}
+	if body == nil {
+		return nil, fmt.Errorf("no corresponding body with chunk root found: %v", chunkRoot.String())


Change this to %s and I think you don't have to call .String() since go will do that automatically.

prestonvanloon · 2018-05-16T02:35:46Z

sharding/shard.go

+	if val == nil {
+		return false, fmt.Errorf("availability not set for header")
+	}
+	availability := *val


I believe this makes a copy of the data. Can't you access val[0]?

cause val is type *[]byte

Yep! I am mistaken about copying the data. With a slice, like you have here, it is copying the internal slice pointers so this is not an expensive operation.

In fact, it is OK to pass in a slice as an argument since it's really just a set of 1 pointer and 2 values. (beginning, cap, len).

https://blog.golang.org/go-slices-usage-and-internals

prestonvanloon · 2018-05-16T02:38:03Z

sharding/shard.go

+	}
+	// 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 this

prestonvanloon · 2018-05-16T02:38:57Z

sharding/shard.go

+
+// dataAvailabilityLookupKey formats a string that will become a lookup
+// key in the shardDB.
+func dataAvailabilityLookupKey(chunkRoot *common.Hash) common.Hash {


prestonvanloon · 2018-05-16T02:39:17Z

sharding/shard.go

+// dataAvailabilityLookupKey formats a string that will become a lookup
+// key in the shardDB.
+func dataAvailabilityLookupKey(chunkRoot *common.Hash) common.Hash {
+	key := fmt.Sprintf("availability-lookup:%s", chunkRoot.String())


I don't think you need to call .String() it should be automatic for the formatter with %s.

prestonvanloon · 2018-05-16T02:39:40Z

sharding/shard.go

+// of the shard for ease of use.
+func canonicalCollationLookupKey(shardID *big.Int, period *big.Int) common.Hash {
+	str := "canonical-collation-lookup:shardID=%s,period=%s"
+	key := fmt.Sprintf(str, shardID.String(), period.String())


I don't think you need to call .String(), it should be automatic.

prestonvanloon · 2018-05-16T02:41:23Z

sharding/shard_test.go

+	}
+
+	if canonicalHeaderHash.String() != headerHash.String() {
+		t.Errorf("header hashes do not match. want=%v. got=%v", headerHash.String(), canonicalHeaderHash.String())


Remove .String() and change %v to %s

terencechain

Great job on the well-written comments! Makes it really to understand the code

terencechain · 2018-05-16T03:15:48Z

sharding/collation.go

+type collationHeaderData struct {
+	ShardID           *big.Int        // the shard ID of the shard.
+	ChunkRoot         *common.Hash    // the root of the chunk tree which identifies collation body.
+	Period            *big.Int        // the period number in which collation to be Pncluded.


s/Pncluded/included

terencechain · 2018-05-16T16:08:31Z

sharding/collation.go

+// Hash takes the keccak256 of the collation header's contents.
+func (h *CollationHeader) Hash() (hash common.Hash) {
+	hw := sha3.NewKeccak256()
+	rlp.Encode(hw, h)


Makes more sense to encode h.data instead of h?

prestonvanloon

The best pull request I have seen all day. Great job

WIP: Shard Chain Type With Useful Methods for Notary/Proposer Clients

Former-commit-id: 74c85fc

Former-commit-id: 32c501e

Former-commit-id: 59b4763

WIP: Shard Chain Type With Useful Methods for Notary/Proposer Clients Former-commit-id: d7f0bdf

Former-commit-id: 5febe72a5a1936ce643488067e0990da810f1f5e [formerly 74c85fc] Former-commit-id: 0cc6d45

Former-commit-id: 9ab2595c130922bc49fc3c9a69f11dd93b6774fb [formerly 32c501e] Former-commit-id: cfe18a1

Former-commit-id: 1fc773af9e07a79f63bf63a097526a7c3303cd14 [formerly 59b4763] Former-commit-id: 6b8162c

WIP: Shard Chain Type With Useful Methods for Notary/Proposer Clients Former-commit-id: 6e12288539871b6e5077f21564a3161dfecd4c0b [formerly d7f0bdf] Former-commit-id: bca3868

sharding: beginning the shard interface

b366896

rauljordan added the Phase 1 label May 2, 2018

rauljordan added this to the Ruby milestone May 2, 2018

rauljordan self-assigned this May 2, 2018

rauljordan added this to To do in Validator Client via automation May 2, 2018

rauljordan mentioned this pull request May 3, 2018

Sharding-Enabled Merkle Trie Design Choices #102

Closed

rauljordan added 3 commits May 3, 2018 10:22

sharding: beginning receiver methods for shard type

290e1ac

sharding: collation header hash and shard receiver methods

03e2212

sharding: fetch body by chunk root

a07b276

rauljordan mentioned this pull request May 3, 2018

Create a Shard Interface/Struct With Useful Receiver Methods in Client Package #99

Closed

rauljordan added 8 commits May 3, 2018 16:31

sharding: make shard func

f732b7d

sharding: fix shard methods and lookup keys

1ab7b5f

sharding: merge from master and fix collation.go conflicts

2cdbad6

sharding: fully functional receiver methods for shard. pre-tests

dbcd93f

sharding: test validate shard id

ad7034b

sharding: added tests for get header by hash

36d9a48

sharding: add test for get collation by hash

53feb62

sharding: tests get collation header by hash passes

841ec3a

rauljordan requested a review from prestonvanloon May 7, 2018 22:06

prestonvanloon requested changes May 7, 2018

View reviewed changes

terencechain reviewed May 9, 2018

View reviewed changes

rauljordan mentioned this pull request May 9, 2018

Clients store chaindata in local KV store and can produce it later #109

Closed

rauljordan added 5 commits May 9, 2018 15:23

sharding: address review comments om #100, fix linter issues

215cec0

sharding: full reorganization of collation struct, encoding, and test…

a38d3fe

…s pass

sharding: rename methods, tests pass.

4305cd8

sharding: use new header.EncodeRLP method

3af40c2

sharding: remove log debug statements

b65f675

prestonvanloon requested changes May 10, 2018

View reviewed changes

terencechain reviewed May 10, 2018

View reviewed changes

terencechain reviewed May 11, 2018

View reviewed changes

rauljordan added 3 commits May 14, 2018 09:53

sharding: improve coverage for #100

841f2e5

sharding: 93% coverage

c5a51c5

sharding: eliminated assumptions about db implementation

08c8623

prestonvanloon reviewed May 16, 2018

View reviewed changes

sharding: address review comments

89b33d4

terencechain approved these changes May 16, 2018

View reviewed changes

rauljordan added 3 commits May 16, 2018 01:13

sharding: fix typo in collation.go

8185cc9

sharding: use .Hex() instead of .String() when logging hexdigests

1f70f08

sharding: merge from master

cdff195

terencechain reviewed May 16, 2018

View reviewed changes

rauljordan added 4 commits May 16, 2018 12:59

sharding: hashes collationHeaderData instead of the full header

31abcdb

sharding: fix broken tests

a3202b7

sharding: remove .String() from big.Int usage

bc6879b

sharding: fix return vals

976ecea

prestonvanloon approved these changes May 16, 2018

View reviewed changes

sharding: return val in SaveCollation simplified

0e2b354

rauljordan merged commit 0228955 into prysmaticlabs:master May 16, 2018

Validator Client automation moved this from To do to Done May 16, 2018

prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018

sharding: address review comments om #100, fix linter issues

74c85fc

prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018

sharding: all relevant tests pass, linter passes, ready for review #100

32c501e

prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018

sharding: improve coverage for #100

59b4763

prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018

Merge pull request #100 from rauljordan/shard-type

d7f0bdf

WIP: Shard Chain Type With Useful Methods for Notary/Proposer Clients

prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018

sharding: address review comments om #100, fix linter issues

0cc6d45

Former-commit-id: 74c85fc

prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018

sharding: all relevant tests pass, linter passes, ready for review #100

cfe18a1

Former-commit-id: 32c501e

prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018

sharding: improve coverage for #100

6b8162c

Former-commit-id: 59b4763

prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018

Merge pull request #100 from rauljordan/shard-type

bca3868

WIP: Shard Chain Type With Useful Methods for Notary/Proposer Clients Former-commit-id: d7f0bdf

prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018

sharding: address review comments om #100, fix linter issues

d1aa843

Former-commit-id: 5febe72a5a1936ce643488067e0990da810f1f5e [formerly 74c85fc] Former-commit-id: 0cc6d45

prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018

sharding: all relevant tests pass, linter passes, ready for review #100

880a5e7

Former-commit-id: 9ab2595c130922bc49fc3c9a69f11dd93b6774fb [formerly 32c501e] Former-commit-id: cfe18a1

prestonvanloon pushed a commit that referenced this pull request Jul 22, 2018

sharding: improve coverage for #100

6e10d9f

Former-commit-id: 1fc773af9e07a79f63bf63a097526a7c3303cd14 [formerly 59b4763] Former-commit-id: 6b8162c

WIP: Shard Chain Type With Useful Methods for Notary/Proposer Clients #100

WIP: Shard Chain Type With Useful Methods for Notary/Proposer Clients #100

Conversation

rauljordan commented May 2, 2018 • edited

Context

Requirements for Merge

Design Rationale

Functions Being Implemented

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prestonvanloon May 7, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prestonvanloon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prestonvanloon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

terencechain left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prestonvanloon left a comment

Choose a reason for hiding this comment

rauljordan commented May 2, 2018 •

edited

prestonvanloon May 7, 2018 •

edited