Merge pull request #97 from getamis/feature/use-rlp-for-header-extra-data

core/types: use rlp to encode/decode istanbul extra-data

Author: bailantaotao

consensus/istanbul/backend/backend.go

@@ -132,11 +132,14 @@ func (sb *simpleBackend) Commit(proposal istanbul.Proposal, seals []byte) error
 		return errInvalidProposal
 	}
 
+	h := block.Header()
 	// Append seals into extra-data
-	err := types.AppendIstanbulCommittedSealExtra(block, seals)
+	err := writeCommittedSeals(h, seals)
 	if err != nil {
 		return err
 	}
+	// update block's header
+	block = block.WithSeal(h)
 
 	sb.logger.Info("Committed", "address", sb.Address(), "hash", proposal.Hash(), "number", proposal.Number().Uint64())
 	// - if the proposed and committed blocks are the same, send the proposed hash

consensus/istanbul/backend/backend_test.go

@@ -134,7 +134,7 @@ func TestCommit(t *testing.T) {
 		},
 		{
 			// invalid signature
-			types.ErrInvalidIstanbulCommittedSeal,
+			errInvalidCommittedSeals,
 			nil,
 			func() *types.Block {
 				chain, engine := newBlockChain(1)

consensus/istanbul/backend/engine.go

@@ -119,7 +119,7 @@ func (sb *simpleBackend) verifyHeader(chain consensus.ChainReader, header *types
 	}
 
 	// Ensure that the extra data format is satisfied
-	if err := types.ValidateIstanbulSeal(header); err != nil {
+	if _, err := types.ExtractIstanbulExtra(header); err != nil {
 		return errInvalidExtraDataFormat
 	}
 
@@ -253,7 +253,10 @@ func (sb *simpleBackend) verifyCommittedSeals(chain consensus.ChainReader, heade
 		return err
 	}
 
-	extra := types.ExtractToIstanbul(header)
+	extra, err := types.ExtractIstanbulExtra(header)
+	if err != nil {
+		return err
+	}
 	// The length of Committed seals should be larger than 0
 	if len(extra.CommittedSeal) == 0 {
 		return errEmptyCommittedSeals
@@ -352,7 +355,11 @@ func (sb *simpleBackend) Prepare(chain consensus.ChainReader, header *types.Head
 	}
 
 	// add validators in snapshot to extraData's validators section
-	header.Extra = types.PrepareIstanbulExtra(header, snap.validators())
+	extra, err := prepareExtra(header, snap.validators())
+	if err != nil {
+		return err
+	}
+	header.Extra = extra
 	return nil
 }
 
@@ -450,13 +457,15 @@ func (sb *simpleBackend) updateBlock(parent *types.Header, block *types.Block) (
 		header.Time = big.NewInt(time)
 	}
 	// sign the hash
-	sighash, err := sb.Sign(sigHash(header).Bytes())
+	seal, err := sb.Sign(sigHash(header).Bytes())
 	if err != nil {
 		return nil, err
 	}
 
-	index := types.ExtractToIstanbulIndex(header)
-	copy(header.Extra[index.Seal:index.Seal+types.IstanbulExtraSeal], sighash)
+	err = writeSeal(header, seal)
+	if err != nil {
+		return nil, err
+	}
 
 	return block.WithSeal(header), nil
 }
@@ -562,8 +571,11 @@ func (sb *simpleBackend) snapshot(chain consensus.ChainReader, number uint64, ha
 			if err := sb.VerifyHeader(chain, genesis, false); err != nil {
 				return nil, err
 			}
-			istanbul := types.ExtractToIstanbul(genesis)
-			snap = newSnapshot(sb.config.Epoch, 0, genesis.Hash(), validator.NewSet(istanbul.Validators, sb.config.ProposerPolicy))
+			istanbulExtra, err := types.ExtractIstanbulExtra(genesis)
+			if err != nil {
+				return nil, err
+			}
+			snap = newSnapshot(sb.config.Epoch, 0, genesis.Hash(), validator.NewSet(istanbulExtra.Validators, sb.config.ProposerPolicy))
 			if err := snap.store(sb.db); err != nil {
 				return nil, err
 			}
@@ -620,32 +632,90 @@ func (sb *simpleBackend) snapshot(chain consensus.ChainReader, number uint64, ha
 func sigHash(header *types.Header) (hash common.Hash) {
 	hasher := sha3.NewKeccak256()
 
-	index := types.ExtractToIstanbulIndex(header)
-	rlp.Encode(hasher, []interface{}{
-		header.ParentHash,
-		header.UncleHash,
-		header.Coinbase,
-		header.Root,
-		header.TxHash,
-		header.ReceiptHash,
-		header.Bloom,
-		header.Difficulty,
-		header.Number,
-		header.GasLimit,
-		header.GasUsed,
-		header.Time,
-		header.Extra[:index.Seal], // Yes, this will panic if extra is too short
-		header.MixDigest,
-		header.Nonce,
-	})
+	// Clean seal is required for calculating proposer seal.
+	rlp.Encode(hasher, types.IstanbulFilteredHeader(header, false))
 	hasher.Sum(hash[:0])
 	return hash
 }
 
 // ecrecover extracts the Ethereum account address from a signed header.
 func ecrecover(header *types.Header) (common.Address, error) {
 	// Retrieve the signature from the header extra-data
-	index := types.ExtractToIstanbulIndex(header)
-	signature := header.Extra[index.Seal : index.Seal+types.IstanbulExtraSeal]
-	return istanbul.GetSignatureAddress(sigHash(header).Bytes(), signature)
+	istanbulExtra, err := types.ExtractIstanbulExtra(header)
+	if err != nil {
+		return common.Address{}, err
+	}
+	return istanbul.GetSignatureAddress(sigHash(header).Bytes(), istanbulExtra.Seal)
+}
+
+// prepareExtra returns a extra-data of the given header and validators
+func prepareExtra(header *types.Header, vals []common.Address) ([]byte, error) {
+	var buf bytes.Buffer
+
+	// compensate the lack bytes if header.Extra is not enough IstanbulExtraVanity bytes.
+	if len(header.Extra) < types.IstanbulExtraVanity {
+		header.Extra = append(header.Extra, bytes.Repeat([]byte{0x00}, types.IstanbulExtraVanity-len(header.Extra))...)
+	}
+	buf.Write(header.Extra[:types.IstanbulExtraVanity])
+
+	ist := &types.IstanbulExtra{
+		Validators:    vals,
+		Seal:          []byte{},
+		CommittedSeal: [][]byte{},
+	}
+
+	payload, err := rlp.EncodeToBytes(&ist)
+	if err != nil {
+		return nil, err
+	}
+
+	return append(buf.Bytes(), payload...), nil
+}
+
+// writeSeal writes the extra-data field of the given header with the given seals.
+// suggest to rename to writeSeal.
+func writeSeal(h *types.Header, seal []byte) error {
+	if len(seal)%types.IstanbulExtraSeal != 0 {
+		return errInvalidSignature
+	}
+
+	istanbulExtra, err := types.ExtractIstanbulExtra(h)
+	if err != nil {
+		return err
+	}
+
+	istanbulExtra.Seal = seal
+	payload, err := rlp.EncodeToBytes(&istanbulExtra)
+	if err != nil {
+		return err
+	}
+
+	h.Extra = append(h.Extra[:types.IstanbulExtraVanity], payload...)
+	return nil
+}
+
+// writeCommittedSeals writes the extra-data field of a block header with given committed seals.
+func writeCommittedSeals(h *types.Header, committedSeals []byte) error {
+	if len(committedSeals)%types.IstanbulExtraSeal != 0 {
+		return errInvalidCommittedSeals
+	}
+
+	istanbulExtra, err := types.ExtractIstanbulExtra(h)
+	if err != nil {
+		return err
+	}
+
+	istanbulExtra.CommittedSeal = make([][]byte, len(committedSeals)/types.IstanbulExtraSeal)
+	for i := 0; i < len(istanbulExtra.CommittedSeal); i++ {
+		istanbulExtra.CommittedSeal[i] = make([]byte, types.IstanbulExtraSeal)
+		copy(istanbulExtra.CommittedSeal[i][:], committedSeals[i*types.IstanbulExtraSeal:])
+	}
+
+	payload, err := rlp.EncodeToBytes(&istanbulExtra)
+	if err != nil {
+		return err
+	}
+
+	h.Extra = append(h.Extra[:types.IstanbulExtraVanity], payload...)
+	return nil
 }

consensus/istanbul/backend/engine_test.go

@@ -35,6 +35,7 @@ import (
 	"github.com/ethereum/go-ethereum/ethdb"
 	"github.com/ethereum/go-ethereum/event"
 	"github.com/ethereum/go-ethereum/params"
+	"github.com/ethereum/go-ethereum/rlp"
 )
 
 // in this test, we can set n to 1, and it means we can process Istanbul and commit a
@@ -105,22 +106,23 @@ func getGenesisAndKeys(n int) (*core.Genesis, []*ecdsa.PrivateKey) {
 }
 
 func appendValidators(genesis *core.Genesis, addrs []common.Address) {
+
 	if len(genesis.ExtraData) < types.IstanbulExtraVanity {
 		genesis.ExtraData = append(genesis.ExtraData, bytes.Repeat([]byte{0x00}, types.IstanbulExtraVanity)...)
 	}
 	genesis.ExtraData = genesis.ExtraData[:types.IstanbulExtraVanity]
 
-	validatorSize := byte(len(addrs))
-	genesis.ExtraData = append(genesis.ExtraData, validatorSize)
-
-	for _, addr := range addrs {
-		genesis.ExtraData = append(genesis.ExtraData, addr[:]...)
+	ist := &types.IstanbulExtra{
+		Validators:    addrs,
+		Seal:          []byte{},
+		CommittedSeal: [][]byte{},
 	}
-	genesis.ExtraData = append(genesis.ExtraData, make([]byte, types.IstanbulExtraSeal)...)
 
-	// committed seal
-	genesis.ExtraData = append(genesis.ExtraData, byte(0x01))
-	genesis.ExtraData = append(genesis.ExtraData, make([]byte, types.IstanbulExtraSeal)...)
+	istPayload, err := rlp.EncodeToBytes(&ist)
+	if err != nil {
+		panic("failed to encode istanbul extra")
+	}
+	genesis.ExtraData = append(genesis.ExtraData, istPayload...)
 }
 
 func makeHeader(parent *types.Block, config *istanbul.Config) *types.Header {
@@ -474,3 +476,120 @@ OUT3:
 		}
 	}
 }
+
+func TestPrepareExtra(t *testing.T) {
+	validators := make([]common.Address, 4)
+	validators[0] = common.BytesToAddress(hexutil.MustDecode("0x44add0ec310f115a0e603b2d7db9f067778eaf8a"))
+	validators[1] = common.BytesToAddress(hexutil.MustDecode("0x294fc7e8f22b3bcdcf955dd7ff3ba2ed833f8212"))
+	validators[2] = common.BytesToAddress(hexutil.MustDecode("0x6beaaed781d2d2ab6350f5c4566a2c6eaac407a6"))
+	validators[3] = common.BytesToAddress(hexutil.MustDecode("0x8be76812f765c24641ec63dc2852b378aba2b440"))
+
+	vanity := make([]byte, types.IstanbulExtraVanity)
+	expectedResult := append(vanity, hexutil.MustDecode("0xf858f8549444add0ec310f115a0e603b2d7db9f067778eaf8a94294fc7e8f22b3bcdcf955dd7ff3ba2ed833f8212946beaaed781d2d2ab6350f5c4566a2c6eaac407a6948be76812f765c24641ec63dc2852b378aba2b44080c0")...)
+
+	h := &types.Header{
+		Extra: vanity,
+	}
+
+	payload, err := prepareExtra(h, validators)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if !reflect.DeepEqual(payload, expectedResult) {
+		t.Errorf("expected: %v, but got: %v", expectedResult, payload)
+	}
+
+	// append useless information to extra-data
+	h.Extra = append(vanity, make([]byte, 15)...)
+
+	payload, err = prepareExtra(h, validators)
+	if !reflect.DeepEqual(payload, expectedResult) {
+		t.Errorf("expected: %v, got: %v", expectedResult, payload)
+	}
+}
+
+func TestWriteSeal(t *testing.T) {
+	vanity := bytes.Repeat([]byte{0x00}, types.IstanbulExtraVanity)
+	istRawData := hexutil.MustDecode("0xf858f8549444add0ec310f115a0e603b2d7db9f067778eaf8a94294fc7e8f22b3bcdcf955dd7ff3ba2ed833f8212946beaaed781d2d2ab6350f5c4566a2c6eaac407a6948be76812f765c24641ec63dc2852b378aba2b44080c0")
+	expectedSeal := append([]byte{1, 2, 3}, bytes.Repeat([]byte{0x00}, types.IstanbulExtraSeal-3)...)
+	expectedIstExtra := &types.IstanbulExtra{
+		Validators: []common.Address{
+			common.BytesToAddress(hexutil.MustDecode("0x44add0ec310f115a0e603b2d7db9f067778eaf8a")),
+			common.BytesToAddress(hexutil.MustDecode("0x294fc7e8f22b3bcdcf955dd7ff3ba2ed833f8212")),
+			common.BytesToAddress(hexutil.MustDecode("0x6beaaed781d2d2ab6350f5c4566a2c6eaac407a6")),
+			common.BytesToAddress(hexutil.MustDecode("0x8be76812f765c24641ec63dc2852b378aba2b440")),
+		},
+		Seal:          expectedSeal,
+		CommittedSeal: [][]byte{},
+	}
+	var expectedErr error
+
+	h := &types.Header{
+		Extra: append(vanity, istRawData...),
+	}
+
+	// normal case
+	err := writeSeal(h, expectedSeal)
+	if err != expectedErr {
+		t.Errorf("expected: %v, but got: %v", expectedErr, err)
+	}
+
+	// verify istanbul extra-data
+	istExtra, err := types.ExtractIstanbulExtra(h)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if !reflect.DeepEqual(istExtra, expectedIstExtra) {
+		t.Errorf("expected: %v, got: %v", expectedIstExtra, istExtra)
+	}
+
+	// invalid seal
+	unexpectedSeal := append(expectedSeal, make([]byte, 1)...)
+	err = writeSeal(h, unexpectedSeal)
+	if err != errInvalidSignature {
+		t.Errorf("expected: %v, got: %v", errInvalidSignature, err)
+	}
+}
+
+func TestWriteCommittedSeals(t *testing.T) {
+	vanity := bytes.Repeat([]byte{0x00}, types.IstanbulExtraVanity)
+	istRawData := hexutil.MustDecode("0xf858f8549444add0ec310f115a0e603b2d7db9f067778eaf8a94294fc7e8f22b3bcdcf955dd7ff3ba2ed833f8212946beaaed781d2d2ab6350f5c4566a2c6eaac407a6948be76812f765c24641ec63dc2852b378aba2b44080c0")
+	expectedCommittedSeal := append([]byte{1, 2, 3}, bytes.Repeat([]byte{0x00}, types.IstanbulExtraSeal-3)...)
+	expectedIstExtra := &types.IstanbulExtra{
+		Validators: []common.Address{
+			common.BytesToAddress(hexutil.MustDecode("0x44add0ec310f115a0e603b2d7db9f067778eaf8a")),
+			common.BytesToAddress(hexutil.MustDecode("0x294fc7e8f22b3bcdcf955dd7ff3ba2ed833f8212")),
+			common.BytesToAddress(hexutil.MustDecode("0x6beaaed781d2d2ab6350f5c4566a2c6eaac407a6")),
+			common.BytesToAddress(hexutil.MustDecode("0x8be76812f765c24641ec63dc2852b378aba2b440")),
+		},
+		Seal:          []byte{},
+		CommittedSeal: [][]byte{expectedCommittedSeal},
+	}
+	var expectedErr error
+
+	h := &types.Header{
+		Extra: append(vanity, istRawData...),
+	}
+
+	// normal case
+	err := writeCommittedSeals(h, expectedCommittedSeal)
+	if err != expectedErr {
+		t.Errorf("expected: %v, but got: %v", expectedErr, err)
+	}
+
+	// verify istanbul extra-data
+	istExtra, err := types.ExtractIstanbulExtra(h)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if !reflect.DeepEqual(istExtra, expectedIstExtra) {
+		t.Errorf("expected: %v, got: %v", expectedIstExtra, istExtra)
+	}
+
+	// invalid seal
+	unexpectedCommittedSeal := append(expectedCommittedSeal, make([]byte, 1)...)
+	err = writeCommittedSeals(h, unexpectedCommittedSeal)
+	if err != errInvalidCommittedSeals {
+		t.Errorf("expected: %v, got: %v", errInvalidCommittedSeals, err)
+	}
+}

consensus/istanbul/backend/snapshot_test.go

@@ -59,7 +59,8 @@ func (ap *testerAccountPool) sign(header *types.Header, validator string) {
 	// Sign the header and embed the signature in extra data
 	hashData := crypto.Keccak256([]byte(sigHash(header).Bytes()))
 	sig, _ := crypto.Sign(hashData, ap.accounts[validator])
-	copy(header.Extra[len(header.Extra)-types.IstanbulExtraSeal:], sig)
+
+	writeSeal(header, sig)
 }
 
 func (ap *testerAccountPool) address(account string) common.Address {
@@ -334,7 +335,8 @@ func TestVoting(t *testing.T) {
 			Mixhash:    types.IstanbulDigest,
 		}
 		b, _ := genesis.ToBlock()
-		genesis.ExtraData = types.PrepareIstanbulExtra(b.Header(), validators)
+		extra, _ := prepareExtra(b.Header(), validators)
+		genesis.ExtraData = extra
 		// Create a pristine blockchain with the genesis injected
 		db, _ := ethdb.NewMemDatabase()
 		genesis.Commit(db)
@@ -357,7 +359,8 @@ func TestVoting(t *testing.T) {
 				Difficulty: defaultDifficulty,
 				MixDigest:  types.IstanbulDigest,
 			}
-			headers[j].Extra = types.PrepareIstanbulExtra(headers[j], validators)
+			extra, _ := prepareExtra(headers[j], validators)
+			headers[j].Extra = extra
 			if j > 0 {
 				headers[j].ParentHash = headers[j-1].Hash()
 			}