Skip to content

Commit c8a3424

Browse files
JonathanOppenheimerpowersliderceyonur
authored
PARTIAL sync: coreth PR #1206: sentinel err tx apis cleanup (#1757)
Signed-off-by: Tsvetan Dimitrov (tsvetan.dimitrov@avalabs.org) Signed-off-by: Jonathan Oppenheimer <jonathan.oppenheimer@avalabs.org> Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com> Co-authored-by: Tsvetan Dimitrov <tsvetan.dimitrov23@gmail.com> Co-authored-by: Ceyhun Onur <ceyhun.onur@avalabs.org>
1 parent f9d536e commit c8a3424

File tree

6 files changed

+75
-61
lines changed

6 files changed

+75
-61
lines changed

‎consensus/dummy/consensus.go‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,16 @@ import (
2424
"github.com/ava-labs/subnet-evm/utils"
2525
)
2626

27-
var errUnclesUnsupported = errors.New("uncles unsupported")
27+
var (
28+
errUnclesUnsupported = errors.New("uncles unsupported")
29+
ErrInvalidBlockGasCost = errors.New("invalid blockGasCost")
30+
errInvalidExcessBlobGasBeforeCancun = errors.New("invalid excessBlobGas before cancun")
31+
errInvalidBlobGasUsedBeforeCancun = errors.New("invalid blobGasUsed before cancun")
32+
errInvalidParentBeaconRootBeforeCancun = errors.New("invalid parentBeaconRoot before cancun")
33+
errMissingParentBeaconRoot = errors.New("header is missing beaconRoot")
34+
errNonEmptyParentBeaconRoot = errors.New("invalid non-empty parentBeaconRoot")
35+
errBlobsNotEnabled = errors.New("blobs not enabled on avalanche networks")
36+
)
2837

2938
type Mode struct {
3039
ModeSkipHeader bool
@@ -173,24 +182,24 @@ func (eng *DummyEngine) verifyHeader(chain consensus.ChainHeaderReader, header *
173182
if !cancun {
174183
switch {
175184
case header.ExcessBlobGas != nil:
176-
return fmt.Errorf("invalid excessBlobGas: have %d, expected nil", *header.ExcessBlobGas)
185+
return fmt.Errorf("%w: have %d, expected nil", errInvalidExcessBlobGasBeforeCancun, *header.ExcessBlobGas)
177186
case header.BlobGasUsed != nil:
178-
return fmt.Errorf("invalid blobGasUsed: have %d, expected nil", *header.BlobGasUsed)
187+
return fmt.Errorf("%w: have %d, expected nil", errInvalidBlobGasUsedBeforeCancun, *header.BlobGasUsed)
179188
case header.ParentBeaconRoot != nil:
180-
return fmt.Errorf("invalid parentBeaconRoot, have %#x, expected nil", *header.ParentBeaconRoot)
189+
return fmt.Errorf("%w: have %#x, expected nil", errInvalidParentBeaconRootBeforeCancun, *header.ParentBeaconRoot)
181190
}
182191
} else {
183192
if header.ParentBeaconRoot == nil {
184-
return errors.New("header is missing beaconRoot")
193+
return errMissingParentBeaconRoot
185194
}
186195
if *header.ParentBeaconRoot != (common.Hash{}) {
187-
return fmt.Errorf("invalid parentBeaconRoot, have %#x, expected empty", *header.ParentBeaconRoot)
196+
return fmt.Errorf("%w: have %#x, expected empty", errNonEmptyParentBeaconRoot, *header.ParentBeaconRoot)
188197
}
189198
if err := eip4844.VerifyEIP4844Header(parent, header); err != nil {
190199
return err
191200
}
192201
if *header.BlobGasUsed > 0 { // VerifyEIP4844Header ensures BlobGasUsed is non-nil
193-
return fmt.Errorf("blobs not enabled on avalanche networks: used %d blob gas, expected 0", *header.BlobGasUsed)
202+
return fmt.Errorf("%w: used %d blob gas, expected 0", errBlobsNotEnabled, *header.BlobGasUsed)
194203
}
195204
}
196205
return nil
@@ -248,7 +257,7 @@ func (eng *DummyEngine) Finalize(chain consensus.ChainHeaderReader, block *types
248257
timestamp,
249258
)
250259
if !utils.BigEqual(blockGasCost, expectedBlockGasCost) {
251-
return fmt.Errorf("invalid blockGasCost: have %d, want %d", blockGasCost, expectedBlockGasCost)
260+
return fmt.Errorf("%w: have %d, want %d", ErrInvalidBlockGasCost, blockGasCost, expectedBlockGasCost)
252261
}
253262
if config.IsSubnetEVM(timestamp) {
254263
// Verify the block fee was paid.

‎plugin/evm/customheader/block_gas_cost.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ var (
2222
errBlockGasCostNil = errors.New("block gas cost is nil")
2323
errNoGasUsed = errors.New("no gas used")
2424

25-
ErrInsufficientBlockGas = errors.New("insufficient gas to cover the block cost")
25+
ErrInsufficientBlockGas = errors.New("insufficient gas to cover the block cost")
26+
errInvalidExtraStateChangeContribution = errors.New("invalid extra state change contribution")
27+
errInvalidBaseFee = errors.New("invalid base fee")
28+
errInvalidRequiredBlockGasCost = errors.New("invalid block gas cost")
2629
)
2730

2831
// BlockGasCost calculates the required block gas cost based on the parent
@@ -135,10 +138,10 @@ func VerifyBlockFee(
135138
extraStateChangeContribution *big.Int,
136139
) error {
137140
if baseFee == nil || baseFee.Sign() <= 0 {
138-
return fmt.Errorf("invalid base fee (%d) in apricot phase 4", baseFee)
141+
return fmt.Errorf("%w: %d", errInvalidBaseFee, baseFee)
139142
}
140143
if requiredBlockGasCost == nil || !requiredBlockGasCost.IsUint64() {
141-
return fmt.Errorf("invalid block gas cost (%d) in apricot phase 4", requiredBlockGasCost)
144+
return fmt.Errorf("%w: %d", errInvalidRequiredBlockGasCost, requiredBlockGasCost)
142145
}
143146
// If the required block gas cost is 0, we don't need to verify the block fee
144147
if requiredBlockGasCost.Sign() == 0 {
@@ -154,7 +157,7 @@ func VerifyBlockFee(
154157
// Add in the external contribution
155158
if extraStateChangeContribution != nil {
156159
if extraStateChangeContribution.Cmp(common.Big0) < 0 {
157-
return fmt.Errorf("invalid extra state change contribution: %d", extraStateChangeContribution)
160+
return fmt.Errorf("%w: %d", errInvalidExtraStateChangeContribution, extraStateChangeContribution)
158161
}
159162
totalBlockFee.Add(totalBlockFee, extraStateChangeContribution)
160163
}

‎plugin/evm/customheader/block_gas_cost_test.go‎

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ func TestVerifyBlockFee(t *testing.T) {
331331
txs []*types.Transaction
332332
receipts []*types.Receipt
333333
extraStateContribution *big.Int
334-
shouldErr bool
334+
expectedErr error
335335
}{
336336
"tx only base fee": {
337337
baseFee: big.NewInt(100),
@@ -344,7 +344,7 @@ func TestVerifyBlockFee(t *testing.T) {
344344
{GasUsed: 1000},
345345
},
346346
extraStateContribution: nil,
347-
shouldErr: true,
347+
expectedErr: ErrInsufficientBlockGas,
348348
},
349349
"tx covers exactly block fee": {
350350
baseFee: big.NewInt(100),
@@ -357,7 +357,6 @@ func TestVerifyBlockFee(t *testing.T) {
357357
{GasUsed: 100_000},
358358
},
359359
extraStateContribution: nil,
360-
shouldErr: false,
361360
},
362361
"txs share block fee": {
363362
baseFee: big.NewInt(100),
@@ -372,7 +371,6 @@ func TestVerifyBlockFee(t *testing.T) {
372371
{GasUsed: 100_000},
373372
},
374373
extraStateContribution: nil,
375-
shouldErr: false,
376374
},
377375
"txs split block fee": {
378376
baseFee: big.NewInt(100),
@@ -387,7 +385,6 @@ func TestVerifyBlockFee(t *testing.T) {
387385
{GasUsed: 100_000},
388386
},
389387
extraStateContribution: nil,
390-
shouldErr: false,
391388
},
392389
"split block fee with extra state contribution": {
393390
baseFee: big.NewInt(100),
@@ -400,7 +397,6 @@ func TestVerifyBlockFee(t *testing.T) {
400397
{GasUsed: 100_000},
401398
},
402399
extraStateContribution: big.NewInt(5_000_000),
403-
shouldErr: false,
404400
},
405401
"extra state contribution insufficient": {
406402
baseFee: big.NewInt(100),
@@ -409,7 +405,7 @@ func TestVerifyBlockFee(t *testing.T) {
409405
txs: nil,
410406
receipts: nil,
411407
extraStateContribution: big.NewInt(9_999_999),
412-
shouldErr: true,
408+
expectedErr: ErrInsufficientBlockGas,
413409
},
414410
"negative extra state contribution": {
415411
baseFee: big.NewInt(100),
@@ -418,7 +414,7 @@ func TestVerifyBlockFee(t *testing.T) {
418414
txs: nil,
419415
receipts: nil,
420416
extraStateContribution: big.NewInt(-1),
421-
shouldErr: true,
417+
expectedErr: errInvalidExtraStateChangeContribution,
422418
},
423419
"extra state contribution covers block fee": {
424420
baseFee: big.NewInt(100),
@@ -427,7 +423,6 @@ func TestVerifyBlockFee(t *testing.T) {
427423
txs: nil,
428424
receipts: nil,
429425
extraStateContribution: big.NewInt(10_000_000),
430-
shouldErr: false,
431426
},
432427
"extra state contribution covers more than block fee": {
433428
baseFee: big.NewInt(100),
@@ -436,7 +431,6 @@ func TestVerifyBlockFee(t *testing.T) {
436431
txs: nil,
437432
receipts: nil,
438433
extraStateContribution: big.NewInt(10_000_001),
439-
shouldErr: false,
440434
},
441435
"tx only base fee after full time window": {
442436
baseFee: big.NewInt(100),
@@ -449,7 +443,6 @@ func TestVerifyBlockFee(t *testing.T) {
449443
{GasUsed: 1000},
450444
},
451445
extraStateContribution: nil,
452-
shouldErr: false,
453446
},
454447
"tx only base fee after large time window": {
455448
baseFee: big.NewInt(100),
@@ -462,7 +455,6 @@ func TestVerifyBlockFee(t *testing.T) {
462455
{GasUsed: 1000},
463456
},
464457
extraStateContribution: nil,
465-
shouldErr: false,
466458
},
467459
"zero block gas cost": {
468460
baseFee: big.NewInt(100),
@@ -484,15 +476,8 @@ func TestVerifyBlockFee(t *testing.T) {
484476
)
485477
bigBlockGasCost := new(big.Int).SetUint64(blockGasCost)
486478

487-
if err := VerifyBlockFee(test.baseFee, bigBlockGasCost, test.txs, test.receipts, test.extraStateContribution); err != nil {
488-
if !test.shouldErr {
489-
t.Fatalf("Unexpected error: %s", err)
490-
}
491-
} else {
492-
if test.shouldErr {
493-
t.Fatal("Should have failed verification")
494-
}
495-
}
479+
err := VerifyBlockFee(test.baseFee, bigBlockGasCost, test.txs, test.receipts, test.extraStateContribution)
480+
require.ErrorIs(t, err, test.expectedErr)
496481
})
497482
}
498483
}

‎plugin/evm/vm.go‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -134,16 +134,21 @@ var (
134134
)
135135

136136
var (
137-
errEmptyBlock = errors.New("empty block")
138-
errUnsupportedFXs = errors.New("unsupported feature extensions")
139-
errInvalidBlock = errors.New("invalid block")
140-
errInvalidNonce = errors.New("invalid nonce")
141-
errUnclesUnsupported = errors.New("uncles unsupported")
142-
errNilBaseFeeSubnetEVM = errors.New("nil base fee is invalid after subnetEVM")
143-
errNilBlockGasCostSubnetEVM = errors.New("nil blockGasCost is invalid after subnetEVM")
144-
errInvalidHeaderPredicateResults = errors.New("invalid header predicate results")
145-
errInitializingLogger = errors.New("failed to initialize logger")
146-
errShuttingDownVM = errors.New("shutting down VM")
137+
errEmptyBlock = errors.New("empty block")
138+
errUnsupportedFXs = errors.New("unsupported feature extensions")
139+
errNilBaseFeeSubnetEVM = errors.New("nil base fee is invalid after subnetEVM")
140+
errNilBlockGasCostSubnetEVM = errors.New("nil blockGasCost is invalid after subnetEVM")
141+
errInvalidBlock = errors.New("invalid block")
142+
errInvalidNonce = errors.New("invalid nonce")
143+
errUnclesUnsupported = errors.New("uncles unsupported")
144+
errInvalidHeaderPredicateResults = errors.New("invalid header predicate results")
145+
errInitializingLogger = errors.New("failed to initialize logger")
146+
errShuttingDownVM = errors.New("shutting down VM")
147+
errFirewoodPruningRequired = errors.New("pruning must be enabled for Firewood")
148+
errFirewoodSnapshotCacheDisabled = errors.New("snapshot cache must be disabled for Firewood")
149+
errFirewoodOfflinePruningUnsupported = errors.New("offline pruning is not supported for Firewood")
150+
errFirewoodStateSyncUnsupported = errors.New("state sync is not yet supported for Firewood")
151+
errPathStateUnsupported = errors.New("path state scheme is not supported")
147152
)
148153

149154
// legacyApiNames maps pre geth v1.10.20 api names to their updated counterparts.
@@ -395,22 +400,22 @@ func (vm *VM) Initialize(
395400
log.Warn("This is untested in production, use at your own risk")
396401
// Firewood only supports pruning for now.
397402
if !vm.config.Pruning {
398-
return errors.New("Pruning must be enabled for Firewood")
403+
return errFirewoodPruningRequired
399404
}
400405
// Firewood does not support iterators, so the snapshot cannot be constructed
401406
if vm.config.SnapshotCache > 0 {
402-
return errors.New("Snapshot cache must be disabled for Firewood")
407+
return errFirewoodSnapshotCacheDisabled
403408
}
404409
if vm.config.OfflinePruning {
405-
return errors.New("Offline pruning is not supported for Firewood")
410+
return errFirewoodOfflinePruningUnsupported
406411
}
407412
if vm.config.StateSyncEnabled {
408-
return errors.New("State sync is not yet supported for Firewood")
413+
return errFirewoodStateSyncUnsupported
409414
}
410415
}
411416
if vm.ethConfig.StateScheme == rawdb.PathScheme {
412417
log.Error("Path state scheme is not supported. Please use HashDB or Firewood state schemes instead")
413-
return errors.New("Path state scheme is not supported")
418+
return errPathStateUnsupported
414419
}
415420

416421
// Create directory for offline pruning

‎plugin/evm/vm_test.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1633,9 +1633,8 @@ func testUncleBlock(t *testing.T, scheme string) {
16331633
if _, err := vm1.ParseBlock(context.Background(), vm2BlkC.Bytes()); err != nil {
16341634
t.Fatalf("VM1 errored parsing blkC: %s", err)
16351635
}
1636-
if _, err := vm1.ParseBlock(context.Background(), uncleBlock.Bytes()); !errors.Is(err, errUnclesUnsupported) {
1637-
t.Fatalf("VM1 should have failed with %q but got %q", errUnclesUnsupported, err.Error())
1638-
}
1636+
_, err = vm1.ParseBlock(context.Background(), uncleBlock.Bytes())
1637+
require.ErrorIs(t, err, errUnclesUnsupported)
16391638
}
16401639

16411640
// Regression test to ensure that a VM that is not able to parse a block that

‎plugin/evm/wrapped_block.go‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,20 @@ var (
3939
errTotalIntrinsicGasCostExceedsClaimed = errors.New("total intrinsic gas cost is greater than claimed gas used")
4040
)
4141

42-
// wrappedBlock implements the snowman.Block interface by wrapping a libevm block
42+
// Sentinel errors for header validation in this file
43+
var (
44+
errInvalidExcessBlobGasBeforeCancun = errors.New("invalid excessBlobGas before cancun")
45+
errInvalidBlobGasUsedBeforeCancun = errors.New("invalid blobGasUsed before cancun")
46+
errInvalidParentBeaconRootBeforeCancun = errors.New("invalid parentBeaconRoot before cancun")
47+
errMissingExcessBlobGas = errors.New("header is missing excessBlobGas")
48+
errMissingBlobGasUsed = errors.New("header is missing blobGasUsed")
49+
errMissingParentBeaconRoot = errors.New("header is missing parentBeaconRoot")
50+
errParentBeaconRootNonEmpty = errors.New("invalid non-empty parentBeaconRoot")
51+
errBlobGasUsedNilInCancun = errors.New("blob gas used must not be nil in Cancun")
52+
errBlobsNotEnabled = errors.New("blobs not enabled on avalanche networks")
53+
)
54+
55+
// wrappedBlock implements the snowman.wrappedBlock interface
4356
type wrappedBlock struct {
4457
id ids.ID
4558
ethBlock *types.Block
@@ -380,31 +393,31 @@ func (b *wrappedBlock) syntacticVerify() error {
380393
// Verify the existence / non-existence of excessBlobGas
381394
cancun := rules.IsCancun
382395
if !cancun && ethHeader.ExcessBlobGas != nil {
383-
return fmt.Errorf("invalid excessBlobGas: have %d, expected nil", *ethHeader.ExcessBlobGas)
396+
return fmt.Errorf("%w: have %d, expected nil", errInvalidExcessBlobGasBeforeCancun, *ethHeader.ExcessBlobGas)
384397
}
385398
if !cancun && ethHeader.BlobGasUsed != nil {
386-
return fmt.Errorf("invalid blobGasUsed: have %d, expected nil", *ethHeader.BlobGasUsed)
399+
return fmt.Errorf("%w: have %d, expected nil", errInvalidBlobGasUsedBeforeCancun, *ethHeader.BlobGasUsed)
387400
}
388401
if cancun && ethHeader.ExcessBlobGas == nil {
389-
return errors.New("header is missing excessBlobGas")
402+
return errMissingExcessBlobGas
390403
}
391404
if cancun && ethHeader.BlobGasUsed == nil {
392-
return errors.New("header is missing blobGasUsed")
405+
return errMissingBlobGasUsed
393406
}
394407
if !cancun && ethHeader.ParentBeaconRoot != nil {
395-
return fmt.Errorf("invalid parentBeaconRoot: have %x, expected nil", *ethHeader.ParentBeaconRoot)
408+
return fmt.Errorf("%w: have %x, expected nil", errInvalidParentBeaconRootBeforeCancun, *ethHeader.ParentBeaconRoot)
396409
}
397410
if cancun {
398411
switch {
399412
case ethHeader.ParentBeaconRoot == nil:
400-
return errors.New("header is missing parentBeaconRoot")
413+
return errMissingParentBeaconRoot
401414
case *ethHeader.ParentBeaconRoot != (common.Hash{}):
402-
return fmt.Errorf("invalid parentBeaconRoot: have %x, expected empty hash", ethHeader.ParentBeaconRoot)
415+
return fmt.Errorf("%w: have %x, expected empty hash", errParentBeaconRootNonEmpty, ethHeader.ParentBeaconRoot)
403416
}
404417
if ethHeader.BlobGasUsed == nil {
405-
return errors.New("blob gas used must not be nil in Cancun")
418+
return errBlobGasUsedNilInCancun
406419
} else if *ethHeader.BlobGasUsed > 0 {
407-
return fmt.Errorf("blobs not enabled on avalanche networks: used %d blob gas, expected 0", *ethHeader.BlobGasUsed)
420+
return fmt.Errorf("%w: used %d blob gas, expected 0", errBlobsNotEnabled, *ethHeader.BlobGasUsed)
408421
}
409422
}
410423

0 commit comments

Comments
 (0)