First implementation of vetting tools. #2

ghost · 2017-07-20T10:37:24Z

First Implementation of vetting tools:
- unused
- gosimple
- staticcheck
- golint
Added TODO notes
Added documentation and hacks to be able to run this plugin on scenarios different from linux.
- Noted techniques are non invasive to the plugin layout.
Next Steps (to be copied in this issue)
- Refactor to comply with eth-block to be a block header refactor/block-header.
- Add support for state trie element retrieval feat/state-trie.
- Extend this program to a repo to be called go-ipld-eth-dump-star, which will be a CLI app where you provide a block hash, and this program will recursively retrieve and add to IPFS block header, ommer list, state trie elements, etc; Until complete the 8 IPLD types. This should be the angular stone to the ambitious project of having the whole eth blockchain on IPFS.
  - To accomplish above, we will need to build a patch to go-ethereum's ethapi package, to retrieve whatever we need.
  - Support for the rest of the 8 IPLD types.

[0x90] eth-block 
  ommers: eth-block-list
  txs: eth-tx-trie
  receipts: eth-tx-receipt-trie
  state: eth-state-trie

[0x91] eth-tx (local data only)
[0x92] eth-tx-receipt (local data only)
[0x93] eth-account-snapshot (local data only)

[0x94] eth-block-list (rlp array)
  [leaves]: eth-block

[0x95] eth-tx-trie (merkle trie)
  [leaves]: eth-tx

[0x96] eth-tx-receipt-trie (merkle trie)
  [leaves]: links to eth-tx-receipt

[0x97] eth-state-trie (secure merkle trie)
  [leaves]: links to eth-account-snapshot

[0x98] eth-storage-trie (secure merkle trie)
  [leaves]: links to raw binary

…blem

…p to 0.0.2

Stebalien

Most of the documentation looks fine (although I haven't thoroughly read the markdown documentation files).

Most of my comments are just addressing questions in the code.

Stebalien · 2017-07-20T18:07:36Z

block.go

-	cid "github.com/ipfs/go-cid"
-	node "github.com/ipfs/go-ipld-format"
-	mh "github.com/multiformats/go-multihash"
+	cid "gx/ipfs/QmTprEaAA2A9bst5XH7exuyi5KzNMK3SEDNN8rBDnKWcUS/go-cid"


We generally don't leave imports in gx form in any repo but go-ipfs.

Got it. TIL: gx-go hook pre-test and gx-go hook post-test.

Stebalien · 2017-07-20T18:53:49Z

block.go

+// Copy is NOT IMPLEMENTED YET
+// Should return a deep copy of this node.
+// TODO
+// TBD how deep we want to copy this node.
 func (b *EthBlock) Copy() node.Node {


sigh... The simplest way to do this is to just encode then decode. That gives you a guaranteed copy 😁. I don't think anything will call this so performance shouldn't be that much of an issue.

However, this method really has no place here because IPLD nodes should be immutable once constructed. It's a remanent from the past at this point. Unfortunately, we still have to update the interface (take a look at the issues) and doing that will require quite a bit of cross-project refactoring.

Stebalien · 2017-07-20T19:06:49Z

block.go

@@ -162,6 +164,7 @@ func (b *EthBlock) MarshalJSON() ([]byte, error) {
 	return json.Marshal(out)
 }

+// Cid returns the content identifier of the block header.


We usually just abbreviate "content identifier" as CID (kind of like how nobody calls http "hyper-text transport protocol").

Stebalien · 2017-07-20T19:07:15Z

block.go

@@ -175,18 +178,25 @@ func (b *EthBlock) Cid() *cid.Cid {
 	return c
 }

+// Parent returns the content identifier of the parent of the block.


content identifier -> CID

Stebalien · 2017-07-20T19:07:21Z

block.go

 func (b *EthBlock) Parent() *cid.Cid {
 	return toCid(MEthBlock, b.header.ParentHash.Bytes())
 }

+// Tx returns the content identifier of the transactionsTrie root of the block.


content identifier -> CID

Stebalien · 2017-07-20T19:10:32Z

block.go

@@ -249,19 +265,27 @@ func (b *EthBlock) ResolveLink(p []string) (*node.Link, []string, error) {
 	return nil, nil, fmt.Errorf("resolved item was not a link")
 }

+// Size returns the size in bytes of the serialized object
 func (b *EthBlock) Size() (uint64, error) {
 	// TODO:


In case you're wondering what this should return... the answer is it doesn't matter. This function will go away as a part of the great go-ipld-format refactor (every IPLD format handles size differently so it's kind of useless at the moment).

Stebalien · 2017-07-20T19:10:54Z

block.go

 func (b *EthBlock) Size() (uint64, error) {
 	// TODO:
 	return 0, nil
 }

+// Stat helps this struct to comply with the Node interface
+// TODO: not sure if stat deserves to stay
 func (b *EthBlock) Stat() (*node.NodeStat, error) {


It has to stay (:sob:) for now but yeah, this will also go away.

Stebalien · 2017-07-20T19:15:50Z

trie.go

@@ -76,34 +77,39 @@ func NewTrieNode(data []byte) (node.Node, error) {
 	}
 }

-func (b *TrieNode) Cid() *cid.Cid {
+// Cid returns the content identifier of the trie


content identifier -> CID.

Stebalien · 2017-07-20T19:16:25Z

trie.go

 	if err != nil {
 		panic(err)
 	}
 	return c
 }

+// HexHash returns the hex hash of the trie


Is "hex hash" the standard name for this in etherland?

Stebalien · 2017-07-20T19:18:01Z

tx.go

 type Tx struct {
 	tx *types.Transaction
 }

-func (b *Tx) Cid() *cid.Cid {
+// Cid returns the content identifier of the transaction.


content identifier -> CID

…will go away'

…. Should be variable in the future

ghost · 2017-07-21T10:05:28Z

@Stebalien All observations solved. Moving on to the eth-block branch.

Stebalien · 2017-07-21T18:17:58Z

block_test.go

-	fmt.Println(c)
+
+	_ = c
+	// f_ = cmt.Println(c)


We should probably be either using this value or not generating it. What's this test for?

It's legacy code. I planned to improve the test on a later PR. Will delete it now.

Stebalien · 2017-07-21T18:18:04Z

block_test.go

@@ -63,5 +64,6 @@ func TestBlockWithOddTransactions(t *testing.T) {
 		t.Fatal(err)
 	}

-	fmt.Printf("%x\n", blk.Tx().Bytes())
+	_ = blk
+	// fmt.Printf("%x\n", blk.Tx().Bytes())


Removed. It is legacy code. I'm properly dealing with the tests for block in the next PR.

ghost · 2017-07-21T22:27:13Z

@Stebalien should we be good to merge this one?

Stebalien

LGTM

Stebalien · 2017-07-21T22:41:17Z

Ah, except I can't merge on this repo... let me ping @whyrusleeping.

Herman Junge added 5 commits July 17, 2017 06:02

Update Dependencies to gx

9ab0116

Add unsupported docker guide (oriented to OSX users like me)

74068bf

Add the hacks directory and an OSX solution to the current plugin pro…

ef2b0c3

…blem

Solve golint observations on block, move codecs to separate file, bum…

43b8101

…p to 0.0.2

Solve golint observations on tx

79e3036

ghost mentioned this pull request Jul 20, 2017

Next steps #1

Open

Solve golint observations on trie node

236ab34

Stebalien approved these changes Jul 20, 2017

View reviewed changes

Herman Junge added 6 commits July 20, 2017 22:52

Apply gx-go hook post-test

5e6335d

Add gx-go hook <function> to the osx hack

4064f61

Rewrite doc to 'Cid returns the cid'

cc3dd60

content identifier -> cid

eeae01b

Move Copy, Size and Stat to the tail of each file. Document them as '…

e3b49f4

…will go away'

Add test values, test. Modify (tmp) trie node to be a state trie node…

62f061b

…. Should be variable in the future

ghost mentioned this pull request Jul 21, 2017

Refactor eth-block to be an IPLD Link of a block header #4

Merged

Stebalien reviewed Jul 21, 2017

View reviewed changes

Get rid of commented code on tests

66ea037

Stebalien approved these changes Jul 21, 2017

View reviewed changes

whyrusleeping merged commit d33d669 into ipfs:master Jul 21, 2017

ghost deleted the feat/vetting branch July 31, 2017 11:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First implementation of vetting tools. #2

First implementation of vetting tools. #2

ghost commented Jul 20, 2017 •

edited by ghost

Loading

Stebalien left a comment

Stebalien Jul 20, 2017

ghost Jul 21, 2017

Stebalien Jul 20, 2017

Stebalien Jul 20, 2017

Stebalien Jul 20, 2017

Stebalien Jul 20, 2017

Stebalien Jul 20, 2017

Stebalien Jul 20, 2017

Stebalien Jul 20, 2017

Stebalien Jul 20, 2017

Stebalien Jul 20, 2017

ghost commented Jul 21, 2017

Stebalien Jul 21, 2017

ghost Jul 21, 2017

Stebalien Jul 21, 2017

ghost Jul 21, 2017

ghost commented Jul 21, 2017

Stebalien left a comment

Stebalien commented Jul 21, 2017

First implementation of vetting tools. #2

First implementation of vetting tools. #2

Conversation

ghost commented Jul 20, 2017 • edited by ghost Loading

Stebalien 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

ghost commented Jul 21, 2017

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

ghost commented Jul 21, 2017

Stebalien left a comment

Choose a reason for hiding this comment

Stebalien commented Jul 21, 2017

ghost commented Jul 20, 2017 •

edited by ghost

Loading