From e4b0cf8f3730625c6a4affa19a9288fd05e3b27b Mon Sep 17 00:00:00 2001 From: Darioush Jalali Date: Wed, 24 Jul 2024 12:32:13 -0700 Subject: [PATCH 1/8] eupgrade/cancun: verify no blobs in header --- consensus/dummy/consensus.go | 3 +++ core/state_processor_test.go | 3 ++- internal/ethapi/api_test.go | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/consensus/dummy/consensus.go b/consensus/dummy/consensus.go index 797966156b..82f7b6047e 100644 --- a/consensus/dummy/consensus.go +++ b/consensus/dummy/consensus.go @@ -263,6 +263,9 @@ func (self *DummyEngine) verifyHeader(chain consensus.ChainHeaderReader, header if err := eip4844.VerifyEIP4844Header(parent, header); err != nil { return err } + if *header.BlobGasUsed > 0 { // VerifyEIP4844Header ensures excessBlobGas is non-nil + return fmt.Errorf("blobs not enabled on avalanche networks: have %d, expected 0", *header.BlobGasUsed) + } } return nil } diff --git a/core/state_processor_test.go b/core/state_processor_test.go index f6dcb789a6..47e5757376 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -236,7 +236,8 @@ func TestStateProcessorErrors(t *testing.T) { want: "could not apply tx 0 [0x6c11015985ce82db691d7b2d017acda296db88b811c3c60dc71449c76256c716]: max fee per gas less than block base fee: address 0x71562b71999873DB5b286dF957af199Ec94617F7, maxFeePerGas: 1, baseFee: 225000000000", }, } { - block := GenerateBadBlock(gspec.ToBlock(), dummy.NewCoinbaseFaker(), tt.txs, gspec.Config) + // FullFaker used to skip header verification that enforces no blobs. + block := GenerateBadBlock(gspec.ToBlock(), dummy.NewFullFaker(), tt.txs, gspec.Config) _, err := blockchain.InsertChain(types.Blocks{block}) if err == nil { t.Fatal("block imported without errors") diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index 9d8a26045a..b74507c1be 100644 --- a/internal/ethapi/api_test.go +++ b/internal/ethapi/api_test.go @@ -1436,7 +1436,8 @@ func setupReceiptBackend(t *testing.T, genBlocks int) (*testBackend, []common.Ha // Set the terminal total difficulty in the config // genesis.Config.TerminalTotalDifficulty = big.NewInt(0) // genesis.Config.TerminalTotalDifficultyPassed = true - backend := newTestBackend(t, genBlocks, genesis, dummy.NewCoinbaseFaker(), func(i int, b *core.BlockGen) { + // FullFaker used to skip header verification that enforces no blobs. + backend := newTestBackend(t, genBlocks, genesis, dummy.NewFullFaker(), func(i int, b *core.BlockGen) { var ( tx *types.Transaction err error From 4ec90b04242a4ea4f3466927d6db8303367276f5 Mon Sep 17 00:00:00 2001 From: Darioush Jalali Date: Wed, 24 Jul 2024 12:34:41 -0700 Subject: [PATCH 2/8] Update consensus/dummy/consensus.go Signed-off-by: Darioush Jalali --- consensus/dummy/consensus.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/dummy/consensus.go b/consensus/dummy/consensus.go index 82f7b6047e..3fc9f48ac8 100644 --- a/consensus/dummy/consensus.go +++ b/consensus/dummy/consensus.go @@ -263,7 +263,7 @@ func (self *DummyEngine) verifyHeader(chain consensus.ChainHeaderReader, header if err := eip4844.VerifyEIP4844Header(parent, header); err != nil { return err } - if *header.BlobGasUsed > 0 { // VerifyEIP4844Header ensures excessBlobGas is non-nil + if *header.BlobGasUsed > 0 { // VerifyEIP4844Header ensures BlobGasUsed is non-nil return fmt.Errorf("blobs not enabled on avalanche networks: have %d, expected 0", *header.BlobGasUsed) } } From 16a9f01437fe81b0e49fa85b8f2e0b8c608f2999 Mon Sep 17 00:00:00 2001 From: Darioush Jalali Date: Wed, 24 Jul 2024 12:59:07 -0700 Subject: [PATCH 3/8] fix ut --- core/state_processor_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/state_processor_test.go b/core/state_processor_test.go index 47e5757376..0c27446964 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -117,7 +117,8 @@ func TestStateProcessorErrors(t *testing.T) { }, GasLimit: params.CortinaGasLimit, } - blockchain, _ = NewBlockChain(db, DefaultCacheConfig, gspec, dummy.NewCoinbaseFaker(), vm.Config{}, common.Hash{}, false) + // FullFaker used to skip header verification that enforces no blobs. + blockchain, _ = NewBlockChain(db, DefaultCacheConfig, gspec, dummy.NewFullFaker(), vm.Config{}, common.Hash{}, false) tooBigInitCode = [params.MaxInitCodeSize + 1]byte{} ) From 6190ed42d7f06485dc7ec895388731ac67375159 Mon Sep 17 00:00:00 2001 From: Darioush Jalali Date: Thu, 25 Jul 2024 10:01:41 -0700 Subject: [PATCH 4/8] review comments --- consensus/dummy/consensus.go | 2 +- plugin/evm/block_verification.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/consensus/dummy/consensus.go b/consensus/dummy/consensus.go index 3fc9f48ac8..717f41af66 100644 --- a/consensus/dummy/consensus.go +++ b/consensus/dummy/consensus.go @@ -264,7 +264,7 @@ func (self *DummyEngine) verifyHeader(chain consensus.ChainHeaderReader, header return err } if *header.BlobGasUsed > 0 { // VerifyEIP4844Header ensures BlobGasUsed is non-nil - return fmt.Errorf("blobs not enabled on avalanche networks: have %d, expected 0", *header.BlobGasUsed) + return fmt.Errorf("blobs not enabled on avalanche networks: used %d blob gas, expected 0", *header.BlobGasUsed) } } return nil diff --git a/plugin/evm/block_verification.go b/plugin/evm/block_verification.go index 38f2bc29fa..f0f8db1658 100644 --- a/plugin/evm/block_verification.go +++ b/plugin/evm/block_verification.go @@ -287,6 +287,11 @@ func (v blockValidator) SyntacticVerify(b *Block, rules params.Rules) error { case *ethHeader.ParentBeaconRoot != (common.Hash{}): return fmt.Errorf("invalid parentBeaconRoot: have %x, expected empty hash", ethHeader.ParentBeaconRoot) } + if ethHeader.BlobGasUsed == nil { + return fmt.Errorf("blob gas used must not be nil in Cancun") + } else if *ethHeader.BlobGasUsed > 0 { + return fmt.Errorf("blobs not enabled on avalanche networks: used %d blob gas, expected 0", *ethHeader.BlobGasUsed) + } } return nil } From c53a5ea41ce1cd72899eb44ed67e5a10a48089d6 Mon Sep 17 00:00:00 2001 From: Darioush Jalali Date: Fri, 26 Jul 2024 09:51:15 -0700 Subject: [PATCH 5/8] add UT --- plugin/evm/vm_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index bcadcb7271..86b79b707c 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -4065,3 +4065,44 @@ func TestParentBeaconRootBlock(t *testing.T) { }) } } + +func TestNoBlobsAllowed(t *testing.T) { + require := require.New(t) + importAmount := uint64(1000000000) + issuer, vm, _, _, _ := GenesisVMWithUTXOs(t, true, genesisJSONCancun, "", "", map[ids.ShortID]uint64{ + testShortIDAddrs[0]: importAmount, + }) + defer func() { require.NoError(vm.Shutdown(context.Background())) }() + + // Build a valid block in Cancun + importTx, err := vm.newImportTx(vm.ctx.XChainID, testEthAddrs[0], initialBaseFee, []*secp256k1.PrivateKey{testKeys[0]}) + if err != nil { + t.Fatal(err) + } + if err := vm.mempool.AddLocalTx(importTx); err != nil { + t.Fatal(err) + } + <-issuer + blk, err := vm.BuildBlock(context.Background()) + if err != nil { + t.Fatalf("Failed to build block with import transaction: %s", err) + } + + // Modify the block to have a non-zero blob gas + ethBlock := blk.(*chain.BlockWrapper).Block.(*Block).ethBlock + header := types.CopyHeader(ethBlock.Header()) + blobGasUsed := uint64(params.BlobTxBlobGasPerBlob) + header.BlobGasUsed = &blobGasUsed + modifiedEthBlock := types.NewBlockWithExtData( + header, nil, nil, nil, new(trie.Trie), ethBlock.ExtData(), false) + modifiedBlock, err := vm.newBlock(modifiedEthBlock) + if err != nil { + t.Fatal(err) + } + + // Verification should fail + _, err = vm.ParseBlock(context.Background(), modifiedBlock.Bytes()) + require.NoError(err) + err = modifiedBlock.Verify(context.Background()) + require.ErrorContains(err, "blobs not enabled on avalanche networks") +} From b069f270cced2d3024c2dfb06f219ca95e8a9675 Mon Sep 17 00:00:00 2001 From: Darioush Jalali Date: Fri, 26 Jul 2024 09:52:48 -0700 Subject: [PATCH 6/8] use require --- plugin/evm/vm_test.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index 86b79b707c..92096a0ce4 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -4076,17 +4076,11 @@ func TestNoBlobsAllowed(t *testing.T) { // Build a valid block in Cancun importTx, err := vm.newImportTx(vm.ctx.XChainID, testEthAddrs[0], initialBaseFee, []*secp256k1.PrivateKey{testKeys[0]}) - if err != nil { - t.Fatal(err) - } - if err := vm.mempool.AddLocalTx(importTx); err != nil { - t.Fatal(err) - } + require.NoError(err) + require.NoError(vm.mempool.AddLocalTx(importTx)) <-issuer blk, err := vm.BuildBlock(context.Background()) - if err != nil { - t.Fatalf("Failed to build block with import transaction: %s", err) - } + require.NoError(err) // Modify the block to have a non-zero blob gas ethBlock := blk.(*chain.BlockWrapper).Block.(*Block).ethBlock @@ -4096,9 +4090,7 @@ func TestNoBlobsAllowed(t *testing.T) { modifiedEthBlock := types.NewBlockWithExtData( header, nil, nil, nil, new(trie.Trie), ethBlock.ExtData(), false) modifiedBlock, err := vm.newBlock(modifiedEthBlock) - if err != nil { - t.Fatal(err) - } + require.NoError(err) // Verification should fail _, err = vm.ParseBlock(context.Background(), modifiedBlock.Bytes()) From 34d6f07489e22eaf906874737048766a1661dbe5 Mon Sep 17 00:00:00 2001 From: Darioush Jalali Date: Fri, 26 Jul 2024 10:26:49 -0700 Subject: [PATCH 7/8] fix ut --- plugin/evm/vm_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index 92096a0ce4..de67fcd6eb 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -4094,7 +4094,7 @@ func TestNoBlobsAllowed(t *testing.T) { // Verification should fail _, err = vm.ParseBlock(context.Background(), modifiedBlock.Bytes()) - require.NoError(err) + require.ErrorContains(err, "blobs not enabled on avalanche networks") err = modifiedBlock.Verify(context.Background()) require.ErrorContains(err, "blobs not enabled on avalanche networks") } From 3f2a17af594b8375f53bf22dc24445bee94ed1ba Mon Sep 17 00:00:00 2001 From: Darioush Jalali Date: Wed, 31 Jul 2024 10:32:47 -0700 Subject: [PATCH 8/8] update UT to make a blob TX --- plugin/evm/vm_test.go | 63 ++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index 20841eca33..ecd5fc626a 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -21,7 +21,9 @@ import ( "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rlp" + "github.com/holiman/uint256" + "github.com/ava-labs/coreth/constants" "github.com/ava-labs/coreth/eth/filters" "github.com/ava-labs/coreth/internal/ethapi" "github.com/ava-labs/coreth/metrics" @@ -41,7 +43,6 @@ import ( "github.com/ava-labs/avalanchego/snow" "github.com/ava-labs/avalanchego/snow/validators" "github.com/ava-labs/avalanchego/utils/cb58" - "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/crypto/bls" "github.com/ava-labs/avalanchego/utils/crypto/secp256k1" "github.com/ava-labs/avalanchego/utils/formatting" @@ -55,6 +56,7 @@ import ( "github.com/ava-labs/avalanchego/vms/secp256k1fx" commonEng "github.com/ava-labs/avalanchego/snow/engine/common" + constantsEng "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/coreth/consensus/dummy" "github.com/ava-labs/coreth/core" @@ -217,9 +219,9 @@ func NewContext() *snow.Context { ctx.ValidatorState = &validators.TestState{ GetSubnetIDF: func(_ context.Context, chainID ids.ID) (ids.ID, error) { subnetID, ok := map[ids.ID]ids.ID{ - constants.PlatformChainID: constants.PrimaryNetworkID, - testXChainID: constants.PrimaryNetworkID, - testCChainID: constants.PrimaryNetworkID, + constantsEng.PlatformChainID: constantsEng.PrimaryNetworkID, + testXChainID: constantsEng.PrimaryNetworkID, + testCChainID: constantsEng.PrimaryNetworkID, }[chainID] if !ok { return ids.Empty, errors.New("unknown chain") @@ -4091,35 +4093,46 @@ func TestParentBeaconRootBlock(t *testing.T) { } func TestNoBlobsAllowed(t *testing.T) { + ctx := context.Background() require := require.New(t) - importAmount := uint64(1000000000) - issuer, vm, _, _, _ := GenesisVMWithUTXOs(t, true, genesisJSONCancun, "", "", map[ids.ShortID]uint64{ - testShortIDAddrs[0]: importAmount, - }) - defer func() { require.NoError(vm.Shutdown(context.Background())) }() - // Build a valid block in Cancun - importTx, err := vm.newImportTx(vm.ctx.XChainID, testEthAddrs[0], initialBaseFee, []*secp256k1.PrivateKey{testKeys[0]}) - require.NoError(err) - require.NoError(vm.mempool.AddLocalTx(importTx)) - <-issuer - blk, err := vm.BuildBlock(context.Background()) + gspec := new(core.Genesis) + err := json.Unmarshal([]byte(genesisJSONCancun), gspec) require.NoError(err) - // Modify the block to have a non-zero blob gas - ethBlock := blk.(*chain.BlockWrapper).Block.(*Block).ethBlock - header := types.CopyHeader(ethBlock.Header()) - blobGasUsed := uint64(params.BlobTxBlobGasPerBlob) - header.BlobGasUsed = &blobGasUsed - modifiedEthBlock := types.NewBlockWithExtData( - header, nil, nil, nil, new(trie.Trie), ethBlock.ExtData(), false) - modifiedBlock, err := vm.newBlock(modifiedEthBlock) + // Make one block with a single blob tx + signer := types.NewCancunSigner(gspec.Config.ChainID) + blockGen := func(_ int, b *core.BlockGen) { + b.SetCoinbase(constants.BlackholeAddr) + fee := big.NewInt(500) + fee.Add(fee, b.BaseFee()) + tx, err := types.SignTx(types.NewTx(&types.BlobTx{ + Nonce: 0, + GasTipCap: uint256.NewInt(1), + GasFeeCap: uint256.MustFromBig(fee), + Gas: params.TxGas, + To: testEthAddrs[0], + BlobFeeCap: uint256.NewInt(1), + BlobHashes: []common.Hash{{1}}, + Value: new(uint256.Int), + }), signer, testKeys[0].ToECDSA()) + require.NoError(err) + b.AddTx(tx) + } + // FullFaker used to skip header verification so we can generate a block with blobs + _, blocks, _, err := core.GenerateChainWithGenesis(gspec, dummy.NewFullFaker(), 1, 10, blockGen) require.NoError(err) + // Create a VM with the genesis (will use header verification) + _, vm, _, _, _ := GenesisVM(t, true, genesisJSONCancun, "", "") + defer func() { require.NoError(vm.Shutdown(ctx)) }() + // Verification should fail - _, err = vm.ParseBlock(context.Background(), modifiedBlock.Bytes()) + vmBlock, err := vm.newBlock(blocks[0]) + require.NoError(err) + _, err = vm.ParseBlock(ctx, vmBlock.Bytes()) require.ErrorContains(err, "blobs not enabled on avalanche networks") - err = modifiedBlock.Verify(context.Background()) + err = vmBlock.Verify(ctx) require.ErrorContains(err, "blobs not enabled on avalanche networks") }