85-block-extra-data #88

david-julien · 2017-07-10T02:43:11Z

No description provided.

coveralls · 2017-07-10T02:46:12Z

Coverage increased (+0.3%) to 77.343% when pulling 5a45c27 on 85-block-extra-data into 0ce1330 on dev.

chadlagore

Interested in what the use case of the extra data is? I know there is some analogue in BTC. Also, why are we enforcing a block size now?

chadlagore · 2017-07-10T03:16:09Z

blockchain/block.go

@@ -28,6 +27,9 @@ type BlockHeader struct {
 	Time uint32
 	// Nonce starts at 0 and increments by 1 for every hash when mining
 	Nonce uint64
+	// ExtraData is an extra field that can be filled with arbitrary data to
+	// be stored in the block
+	ExtraData []byte


I know there is some analogue to this in bitcoin. Interested, why are we adding it to our chain?

I was thinking its a standard blockchain feature, lets you store data for "eternity" and will potentially help us with future applications of our blockchain.

Should we make this an array to indicate that it should be size-limited? (can change later too).

This is what chad and i were talking about on slack, I think we decided to not limit any size for now. I made a ticket for it.

chadlagore · 2017-07-10T03:18:38Z

blockchain/block.go


 	return buf
 }

+// Len returns the length in bytes of the BlockHeader.
+func (bh *BlockHeader) Len() int {
+	l := 2*(32/8) + 64/8 + 2*HashLen


unsafe doesn't seem ideal, but isn't there some notion of sizeof in golang?https://stackoverflow.com/questions/26975738/how-to-get-memory-size-of-variable

been looking in to this, unsafe.Sizeof causes some issues because in the ExtraData byte array, Sizeof returns the size of the slice descriptor, not the size of the memory referenced by the slice which is what we want as we are looking for the length when marshalled.

Statically setting the length here should be safe if we have tests that will fail once someone messes with it.

chadlagore · 2017-07-10T03:22:21Z

blockchain/block.go

-	BlockHeaderLen = 2*(32/8) + 64/8 + 2*HashLen
+	// MaxBlockSize is the maximum size of a block in bytes when marshaled
+	// (about 250K).
+	MaxBlockSize = 1 << 18


Block size is busy blowing the bitcoin universe up - are we sure we want to enforce it?

https://medium.com/@olivierjanss/the-solution-to-the-block-size-debate-42c3814ff481

https://www.bitcoinunlimited.info/

Did you guys discuss this on Saturday? I'm game if we're consciously making the decision.

Bringing this to slack

chadlagore · 2017-07-10T03:25:51Z

blockchain/validation_test.go

+
+	valid, code := bc.ValidGenesisBlock(gb)
+
+	if valid {


Use assert if we can? 😄

… are now returning len(x.marshal())

coveralls · 2017-07-11T00:21:04Z

Coverage decreased (-0.5%) to 76.587% when pulling 89c0706 on 85-block-extra-data into 0ce1330 on dev.

coveralls · 2017-07-11T00:22:49Z

Coverage decreased (-0.5%) to 76.587% when pulling 89c0706 on 85-block-extra-data into 0ce1330 on dev.

jordanschalm · 2017-07-11T02:48:38Z

blockchain/block.go

@@ -28,6 +27,9 @@ type BlockHeader struct {
 	Time uint32
 	// Nonce starts at 0 and increments by 1 for every hash when mining
 	Nonce uint64
+	// ExtraData is an extra field that can be filled with arbitrary data to
+	// be stored in the block
+	ExtraData []byte


Should we make this an array to indicate that it should be size-limited? (can change later too).

jordanschalm · 2017-07-11T02:49:50Z

blockchain/validation.go

@@ -73,7 +73,7 @@ const (
 	BadGenesisBlockNumber
 	// BadGenesisTarget is returned when the genesis block's target is invalid.
 	BadGenesisTarget
-	// BadGenesisTime is returned when teh gensis block's time is invalid.
+	// BadGenesisTime is returned when the gensis block's time is invalid.


there's another typo here lol (gensis -> genesis)

Gensyscalls

bfbachmann

Wow! Great work!

bfbachmann · 2017-07-11T04:21:03Z

blockchain/block.go

-		l += t.Len()
-	}
-	return l
+	return len(b.Marshal())


bfbachmann · 2017-07-11T04:23:19Z

blockchain/validation.go

@@ -73,7 +73,7 @@ const (
 	BadGenesisBlockNumber
 	// BadGenesisTarget is returned when the genesis block's target is invalid.
 	BadGenesisTarget
-	// BadGenesisTime is returned when teh gensis block's time is invalid.
+	// BadGenesisTime is returned when the gensis block's time is invalid.


Gensyscalls

bfbachmann · 2017-07-11T04:24:19Z

blockchain/validation.go

@@ -190,6 +190,7 @@ func ValidCloudBase(t *Transaction) (bool, CloudBaseTransactionCode) {

 // ValidGenesisBlock checks whether a block is a valid genesis block.
 func (bc *BlockChain) ValidGenesisBlock(gb *Block) (bool, GenesisBlockCode) {
+


This is my favorite line

bfbachmann · 2017-07-11T04:24:45Z

blockchain/validation.go

 	if b == nil {
 		return false, NilBlock
 	}

-	// Check if the block is the genesis block
+	// Check if the block is the genesis block.


jordanschalm · 2017-07-11T20:30:08Z

@david-julien Does this build for you locally? I get:

app/worker.go:76: work.Miner undefined (type BlockWork has no field or method Miner)
app/worker.go:76: too many arguments in call to chain.AppendBlock
	have (*blockchain.Block, <T>)
	want (*blockchain.Block)

Maybe why Travis is failing

coveralls · 2017-07-12T00:25:24Z

Coverage decreased (-0.4%) to 68.185% when pulling 14b93ad on 85-block-extra-data into 2fef206 on dev.

coveralls · 2017-07-12T01:06:34Z

Coverage increased (+0.07%) to 68.703% when pulling eb76c5c on 85-block-extra-data into 2fef206 on dev.

coveralls · 2017-07-12T04:45:42Z

Coverage increased (+0.06%) to 70.05% when pulling d4c1fc0 on 85-block-extra-data into 6788fe6 on dev.

ExtraData field added to block

5a45c27

david-julien requested review from jordanschalm, chadlagore and bfbachmann July 10, 2017 02:43

chadlagore reviewed Jul 10, 2017

View reviewed changes

update after PR review: remove block size checks. All Len() functions…

5cd6ae1

… are now returning len(x.marshal())

david-julien changed the title ~~ExtraData field added to block~~ 85-block-extra-data Jul 11, 2017

remove max block size constant

89c0706

jordanschalm approved these changes Jul 11, 2017

View reviewed changes

bfbachmann approved these changes Jul 11, 2017

View reviewed changes

typo fix

57a796a

pulled dev

14b93ad

added transaction .Len() tests and fixed txBody marshalling error

eb76c5c

Merge branch 'dev' into 85-block-extra-data

d4c1fc0

jordanschalm merged commit f4f8b68 into dev Jul 12, 2017

jordanschalm deleted the 85-block-extra-data branch July 12, 2017 04:47

david-julien mentioned this pull request Jul 14, 2017

Incoming Block Handling Logic #94

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

85-block-extra-data #88

85-block-extra-data #88

david-julien commented Jul 10, 2017

coveralls commented Jul 10, 2017

chadlagore left a comment

chadlagore Jul 10, 2017

david-julien Jul 10, 2017

jordanschalm Jul 11, 2017

david-julien Jul 11, 2017

chadlagore Jul 10, 2017

david-julien Jul 10, 2017 •

edited

Loading

chadlagore Jul 10, 2017 •

edited

Loading

david-julien Jul 10, 2017

chadlagore Jul 10, 2017

coveralls commented Jul 11, 2017

coveralls commented Jul 11, 2017

jordanschalm Jul 11, 2017

jordanschalm Jul 11, 2017

bfbachmann Jul 11, 2017

bfbachmann left a comment

bfbachmann Jul 11, 2017

bfbachmann Jul 11, 2017

bfbachmann Jul 11, 2017

bfbachmann Jul 11, 2017

jordanschalm commented Jul 11, 2017

coveralls commented Jul 12, 2017

coveralls commented Jul 12, 2017

coveralls commented Jul 12, 2017

		@@ -190,6 +190,7 @@ func ValidCloudBase(t *Transaction) (bool, CloudBaseTransactionCode) {

		// ValidGenesisBlock checks whether a block is a valid genesis block.
		func (bc BlockChain) ValidGenesisBlock(gb Block) (bool, GenesisBlockCode) {

85-block-extra-data #88

85-block-extra-data #88

Conversation

david-julien commented Jul 10, 2017

coveralls commented Jul 10, 2017

chadlagore 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

david-julien Jul 10, 2017 • edited Loading

Choose a reason for hiding this comment

chadlagore Jul 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jul 11, 2017

coveralls commented Jul 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bfbachmann 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

jordanschalm commented Jul 11, 2017

coveralls commented Jul 12, 2017

coveralls commented Jul 12, 2017

coveralls commented Jul 12, 2017

david-julien Jul 10, 2017 •

edited

Loading

chadlagore Jul 10, 2017 •

edited

Loading