diff --git a/CHANGELOG.md b/CHANGELOG.md index 753cc73acd2..6e4143ca0e6 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/codec/address/bech32_codec.go b/codec/address/bech32_codec.go index 45e9d2c0117..ea6b669e682 100644 --- a/codec/address/bech32_codec.go +++ b/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 } diff --git a/fuzz/oss-fuzz-build.sh b/fuzz/oss-fuzz-build.sh index 733498d691b..882b77f5242 100644 --- a/fuzz/oss-fuzz-build.sh +++ b/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 diff --git a/fuzz/tests/types_verifyaddressformat_test.go b/fuzz/tests/types_verifyaddressformat_test.go deleted file mode 100644 index 55e0014cc7c..00000000000 --- a/fuzz/tests/types_verifyaddressformat_test.go +++ /dev/null @@ -1,15 +0,0 @@ -//go:build gofuzz || go1.18 - -package tests - -import ( - "testing" - - "github.com/cosmos/cosmos-sdk/types" -) - -func FuzzTypesVerifyAddressFormat(f *testing.F) { - f.Fuzz(func(t *testing.T, data []byte) { - _ = types.VerifyAddressFormat(data) - }) -} diff --git a/types/address.go b/types/address.go index eca97cee297..3cafd383eb8 100644 --- a/types/address.go +++ b/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 diff --git a/types/address_test.go b/types/address_test.go index 414751c7ea9..5d7fb78d80f 100644 --- a/types/address_test.go +++ b/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} diff --git a/types/config.go b/types/config.go index 09a1ce69f5b..7f638f255e3 100644 --- a/types/config.go +++ b/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 diff --git a/types/config_test.go b/types/config_test.go index 51579f0b2f3..266fbe64b99 100644 --- a/types/config_test.go +++ b/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()) } diff --git a/x/auth/types/credentials_test.go b/x/auth/types/credentials_test.go index f31339ac564..12ddf0a24fd 100644 --- a/x/auth/types/credentials_test.go +++ b/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())