Skip to content

Commit

Permalink
refactor(types): audit QA (#21355)
Browse files Browse the repository at this point in the history
  • Loading branch information
akhilkumarpilli authored Aug 27, 2024
1 parent 03d3a93 commit e98b8e9
Show file tree
Hide file tree
Showing 7 changed files with 162 additions and 61 deletions.
2 changes: 0 additions & 2 deletions api/cosmos/base/abci/v1beta1/abci.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions proto/cosmos/base/abci/v1beta1/abci.proto
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ message TxResponse {
// these events include those emitted by processing all the messages and those
// emitted from the ante. Whereas Logs contains the events, with
// additional metadata, emitted only by processing the messages.
//
// Since: cosmos-sdk 0.42.11, 0.44.5, 0.45
repeated cometbft.abci.v1.Event events = 13
[(gogoproto.nullable) = false, (cosmos_proto.field_added_in) = "cosmos-sdk 0.45"];
}
Expand Down
2 changes: 0 additions & 2 deletions types/abci.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

83 changes: 36 additions & 47 deletions types/address_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,6 @@ import (
"github.com/cosmos/cosmos-sdk/types/bech32/legacybech32" //nolint:staticcheck // we're using this to support the legacy way of dealing with bech32
)

const (
pubStr = "pub"
valoper = "valoper"
valoperpub = "valoperpub"
valcons = "valcons"
valconspub = "valconspub"
)

type addressTestSuite struct {
suite.Suite
}
Expand All @@ -42,17 +34,22 @@ func (s *addressTestSuite) SetupSuite() {
s.T().Parallel()
}

var invalidStrs = []string{
"hello, world!",
"0xAA",
"AAA",
types.Bech32PrefixAccAddr + "AB0C",
types.Bech32PrefixAccPub + "1234",
types.Bech32PrefixValAddr + "5678",
types.Bech32PrefixValPub + "BBAB",
types.Bech32PrefixConsAddr + "FF04",
types.Bech32PrefixConsPub + "6789",
}
var (
invalidStrs = []string{
"hello, world!",
"0xAA",
"AAA",
types.Bech32PrefixAccAddr + "AB0C",
types.Bech32PrefixAccPub + "1234",
types.Bech32PrefixValAddr + "5678",
types.Bech32PrefixValPub + "BBAB",
types.Bech32PrefixConsAddr + "FF04",
types.Bech32PrefixConsPub + "6789",
}

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}
)

func (s *addressTestSuite) testMarshal(original, res interface{}, marshal func() ([]byte, error), unmarshal func([]byte) error) {
bz, err := marshal()
Expand Down Expand Up @@ -161,6 +158,12 @@ func (s *addressTestSuite) TestUnmarshalYAMLWithInvalidInput() {
s.Require().Equal(types.ErrEmptyHexAddress, err)
}

func setConfigWithPrefix(conf *types.Config, prefix string) {
conf.SetBech32PrefixForAccount(prefix, types.GetBech32PrefixAccPub(prefix))
conf.SetBech32PrefixForValidator(types.GetBech32PrefixValAddr(prefix), types.GetBech32PrefixValPub(prefix))
conf.SetBech32PrefixForConsensusNode(types.GetBech32PrefixConsAddr(prefix), types.GetBech32PrefixConsPub(prefix))
}

// Test that the account address cache ignores the bech32 prefix setting, retrieving bech32 addresses from the cache.
// This will cause the AccAddress.String() to print out unexpected prefixes if the config was changed between bech32 lookups.
// See https://github.com/cosmos/cosmos-sdk/issues/15317.
Expand All @@ -173,18 +176,14 @@ func (s *addressTestSuite) TestAddrCache() {
// Set SDK bech32 prefixes to 'osmo'
prefix := "osmo"
conf := types.GetConfig()
conf.SetBech32PrefixForAccount(prefix, prefix+pubStr)
conf.SetBech32PrefixForValidator(prefix+valoper, prefix+valoperpub)
conf.SetBech32PrefixForConsensusNode(prefix+valcons, prefix+valconspub)
setConfigWithPrefix(conf, prefix)

acc := types.AccAddress(pub.Address())
osmoAddrBech32 := acc.String()

// Set SDK bech32 to 'cosmos'
prefix = "cosmos"
conf.SetBech32PrefixForAccount(prefix, prefix+pubStr)
conf.SetBech32PrefixForValidator(prefix+valoper, prefix+valoperpub)
conf.SetBech32PrefixForConsensusNode(prefix+valcons, prefix+valconspub)
setConfigWithPrefix(conf, prefix)

// We name this 'addrCosmos' to prove a point, but the bech32 address will still begin with 'osmo' due to the cache behavior.
addrCosmos := types.AccAddress(pub.Address())
Expand All @@ -210,18 +209,14 @@ func (s *addressTestSuite) TestAddrCacheDisabled() {
// Set SDK bech32 prefixes to 'osmo'
prefix := "osmo"
conf := types.GetConfig()
conf.SetBech32PrefixForAccount(prefix, prefix+pubStr)
conf.SetBech32PrefixForValidator(prefix+valoper, prefix+valoperpub)
conf.SetBech32PrefixForConsensusNode(prefix+valcons, prefix+valconspub)
setConfigWithPrefix(conf, prefix)

acc := types.AccAddress(pub.Address())
osmoAddrBech32 := acc.String()

// Set SDK bech32 to 'cosmos'
prefix = "cosmos"
conf.SetBech32PrefixForAccount(prefix, prefix+pubStr)
conf.SetBech32PrefixForValidator(prefix+valoper, prefix+valoperpub)
conf.SetBech32PrefixForConsensusNode(prefix+valcons, prefix+valconspub)
setConfigWithPrefix(conf, prefix)

addrCosmos := types.AccAddress(pub.Address())
cosmosAddrBech32 := addrCosmos.String()
Expand Down Expand Up @@ -409,8 +404,6 @@ func (s *addressTestSuite) TestAddressInterface() {
}

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}
type args struct {
prefix string
bs []byte
Expand Down Expand Up @@ -442,8 +435,6 @@ func (s *addressTestSuite) TestBech32ifyAddressBytes() {
}

func (s *addressTestSuite) TestMustBech32ifyAddressBytes() {
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}
type args struct {
prefix string
bs []byte
Expand Down Expand Up @@ -536,7 +527,7 @@ func (s *addressTestSuite) TestGetFromBech32() {
}

func (s *addressTestSuite) TestMustValAddressFromBech32() {
valAddress1 := types.ValAddress([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19})
valAddress1 := types.ValAddress(addr20byte)
valAddress2 := types.MustValAddressFromBech32(valAddress1.String())

s.Require().Equal(valAddress1, valAddress2)
Expand Down Expand Up @@ -589,7 +580,7 @@ func (s *addressTestSuite) TestGetBech32PrefixConsPub() {
}

func (s *addressTestSuite) TestMustAccAddressFromBech32() {
accAddress1 := types.AccAddress([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19})
accAddress1 := types.AccAddress(addr20byte)
accAddress2 := types.MustAccAddressFromBech32(accAddress1.String())
s.Require().Equal(accAddress1, accAddress2)
}
Expand Down Expand Up @@ -628,10 +619,9 @@ func (s *addressTestSuite) TestUnmarshalYAMLAccAddressWithEmptyString() {
}

func (s *addressTestSuite) TestFormatAccAddressAsString() {
addr20byte := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}
accAddr := types.AccAddress(addr20byte)

actual := fmt.Sprintf("%s", accAddr) // this will call internally the method AccAddress.Format
actual := fmt.Sprintf("%s", accAddr) // this will internally calls AccAddress.Format method

hrp := types.GetConfig().GetBech32AccountAddrPrefix()
expected, err := bech32.ConvertAndEncode(hrp, addr20byte)
Expand All @@ -640,9 +630,9 @@ func (s *addressTestSuite) TestFormatAccAddressAsString() {
}

func (s *addressTestSuite) TestFormatAccAddressAsPointer() {
accAddr := types.AccAddress([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19})
accAddr := types.AccAddress(addr20byte)
ptrAddr := &accAddr
actual := fmt.Sprintf("%p", ptrAddr) // this will call internally the method AccAddress.Format
actual := fmt.Sprintf("%p", ptrAddr) // this will internally calls AccAddress.Format method
expected := fmt.Sprintf("0x%x", uintptr(unsafe.Pointer(&accAddr)))
s.Require().Equal(expected, actual)
}
Expand Down Expand Up @@ -684,7 +674,6 @@ func (s *addressTestSuite) TestUnmarshalYAMLValAddressWithEmptyString() {
}

func (s *addressTestSuite) TestFormatValAddressAsString() {
addr20byte := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}
accAddr := types.ValAddress(addr20byte)

actual := fmt.Sprintf("%s", accAddr)
Expand All @@ -696,15 +685,15 @@ func (s *addressTestSuite) TestFormatValAddressAsString() {
}

func (s *addressTestSuite) TestFormatValAddressAsPointer() {
accAddr := types.AccAddress([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19})
accAddr := types.AccAddress(addr20byte)
ptrAddr := &accAddr
actual := fmt.Sprintf("%p", ptrAddr)
expected := uintptr(unsafe.Pointer(&accAddr))
s.Require().Equal(fmt.Sprintf("0x%x", expected), actual)
}

func (s *addressTestSuite) TestFormatValAddressWhenVerbIsDifferentFromSOrP() {
myAddr := types.ValAddress([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19})
myAddr := types.ValAddress(addr20byte)
exp := "000102030405060708090A0B0C0D0E0F10111213"
spec := []string{"%v", "%#v", "%t", "%b", "%c", "%d", "%o", "%O", "%x", "%X", "%U", "%e", "%E", "%f", "%F", "%g", "%G"}
for _, v := range spec {
Expand Down Expand Up @@ -740,7 +729,7 @@ func (s *addressTestSuite) TestUnmarshalYAMLConsAddressWithEmptyString() {
}

func (s *addressTestSuite) TestFormatConsAddressAsString() {
consAddr := types.ConsAddress([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19})
consAddr := types.ConsAddress(addr20byte)
actual := fmt.Sprintf("%s", consAddr)

hrp := types.GetConfig().GetBech32ConsensusAddrPrefix()
Expand All @@ -750,7 +739,7 @@ func (s *addressTestSuite) TestFormatConsAddressAsString() {
}

func (s *addressTestSuite) TestFormatConsAddressAsPointer() {
consAddr := types.ConsAddress([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19})
consAddr := types.ConsAddress(addr20byte)

ptrAddr := &consAddr
actual := fmt.Sprintf("%p", ptrAddr)
Expand All @@ -759,7 +748,7 @@ func (s *addressTestSuite) TestFormatConsAddressAsPointer() {
}

func (s *addressTestSuite) TestFormatConsAddressWhenVerbIsDifferentFromSOrP() {
myAddr := types.ConsAddress([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19})
myAddr := types.ConsAddress(addr20byte)
exp := "000102030405060708090A0B0C0D0E0F10111213"
spec := []string{"%v", "%#v", "%t", "%b", "%c", "%d", "%o", "%O", "%x", "%X", "%U", "%e", "%E", "%f", "%F", "%g", "%G"}
for _, v := range spec {
Expand Down
2 changes: 1 addition & 1 deletion types/coin.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ func (coins Coins) Validate() error {
}
}

// IsSorted returns true when coins are order ASC sorted with denoms.
// IsSorted returns true when coins are sorted in ASC order by denoms.
func (coins Coins) IsSorted() bool {
for i := 1; i < len(coins); i++ {
if coins[i-1].Denom > coins[i].Denom {
Expand Down
14 changes: 7 additions & 7 deletions types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ type Context struct {
chainID string // Deprecated: Use HeaderService for chainID and CometService for the rest
txBytes []byte
logger log.Logger
voteInfo []abci.VoteInfo // Deprecated: use Cometinfo.LastCommit.Votes instead, will be removed after 0.51
voteInfo []abci.VoteInfo // Deprecated: use Cometinfo.LastCommit.Votes instead, will be removed after 0.52
gasMeter storetypes.GasMeter
blockGasMeter storetypes.GasMeter
checkTx bool // Deprecated: use execMode instead, will be removed after 0.51
recheckTx bool // if recheckTx == true, then checkTx must also be true // Deprecated: use execMode instead, will be removed after 0.51
checkTx bool // Deprecated: use execMode instead, will be removed after 0.52
recheckTx bool // if recheckTx == true, then checkTx must also be true // Deprecated: use execMode instead, will be removed after 0.52
sigverifyTx bool // when run simulation, because the private key corresponding to the account in the genesis.json randomly generated, we must skip the sigverify.
execMode ExecMode
minGasPrice DecCoins
Expand Down Expand Up @@ -102,7 +102,7 @@ func (c Context) HeaderHash() []byte {
return hash
}

// Deprecated: getting consensus params from the context is deprecated and will be removed after 0.51
// Deprecated: getting consensus params from the context is deprecated and will be removed after 0.52
// Querying the consensus module for the parameters is required in server/v2
func (c Context) ConsensusParams() cmtproto.ConsensusParams {
return c.consParams
Expand Down Expand Up @@ -138,7 +138,7 @@ func NewContext(ms storetypes.MultiStore, isCheckTx bool, logger log.Logger) Con
kvGasConfig: storetypes.KVGasConfig(),
transientKVGasConfig: storetypes.TransientGasConfig(),
headerInfo: header.Info{
Time: h.Time.UTC(),
Time: h.Time,
},
}
}
Expand All @@ -161,7 +161,7 @@ func (c Context) WithBlockHeader(header cmtproto.Header) Context {
header.Time = header.Time.UTC()
c.header = header

// when calling withBlockheader on a new context chainID in the struct is empty
// when calling withBlockheader on a new context, chainID in the struct will be empty
c.chainID = header.ChainID
return c
}
Expand Down Expand Up @@ -208,7 +208,7 @@ func (c Context) WithLogger(logger log.Logger) Context {
}

// WithVoteInfos returns a Context with an updated consensus VoteInfo.
// Deprecated: use WithCometinfo() instead, will be removed after 0.51
// Deprecated: use WithCometinfo() instead, will be removed after 0.52
func (c Context) WithVoteInfos(voteInfo []abci.VoteInfo) Context {
c.voteInfo = voteInfo
return c
Expand Down
Loading

0 comments on commit e98b8e9

Please sign in to comment.