Skip to content

Commit

Permalink
Refactor header.Extra verification (#788)
Browse files Browse the repository at this point in the history
  • Loading branch information
StephenButtolph authored and ceyonur committed Feb 26, 2025
1 parent a0e0781 commit e501c94
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 41 deletions.
25 changes: 10 additions & 15 deletions consensus/dummy/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"github.com/ava-labs/subnet-evm/trie"
"github.com/ava-labs/subnet-evm/vmerrs"
"github.com/ethereum/go-ethereum/common"

customheader "github.com/ava-labs/subnet-evm/plugin/evm/header"
)

var (
Expand Down Expand Up @@ -200,25 +202,18 @@ func (eng *DummyEngine) verifyHeaderGasFields(config *params.ChainConfig, header

// modified from consensus.go
func (eng *DummyEngine) verifyHeader(chain consensus.ChainHeaderReader, header *types.Header, parent *types.Header, uncle bool) error {
config := chain.Config()
// Ensure that we do not verify an uncle
if uncle {
return errUnclesUnsupported
}
switch {
case config.IsDurango(header.Time):
if len(header.Extra) < params.DynamicFeeExtraDataSize {
return fmt.Errorf("expected extra-data field length >= %d, found %d", params.DynamicFeeExtraDataSize, len(header.Extra))
}
case config.IsSubnetEVM(header.Time):
if len(header.Extra) != params.DynamicFeeExtraDataSize {
return fmt.Errorf("expected extra-data field to be: %d, but found %d", params.DynamicFeeExtraDataSize, len(header.Extra))
}
default:
if uint64(len(header.Extra)) > params.MaximumExtraDataSize {
return fmt.Errorf("extra-data too long: %d > %d", len(header.Extra), params.MaximumExtraDataSize)
}

// Verify the extra data is well-formed.
config := chain.Config()
rules := config.GetAvalancheRules(header.Time)
if err := customheader.VerifyExtra(rules, header.Extra); err != nil {
return err
}

// Ensure gas-related header fields are correct
if err := eng.verifyHeaderGasFields(config, header, parent, chain); err != nil {
return err
Expand All @@ -242,7 +237,7 @@ func (eng *DummyEngine) verifyHeader(chain consensus.ChainHeaderReader, header *
return consensus.ErrInvalidNumber
}
// Verify the existence / non-existence of excessBlobGas
cancun := chain.Config().IsCancun(header.Number, header.Time)
cancun := config.IsCancun(header.Number, header.Time)
if !cancun {
switch {
case header.ExcessBlobGas != nil:
Expand Down
2 changes: 1 addition & 1 deletion consensus/dummy/dynamic_fee_window.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"fmt"

"github.com/ava-labs/avalanchego/utils/wrappers"
"github.com/ava-labs/coreth/params"
"github.com/ava-labs/subnet-evm/params"
"github.com/ethereum/go-ethereum/common/math"
)

Expand Down
2 changes: 1 addition & 1 deletion consensus/dummy/dynamic_fee_window_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"strconv"
"testing"

"github.com/ava-labs/coreth/params"
"github.com/ava-labs/subnet-evm/params"
"github.com/ethereum/go-ethereum/common/math"
"github.com/stretchr/testify/require"
)
Expand Down
28 changes: 4 additions & 24 deletions plugin/evm/block_verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/ava-labs/subnet-evm/core/types"
"github.com/ava-labs/subnet-evm/params"
"github.com/ava-labs/subnet-evm/plugin/evm/header"
"github.com/ava-labs/subnet-evm/trie"
)

Expand Down Expand Up @@ -58,30 +59,9 @@ func (v blockValidator) SyntacticVerify(b *Block, rules params.Rules) error {
return fmt.Errorf("invalid mix digest: %v", ethHeader.MixDigest)
}

// Check that the size of the header's Extra data field is correct for [rules].
headerExtraDataSize := len(ethHeader.Extra)
switch {
case rules.IsDurango:
if headerExtraDataSize < params.DynamicFeeExtraDataSize {
return fmt.Errorf(
"expected header ExtraData to be len >= %d but got %d",
params.DynamicFeeExtraDataSize, len(ethHeader.Extra),
)
}
case rules.IsSubnetEVM:
if headerExtraDataSize != params.DynamicFeeExtraDataSize {
return fmt.Errorf(
"expected header ExtraData to be len %d but got %d",
params.DynamicFeeExtraDataSize, headerExtraDataSize,
)
}
default:
if uint64(headerExtraDataSize) > params.MaximumExtraDataSize {
return fmt.Errorf(
"expected header ExtraData to be <= %d but got %d",
params.MaximumExtraDataSize, headerExtraDataSize,
)
}
// Verify the extra data is well-formed.
if err := header.VerifyExtra(rules.AvalancheRules, ethHeader.Extra); err != nil {
return err
}

if rules.IsSubnetEVM {
Expand Down
49 changes: 49 additions & 0 deletions plugin/evm/header/extra.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// (c) 2025, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package header

import (
"errors"
"fmt"

"github.com/ava-labs/subnet-evm/params"
)

var errInvalidExtraLength = errors.New("invalid header.Extra length")

// VerifyExtra verifies that the header's Extra field is correctly formatted for
// [rules].
func VerifyExtra(rules params.AvalancheRules, extra []byte) error {
extraLen := len(extra)
switch {
case rules.IsDurango:
if extraLen < params.DynamicFeeExtraDataSize {
return fmt.Errorf(
"%w: expected >= %d but got %d",
errInvalidExtraLength,
params.DynamicFeeExtraDataSize,
extraLen,
)
}
case rules.IsSubnetEVM:
if extraLen != params.DynamicFeeExtraDataSize {
return fmt.Errorf(
"%w: expected %d but got %d",
errInvalidExtraLength,
params.DynamicFeeExtraDataSize,
extraLen,
)
}
default:
if uint64(extraLen) > params.MaximumExtraDataSize {
return fmt.Errorf(
"%w: expected <= %d but got %d",
errInvalidExtraLength,
params.MaximumExtraDataSize,
extraLen,
)
}
}
return nil
}
87 changes: 87 additions & 0 deletions plugin/evm/header/extra_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// (c) 2025, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package header

import (
"testing"

"github.com/ava-labs/subnet-evm/params"
"github.com/stretchr/testify/require"
)

func TestVerifyExtra(t *testing.T) {
tests := []struct {
name string
rules params.AvalancheRules
extra []byte
expected error
}{
{
name: "initial_valid",
rules: params.AvalancheRules{},
extra: make([]byte, params.MaximumExtraDataSize),
expected: nil,
},
{
name: "initial_invalid",
rules: params.AvalancheRules{},
extra: make([]byte, params.MaximumExtraDataSize+1),
expected: errInvalidExtraLength,
},
{
name: "subnet_evm_valid",
rules: params.AvalancheRules{
IsSubnetEVM: true,
},
extra: make([]byte, params.DynamicFeeExtraDataSize),
expected: nil,
},
{
name: "subnet_invalid_less",
rules: params.AvalancheRules{
IsSubnetEVM: true,
},
extra: make([]byte, params.DynamicFeeExtraDataSize-1),
expected: errInvalidExtraLength,
},
{
name: "subnet_invalid_more",
rules: params.AvalancheRules{
IsSubnetEVM: true,
},
extra: make([]byte, params.DynamicFeeExtraDataSize+1),
expected: errInvalidExtraLength,
},
{
name: "durango_valid_min",
rules: params.AvalancheRules{
IsDurango: true,
},
extra: make([]byte, params.DynamicFeeExtraDataSize),
expected: nil,
},
{
name: "durango_valid_extra",
rules: params.AvalancheRules{
IsDurango: true,
},
extra: make([]byte, params.DynamicFeeExtraDataSize+1),
expected: nil,
},
{
name: "durango_invalid",
rules: params.AvalancheRules{
IsDurango: true,
},
extra: make([]byte, params.DynamicFeeExtraDataSize-1),
expected: errInvalidExtraLength,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := VerifyExtra(test.rules, test.extra)
require.ErrorIs(t, err, test.expected)
})
}
}

0 comments on commit e501c94

Please sign in to comment.