PR review refactoring and bug fix

consensus/clique/clique.go

@@ -730,46 +730,27 @@ func CliqueRLP(header *types.Header) []byte {
 }
 
 func encodeSigHeader(w io.Writer, header *types.Header) {
-	var err error
-	if header.BaseFee == nil {
-		err = rlp.Encode(w, []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[:len(header.Extra)-crypto.SignatureLength], // Yes, this will panic if extra is too short
-			header.MixDigest,
-			header.Nonce,
-		})
-	} else {
-		err = rlp.Encode(w, []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[:len(header.Extra)-crypto.SignatureLength], // Yes, this will panic if extra is too short
-			header.MixDigest,
-			header.Nonce,
-			header.BaseFee,
-		})
+	headerFields := []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[:len(header.Extra)-crypto.SignatureLength], // Yes, this will panic if extra is too short
+		header.MixDigest,
+		header.Nonce,
 	}
-	if err != nil {
+	if header.BaseFee != nil {
+		headerFields = append(headerFields, header.BaseFee)
+	}
+	if err := rlp.Encode(w, headerFields); err != nil {
 		panic("can't encode: " + err.Error())
 	}
 }

consensus/ethash/consensus.go

@@ -613,41 +613,25 @@ func (ethash *Ethash) FinalizeAndAssemble(chain consensus.ChainReader, header *t
 // SealHash returns the hash of a block prior to it being sealed.
 func (ethash *Ethash) SealHash(header *types.Header) (hash common.Hash) {
 	hasher := sha3.NewLegacyKeccak256()
-
-	if header.BaseFee == nil {
-		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,
-		})
-	} else {
-		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,
-			header.BaseFee,
-		})
+	encodedHeader := []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,
 	}
+	if header.BaseFee != nil {
+		encodedHeader = append(encodedHeader, header.BaseFee)
+	}
+	rlp.Encode(hasher, encodedHeader)
 
 	hasher.Sum(hash[:0])
 	return hash

consensus/misc/forks.go

@@ -62,7 +62,8 @@ func VerifyEIP1559BaseFee(config *params.ChainConfig, header, parent *types.Head
 		}
 		return nil
 	}
-	// Verify the BaseFee is valid if we are past the EIP1559 activation block
+	// If we are past the EIP1559 activation block verify the header's BaseFee is valid by deriving
+	// it from the parent header and validating that they are the same
 	if config.IsEIP1559(header.Number) {
 		if parent.BaseFee == nil {
 			return errMissingParentBaseFee
@@ -91,7 +92,7 @@ func VerifyEIP1559BaseFee(config *params.ChainConfig, header, parent *types.Head
 			}
 			expectedBaseFee.Set(new(big.Int).Add(parent.BaseFee, max))
 		}
-		if expectedBaseFee.Cmp(header.BaseFee) > 0 {
+		if expectedBaseFee.Cmp(header.BaseFee) != 0 {
 			return errInvalidBaseFee
 		}
 		return nil

core/types/block.go

@@ -100,26 +100,7 @@ type headerMarshaling struct {
 
 // EncodeRLP implements rlp.Encoder
 func (h *Header) EncodeRLP(w io.Writer) error {
-	if h.BaseFee == nil {
-		return rlp.Encode(w, []interface{}{
-			h.ParentHash,
-			h.UncleHash,
-			h.Coinbase,
-			h.Root,
-			h.TxHash,
-			h.ReceiptHash,
-			h.Bloom,
-			h.Difficulty,
-			h.Number,
-			h.GasLimit,
-			h.GasUsed,
-			h.Time,
-			h.Extra,
-			h.MixDigest,
-			h.Nonce,
-		})
-	}
-	return rlp.Encode(w, []interface{}{
+	encodedHeader := []interface{}{
 		h.ParentHash,
 		h.UncleHash,
 		h.Coinbase,
@@ -135,8 +116,11 @@ func (h *Header) EncodeRLP(w io.Writer) error {
 		h.Extra,
 		h.MixDigest,
 		h.Nonce,
-		h.BaseFee,
-	})
+	}
+	if h.BaseFee != nil {
+		encodedHeader = append(encodedHeader, h.BaseFee)
+	}
+	return rlp.Encode(w, encodedHeader)
 }
 
 // DecodeRLP implements rlp.Decoder

core/types/transaction_signing.go

@@ -153,28 +153,19 @@ func (s EIP155Signer) SignatureValues(tx *Transaction, sig []byte) (R, S, V *big
 // Hash returns the hash to be signed by the sender.
 // It does not uniquely identify the transaction.
 func (s EIP155Signer) Hash(tx *Transaction) common.Hash {
-	if tx.data.GasPremium == nil && tx.data.FeeCap == nil {
-		return rlpHash([]interface{}{
-			tx.data.AccountNonce,
-			tx.data.Price,
-			tx.data.GasLimit,
-			tx.data.Recipient,
-			tx.data.Amount,
-			tx.data.Payload,
-			s.chainId, uint(0), uint(0),
-		})
-	}
-	return rlpHash([]interface{}{
+	txFields := []interface{}{
 		tx.data.AccountNonce,
 		tx.data.Price,
 		tx.data.GasLimit,
 		tx.data.Recipient,
 		tx.data.Amount,
 		tx.data.Payload,
-		tx.data.GasPremium,
-		tx.data.FeeCap,
-		s.chainId, uint(0), uint(0),
-	})
+	}
+	if tx.data.GasPremium != nil && tx.data.FeeCap != nil {
+		txFields = append(txFields, tx.data.GasPremium, tx.data.FeeCap)
+	}
+	txFields = append(txFields, s.chainId, uint(0), uint(0))
+	return rlpHash(txFields)
 }
 
 // HomesteadTransaction implements TransactionInterface using the
@@ -218,26 +209,18 @@ func (fs FrontierSigner) SignatureValues(tx *Transaction, sig []byte) (r, s, v *
 // Hash returns the hash to be signed by the sender.
 // It does not uniquely identify the transaction.
 func (fs FrontierSigner) Hash(tx *Transaction) common.Hash {
-	if tx.data.GasPremium == nil && tx.data.FeeCap == nil {
-		return rlpHash([]interface{}{
-			tx.data.AccountNonce,
-			tx.data.Price,
-			tx.data.GasLimit,
-			tx.data.Recipient,
-			tx.data.Amount,
-			tx.data.Payload,
-		})
-	}
-	return rlpHash([]interface{}{
+	txFields := []interface{}{
 		tx.data.AccountNonce,
 		tx.data.Price,
 		tx.data.GasLimit,
 		tx.data.Recipient,
 		tx.data.Amount,
 		tx.data.Payload,
-		tx.data.GasPremium,
-		tx.data.FeeCap,
-	})
+	}
+	if tx.data.GasPremium != nil && tx.data.FeeCap != nil {
+		txFields = append(txFields, tx.data.GasPremium, tx.data.FeeCap)
+	}
+	return rlpHash(txFields)
 }
 
 func (fs FrontierSigner) Sender(tx *Transaction) (common.Address, error) {

eth/gasprice/gasprice.go

@@ -418,17 +418,18 @@ func (gpo *Oracle) getBlockPrices(ctx context.Context, signer types.Signer, bloc
 
 	for _, tx := range txs.txs {
 		sender, err := types.Sender(signer, tx)
-		if err == nil && sender != block.Coinbase() {
-			price := tx.GasPrice()
-			if price == nil {
-				price = new(big.Int).Add(block.BaseFee(), tx.GasPremium())
-				if price.Cmp(tx.FeeCap()) > 0 {
-					price.Set(tx.FeeCap())
-				}
-			}
-			ch <- getBlockPricesResult{price, nil}
-			return
+		if err != nil || sender == block.Coinbase() {
+			continue
 		}
+		price := tx.GasPrice()
+		if price == nil {
+			price = new(big.Int).Add(block.BaseFee(), tx.GasPremium())
+			if price.Cmp(tx.FeeCap()) > 0 {
+				price.Set(tx.FeeCap())
+			}
+		}
+		ch <- getBlockPricesResult{price, nil}
+		return
 	}
 	ch <- getBlockPricesResult{nil, nil}
 }
@@ -451,17 +452,18 @@ func (gpo *Oracle) getBlockPremiums(ctx context.Context, signer types.Signer, bl
 
 	for _, tx := range txs.txs {
 		sender, err := types.Sender(signer, tx)
-		if err == nil && sender != block.Coinbase() {
-			premium := tx.GasPremium()
-			if premium == nil {
-				premium = new(big.Int).Sub(tx.GasPrice(), block.BaseFee())
-				if premium.Cmp(common.Big0) < 0 {
-					premium.Set(common.Big0)
-				}
-			}
-			ch <- getBlockPremiumsResult{premium, nil}
-			return
+		if err != nil || sender == block.Coinbase() {
+			continue
 		}
+		premium := tx.GasPremium()
+		if premium == nil {
+			premium = new(big.Int).Sub(tx.GasPrice(), block.BaseFee())
+			if premium.Cmp(common.Big0) < 0 {
+				premium.Set(common.Big0)
+			}
+		}
+		ch <- getBlockPremiumsResult{premium, nil}
+		return
 	}
 	ch <- getBlockPremiumsResult{nil, nil}
 }
@@ -482,14 +484,15 @@ func (gpo *Oracle) getBlockCaps(ctx context.Context, signer types.Signer, blockN
 
 	for _, tx := range txs {
 		sender, err := types.Sender(signer, tx)
-		if err == nil && sender != block.Coinbase() {
-			cap := tx.FeeCap()
-			if cap == nil {
-				cap = tx.GasPrice()
-			}
-			ch <- getBlockCapsResult{cap, nil}
-			return
+		if err != nil || sender == block.Coinbase() {
+			continue
 		}
+		cap := tx.FeeCap()
+		if cap == nil {
+			cap = tx.GasPrice()
+		}
+		ch <- getBlockCapsResult{cap, nil}
+		return
 	}
 	ch <- getBlockCapsResult{nil, nil}
 }