diff --git a/.avalanche-golangci.yml b/.avalanche-golangci.yml index 2d0b812652..31cf795e0d 100644 --- a/.avalanche-golangci.yml +++ b/.avalanche-golangci.yml @@ -104,12 +104,12 @@ linters: forbidigo: # Forbid the following identifiers (list of regexp). forbid: - # - pattern: require\.Error$(# ErrorIs should be used instead)? - # - pattern: require\.ErrorContains$(# ErrorIs should be used instead)? - # - pattern: require\.EqualValues$(# Equal should be used instead)? - # - pattern: require\.NotEqualValues$(# NotEqual should be used instead)? + - pattern: require\.Error$(# ErrorIs should be used instead)? + - pattern: require\.ErrorContains$(# ErrorIs should be used instead)? + - pattern: require\.EqualValues$(# Equal should be used instead)? + - pattern: require\.NotEqualValues$(# NotEqual should be used instead)? - pattern: ^(t|b|tb|f)\.(Fatal|Fatalf|Error|Errorf)$(# the require library should be used instead)? - # - pattern: ^sort\.(Slice|Strings)$(# the slices package should be used instead)? + - pattern: ^sort\.(Slice|Strings)$(# the slices package should be used instead)? # Exclude godoc examples from forbidigo checks. exclude-godoc-examples: false gosec: diff --git a/accounts/abi/abi_extra_test.go b/accounts/abi/abi_extra_test.go index 37333ad02d..f8580747db 100644 --- a/accounts/abi/abi_extra_test.go +++ b/accounts/abi/abi_extra_test.go @@ -81,7 +81,7 @@ func TestUnpackInputIntoInterface(t *testing.T) { err = abi.UnpackInputIntoInterface(&v, "receive", data, test.strictMode) // skips 4 byte selector if test.expectedErrorSubstring != "" { - require.ErrorContains(t, err, test.expectedErrorSubstring) + require.ErrorContains(t, err, test.expectedErrorSubstring) //nolint:forbidigo // uses upstream code } else { require.NoError(t, err) require.Equal(t, input, v) diff --git a/network/network.go b/network/network.go index fd0a064c90..06e120e21c 100644 --- a/network/network.go +++ b/network/network.go @@ -35,6 +35,7 @@ const ( var ( errAcquiringSemaphore = errors.New("error acquiring semaphore") + errEmptyNodeID = errors.New("cannot send request to empty nodeID") errExpiredRequest = errors.New("expired request") errNoPeersFound = errors.New("no peers found matching version") _ Network = (*network)(nil) @@ -187,7 +188,7 @@ func (n *network) SendAppRequestAny(ctx context.Context, minVersion *version.App // SendAppRequest sends request message bytes to specified nodeID, notifying the responseHandler on response or failure func (n *network) SendAppRequest(ctx context.Context, nodeID ids.NodeID, request []byte, responseHandler message.ResponseHandler) error { if nodeID == ids.EmptyNodeID { - return fmt.Errorf("cannot send request to empty nodeID, nodeID=%s, requestLen=%d", nodeID, len(request)) + return fmt.Errorf("%w, nodeID=%s, requestLen=%d", errEmptyNodeID, nodeID, len(request)) } // If the context was cancelled, we can skip sending this request. diff --git a/network/network_test.go b/network/network_test.go index e21148533d..9c2cae5511 100644 --- a/network/network_test.go +++ b/network/network_test.go @@ -248,10 +248,8 @@ func TestRequestRequestsRoutingAndResponse(t *testing.T) { } // ensure empty nodeID is not allowed - require.ErrorContains(t, - net.SendAppRequest(context.Background(), ids.EmptyNodeID, []byte("hello there"), nil), - "cannot send request to empty nodeID", - ) + err = net.SendAppRequest(context.Background(), ids.EmptyNodeID, []byte("hello there"), nil) + require.ErrorIs(t, err, errEmptyNodeID) } func TestAppRequestOnShutdown(t *testing.T) { @@ -554,7 +552,8 @@ func TestNetworkPropagatesRequestHandlerError(t *testing.T) { ctx := snowtest.Context(t, snowtest.CChainID) clientNetwork, err := NewNetwork(ctx, sender, codecManager, 1, prometheus.NewRegistry()) require.NoError(t, err) - clientNetwork.SetRequestHandler(&testRequestHandler{err: errors.New("fail")}) // Return an error from the request handler + errTest := errors.New("test error") + clientNetwork.SetRequestHandler(&testRequestHandler{err: errTest}) // Return an error from the request handler require.NoError(t, clientNetwork.Connected(context.Background(), nodeID, defaultPeerVersion)) @@ -565,7 +564,8 @@ func TestNetworkPropagatesRequestHandlerError(t *testing.T) { require.NoError(t, err) // Check that if the request handler returns an error, it is propagated as a fatal error. - require.ErrorContains(t, clientNetwork.AppRequest(context.Background(), nodeID, requestID, time.Now().Add(time.Second), requestMessage), "fail") + err = clientNetwork.AppRequest(context.Background(), nodeID, requestID, time.Now().Add(time.Second), requestMessage) + require.ErrorIs(t, err, errTest) } func TestNetworkAppRequestAfterShutdown(t *testing.T) { diff --git a/plugin/evm/atomic/export_tx.go b/plugin/evm/atomic/export_tx.go index 097ee97e72..f4ce67c865 100644 --- a/plugin/evm/atomic/export_tx.go +++ b/plugin/evm/atomic/export_tx.go @@ -36,10 +36,10 @@ var ( _ secp256k1fx.UnsignedTx = (*UnsignedExportTx)(nil) ErrExportNonAVAXInputBanff = errors.New("export input cannot contain non-AVAX in Banff") ErrExportNonAVAXOutputBanff = errors.New("export output cannot contain non-AVAX in Banff") + ErrInsufficientFunds = errors.New("insufficient funds") ErrInvalidNonce = errors.New("invalid nonce") ErrNoExportOutputs = errors.New("tx has no export outputs") errOverflowExport = errors.New("overflow when computing export amount + txFee") - errInsufficientFunds = errors.New("insufficient funds") ) // UnsignedExportTx is an unsigned ExportTx @@ -323,14 +323,14 @@ func (utx *UnsignedExportTx) EVMStateTransfer(ctx *snow.Context, state StateDB) uint256.NewInt(X2CRate.Uint64()), ) if state.GetBalance(from.Address).Cmp(amount) < 0 { - return errInsufficientFunds + return ErrInsufficientFunds } state.SubBalance(from.Address, amount) } else { log.Debug("export_tx", "dest", utx.DestinationChain, "addr", from.Address, "amount", from.Amount, "assetID", from.AssetID) amount := new(big.Int).SetUint64(from.Amount) if state.GetBalanceMultiCoin(from.Address, common.Hash(from.AssetID)).Cmp(amount) < 0 { - return errInsufficientFunds + return ErrInsufficientFunds } state.SubBalanceMultiCoin(from.Address, common.Hash(from.AssetID), amount) } @@ -393,7 +393,7 @@ func getSpendableFunds( } if amount > 0 { - return nil, nil, errInsufficientFunds + return nil, nil, ErrInsufficientFunds } return inputs, signers, nil @@ -487,7 +487,7 @@ func getSpendableAVAXWithFee( } if amount > 0 { - return nil, nil, errInsufficientFunds + return nil, nil, ErrInsufficientFunds } return inputs, signers, nil diff --git a/plugin/evm/atomic/state/atomic_trie_iterator_test.go b/plugin/evm/atomic/state/atomic_trie_iterator_test.go index 3780c046e2..dfbdc97518 100644 --- a/plugin/evm/atomic/state/atomic_trie_iterator_test.go +++ b/plugin/evm/atomic/state/atomic_trie_iterator_test.go @@ -100,5 +100,6 @@ func TestIteratorHandlesInvalidData(t *testing.T) { require.NoError(err) for iter.Next() { } - require.Error(iter.Error()) + err = iter.Error() + require.ErrorIs(err, errKeyLength) } diff --git a/plugin/evm/atomic/tx.go b/plugin/evm/atomic/tx.go index 6602d1c6e1..aff5079957 100644 --- a/plugin/evm/atomic/tx.go +++ b/plugin/evm/atomic/tx.go @@ -37,7 +37,7 @@ const ( var ( ErrWrongNetworkID = errors.New("tx was issued with a different network ID") ErrNilTx = errors.New("tx is nil") - errNoValueOutput = errors.New("output has no value") + ErrNoValueOutput = errors.New("output has no value") ErrNoValueInput = errors.New("input has no value") ErrNoGasUsed = errors.New("no gas used") errNilOutput = errors.New("nil output") @@ -97,7 +97,7 @@ func (out *EVMOutput) Verify() error { case out == nil: return errNilOutput case out.Amount == 0: - return errNoValueOutput + return ErrNoValueOutput case out.AssetID == ids.Empty: return errEmptyAssetID } diff --git a/plugin/evm/atomic/vm/export_tx_test.go b/plugin/evm/atomic/vm/export_tx_test.go index 25c08df585..ebc49b6d27 100644 --- a/plugin/evm/atomic/vm/export_tx_test.go +++ b/plugin/evm/atomic/vm/export_tx_test.go @@ -134,7 +134,7 @@ func TestExportTxEVMStateTransfer(t *testing.T) { avaxBalance *uint256.Int balances map[ids.ID]*big.Int expectedNonce uint64 - shouldErr bool + expectedError error }{ { name: "no transfers", @@ -144,7 +144,6 @@ func TestExportTxEVMStateTransfer(t *testing.T) { customAssetID: big.NewInt(int64(customAmount)), }, expectedNonce: 0, - shouldErr: false, }, { name: "spend half AVAX", @@ -161,7 +160,6 @@ func TestExportTxEVMStateTransfer(t *testing.T) { customAssetID: big.NewInt(int64(customAmount)), }, expectedNonce: 1, - shouldErr: false, }, { name: "spend all AVAX", @@ -178,7 +176,6 @@ func TestExportTxEVMStateTransfer(t *testing.T) { customAssetID: big.NewInt(int64(customAmount)), }, expectedNonce: 1, - shouldErr: false, }, { name: "spend too much AVAX", @@ -195,7 +192,7 @@ func TestExportTxEVMStateTransfer(t *testing.T) { customAssetID: big.NewInt(int64(customAmount)), }, expectedNonce: 1, - shouldErr: true, + expectedError: atomic.ErrInsufficientFunds, }, { name: "spend half custom", @@ -212,7 +209,6 @@ func TestExportTxEVMStateTransfer(t *testing.T) { customAssetID: big.NewInt(int64(customAmount / 2)), }, expectedNonce: 1, - shouldErr: false, }, { name: "spend all custom", @@ -229,7 +225,6 @@ func TestExportTxEVMStateTransfer(t *testing.T) { customAssetID: big.NewInt(0), }, expectedNonce: 1, - shouldErr: false, }, { name: "spend too much custom", @@ -246,7 +241,7 @@ func TestExportTxEVMStateTransfer(t *testing.T) { customAssetID: big.NewInt(0), }, expectedNonce: 1, - shouldErr: true, + expectedError: atomic.ErrInsufficientFunds, }, { name: "spend everything", @@ -269,7 +264,6 @@ func TestExportTxEVMStateTransfer(t *testing.T) { customAssetID: big.NewInt(0), }, expectedNonce: 1, - shouldErr: false, }, { name: "spend everything wrong nonce", @@ -292,7 +286,7 @@ func TestExportTxEVMStateTransfer(t *testing.T) { customAssetID: big.NewInt(0), }, expectedNonce: 1, - shouldErr: true, + expectedError: atomic.ErrInvalidNonce, }, { name: "spend everything changing nonces", @@ -315,7 +309,7 @@ func TestExportTxEVMStateTransfer(t *testing.T) { customAssetID: big.NewInt(0), }, expectedNonce: 1, - shouldErr: true, + expectedError: atomic.ErrInvalidNonce, }, } for _, test := range tests { @@ -394,11 +388,10 @@ func TestExportTxEVMStateTransfer(t *testing.T) { wrappedStateDB := extstate.New(statedb) err = newTx.EVMStateTransfer(vm.Ctx, wrappedStateDB) - if test.shouldErr { - require.Error(t, err) + require.ErrorIs(t, err, test.expectedError) + if test.expectedError != nil { return } - require.NoError(t, err) avaxBalance := wrappedStateDB.GetBalance(ethAddr) require.Zero(t, avaxBalance.Cmp(test.avaxBalance), "address balance %s equal %s not %s", addr.String(), avaxBalance, test.avaxBalance) @@ -1055,23 +1048,21 @@ func TestExportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrNilTx.Error(), + expectedErr: atomic.ErrNilTx, }, "valid export tx": { generate: func() atomic.UnsignedAtomicTx { return exportTx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: "", + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.NoUpgrades), }, "valid export tx banff": { generate: func() atomic.UnsignedAtomicTx { return exportTx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.Banff), - expectedErr: "", + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.Banff), }, "incorrect networkID": { generate: func() atomic.UnsignedAtomicTx { @@ -1081,7 +1072,7 @@ func TestExportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrWrongNetworkID.Error(), + expectedErr: atomic.ErrWrongNetworkID, }, "incorrect blockchainID": { generate: func() atomic.UnsignedAtomicTx { @@ -1091,7 +1082,7 @@ func TestExportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrWrongChainID.Error(), + expectedErr: atomic.ErrWrongChainID, }, "incorrect destination chain": { generate: func() atomic.UnsignedAtomicTx { @@ -1101,7 +1092,7 @@ func TestExportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrWrongChainID.Error(), // TODO make this error more specific to destination not just chainID + expectedErr: atomic.ErrWrongChainID, // TODO make this error more specific to destination not just chainID }, "no exported outputs": { generate: func() atomic.UnsignedAtomicTx { @@ -1111,7 +1102,7 @@ func TestExportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrNoExportOutputs.Error(), + expectedErr: atomic.ErrNoExportOutputs, }, "unsorted outputs": { generate: func() atomic.UnsignedAtomicTx { @@ -1124,7 +1115,7 @@ func TestExportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrOutputsNotSorted.Error(), + expectedErr: atomic.ErrOutputsNotSorted, }, "invalid exported output": { generate: func() atomic.UnsignedAtomicTx { @@ -1134,7 +1125,7 @@ func TestExportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: "nil transferable output is not valid", + expectedErr: avax.ErrNilTransferableOutput, }, "unsorted EVM inputs before AP1": { generate: func() atomic.UnsignedAtomicTx { @@ -1145,9 +1136,8 @@ func TestExportTxVerify(t *testing.T) { } return &tx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: "", + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.NoUpgrades), }, "unsorted EVM inputs after AP1": { generate: func() atomic.UnsignedAtomicTx { @@ -1160,7 +1150,7 @@ func TestExportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.ApricotPhase1), - expectedErr: atomic.ErrInputsNotSortedUnique.Error(), + expectedErr: atomic.ErrInputsNotSortedUnique, }, "EVM input with amount 0": { generate: func() atomic.UnsignedAtomicTx { @@ -1177,7 +1167,7 @@ func TestExportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrNoValueInput.Error(), + expectedErr: atomic.ErrNoValueInput, }, "non-unique EVM input before AP1": { generate: func() atomic.UnsignedAtomicTx { @@ -1185,9 +1175,8 @@ func TestExportTxVerify(t *testing.T) { tx.Ins = []atomic.EVMInput{tx.Ins[0], tx.Ins[0]} return &tx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: "", + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.NoUpgrades), }, "non-unique EVM input after AP1": { generate: func() atomic.UnsignedAtomicTx { @@ -1197,7 +1186,7 @@ func TestExportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.ApricotPhase1), - expectedErr: atomic.ErrInputsNotSortedUnique.Error(), + expectedErr: atomic.ErrInputsNotSortedUnique, }, "non-AVAX input Apricot Phase 6": { generate: func() atomic.UnsignedAtomicTx { @@ -1212,9 +1201,8 @@ func TestExportTxVerify(t *testing.T) { } return &tx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.ApricotPhase6), - expectedErr: "", + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.ApricotPhase6), }, "non-AVAX output Apricot Phase 6": { generate: func() atomic.UnsignedAtomicTx { @@ -1234,9 +1222,8 @@ func TestExportTxVerify(t *testing.T) { } return &tx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.ApricotPhase6), - expectedErr: "", + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.ApricotPhase6), }, "non-AVAX input Banff": { generate: func() atomic.UnsignedAtomicTx { @@ -1253,7 +1240,7 @@ func TestExportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.Banff), - expectedErr: atomic.ErrExportNonAVAXInputBanff.Error(), + expectedErr: atomic.ErrExportNonAVAXInputBanff, }, "non-AVAX output Banff": { generate: func() atomic.UnsignedAtomicTx { @@ -1275,7 +1262,7 @@ func TestExportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.Banff), - expectedErr: atomic.ErrExportNonAVAXOutputBanff.Error(), + expectedErr: atomic.ErrExportNonAVAXOutputBanff, }, } diff --git a/plugin/evm/atomic/vm/import_tx_test.go b/plugin/evm/atomic/vm/import_tx_test.go index ad0763854d..27344f2692 100644 --- a/plugin/evm/atomic/vm/import_tx_test.go +++ b/plugin/evm/atomic/vm/import_tx_test.go @@ -134,23 +134,21 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrNilTx.Error(), + expectedErr: atomic.ErrNilTx, }, "valid import tx": { generate: func() atomic.UnsignedAtomicTx { return importTx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: "", // Expect this transaction to be valid in Apricot Phase 0 + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.NoUpgrades), }, "valid import tx banff": { generate: func() atomic.UnsignedAtomicTx { return importTx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.Banff), - expectedErr: "", // Expect this transaction to be valid in Banff + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.Banff), }, "invalid network ID": { generate: func() atomic.UnsignedAtomicTx { @@ -160,7 +158,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrWrongNetworkID.Error(), + expectedErr: atomic.ErrWrongNetworkID, }, "invalid blockchain ID": { generate: func() atomic.UnsignedAtomicTx { @@ -170,7 +168,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrWrongChainID.Error(), + expectedErr: atomic.ErrWrongChainID, }, "P-chain source before AP5": { generate: func() atomic.UnsignedAtomicTx { @@ -180,7 +178,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrWrongChainID.Error(), + expectedErr: atomic.ErrWrongChainID, }, "P-chain source after AP5": { generate: func() atomic.UnsignedAtomicTx { @@ -199,7 +197,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.ApricotPhase5), - expectedErr: atomic.ErrWrongChainID.Error(), + expectedErr: atomic.ErrWrongChainID, }, "no inputs": { generate: func() atomic.UnsignedAtomicTx { @@ -209,7 +207,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrNoImportInputs.Error(), + expectedErr: atomic.ErrNoImportInputs, }, "inputs sorted incorrectly": { generate: func() atomic.UnsignedAtomicTx { @@ -222,7 +220,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: atomic.ErrInputsNotSortedUnique.Error(), + expectedErr: atomic.ErrInputsNotSortedUnique, }, "invalid input": { generate: func() atomic.UnsignedAtomicTx { @@ -235,7 +233,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: "atomic input failed verification", + expectedErr: avax.ErrNilTransferableInput, }, "unsorted outputs phase 0 passes verification": { generate: func() atomic.UnsignedAtomicTx { @@ -246,9 +244,8 @@ func TestImportTxVerify(t *testing.T) { } return &tx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: "", + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.NoUpgrades), }, "non-unique outputs phase 0 passes verification": { generate: func() atomic.UnsignedAtomicTx { @@ -259,9 +256,8 @@ func TestImportTxVerify(t *testing.T) { } return &tx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: "", + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.NoUpgrades), }, "unsorted outputs phase 1 fails verification": { generate: func() atomic.UnsignedAtomicTx { @@ -274,7 +270,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.ApricotPhase1), - expectedErr: atomic.ErrOutputsNotSorted.Error(), + expectedErr: atomic.ErrOutputsNotSorted, }, "non-unique outputs phase 1 passes verification": { generate: func() atomic.UnsignedAtomicTx { @@ -285,9 +281,8 @@ func TestImportTxVerify(t *testing.T) { } return &tx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.ApricotPhase1), - expectedErr: "", + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.ApricotPhase1), }, "outputs not sorted and unique phase 2 fails verification": { generate: func() atomic.UnsignedAtomicTx { @@ -300,7 +295,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.ApricotPhase2), - expectedErr: atomic.ErrOutputsNotSortedUnique.Error(), + expectedErr: atomic.ErrOutputsNotSortedUnique, }, "outputs not sorted phase 2 fails verification": { generate: func() atomic.UnsignedAtomicTx { @@ -313,7 +308,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.ApricotPhase2), - expectedErr: atomic.ErrOutputsNotSortedUnique.Error(), + expectedErr: atomic.ErrOutputsNotSortedUnique, }, "invalid EVMOutput fails verification": { generate: func() atomic.UnsignedAtomicTx { @@ -329,7 +324,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.NoUpgrades), - expectedErr: "EVM Output failed verification", + expectedErr: atomic.ErrNoValueOutput, }, "no outputs apricot phase 3": { generate: func() atomic.UnsignedAtomicTx { @@ -339,7 +334,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.ApricotPhase3), - expectedErr: atomic.ErrNoEVMOutputs.Error(), + expectedErr: atomic.ErrNoEVMOutputs, }, "non-AVAX input Apricot Phase 6": { generate: func() atomic.UnsignedAtomicTx { @@ -361,9 +356,8 @@ func TestImportTxVerify(t *testing.T) { } return &tx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.ApricotPhase6), - expectedErr: "", + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.ApricotPhase6), }, "non-AVAX output Apricot Phase 6": { generate: func() atomic.UnsignedAtomicTx { @@ -377,9 +371,8 @@ func TestImportTxVerify(t *testing.T) { } return &tx }, - ctx: ctx, - rules: extrastest.ForkToRules(upgradetest.ApricotPhase6), - expectedErr: "", + ctx: ctx, + rules: extrastest.ForkToRules(upgradetest.ApricotPhase6), }, "non-AVAX input Banff": { generate: func() atomic.UnsignedAtomicTx { @@ -403,7 +396,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.Banff), - expectedErr: atomic.ErrImportNonAVAXInputBanff.Error(), + expectedErr: atomic.ErrImportNonAVAXInputBanff, }, "non-AVAX output Banff": { generate: func() atomic.UnsignedAtomicTx { @@ -419,7 +412,7 @@ func TestImportTxVerify(t *testing.T) { }, ctx: ctx, rules: extrastest.ForkToRules(upgradetest.Banff), - expectedErr: atomic.ErrImportNonAVAXOutputBanff.Error(), + expectedErr: atomic.ErrImportNonAVAXOutputBanff, }, } for name, test := range tests { @@ -901,7 +894,7 @@ func TestImportTxSemanticVerify(t *testing.T) { require.NoError(t, tx.Sign(atomic.Codec, [][]*secp256k1.PrivateKey{{key}})) return tx }, - semanticVerifyErr: "failed to fetch import UTXOs from", + semanticVerifyErr: errFailedToFetchImportUTXOs, }, "garbage UTXO": { setup: func(t *testing.T, vm *VM, sharedMemory *avalancheatomic.Memory) *atomic.Tx { @@ -937,7 +930,7 @@ func TestImportTxSemanticVerify(t *testing.T) { require.NoError(t, tx.Sign(atomic.Codec, [][]*secp256k1.PrivateKey{{key}})) return tx }, - semanticVerifyErr: "failed to unmarshal UTXO", + semanticVerifyErr: errFailedToUnmarshalUTXO, }, "UTXO AssetID mismatch": { setup: func(t *testing.T, vm *VM, sharedMemory *avalancheatomic.Memory) *atomic.Tx { @@ -967,7 +960,7 @@ func TestImportTxSemanticVerify(t *testing.T) { require.NoError(t, tx.Sign(atomic.Codec, [][]*secp256k1.PrivateKey{{key}})) return tx }, - semanticVerifyErr: ErrAssetIDMismatch.Error(), + semanticVerifyErr: ErrAssetIDMismatch, }, "insufficient AVAX funds": { setup: func(t *testing.T, vm *VM, sharedMemory *avalancheatomic.Memory) *atomic.Tx { @@ -996,7 +989,7 @@ func TestImportTxSemanticVerify(t *testing.T) { require.NoError(t, tx.Sign(atomic.Codec, [][]*secp256k1.PrivateKey{{key}})) return tx }, - semanticVerifyErr: "import tx flow check failed due to", + semanticVerifyErr: avax.ErrInsufficientFunds, }, "insufficient non-AVAX funds": { setup: func(t *testing.T, vm *VM, sharedMemory *avalancheatomic.Memory) *atomic.Tx { @@ -1026,7 +1019,7 @@ func TestImportTxSemanticVerify(t *testing.T) { require.NoError(t, tx.Sign(atomic.Codec, [][]*secp256k1.PrivateKey{{key}})) return tx }, - semanticVerifyErr: "import tx flow check failed due to", + semanticVerifyErr: avax.ErrInsufficientFunds, }, "no signatures": { setup: func(t *testing.T, vm *VM, sharedMemory *avalancheatomic.Memory) *atomic.Tx { @@ -1055,7 +1048,7 @@ func TestImportTxSemanticVerify(t *testing.T) { require.NoError(t, tx.Sign(atomic.Codec, nil)) return tx }, - semanticVerifyErr: "import tx contained mismatched number of inputs/credentials", + semanticVerifyErr: errIncorrectNumCredentials, }, "incorrect signature": { setup: func(t *testing.T, vm *VM, sharedMemory *avalancheatomic.Memory) *atomic.Tx { @@ -1087,7 +1080,7 @@ func TestImportTxSemanticVerify(t *testing.T) { require.NoError(t, tx.Sign(atomic.Codec, [][]*secp256k1.PrivateKey{{incorrectKey}})) return tx }, - semanticVerifyErr: "import tx transfer failed verification", + semanticVerifyErr: secp256k1fx.ErrWrongSig, }, "non-unique EVM Outputs": { setup: func(t *testing.T, vm *VM, sharedMemory *avalancheatomic.Memory) *atomic.Tx { @@ -1124,7 +1117,7 @@ func TestImportTxSemanticVerify(t *testing.T) { return tx }, fork: upgradetest.ApricotPhase3, - semanticVerifyErr: atomic.ErrOutputsNotSortedUnique.Error(), + semanticVerifyErr: atomic.ErrOutputsNotSortedUnique, }, } @@ -1247,24 +1240,22 @@ func executeTxTest(t *testing.T, test atomicTxTest) { backend := NewVerifierBackend(vm, rules) err := backend.SemanticVerify(tx, lastAcceptedBlock, baseFee) - if len(test.semanticVerifyErr) > 0 { - require.ErrorContains(t, err, test.semanticVerifyErr) + require.ErrorIs(t, err, test.semanticVerifyErr) + if test.semanticVerifyErr != nil { // If SemanticVerify failed for the expected reason, return early return } - require.NoError(t, err) // Retrieve dummy state to test that EVMStateTransfer works correctly statedb, err := vm.Ethereum().BlockChain().StateAt(lastAcceptedBlock.GetEthBlock().Root()) require.NoError(t, err) wrappedStateDB := extstate.New(statedb) err = tx.UnsignedAtomicTx.EVMStateTransfer(vm.Ctx, wrappedStateDB) - if len(test.evmStateTransferErr) > 0 { - require.ErrorContains(t, err, test.evmStateTransferErr) + require.ErrorIs(t, err, test.evmStateTransferErr) + if test.evmStateTransferErr != nil { // If EVMStateTransfer failed for the expected reason, return early return } - require.NoError(t, err) require.NoError(t, vm.AtomicMempool.AddLocalTx(tx)) @@ -1282,12 +1273,12 @@ func executeTxTest(t *testing.T, test atomicTxTest) { require.NoError(t, blk.Verify(context.Background())) err = blk.Accept(context.Background()) - if len(test.acceptErr) > 0 { - require.ErrorContains(t, err, test.acceptErr) + require.ErrorIs(t, err, test.acceptErr) + + if test.acceptErr != nil { // If Accept failed for the expected reason, return early return } - require.NoErrorf(t, err, "Accept failed unexpectedly due to: %s", err) if test.checkState != nil { test.checkState(t, vm) diff --git a/plugin/evm/atomic/vm/tx_semantic_verifier.go b/plugin/evm/atomic/vm/tx_semantic_verifier.go index 8a69fca048..31cea11319 100644 --- a/plugin/evm/atomic/vm/tx_semantic_verifier.go +++ b/plugin/evm/atomic/vm/tx_semantic_verifier.go @@ -28,6 +28,7 @@ var ( ErrAssetIDMismatch = errors.New("asset IDs in the input don't match the utxo") ErrConflictingAtomicInputs = errors.New("invalid block due to conflicting atomic inputs") errFailedToFetchImportUTXOs = errors.New("failed to fetch import UTXOs") + errFailedToUnmarshalUTXO = errors.New("failed to unmarshal UTXO") errRejectedParent = errors.New("rejected parent") errIncorrectNumCredentials = errors.New("incorrect number of credentials") errIncorrectNumSignatures = errors.New("incorrect number of signatures") @@ -120,7 +121,7 @@ func (s *semanticVerifier) ImportTx(utx *atomic.UnsignedImportTx) error { } if len(stx.Creds) != len(utx.ImportedInputs) { - return fmt.Errorf("import tx contained mismatched number of inputs/credentials (%d vs. %d)", len(utx.ImportedInputs), len(stx.Creds)) + return fmt.Errorf("%w: (%d vs. %d)", errIncorrectNumCredentials, len(utx.ImportedInputs), len(stx.Creds)) } if !backend.Bootstrapped { @@ -144,7 +145,7 @@ func (s *semanticVerifier) ImportTx(utx *atomic.UnsignedImportTx) error { utxo := &avax.UTXO{} if _, err := atomic.Codec.Unmarshal(utxoBytes, utxo); err != nil { - return fmt.Errorf("failed to unmarshal UTXO: %w", err) + return fmt.Errorf("%w: %w", errFailedToUnmarshalUTXO, err) } cred := stx.Creds[i] diff --git a/plugin/evm/atomic/vm/tx_test.go b/plugin/evm/atomic/vm/tx_test.go index 8da9ed3ac3..26f565cc0f 100644 --- a/plugin/evm/atomic/vm/tx_test.go +++ b/plugin/evm/atomic/vm/tx_test.go @@ -53,7 +53,7 @@ type atomicTxVerifyTest struct { ctx *snow.Context generate func() atomic.UnsignedAtomicTx rules *extras.Rules - expectedErr string + expectedErr error } // executeTxVerifyTest tests @@ -61,11 +61,7 @@ func executeTxVerifyTest(t *testing.T, test atomicTxVerifyTest) { require := require.New(t) atomicTx := test.generate() err := atomicTx.Verify(test.ctx, *test.rules) - if len(test.expectedErr) == 0 { - require.NoError(err) - } else { - require.ErrorContains(err, test.expectedErr, "expected tx verify to fail with specified error") - } + require.ErrorIs(err, test.expectedErr) } type atomicTxTest struct { @@ -74,7 +70,7 @@ type atomicTxTest struct { // define a string that should be contained in the error message if the tx fails verification // at some point. If the strings are empty, then the tx should pass verification at the // respective step. - semanticVerifyErr, evmStateTransferErr, acceptErr string + semanticVerifyErr, evmStateTransferErr, acceptErr error // checkState is called iff building and verifying a block containing the transaction is successful. Verifies // the state of the VM following the block's acceptance. checkState func(t *testing.T, vm *VM) diff --git a/plugin/evm/atomic/vm/vm.go b/plugin/evm/atomic/vm/vm.go index f3e8a25409..4dd3d1ef1a 100644 --- a/plugin/evm/atomic/vm/vm.go +++ b/plugin/evm/atomic/vm/vm.go @@ -60,6 +60,8 @@ var ( _ block.ChainVM = (*VM)(nil) _ block.BuildBlockWithContextChainVM = (*VM)(nil) _ block.StateSyncableVM = (*VM)(nil) + + errAtomicGasExceedsLimit = errors.New("atomic gas used exceeds atomic gas limit") ) const ( @@ -719,7 +721,7 @@ func (vm *VM) onExtraStateChange(block *types.Block, parent *types.Header, state } if !utils.BigLessOrEqualUint64(batchGasUsed, atomicGasLimit) { - return nil, nil, fmt.Errorf("atomic gas used (%d) by block (%s), exceeds atomic gas limit (%d)", batchGasUsed, block.Hash().Hex(), atomicGasLimit) + return nil, nil, fmt.Errorf("%w: (%d) by block (%s), limit (%d)", errAtomicGasExceedsLimit, batchGasUsed, block.Hash().Hex(), atomicGasLimit) } } return batchContribution, batchGasUsed, nil diff --git a/plugin/evm/atomic/vm/vm_test.go b/plugin/evm/atomic/vm/vm_test.go index 7bb24104e7..81e4355871 100644 --- a/plugin/evm/atomic/vm/vm_test.go +++ b/plugin/evm/atomic/vm/vm_test.go @@ -1035,7 +1035,7 @@ func TestExtraStateChangeAtomicGasLimitExceeded(t *testing.T) { // Hack: test [onExtraStateChange] directly to ensure it catches the atomic gas limit error correctly. _, _, err = vm2.onExtraStateChange(ethBlk2, nil, state) - require.ErrorContains(err, "exceeds atomic gas limit") + require.ErrorIs(err, errAtomicGasExceedsLimit) } // Regression test to ensure that a VM that is not able to parse a block that diff --git a/plugin/evm/config/config_test.go b/plugin/evm/config/config_test.go index 564fa9a5d7..5398e4236a 100644 --- a/plugin/evm/config/config_test.go +++ b/plugin/evm/config/config_test.go @@ -119,7 +119,7 @@ func TestUnmarshalConfig(t *testing.T) { var tmp Config err := json.Unmarshal(tt.givenJSON, &tmp) if tt.expectedErr { - require.Error(t, err) + require.Error(t, err) //nolint:forbidigo // uses standard library } else { require.NoError(t, err) tmp.deprecate() @@ -131,11 +131,10 @@ func TestUnmarshalConfig(t *testing.T) { func TestGetConfig(t *testing.T) { tests := []struct { - name string - configJSON []byte - networkID uint32 - expected func(*testing.T, Config) - expectError bool + name string + configJSON []byte + networkID uint32 + expected func(*testing.T, Config) }{ { name: "custom config values", @@ -172,10 +171,6 @@ func TestGetConfig(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { config, _, err := GetConfig(tt.configJSON, tt.networkID) - if tt.expectError { - require.Error(t, err) - return - } require.NoError(t, err) tt.expected(t, config) }) diff --git a/plugin/evm/log/log_test.go b/plugin/evm/log/log_test.go index 4ac8a66698..9d35d470b6 100644 --- a/plugin/evm/log/log_test.go +++ b/plugin/evm/log/log_test.go @@ -42,7 +42,7 @@ func TestInitLogger(t *testing.T) { require := require.New(t) _, err := InitLogger("alias", test.logLevel, true, os.Stderr) if test.expectedErr { - require.ErrorContains(err, "unknown level") + require.ErrorContains(err, "unknown level") //nolint:forbidigo // uses upstream code } else { require.NoError(err) } diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index 97f519efe9..580d77a852 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -519,9 +519,9 @@ func testReorgProtection(t *testing.T, scheme string) { // should NEVER happen. However, the VM defends against this // just in case. err = vm1.SetPreference(context.Background(), vm1BlkC.ID()) - require.ErrorContains(err, "cannot orphan finalized block") + require.ErrorContains(err, "cannot orphan finalized block") //nolint:forbidigo // uses upstream code err = vm1BlkC.Accept(context.Background()) - require.ErrorContains(err, "expected accepted block to have parent") + require.ErrorContains(err, "expected accepted block to have parent") //nolint:forbidigo // uses upstream code } // Regression test to ensure that a VM that accepts block C while preferring @@ -769,7 +769,7 @@ func testStickyPreference(t *testing.T, scheme string) { // Attempt to accept out of order err = vm1BlkD.Accept(context.Background()) - require.ErrorContains(err, "expected accepted block to have parent") + require.ErrorContains(err, "expected accepted block to have parent") //nolint:forbidigo // uses upstream code // Accept in order require.NoError(vm1BlkC.Accept(context.Background())) @@ -1265,7 +1265,7 @@ func TestSkipChainConfigCheckCompatible(t *testing.T) { upgradetest.SetTimesTo(&newCTX.NetworkUpgrades, fork, upgrade.InitiallyActiveTime) genesis := []byte(vmtest.GenesisJSON(paramstest.ForkToChainConfig[fork])) err = reinitVM.Initialize(context.Background(), newCTX, tvm.DB, genesis, []byte{}, []byte{}, []*commonEng.Fx{}, tvm.AppSender) - require.ErrorContains(err, "mismatching Cancun fork timestamp in database") + require.ErrorContains(err, "mismatching Cancun fork timestamp in database") //nolint:forbidigo // uses upstream code // try again with skip-upgrade-check reinitVM = newDefaultTestVM() @@ -1280,47 +1280,41 @@ func TestParentBeaconRootBlock(t *testing.T) { name string fork upgradetest.Fork beaconRoot *common.Hash - expectedError bool - errString string + expectedError error }{ { name: "non-empty parent beacon root in Durango", fork: upgradetest.Durango, beaconRoot: &common.Hash{0x01}, - expectedError: true, - // err string wont work because it will also fail with blob gas is non-empty (zeroed) + expectedError: errInvalidParentBeaconRootBeforeCancun, }, { name: "empty parent beacon root in Durango", fork: upgradetest.Durango, beaconRoot: &common.Hash{}, - expectedError: true, + expectedError: errInvalidParentBeaconRootBeforeCancun, }, { - name: "nil parent beacon root in Durango", - fork: upgradetest.Durango, - beaconRoot: nil, - expectedError: false, + name: "nil parent beacon root in Durango", + fork: upgradetest.Durango, + beaconRoot: nil, }, { name: "non-empty parent beacon root in E-Upgrade (Cancun)", fork: upgradetest.Etna, beaconRoot: &common.Hash{0x01}, - expectedError: true, - errString: "expected empty hash", + expectedError: errParentBeaconRootNonEmpty, }, { - name: "empty parent beacon root in E-Upgrade (Cancun)", - fork: upgradetest.Etna, - beaconRoot: &common.Hash{}, - expectedError: false, + name: "empty parent beacon root in E-Upgrade (Cancun)", + fork: upgradetest.Etna, + beaconRoot: &common.Hash{}, }, { name: "nil parent beacon root in E-Upgrade (Cancun)", fork: upgradetest.Etna, beaconRoot: nil, - expectedError: true, - errString: "header is missing parentBeaconRoot", + expectedError: errMissingParentBeaconRoot, }, } @@ -1350,18 +1344,10 @@ func TestParentBeaconRootBlock(t *testing.T) { parentBeaconBlock, err := wrapBlock(parentBeaconEthBlock, vm) require.NoError(err) - errCheck := func(err error) { - if test.expectedError { - require.ErrorContains(err, test.errString) - } else { - require.NoError(err) - } - } - _, err = vm.ParseBlock(context.Background(), parentBeaconBlock.Bytes()) - errCheck(err) + require.ErrorIs(err, test.expectedError) err = parentBeaconBlock.Verify(context.Background()) - errCheck(err) + require.ErrorIs(err, test.expectedError) }) } } @@ -1759,7 +1745,7 @@ func TestCreateHandlers(t *testing.T) { }) } require.NoError(t, client.BatchCall(batch)) - require.ErrorContains(t, batch[0].Error, "batch too large") + require.ErrorContains(t, batch[0].Error, "batch too large") //nolint:forbidigo // uses upstream code // All other elements should have an error indicating there's no response for _, elem := range batch[1:] { diff --git a/plugin/evm/vm_warp_test.go b/plugin/evm/vm_warp_test.go index c4aa7fdc39..aee4de3130 100644 --- a/plugin/evm/vm_warp_test.go +++ b/plugin/evm/vm_warp_test.go @@ -141,9 +141,9 @@ func testSendWarpMessage(t *testing.T, scheme string) { // Verify the signature cannot be fetched before the block is accepted _, err = vm.warpBackend.GetMessageSignature(context.TODO(), unsignedMessage) - require.ErrorContains(err, "unknown payload type") + require.ErrorIs(err, warp.ErrVerifyWarpMessage) _, err = vm.warpBackend.GetBlockSignature(context.TODO(), blk.ID()) - require.ErrorContains(err, "failed to get block") + require.ErrorIs(err, warp.ErrValidateBlock) require.NoError(vm.SetPreference(context.Background(), blk.ID())) require.NoError(blk.Accept(context.Background())) diff --git a/plugin/evm/vmsync/registry.go b/plugin/evm/vmsync/registry.go index 46045df839..f6895807e3 100644 --- a/plugin/evm/vmsync/registry.go +++ b/plugin/evm/vmsync/registry.go @@ -5,6 +5,7 @@ package vmsync import ( "context" + "errors" "fmt" "github.com/ava-labs/libevm/log" @@ -15,6 +16,8 @@ import ( synccommon "github.com/ava-labs/coreth/sync" ) +var errSyncerAlreadyRegistered = errors.New("syncer already registered") + // SyncerTask represents a single syncer with its name for identification. type SyncerTask struct { name string @@ -39,7 +42,7 @@ func NewSyncerRegistry() *SyncerRegistry { func (r *SyncerRegistry) Register(syncer synccommon.Syncer) error { id := syncer.ID() if r.registeredNames[id] { - return fmt.Errorf("syncer with id '%s' is already registered", id) + return fmt.Errorf("%w with id '%s'", errSyncerAlreadyRegistered, id) } r.registeredNames[id] = true diff --git a/plugin/evm/vmsync/registry_test.go b/plugin/evm/vmsync/registry_test.go index 867c2641e1..2a85661444 100644 --- a/plugin/evm/vmsync/registry_test.go +++ b/plugin/evm/vmsync/registry_test.go @@ -67,7 +67,7 @@ func TestSyncerRegistry_Register(t *testing.T) { tests := []struct { name string registrations []*mockSyncer - expectedError string + expectedError error expectedCount int }{ { @@ -76,7 +76,6 @@ func TestSyncerRegistry_Register(t *testing.T) { newMockSyncer("Syncer1", nil), newMockSyncer("Syncer2", nil), }, - expectedError: "", expectedCount: 2, }, { @@ -85,7 +84,7 @@ func TestSyncerRegistry_Register(t *testing.T) { newMockSyncer("Syncer1", nil), newMockSyncer("Syncer1", nil), }, - expectedError: "syncer with id 'Syncer1' is already registered", + expectedError: errSyncerAlreadyRegistered, expectedCount: 1, }, { @@ -114,17 +113,13 @@ func TestSyncerRegistry_Register(t *testing.T) { } // Check error expectations. - if tt.expectedError != "" { - require.ErrorContains(t, errLast, tt.expectedError) - } else { - require.NoError(t, errLast) - } + require.ErrorIs(t, errLast, tt.expectedError) // Verify registration count. require.Len(t, registry.syncers, tt.expectedCount) // Verify registration order for successful cases. - if tt.expectedError == "" { + if tt.expectedError == nil { for i, reg := range tt.registrations { require.Equal(t, reg.name, registry.syncers[i].name) require.Equal(t, reg, registry.syncers[i].syncer) @@ -135,10 +130,11 @@ func TestSyncerRegistry_Register(t *testing.T) { } func TestSyncerRegistry_RunSyncerTasks(t *testing.T) { + errFoo := errors.New("foo") tests := []struct { name string syncers []syncerConfig - expectedError string + expectedError error assertState func(t *testing.T, mockSyncers []*mockSyncer) }{ { @@ -155,10 +151,10 @@ func TestSyncerRegistry_RunSyncerTasks(t *testing.T) { }, { name: "error returned", syncers: []syncerConfig{ - {"Syncer1", errors.New("wait failed")}, + {"Syncer1", errFoo}, {"Syncer2", nil}, }, - expectedError: "Syncer1 failed", + expectedError: errFoo, assertState: func(t *testing.T, mockSyncers []*mockSyncer) { // First syncer should be started and waited on (but wait failed). require.True(t, mockSyncers[0].started, "First syncer should have been started") @@ -184,11 +180,7 @@ func TestSyncerRegistry_RunSyncerTasks(t *testing.T) { err := registry.RunSyncerTasks(ctx, newTestClientSummary(t)) - if tt.expectedError != "" { - require.ErrorContains(t, err, tt.expectedError) - } else { - require.NoError(t, err) - } + require.ErrorIs(t, err, tt.expectedError) // Use custom assertion function for each test case. tt.assertState(t, mockSyncers) diff --git a/plugin/evm/wrapped_block.go b/plugin/evm/wrapped_block.go index 8ff47e5cba..b0ddaa7b4c 100644 --- a/plugin/evm/wrapped_block.go +++ b/plugin/evm/wrapped_block.go @@ -488,12 +488,12 @@ func (b *wrappedBlock) syntacticVerify() error { } } else { switch { + case ethHeader.ParentBeaconRoot != nil: + return fmt.Errorf("%w: have %x, expected nil", errInvalidParentBeaconRootBeforeCancun, *ethHeader.ParentBeaconRoot) case ethHeader.ExcessBlobGas != nil: return fmt.Errorf("%w: have %d, expected nil", errInvalidExcessBlobGasBeforeCancun, *ethHeader.ExcessBlobGas) case ethHeader.BlobGasUsed != nil: return fmt.Errorf("%w: have %d, expected nil", errInvalidBlobGasUsedBeforeCancun, *ethHeader.BlobGasUsed) - case ethHeader.ParentBeaconRoot != nil: - return fmt.Errorf("%w: have %x, expected nil", errInvalidParentBeaconRootBeforeCancun, *ethHeader.ParentBeaconRoot) } } diff --git a/precompile/contracts/warp/config.go b/precompile/contracts/warp/config.go index 3886942ba8..d0b60e7dd1 100644 --- a/precompile/contracts/warp/config.go +++ b/precompile/contracts/warp/config.go @@ -32,6 +32,7 @@ var ( ) var ( + ErrInvalidQuorumRatio = errors.New("invalid warp quorum ratio") errOverflowSignersGasCost = errors.New("overflow calculating warp signers gas cost") errInvalidPredicateBytes = errors.New("cannot unpack predicate bytes") errInvalidWarpMsg = errors.New("cannot unpack warp message") @@ -95,11 +96,11 @@ func (c *Config) Verify(chainConfig precompileconfig.ChainConfig) error { } if c.QuorumNumerator > WarpQuorumDenominator { - return fmt.Errorf("cannot specify quorum numerator (%d) > quorum denominator (%d)", c.QuorumNumerator, WarpQuorumDenominator) + return fmt.Errorf("%w: cannot specify numerator (%d) > denominator (%d)", ErrInvalidQuorumRatio, c.QuorumNumerator, WarpQuorumDenominator) } // If a non-default quorum numerator is specified and it is less than the minimum, return an error if c.QuorumNumerator != 0 && c.QuorumNumerator < WarpQuorumNumeratorMinimum { - return fmt.Errorf("cannot specify quorum numerator (%d) < min quorum numerator (%d)", c.QuorumNumerator, WarpQuorumNumeratorMinimum) + return fmt.Errorf("%w: cannot specify numerator (%d) < min numerator (%d)", ErrInvalidQuorumRatio, c.QuorumNumerator, WarpQuorumNumeratorMinimum) } return nil } diff --git a/precompile/contracts/warp/config_test.go b/precompile/contracts/warp/config_test.go index 577d881153..d69ad0c38d 100644 --- a/precompile/contracts/warp/config_test.go +++ b/precompile/contracts/warp/config_test.go @@ -4,7 +4,6 @@ package warp import ( - "fmt" "testing" "go.uber.org/mock/gomock" @@ -18,11 +17,11 @@ func TestVerify(t *testing.T) { tests := map[string]precompiletest.ConfigVerifyTest{ "quorum numerator less than minimum": { Config: NewConfig(utils.NewUint64(3), WarpQuorumNumeratorMinimum-1, false), - ExpectedError: fmt.Sprintf("cannot specify quorum numerator (%d) < min quorum numerator (%d)", WarpQuorumNumeratorMinimum-1, WarpQuorumNumeratorMinimum), + ExpectedError: ErrInvalidQuorumRatio, }, "quorum numerator greater than quorum denominator": { Config: NewConfig(utils.NewUint64(3), WarpQuorumDenominator+1, false), - ExpectedError: fmt.Sprintf("cannot specify quorum numerator (%d) > quorum denominator (%d)", WarpQuorumDenominator+1, WarpQuorumDenominator), + ExpectedError: ErrInvalidQuorumRatio, }, "default quorum numerator": { Config: NewDefaultConfig(utils.NewUint64(3)), @@ -40,7 +39,7 @@ func TestVerify(t *testing.T) { config.EXPECT().IsDurango(gomock.Any()).Return(false) return config }(), - ExpectedError: errWarpCannotBeActivated.Error(), + ExpectedError: errWarpCannotBeActivated, }, } precompiletest.RunVerifyTests(t, tests) diff --git a/precompile/contracts/warp/contract_test.go b/precompile/contracts/warp/contract_test.go index ba3cdcb805..39ccaa1730 100644 --- a/precompile/contracts/warp/contract_test.go +++ b/precompile/contracts/warp/contract_test.go @@ -110,7 +110,7 @@ func getBlockchainIDTests(tb testing.TB, rules extras.AvalancheRules) []precompi }, SuppliedGas: gasConfig.GetBlockchainID - 1, ReadOnly: false, - ExpectedErr: vm.ErrOutOfGas.Error(), + ExpectedErr: vm.ErrOutOfGas, Rules: rules, }, } @@ -153,7 +153,7 @@ func sendWarpMessageTests(tb testing.TB, rules extras.AvalancheRules) []precompi InputFn: func(testing.TB) []byte { return sendWarpMessageInput }, SuppliedGas: gasConfig.SendWarpMessageCost(len(sendWarpMessageInput[4:])), ReadOnly: true, - ExpectedErr: vm.ErrWriteProtection.Error(), + ExpectedErr: vm.ErrWriteProtection, Rules: rules, }, { @@ -162,7 +162,7 @@ func sendWarpMessageTests(tb testing.TB, rules extras.AvalancheRules) []precompi InputFn: func(testing.TB) []byte { return sendWarpMessageInput }, SuppliedGas: gasConfig.SendWarpMessageBase - 1, ReadOnly: false, - ExpectedErr: vm.ErrOutOfGas.Error(), + ExpectedErr: vm.ErrOutOfGas, Rules: rules, }, { @@ -171,7 +171,7 @@ func sendWarpMessageTests(tb testing.TB, rules extras.AvalancheRules) []precompi InputFn: func(testing.TB) []byte { return sendWarpMessageInput }, SuppliedGas: gasConfig.SendWarpMessageCost(len(sendWarpMessageInput[4:])) - 1, ReadOnly: false, - ExpectedErr: vm.ErrOutOfGas.Error(), + ExpectedErr: vm.ErrOutOfGas, Rules: rules, }, { @@ -182,7 +182,7 @@ func sendWarpMessageTests(tb testing.TB, rules extras.AvalancheRules) []precompi }, SuppliedGas: gasConfig.SendWarpMessageBase, ReadOnly: false, - ExpectedErr: errInvalidSendInput.Error(), + ExpectedErr: errInvalidSendInput, Rules: rules, }, { @@ -428,7 +428,7 @@ func getVerifiedWarpMessageTests(tb testing.TB, rules extras.AvalancheRules) []p Predicates: []predicate.Predicate{warpMessagePredicate}, SuppliedGas: gasConfig.GetVerifiedWarpMessageBase - 1, ReadOnly: false, - ExpectedErr: vm.ErrOutOfGas.Error(), + ExpectedErr: vm.ErrOutOfGas, Rules: rules, }, { @@ -441,7 +441,7 @@ func getVerifiedWarpMessageTests(tb testing.TB, rules extras.AvalancheRules) []p }, SuppliedGas: gasConfig.GetVerifiedWarpMessageCost(len(warpMessagePredicate)) - 1, ReadOnly: false, - ExpectedErr: vm.ErrOutOfGas.Error(), + ExpectedErr: vm.ErrOutOfGas, Rules: rules, }, { @@ -454,7 +454,7 @@ func getVerifiedWarpMessageTests(tb testing.TB, rules extras.AvalancheRules) []p }, SuppliedGas: gasConfig.GetVerifiedWarpMessageCost(len(invalidPackedPredicate)), ReadOnly: false, - ExpectedErr: errInvalidPredicateBytes.Error(), + ExpectedErr: errInvalidPredicateBytes, Rules: rules, }, { @@ -467,7 +467,7 @@ func getVerifiedWarpMessageTests(tb testing.TB, rules extras.AvalancheRules) []p }, SuppliedGas: gasConfig.GetVerifiedWarpMessageCost(len(invalidWarpMsgPredicate)), ReadOnly: false, - ExpectedErr: errInvalidWarpMsg.Error(), + ExpectedErr: errInvalidWarpMsg, Rules: rules, }, { @@ -480,7 +480,7 @@ func getVerifiedWarpMessageTests(tb testing.TB, rules extras.AvalancheRules) []p }, SuppliedGas: gasConfig.GetVerifiedWarpMessageCost(len(invalidAddressedPredicate)), ReadOnly: false, - ExpectedErr: errInvalidAddressedPayload.Error(), + ExpectedErr: errInvalidAddressedPayload, Rules: rules, }, { @@ -491,7 +491,7 @@ func getVerifiedWarpMessageTests(tb testing.TB, rules extras.AvalancheRules) []p }, SuppliedGas: gasConfig.GetVerifiedWarpMessageBase, ReadOnly: false, - ExpectedErr: errInvalidIndexInput.Error(), + ExpectedErr: errInvalidIndexInput, Rules: rules, }, { @@ -504,7 +504,7 @@ func getVerifiedWarpMessageTests(tb testing.TB, rules extras.AvalancheRules) []p }, SuppliedGas: gasConfig.GetVerifiedWarpMessageBase, ReadOnly: false, - ExpectedErr: errInvalidIndexInput.Error(), + ExpectedErr: errInvalidIndexInput, Rules: rules, }, { @@ -517,7 +517,7 @@ func getVerifiedWarpMessageTests(tb testing.TB, rules extras.AvalancheRules) []p }, SuppliedGas: gasConfig.GetVerifiedWarpMessageBase, ReadOnly: false, - ExpectedErr: errInvalidIndexInput.Error(), + ExpectedErr: errInvalidIndexInput, Rules: rules, }, } @@ -718,7 +718,7 @@ func getVerifiedWarpBlockHashTests(tb testing.TB, rules extras.AvalancheRules) [ Predicates: []predicate.Predicate{warpMessagePredicate}, SuppliedGas: gasConfig.GetVerifiedWarpMessageBase - 1, ReadOnly: false, - ExpectedErr: vm.ErrOutOfGas.Error(), + ExpectedErr: vm.ErrOutOfGas, Rules: rules, }, { @@ -731,7 +731,7 @@ func getVerifiedWarpBlockHashTests(tb testing.TB, rules extras.AvalancheRules) [ }, SuppliedGas: gasConfig.GetVerifiedWarpMessageCost(len(warpMessagePredicate)) - 1, ReadOnly: false, - ExpectedErr: vm.ErrOutOfGas.Error(), + ExpectedErr: vm.ErrOutOfGas, Rules: rules, }, { @@ -744,7 +744,7 @@ func getVerifiedWarpBlockHashTests(tb testing.TB, rules extras.AvalancheRules) [ }, SuppliedGas: gasConfig.GetVerifiedWarpMessageCost(len(invalidPackedPredicate)), ReadOnly: false, - ExpectedErr: errInvalidPredicateBytes.Error(), + ExpectedErr: errInvalidPredicateBytes, Rules: rules, }, { @@ -757,7 +757,7 @@ func getVerifiedWarpBlockHashTests(tb testing.TB, rules extras.AvalancheRules) [ }, SuppliedGas: gasConfig.GetVerifiedWarpMessageCost(len(invalidWarpMsgPredicate)), ReadOnly: false, - ExpectedErr: errInvalidWarpMsg.Error(), + ExpectedErr: errInvalidWarpMsg, Rules: rules, }, { @@ -770,7 +770,7 @@ func getVerifiedWarpBlockHashTests(tb testing.TB, rules extras.AvalancheRules) [ }, SuppliedGas: gasConfig.GetVerifiedWarpMessageCost(len(invalidHashPredicate)), ReadOnly: false, - ExpectedErr: errInvalidBlockHashPayload.Error(), + ExpectedErr: errInvalidBlockHashPayload, Rules: rules, }, { @@ -781,7 +781,7 @@ func getVerifiedWarpBlockHashTests(tb testing.TB, rules extras.AvalancheRules) [ }, SuppliedGas: gasConfig.GetVerifiedWarpMessageBase, ReadOnly: false, - ExpectedErr: errInvalidIndexInput.Error(), + ExpectedErr: errInvalidIndexInput, Rules: rules, }, { @@ -794,7 +794,7 @@ func getVerifiedWarpBlockHashTests(tb testing.TB, rules extras.AvalancheRules) [ }, SuppliedGas: gasConfig.GetVerifiedWarpMessageBase, ReadOnly: false, - ExpectedErr: errInvalidIndexInput.Error(), + ExpectedErr: errInvalidIndexInput, Rules: rules, }, { @@ -807,7 +807,7 @@ func getVerifiedWarpBlockHashTests(tb testing.TB, rules extras.AvalancheRules) [ }, SuppliedGas: gasConfig.GetVerifiedWarpMessageBase, ReadOnly: false, - ExpectedErr: errInvalidIndexInput.Error(), + ExpectedErr: errInvalidIndexInput, Rules: rules, }, } diff --git a/precompile/modules/registerer.go b/precompile/modules/registerer.go index 0b0f0df451..27c094e828 100644 --- a/precompile/modules/registerer.go +++ b/precompile/modules/registerer.go @@ -4,6 +4,7 @@ package modules import ( + "errors" "fmt" "sort" @@ -32,6 +33,9 @@ var ( End: common.HexToAddress("0x03000000000000000000000000000000000000ff"), }, } + + errBlackholeAddress = fmt.Errorf("cannot register module that overlaps with blackhole address %s", constants.BlackholeAddr) + errAddressNotInReservedRange = errors.New("address is not in a reserved range for custom precompiles") ) // ReservedAddress returns true if [addr] is in a reserved range for custom precompiles @@ -51,10 +55,10 @@ func RegisterModule(stm Module) error { key := stm.ConfigKey if address == constants.BlackholeAddr { - return fmt.Errorf("address %s overlaps with blackhole address", address) + return fmt.Errorf("%w: address %s ", errBlackholeAddress, address) } if !ReservedAddress(address) { - return fmt.Errorf("address %s not in a reserved range", address) + return fmt.Errorf("%w: address %s ", errAddressNotInReservedRange, address) } for _, registeredModule := range registeredModules { diff --git a/precompile/modules/registerer_test.go b/precompile/modules/registerer_test.go index 269404f57b..168ade5380 100644 --- a/precompile/modules/registerer_test.go +++ b/precompile/modules/registerer_test.go @@ -51,10 +51,10 @@ func TestRegisterModuleInvalidAddresses(t *testing.T) { Address: constants.BlackholeAddr, } err := RegisterModule(m) - require.ErrorContains(t, err, "overlaps with blackhole address") + require.ErrorIs(t, err, errBlackholeAddress) // Test an address outside of the reserved ranges cannot be registered m.Address = common.BigToAddress(big.NewInt(1)) err = RegisterModule(m) - require.ErrorContains(t, err, "not in a reserved range") + require.ErrorIs(t, err, errAddressNotInReservedRange) } diff --git a/precompile/precompiletest/test_config.go b/precompile/precompiletest/test_config.go index e0d5a0141e..2824ca24ad 100644 --- a/precompile/precompiletest/test_config.go +++ b/precompile/precompiletest/test_config.go @@ -16,7 +16,7 @@ import ( type ConfigVerifyTest struct { Config precompileconfig.Config ChainConfig precompileconfig.ChainConfig - ExpectedError string + ExpectedError error } // ConfigEqualTest is a test case for comparing two configs @@ -40,11 +40,7 @@ func RunVerifyTests(t *testing.T, tests map[string]ConfigVerifyTest) { chainConfig = mockChainConfig } err := test.Config.Verify(chainConfig) - if test.ExpectedError == "" { - require.NoError(err) - } else { - require.ErrorContains(err, test.ExpectedError) - } + require.ErrorIs(err, test.ExpectedError) }) } } diff --git a/precompile/precompiletest/test_precompile.go b/precompile/precompiletest/test_precompile.go index 3c853656e2..cfd78c8060 100644 --- a/precompile/precompiletest/test_precompile.go +++ b/precompile/precompiletest/test_precompile.go @@ -52,7 +52,7 @@ type PrecompileTest struct { // ExpectedRes is the expected raw byte result returned by the precompile ExpectedRes []byte // ExpectedErr is the expected error returned by the precompile - ExpectedErr string + ExpectedErr error // ChainConfig is the chain config to use for the precompile's block context // If nil, the default chain config will be used. ChainConfig precompileconfig.ChainConfig @@ -77,11 +77,7 @@ func (test PrecompileTest) Run(t *testing.T, module modules.Module) { if runParams.Input != nil { ret, remainingGas, err := module.Contract.Run(runParams.AccessibleState, runParams.Caller, runParams.ContractAddress, runParams.Input, runParams.SuppliedGas, runParams.ReadOnly) - if len(test.ExpectedErr) != 0 { - require.ErrorContains(t, err, test.ExpectedErr) - } else { - require.NoError(t, err) - } + require.ErrorIs(t, err, test.ExpectedErr) require.Equal(t, uint64(0), remainingGas) require.Equal(t, test.ExpectedRes, ret) } @@ -105,11 +101,7 @@ func (test PrecompileTest) Bench(b *testing.B, module modules.Module) { snapshot := stateDB.Snapshot() ret, remainingGas, err := module.Contract.Run(runParams.AccessibleState, runParams.Caller, runParams.ContractAddress, runParams.Input, runParams.SuppliedGas, runParams.ReadOnly) - if len(test.ExpectedErr) != 0 { - require.ErrorContains(b, err, test.ExpectedErr) - } else { - require.NoError(b, err) - } + require.ErrorIs(b, err, test.ExpectedErr) require.Equal(b, uint64(0), remainingGas) require.Equal(b, test.ExpectedRes, ret) @@ -145,11 +137,7 @@ func (test PrecompileTest) Bench(b *testing.B, module modules.Module) { // the benchmark should catch the error here. stateDB.RevertToSnapshot(snapshot) ret, remainingGas, err = module.Contract.Run(runParams.AccessibleState, runParams.Caller, runParams.ContractAddress, runParams.Input, runParams.SuppliedGas, runParams.ReadOnly) - if len(test.ExpectedErr) != 0 { - require.ErrorContains(b, err, test.ExpectedErr) - } else { - require.NoError(b, err) - } + require.ErrorIs(b, err, test.ExpectedErr) require.Equal(b, uint64(0), remainingGas) require.Equal(b, test.ExpectedRes, ret) diff --git a/sync/client/client_test.go b/sync/client/client_test.go index 52ff8de1d0..8c308eea54 100644 --- a/sync/client/client_test.go +++ b/sync/client/client_test.go @@ -181,7 +181,7 @@ func TestGetBlocks(t *testing.T) { request message.BlockRequest getResponse func(t *testing.T, request message.BlockRequest) []byte assertResponse func(t *testing.T, response []*types.Block) - expectedErr string + expectedErr error }{ "normal resonse": { request: message.BlockRequest{ @@ -228,7 +228,7 @@ func TestGetBlocks(t *testing.T) { getResponse: func(_ *testing.T, _ message.BlockRequest) []byte { return []byte("gibberish") }, - expectedErr: errUnmarshalResponse.Error(), + expectedErr: errUnmarshalResponse, }, "invalid value replacing block": { request: message.BlockRequest{ @@ -249,7 +249,7 @@ func TestGetBlocks(t *testing.T) { return responseBytes }, - expectedErr: "failed to unmarshal response: rlp: expected List", + expectedErr: errUnmarshalResponse, }, "incorrect starting point": { request: message.BlockRequest{ @@ -268,7 +268,7 @@ func TestGetBlocks(t *testing.T) { return response }, - expectedErr: errHashMismatch.Error(), + expectedErr: errHashMismatch, }, "missing link in between blocks": { request: message.BlockRequest{ @@ -291,7 +291,7 @@ func TestGetBlocks(t *testing.T) { return responseBytes }, - expectedErr: errHashMismatch.Error(), + expectedErr: errHashMismatch, }, "no blocks": { request: message.BlockRequest{ @@ -308,7 +308,7 @@ func TestGetBlocks(t *testing.T) { return responseBytes }, - expectedErr: errEmptyResponse.Error(), + expectedErr: errEmptyResponse, }, "more than requested blocks": { request: message.BlockRequest{ @@ -327,7 +327,7 @@ func TestGetBlocks(t *testing.T) { return responseBytes }, - expectedErr: errTooManyBlocks.Error(), + expectedErr: errTooManyBlocks, }, } for name, test := range tests { @@ -336,7 +336,7 @@ func TestGetBlocks(t *testing.T) { defer cancel() responseBytes := test.getResponse(t, test.request) - if len(test.expectedErr) == 0 { + if test.expectedErr == nil { testNetClient.testResponse(1, nil, responseBytes) } else { attempted := false @@ -349,11 +349,10 @@ func TestGetBlocks(t *testing.T) { } blockResponse, err := stateSyncClient.GetBlocks(ctx, test.request.Hash, test.request.Height, test.request.Parents) - if len(test.expectedErr) != 0 { - require.ErrorContains(t, err, test.expectedErr) + require.ErrorIs(t, err, test.expectedErr) + if test.expectedErr != nil { return } - require.NoError(t, err) test.assertResponse(t, blockResponse) }) diff --git a/warp/backend.go b/warp/backend.go index d410c9db57..7c2916b737 100644 --- a/warp/backend.go +++ b/warp/backend.go @@ -22,6 +22,8 @@ import ( var ( _ Backend = (*backend)(nil) + ErrValidateBlock = errors.New("failed to validate block message") + ErrVerifyWarpMessage = errors.New("failed to verify warp message") errParsingOffChainMessage = errors.New("failed to parse off-chain message") messageCacheSize = 500 @@ -137,7 +139,7 @@ func (b *backend) GetMessageSignature(ctx context.Context, unsignedMessage *aval } if err := b.Verify(ctx, unsignedMessage, nil); err != nil { - return nil, fmt.Errorf("failed to validate warp message: %w", err) + return nil, fmt.Errorf("%w: %w", ErrVerifyWarpMessage, err) } return b.signMessage(unsignedMessage) } @@ -160,7 +162,7 @@ func (b *backend) GetBlockSignature(ctx context.Context, blockID ids.ID) ([]byte } if err := b.verifyBlockMessage(ctx, blockHashPayload); err != nil { - return nil, fmt.Errorf("failed to validate block message: %w", err) + return nil, fmt.Errorf("%w: %w", ErrValidateBlock, err) } sig, err := b.signMessage(unsignedMessage) diff --git a/warp/backend_test.go b/warp/backend_test.go index ca5142d22e..982017f5c9 100644 --- a/warp/backend_test.go +++ b/warp/backend_test.go @@ -74,7 +74,7 @@ func TestAddAndGetUnknownMessage(t *testing.T) { // Try getting a signature for a message that was not added. _, err = backend.GetMessageSignature(context.TODO(), testUnsignedMessage) - require.ErrorContains(t, err, "unknown payload type") + require.ErrorIs(t, err, ErrVerifyWarpMessage) } func TestGetBlockSignature(t *testing.T) { @@ -103,7 +103,7 @@ func TestGetBlockSignature(t *testing.T) { require.Equal(expectedSig, signature) _, err = backend.GetBlockSignature(context.TODO(), ids.GenerateTestID()) - require.ErrorContains(err, "failed to get block") + require.ErrorIs(err, ErrValidateBlock) } func TestZeroSizedCache(t *testing.T) {