refactor(types)!: removed global config for verifier (#18607)

Co-authored-by: Julian Toledano <jtoledanodiaz@gmail.com>
Co-authored-by: Julián Toledano <JulianToledano@users.noreply.github.com>

CHANGELOG.md

@@ -131,6 +131,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
 * (x/auth) [#18351](https://github.com/cosmos/cosmos-sdk/pull/18351) Auth module was moved to its own go.mod `cosmossdk.io/x/auth`
 * (types) [#18372](https://github.com/cosmos/cosmos-sdk/pull/18372) Removed global configuration for coin type and purpose. Setters and getters should be removed and access directly to defined types.
 * (types) [#18695](https://github.com/cosmos/cosmos-sdk/pull/18695) Removed global configuration for txEncoder.
+* (types) [#18607](https://github.com/cosmos/cosmos-sdk/pull/18607) Removed address verifier from global config, moved verifier function to bech32 codec.
 * (server) [#18909](https://github.com/cosmos/cosmos-sdk/pull/18909) Remove configuration endpoint on grpc reflection endpoint in favour of auth module bech32prefix endpoint already exposed.
 
 ### Client Breaking Changes

codec/address/bech32_codec.go

@@ -7,7 +7,7 @@ import (
 	"cosmossdk.io/core/address"
 	errorsmod "cosmossdk.io/errors"
 
-	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkAddress "github.com/cosmos/cosmos-sdk/types/address"
 	"github.com/cosmos/cosmos-sdk/types/bech32"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
@@ -33,12 +33,12 @@ func (bc Bech32Codec) StringToBytes(text string) ([]byte, error) {
 		return nil, err
 	}
 
-	if hrp != bc.Bech32Prefix {
-		return nil, errorsmod.Wrapf(sdkerrors.ErrLogic, "hrp does not match bech32 prefix: expected '%s' got '%s'", bc.Bech32Prefix, hrp)
+	if len(bz) > sdkAddress.MaxAddrLen {
+		return nil, errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "address max length is %d, got %d", sdkAddress.MaxAddrLen, len(bz))
 	}
 
-	if err := sdk.VerifyAddressFormat(bz); err != nil {
-		return nil, err
+	if hrp != bc.Bech32Prefix {
+		return nil, errorsmod.Wrapf(sdkerrors.ErrLogic, "hrp does not match bech32 prefix: expected '%s' got '%s'", bc.Bech32Prefix, hrp)
 	}
 
 	return bz, nil
@@ -55,5 +55,9 @@ func (bc Bech32Codec) BytesToString(bz []byte) (string, error) {
 		return "", err
 	}
 
+	if len(bz) > sdkAddress.MaxAddrLen {
+		return "", errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "address max length is %d, got %d", sdkAddress.MaxAddrLen, len(bz))
+	}
+
 	return text, nil
 }

fuzz/oss-fuzz-build.sh

@@ -36,7 +36,6 @@ build_go_fuzzer FuzzTendermintAminoDecodeTime fuzz_tendermint_amino_decodetime
 build_go_fuzzer FuzzTypesParseCoin fuzz_types_parsecoin
 build_go_fuzzer FuzzTypesParseDecCoin fuzz_types_parsedeccoin
 build_go_fuzzer FuzzTypesParseTimeBytes fuzz_types_parsetimebytes
-build_go_fuzzer FuzzTypesVerifyAddressFormat fuzz_types_verifyaddressformat
 build_go_fuzzer FuzzTypesDecSetString fuzz_types_dec_setstring
 
 build_go_fuzzer FuzzUnknownProto fuzz_unknownproto

types/address.go

@@ -6,20 +6,16 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"strings"
 	"sync"
 	"sync/atomic"
 
 	"github.com/hashicorp/golang-lru/simplelru"
 	"sigs.k8s.io/yaml"
 
-	errorsmod "cosmossdk.io/errors"
-
+	addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	"github.com/cosmos/cosmos-sdk/internal/conv"
-	"github.com/cosmos/cosmos-sdk/types/address"
 	"github.com/cosmos/cosmos-sdk/types/bech32"
-	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 const (
@@ -157,28 +153,6 @@ func AccAddressFromHexUnsafe(address string) (addr AccAddress, err error) {
 	return AccAddress(bz), err
 }
 
-// VerifyAddressFormat verifies that the provided bytes form a valid address
-// according to the default address rules or a custom address verifier set by
-// GetConfig().SetAddressVerifier().
-// TODO make an issue to get rid of global Config
-// ref: https://github.com/cosmos/cosmos-sdk/issues/9690
-func VerifyAddressFormat(bz []byte) error {
-	verifier := GetConfig().GetAddressVerifier()
-	if verifier != nil {
-		return verifier(bz)
-	}
-
-	if len(bz) == 0 {
-		return errorsmod.Wrap(sdkerrors.ErrUnknownAddress, "addresses cannot be empty")
-	}
-
-	if len(bz) > address.MaxAddrLen {
-		return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "address max length is %d, got %d", address.MaxAddrLen, len(bz))
-	}
-
-	return nil
-}
-
 // MustAccAddressFromBech32 calls AccAddressFromBech32 and panics on error.
 func MustAccAddressFromBech32(address string) AccAddress {
 	addr, err := AccAddressFromBech32(address)
@@ -191,23 +165,10 @@ func MustAccAddressFromBech32(address string) AccAddress {
 
 // AccAddressFromBech32 creates an AccAddress from a Bech32 string.
 func AccAddressFromBech32(address string) (addr AccAddress, err error) {
-	if len(strings.TrimSpace(address)) == 0 {
-		return AccAddress{}, errors.New("empty address string is not allowed")
-	}
-
 	bech32PrefixAccAddr := GetConfig().GetBech32AccountAddrPrefix()
 
-	bz, err := GetFromBech32(address, bech32PrefixAccAddr)
-	if err != nil {
-		return nil, err
-	}
-
-	err = VerifyAddressFormat(bz)
-	if err != nil {
-		return nil, err
-	}
-
-	return AccAddress(bz), nil
+	addrCdc := addresscodec.NewBech32Codec(bech32PrefixAccAddr)
+	return addrCdc.StringToBytes(address)
 }
 
 // Returns boolean for whether two AccAddresses are Equal
@@ -343,23 +304,10 @@ func ValAddressFromHex(address string) (addr ValAddress, err error) {
 
 // ValAddressFromBech32 creates a ValAddress from a Bech32 string.
 func ValAddressFromBech32(address string) (addr ValAddress, err error) {
-	if len(strings.TrimSpace(address)) == 0 {
-		return ValAddress{}, errors.New("empty address string is not allowed")
-	}
-
 	bech32PrefixValAddr := GetConfig().GetBech32ValidatorAddrPrefix()
 
-	bz, err := GetFromBech32(address, bech32PrefixValAddr)
-	if err != nil {
-		return nil, err
-	}
-
-	err = VerifyAddressFormat(bz)
-	if err != nil {
-		return nil, err
-	}
-
-	return ValAddress(bz), nil
+	addrCdc := addresscodec.NewBech32Codec(bech32PrefixValAddr)
+	return addrCdc.StringToBytes(address)
 }
 
 // MustValAddressFromBech32 calls ValAddressFromBech32 and panics on error.
@@ -508,23 +456,10 @@ func ConsAddressFromHex(address string) (addr ConsAddress, err error) {
 
 // ConsAddressFromBech32 creates a ConsAddress from a Bech32 string.
 func ConsAddressFromBech32(address string) (addr ConsAddress, err error) {
-	if len(strings.TrimSpace(address)) == 0 {
-		return ConsAddress{}, errors.New("empty address string is not allowed")
-	}
-
 	bech32PrefixConsAddr := GetConfig().GetBech32ConsensusAddrPrefix()
 
-	bz, err := GetFromBech32(address, bech32PrefixConsAddr)
-	if err != nil {
-		return nil, err
-	}
-
-	err = VerifyAddressFormat(bz)
-	if err != nil {
-		return nil, err
-	}
-
-	return ConsAddress(bz), nil
+	addrCdc := addresscodec.NewBech32Codec(bech32PrefixConsAddr)
+	return addrCdc.StringToBytes(address)
 }
 
 // get ConsAddress from pubkey

types/address_test.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"crypto/rand"
 	"encoding/hex"
-	"fmt"
 	mathrand "math/rand"
 	"strings"
 	"testing"
@@ -381,66 +380,6 @@ func (s *addressTestSuite) TestAddressInterface() {
 	}
 }
 
-func (s *addressTestSuite) TestVerifyAddressFormat() {
-	addr0 := make([]byte, 0)
-	addr5 := make([]byte, 5)
-	addr20 := make([]byte, 20)
-	addr32 := make([]byte, 32)
-	addr256 := make([]byte, 256)
-
-	err := types.VerifyAddressFormat(addr0)
-	s.Require().EqualError(err, "addresses cannot be empty: unknown address")
-	err = types.VerifyAddressFormat(addr5)
-	s.Require().NoError(err)
-	err = types.VerifyAddressFormat(addr20)
-	s.Require().NoError(err)
-	err = types.VerifyAddressFormat(addr32)
-	s.Require().NoError(err)
-	err = types.VerifyAddressFormat(addr256)
-	s.Require().EqualError(err, "address max length is 255, got 256: unknown address")
-}
-
-func (s *addressTestSuite) TestCustomAddressVerifier() {
-	// Create a 10 byte address
-	addr := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}
-	accBech := types.AccAddress(addr).String()
-	valBech := types.ValAddress(addr).String()
-	consBech := types.ConsAddress(addr).String()
-	// Verify that the default logic doesn't reject this 10 byte address
-	// The default verifier is nil, we're only checking address length is
-	// between 1-255 bytes.
-	err := types.VerifyAddressFormat(addr)
-	s.Require().Nil(err)
-	_, err = types.AccAddressFromBech32(accBech)
-	s.Require().Nil(err)
-	_, err = types.ValAddressFromBech32(valBech)
-	s.Require().Nil(err)
-	_, err = types.ConsAddressFromBech32(consBech)
-	s.Require().Nil(err)
-
-	// Set a custom address verifier only accepts 20 byte addresses
-	types.GetConfig().SetAddressVerifier(func(bz []byte) error {
-		n := len(bz)
-		if n == 20 {
-			return nil
-		}
-		return fmt.Errorf("incorrect address length %d", n)
-	})
-
-	// Verify that the custom logic rejects this 10 byte address
-	err = types.VerifyAddressFormat(addr)
-	s.Require().NotNil(err)
-	_, err = types.AccAddressFromBech32(accBech)
-	s.Require().NotNil(err)
-	_, err = types.ValAddressFromBech32(valBech)
-	s.Require().NotNil(err)
-	_, err = types.ConsAddressFromBech32(consBech)
-	s.Require().NotNil(err)
-
-	// Reinitialize the global config to default address verifier (nil)
-	types.GetConfig().SetAddressVerifier(nil)
-}
-
 func (s *addressTestSuite) TestBech32ifyAddressBytes() {
 	addr10byte := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}
 	addr20byte := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}

types/config.go

@@ -15,7 +15,6 @@ const DefaultKeyringServiceName = "cosmos"
 type Config struct {
 	fullFundraiserPath  string
 	bech32AddressPrefix map[string]string
-	addressVerifier     func([]byte) error
 	mtx                 sync.RWMutex
 
 	sealed   bool
@@ -97,13 +96,6 @@ func (config *Config) SetBech32PrefixForConsensusNode(addressPrefix, pubKeyPrefi
 	config.bech32AddressPrefix["consensus_pub"] = pubKeyPrefix
 }
 
-// SetAddressVerifier builds the Config with the provided function for verifying that addresses
-// have the correct format
-func (config *Config) SetAddressVerifier(addressVerifier func([]byte) error) {
-	config.assertNotSealed()
-	config.addressVerifier = addressVerifier
-}
-
 // Set the FullFundraiserPath (BIP44Prefix) on the config.
 //
 // Deprecated: This method is supported for backward compatibility only and will be removed in a future release. Use SetPurpose and SetCoinType instead.
@@ -159,18 +151,6 @@ func (config *Config) GetBech32ConsensusPubPrefix() string {
 	return config.bech32AddressPrefix["consensus_pub"]
 }
 
-// GetAddressVerifier returns the function to verify that addresses have the correct format
-func (config *Config) GetAddressVerifier() func([]byte) error {
-	return config.addressVerifier
-}
-
-// GetFullFundraiserPath returns the BIP44Prefix.
-//
-// Deprecated: This method is supported for backward compatibility only and will be removed in a future release. Use GetFullBIP44Path instead.
-func (config *Config) GetFullFundraiserPath() string {
-	return config.fullFundraiserPath
-}
-
 func KeyringServiceName() string {
 	if len(version.Name) == 0 {
 		return DefaultKeyringServiceName

types/config_test.go

@@ -16,18 +16,6 @@ func TestConfigTestSuite(t *testing.T) {
 	suite.Run(t, new(configTestSuite))
 }
 
-func (s *configTestSuite) TestConfig_SetFullFundraiserPath() {
-	config := sdk.NewConfig()
-	config.SetFullFundraiserPath("test/path")
-	s.Require().Equal("test/path", config.GetFullFundraiserPath())
-
-	config.SetFullFundraiserPath("test/poth")
-	s.Require().Equal("test/poth", config.GetFullFundraiserPath())
-
-	config.Seal()
-	s.Require().Panics(func() { config.SetFullFundraiserPath("x/test/path") })
-}
-
 func (s *configTestSuite) TestKeyringServiceName() {
 	s.Require().Equal(sdk.DefaultKeyringServiceName, sdk.KeyringServiceName())
 }

x/auth/types/credentials_test.go

@@ -20,13 +20,11 @@ func TestNewModuleCrendentials(t *testing.T) {
 
 	expected := sdk.MustAccAddressFromBech32("cosmos1fpn0w0yf4x300llf5r66jnfhgj4ul6cfahrvqsskwkhsw6sv84wsmz359y")
 
-	credential, err := authtypes.NewModuleCredential("group")
+	_, err = authtypes.NewModuleCredential("group")
 	require.NoError(t, err, "must be able to create a Root Module credential (see ADR-33)")
-	require.NoError(t, sdk.VerifyAddressFormat(credential.Address()))
 
-	credential, err = authtypes.NewModuleCredential("group", [][]byte{{0x20}, {0x0}}...)
+	credential, err := authtypes.NewModuleCredential("group", [][]byte{{0x20}, {0x0}}...)
 	require.NoError(t, err)
-	require.NoError(t, sdk.VerifyAddressFormat(credential.Address()))
 	addr, err := sdk.AccAddressFromHexUnsafe(credential.Address().String())
 	require.NoError(t, err)
 	require.Equal(t, expected.String(), addr.String())