Ban usage of require.Error

.golangci.yml

@@ -20,6 +20,7 @@ linters:
     - depguard
     - errcheck
     - exportloopref
+    - forbidigo
     - goconst
     - gocritic
     - gofmt
@@ -49,11 +50,28 @@ linters:
     # - lll
 
 linters-settings:
+  depguard:
+    list-type: blacklist
+    packages-with-error-message:
+      - io/ioutil: 'io/ioutil is deprecated. Use package io or os instead.'
+      - github.com/stretchr/testify/assert: 'github.com/stretchr/testify/require should be used instead.'
+    include-go-root: true
   errorlint:
     # Check for plain type assertions and type switches.
     asserts: false
     # Check for plain error comparisons.
     comparison: false
+  # https://golangci-lint.run/usage/linters/#forbidigo
+  forbidigo:
+    # Forbid the following identifiers (list of regexp).
+    forbid:
+      - 'require\.Error$(# ErrorIs should be used instead)?'
+      - 'require\.ErrorContains$(# ErrorIs should be used instead)?'
+    exclude_godoc_examples: false
+  # https://golangci-lint.run/usage/linters#gosec
+  gosec:
+    excludes:
+      - G107 # https://securego.io/docs/rules/g107.html
   revive:
     rules:
       # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#bool-literal-in-expr
@@ -103,13 +121,3 @@ linters-settings:
       - "all"
       - "-SA6002" # argument should be pointer-like to avoid allocation, for sync.Pool
       - "-SA1019" # deprecated packages e.g., golang.org/x/crypto/ripemd160
-  # https://golangci-lint.run/usage/linters#gosec
-  gosec:
-    excludes:
-      - G107 # https://securego.io/docs/rules/g107.html
-  depguard:
-    list-type: blacklist
-    packages-with-error-message:
-      - io/ioutil: 'io/ioutil is deprecated. Use package io or os instead.'
-      - github.com/stretchr/testify/assert: 'github.com/stretchr/testify/require should be used instead.'
-    include-go-root: true

chains/atomic/test_shared_memory.go

@@ -193,6 +193,7 @@ func TestSharedMemoryLargeIndexed(t *testing.T, chainID0, chainID1 ids.ID, sm0,
 
 func TestSharedMemoryCantDuplicatePut(t *testing.T, _, chainID1 ids.ID, sm0, _ SharedMemory, _ database.Database) {
 	require := require.New(t)
+
 	err := sm0.Apply(map[ids.ID]*Requests{chainID1: {PutRequests: []*Element{
 		{
 			Key:   []byte{0},
@@ -203,26 +204,32 @@ func TestSharedMemoryCantDuplicatePut(t *testing.T, _, chainID1 ids.ID, sm0, _ S
 			Value: []byte{2},
 		},
 	}}})
-	require.Error(err, "shouldn't be able to write duplicated keys")
+	// TODO: require error to be errDuplicatedOperation
+	require.Error(err) //nolint:forbidigo // currently returns grpc errors too
+
 	err = sm0.Apply(map[ids.ID]*Requests{chainID1: {PutRequests: []*Element{{
 		Key:   []byte{0},
 		Value: []byte{1},
 	}}}})
 	require.NoError(err)
+
 	err = sm0.Apply(map[ids.ID]*Requests{chainID1: {PutRequests: []*Element{{
 		Key:   []byte{0},
 		Value: []byte{1},
 	}}}})
-	require.Error(err, "shouldn't be able to write duplicated keys")
+	// TODO: require error to be errDuplicatedOperation
+	require.Error(err) //nolint:forbidigo // currently returns grpc errors too
 }
 
 func TestSharedMemoryCantDuplicateRemove(t *testing.T, _, chainID1 ids.ID, sm0, _ SharedMemory, _ database.Database) {
 	require := require.New(t)
+
 	err := sm0.Apply(map[ids.ID]*Requests{chainID1: {RemoveRequests: [][]byte{{0}}}})
 	require.NoError(err)
 
 	err = sm0.Apply(map[ids.ID]*Requests{chainID1: {RemoveRequests: [][]byte{{0}}}})
-	require.Error(err, "shouldn't be able to remove duplicated keys")
+	// TODO: require error to be errDuplicatedOperation
+	require.Error(err) //nolint:forbidigo // currently returns grpc errors too
 }
 
 func TestSharedMemoryCommitOnPut(t *testing.T, _, chainID1 ids.ID, sm0, _ SharedMemory, db database.Database) {

ids/aliases.go

@@ -4,21 +4,36 @@
 package ids
 
 import (
+	"errors"
 	"fmt"
 	"sync"
 )
 
+var (
+	errNoIDWithAlias      = errors.New("there is no ID with alias")
+	errNoAliasForID       = errors.New("there is no alias for ID")
+	errAliasAlreadyMapped = errors.New("alias already mapped to an ID")
+)
+
 // AliaserReader allows one to lookup the aliases given to an ID.
 type AliaserReader interface {
+	// Lookup returns the ID associated with alias
 	Lookup(alias string) (ID, error)
+
+	// PrimaryAlias returns the first alias of [id]
 	PrimaryAlias(id ID) (string, error)
+
+	// Aliases returns the aliases of an ID
 	Aliases(id ID) ([]string, error)
 }
 
-// Aliaser allows one to give an ID aliases. An ID can have arbitrarily many
-// aliases; two IDs may not have the same alias.
+// AliaserWriter allows one to give an ID aliases. An ID can have arbitrarily
+// many aliases; two IDs may not have the same alias.
 type AliaserWriter interface {
+	// Alias gives [id] the alias [alias]
 	Alias(id ID, alias string) error
+
+	// RemoveAliases of the provided ID
 	RemoveAliases(id ID)
 }
 
@@ -27,6 +42,9 @@ type AliaserWriter interface {
 type Aliaser interface {
 	AliaserReader
 	AliaserWriter
+
+	// PrimaryAliasOrDefault returns the first alias of [id], or ID string as a
+	// default if no alias exists
 	PrimaryAliasOrDefault(id ID) string
 }
 
@@ -43,30 +61,27 @@ func NewAliaser() Aliaser {
 	}
 }
 
-// Lookup returns the ID associated with alias
 func (a *aliaser) Lookup(alias string) (ID, error) {
 	a.lock.RLock()
 	defer a.lock.RUnlock()
 
 	if id, ok := a.dealias[alias]; ok {
 		return id, nil
 	}
-	return ID{}, fmt.Errorf("there is no ID with alias %s", alias)
+	return ID{}, fmt.Errorf("%w: %s", errNoIDWithAlias, alias)
 }
 
-// PrimaryAlias returns the first alias of [id]
 func (a *aliaser) PrimaryAlias(id ID) (string, error) {
 	a.lock.RLock()
 	defer a.lock.RUnlock()
 
 	aliases := a.aliases[id]
 	if len(aliases) == 0 {
-		return "", fmt.Errorf("there is no alias for ID %s", id)
+		return "", fmt.Errorf("%w: %s", errNoAliasForID, id)
 	}
 	return aliases[0], nil
 }
 
-// PrimaryAliasOrDefault returns the first alias of [id], or ID string as default
 func (a *aliaser) PrimaryAliasOrDefault(id ID) string {
 	alias, err := a.PrimaryAlias(id)
 	if err != nil {
@@ -75,29 +90,26 @@ func (a *aliaser) PrimaryAliasOrDefault(id ID) string {
 	return alias
 }
 
-// Aliases returns the aliases of an ID
 func (a *aliaser) Aliases(id ID) ([]string, error) {
 	a.lock.RLock()
 	defer a.lock.RUnlock()
 
 	return a.aliases[id], nil
 }
 
-// Alias gives [id] the alias [alias]
 func (a *aliaser) Alias(id ID, alias string) error {
 	a.lock.Lock()
 	defer a.lock.Unlock()
 
 	if _, exists := a.dealias[alias]; exists {
-		return fmt.Errorf("%s is already used as an alias for an ID", alias)
+		return fmt.Errorf("%w: %s", errAliasAlreadyMapped, alias)
 	}
 
 	a.dealias[alias] = id
 	a.aliases[id] = append(a.aliases[id], alias)
 	return nil
 }
 
-// RemoveAliases of the provided ID
 func (a *aliaser) RemoveAliases(id ID) {
 	a.lock.Lock()
 	defer a.lock.Unlock()

ids/test_aliases.go

@@ -17,7 +17,8 @@ var AliasTests = []func(require *require.Assertions, r AliaserReader, w AliaserW
 
 func AliaserLookupErrorTest(require *require.Assertions, r AliaserReader, _ AliaserWriter) {
 	_, err := r.Lookup("Batman")
-	require.Error(err, "expected an error due to missing alias")
+	// TODO: require error to be errNoIDWithAlias
+	require.Error(err) //nolint:forbidigo // currently returns grpc errors too
 }
 
 func AliaserLookupTest(require *require.Assertions, r AliaserReader, w AliaserWriter) {
@@ -63,7 +64,8 @@ func AliaserPrimaryAliasTest(require *require.Assertions, r AliaserReader, w Ali
 	require.NoError(err)
 
 	_, err = r.PrimaryAlias(id1)
-	require.Error(err)
+	// TODO: require error to be errNoAliasForID
+	require.Error(err) //nolint:forbidigo // currently returns grpc errors too
 
 	expected := "Batman"
 	res, err := r.PrimaryAlias(id2)
@@ -78,7 +80,8 @@ func AliaserAliasClashTest(require *require.Assertions, _ AliaserReader, w Alias
 	require.NoError(err)
 
 	err = w.Alias(id2, "Batman")
-	require.Error(err)
+	// TODO: require error to be errAliasAlreadyMapped
+	require.Error(err) //nolint:forbidigo // currently returns grpc errors too
 }
 
 func AliaserRemoveAliasTest(require *require.Assertions, r AliaserReader, w AliaserWriter) {
@@ -93,7 +96,8 @@ func AliaserRemoveAliasTest(require *require.Assertions, r AliaserReader, w Alia
 	w.RemoveAliases(id1)
 
 	_, err = r.PrimaryAlias(id1)
-	require.Error(err)
+	// TODO: require error to be errNoAliasForID
+	require.Error(err) //nolint:forbidigo // currently returns grpc errors too
 
 	err = w.Alias(id2, "Batman")
 	require.NoError(err)

vms/platformvm/api/static_service.go

@@ -30,9 +30,10 @@ import (
 // state of the network.
 
 var (
-	errUTXOHasNoValue       = errors.New("genesis UTXO has no value")
-	errValidatorAddsNoValue = errors.New("validator would have already unstaked")
-	errStakeOverflow        = errors.New("validator stake exceeds limit")
+	errUTXOHasNoValue         = errors.New("genesis UTXO has no value")
+	errValidatorHasNoWeight   = errors.New("validator has not weight")
+	errValidatorAlreadyExited = errors.New("validator would have already unstaked")
+	errStakeOverflow          = errors.New("validator stake exceeds limit")
 
 	_ utils.Sortable[UTXO] = UTXO{}
 )
@@ -278,10 +279,10 @@ func (*StaticService) BuildGenesis(_ *http.Request, args *BuildGenesisArgs, repl
 		}
 
 		if weight == 0 {
-			return errValidatorAddsNoValue
+			return errValidatorHasNoWeight
 		}
 		if uint64(vdr.EndTime) <= uint64(args.Time) {
-			return errValidatorAddsNoValue
+			return errValidatorAlreadyExited
 		}
 
 		owner := &secp256k1fx.OutputOwners{

vms/platformvm/api/static_service_test.go

@@ -16,13 +16,10 @@ import (
 	"github.com/ava-labs/avalanchego/vms/platformvm/genesis"
 )
 
-const testNetworkID = 10 // To be used in tests
-
 func TestBuildGenesisInvalidUTXOBalance(t *testing.T) {
 	require := require.New(t)
 	nodeID := ids.NodeID{1, 2, 3}
-	hrp := constants.NetworkIDToHRP[testNetworkID]
-	addr, err := address.FormatBech32(hrp, nodeID.Bytes())
+	addr, err := address.FormatBech32(constants.UnitTestHRP, nodeID.Bytes())
 	require.NoError(err)
 
 	utxo := UTXO{
@@ -59,14 +56,14 @@ func TestBuildGenesisInvalidUTXOBalance(t *testing.T) {
 	reply := BuildGenesisReply{}
 
 	ss := StaticService{}
-	require.Error(ss.BuildGenesis(nil, &args, &reply), "should have errored due to an invalid balance")
+	err = ss.BuildGenesis(nil, &args, &reply)
+	require.ErrorIs(err, errUTXOHasNoValue)
 }
 
-func TestBuildGenesisInvalidAmount(t *testing.T) {
+func TestBuildGenesisInvalidStakeWeight(t *testing.T) {
 	require := require.New(t)
 	nodeID := ids.NodeID{1, 2, 3}
-	hrp := constants.NetworkIDToHRP[testNetworkID]
-	addr, err := address.FormatBech32(hrp, nodeID.Bytes())
+	addr, err := address.FormatBech32(constants.UnitTestHRP, nodeID.Bytes())
 	require.NoError(err)
 
 	utxo := UTXO{
@@ -103,14 +100,14 @@ func TestBuildGenesisInvalidAmount(t *testing.T) {
 	reply := BuildGenesisReply{}
 
 	ss := StaticService{}
-	require.Error(ss.BuildGenesis(nil, &args, &reply), "should have errored due to an invalid amount")
+	err = ss.BuildGenesis(nil, &args, &reply)
+	require.ErrorIs(err, errValidatorHasNoWeight)
 }
 
 func TestBuildGenesisInvalidEndtime(t *testing.T) {
 	require := require.New(t)
 	nodeID := ids.NodeID{1, 2, 3}
-	hrp := constants.NetworkIDToHRP[testNetworkID]
-	addr, err := address.FormatBech32(hrp, nodeID.Bytes())
+	addr, err := address.FormatBech32(constants.UnitTestHRP, nodeID.Bytes())
 	require.NoError(err)
 
 	utxo := UTXO{
@@ -148,14 +145,14 @@ func TestBuildGenesisInvalidEndtime(t *testing.T) {
 	reply := BuildGenesisReply{}
 
 	ss := StaticService{}
-	require.Error(ss.BuildGenesis(nil, &args, &reply), "should have errored due to an invalid end time")
+	err = ss.BuildGenesis(nil, &args, &reply)
+	require.ErrorIs(err, errValidatorAlreadyExited)
 }
 
 func TestBuildGenesisReturnsSortedValidators(t *testing.T) {
 	require := require.New(t)
 	nodeID := ids.NodeID{1}
-	hrp := constants.NetworkIDToHRP[testNetworkID]
-	addr, err := address.FormatBech32(hrp, nodeID.Bytes())
+	addr, err := address.FormatBech32(constants.UnitTestHRP, nodeID.Bytes())
 	require.NoError(err)
 
 	utxo := UTXO{

vms/platformvm/blocks/builder/builder.go

@@ -34,9 +34,9 @@ const targetBlockSize = 128 * units.KiB
 var (
 	_ Builder = (*builder)(nil)
 
-	errEndOfTime       = errors.New("program time is suspiciously far in the future")
-	errNoPendingBlocks = errors.New("no pending blocks")
-	errChainNotSynced  = errors.New("chain not synced")
+	ErrEndOfTime       = errors.New("program time is suspiciously far in the future")
+	ErrNoPendingBlocks = errors.New("no pending blocks")
+	ErrChainNotSynced  = errors.New("chain not synced")
 )
 
 type Builder interface {
@@ -126,7 +126,7 @@ func (b *builder) Preferred() (snowman.Block, error) {
 // AddUnverifiedTx verifies a transaction and attempts to add it to the mempool
 func (b *builder) AddUnverifiedTx(tx *txs.Tx) error {
 	if !b.txExecutorBackend.Bootstrapped.Get() {
-		return errChainNotSynced
+		return ErrChainNotSynced
 	}
 
 	txID := tx.ID()
@@ -369,7 +369,7 @@ func buildBlock(
 	// If there is no reason to build a block, don't.
 	if !builder.Mempool.HasTxs() && !forceAdvanceTime {
 		builder.txExecutorBackend.Ctx.Log.Debug("no pending txs to issue into a block")
-		return nil, errNoPendingBlocks
+		return nil, ErrNoPendingBlocks
 	}
 
 	// Issue a block with as many transactions as possible.
@@ -394,7 +394,7 @@ func getNextStakerToReward(
 	preferredState state.Chain,
 ) (ids.ID, bool, error) {
 	if !chainTimestamp.Before(mockable.MaxTime) {
-		return ids.Empty, false, errEndOfTime
+		return ids.Empty, false, ErrEndOfTime
 	}
 
 	currentStakerIterator, err := preferredState.GetCurrentStakerIterator()