Skip to content

Commit

Permalink
fix: accaddr cachefix (backport #15433) (#16823)
Browse files Browse the repository at this point in the history
Co-authored-by: KyleMoser <KyleMoser@users.noreply.github.com>
Co-authored-by: HuangYi <huang@crypto.com>
  • Loading branch information
3 people committed Jul 5, 2023
1 parent a763135 commit b39cdb2
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 16 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

* (x/auth) [#16554](https://github.com/cosmos/cosmos-sdk/pull/16554) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking.
* [#16588](https://github.com/cosmos/cosmos-sdk/pull/16588) Propogate the Snapshotter's failure to the caller, (it will create a empty snapshot silently before).
* (types) [#15433](https://github.com/cosmos/cosmos-sdk/pull/15433) Allow disabling of account address caches (for printing bech32 account addresses).

## [v0.46.13](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.13) - 2023-06-08

Expand Down
61 changes: 45 additions & 16 deletions types/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"strings"
"sync"
"sync/atomic"

"github.com/hashicorp/golang-lru/simplelru"
"sigs.k8s.io/yaml"
Expand Down Expand Up @@ -83,6 +84,8 @@ var (
consAddrCache *simplelru.LRU
valAddrMu sync.Mutex
valAddrCache *simplelru.LRU

isCachingEnabled atomic.Bool
)

// sentinel errors
Expand All @@ -92,6 +95,8 @@ var (

func init() {
var err error
SetAddrCacheEnabled(true)

// in total the cache size is 61k entries. Key is 32 bytes and value is around 50-70 bytes.
// That will make around 92 * 61k * 2 (LRU) bytes ~ 11 MB
if accAddrCache, err = simplelru.NewLRU(60000, nil); err != nil {
Expand All @@ -105,6 +110,16 @@ func init() {
}
}

// SetAddrCacheEnabled enables or disables accAddrCache, consAddrCache, and valAddrCache. By default, caches are enabled.
func SetAddrCacheEnabled(enabled bool) {
isCachingEnabled.Store(enabled)
}

// IsAddrCacheEnabled returns if the address caches are enabled.
func IsAddrCacheEnabled() bool {
return isCachingEnabled.Load()
}

// Address is a common interface for different types of addresses used by the SDK
type Address interface {
Equals(Address) bool
Expand Down Expand Up @@ -285,11 +300,15 @@ func (aa AccAddress) String() string {
}

key := conv.UnsafeBytesToStr(aa)
accAddrMu.Lock()
defer accAddrMu.Unlock()
addr, ok := accAddrCache.Get(key)
if ok {
return addr.(string)

if IsAddrCacheEnabled() {
accAddrMu.Lock()
defer accAddrMu.Unlock()

addr, ok := accAddrCache.Get(key)
if ok {
return addr.(string)
}
}
return cacheBech32Addr(GetConfig().GetBech32AccountAddrPrefix(), aa, accAddrCache, key)
}
Expand Down Expand Up @@ -435,11 +454,15 @@ func (va ValAddress) String() string {
}

key := conv.UnsafeBytesToStr(va)
valAddrMu.Lock()
defer valAddrMu.Unlock()
addr, ok := valAddrCache.Get(key)
if ok {
return addr.(string)

if IsAddrCacheEnabled() {
valAddrMu.Lock()
defer valAddrMu.Unlock()

addr, ok := valAddrCache.Get(key)
if ok {
return addr.(string)
}
}
return cacheBech32Addr(GetConfig().GetBech32ValidatorAddrPrefix(), va, valAddrCache, key)
}
Expand Down Expand Up @@ -590,11 +613,15 @@ func (ca ConsAddress) String() string {
}

key := conv.UnsafeBytesToStr(ca)
consAddrMu.Lock()
defer consAddrMu.Unlock()
addr, ok := consAddrCache.Get(key)
if ok {
return addr.(string)

if IsAddrCacheEnabled() {
consAddrMu.Lock()
defer consAddrMu.Unlock()

addr, ok := consAddrCache.Get(key)
if ok {
return addr.(string)
}
}
return cacheBech32Addr(GetConfig().GetBech32ConsensusAddrPrefix(), ca, consAddrCache, key)
}
Expand Down Expand Up @@ -674,6 +701,8 @@ func cacheBech32Addr(prefix string, addr []byte, cache *simplelru.LRU, cacheKey
if err != nil {
panic(err)
}
cache.Add(cacheKey, bech32Addr)
if IsAddrCacheEnabled() {
cache.Add(cacheKey, bech32Addr)
}
return bech32Addr
}
71 changes: 71 additions & 0 deletions types/address_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,77 @@ func (s *addressTestSuite) TestRandBech32AccAddrConsistency() {
s.Require().Equal(types.ErrEmptyHexAddress, err)
}

// 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.
func (s *addressTestSuite) TestAddrCache() {
// Use a random key
pubBz := make([]byte, ed25519.PubKeySize)
pub := &ed25519.PubKey{Key: pubBz}
rand.Read(pub.Key)

// Set SDK bech32 prefixes to 'osmo'
prefix := "osmo"
conf := types.GetConfig()
conf.SetBech32PrefixForAccount(prefix, prefix+"pub")
conf.SetBech32PrefixForValidator(prefix+"valoper", prefix+"valoperpub")
conf.SetBech32PrefixForConsensusNode(prefix+"valcons", prefix+"valconspub")

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

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

// 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())
cosmosAddrBech32 := addrCosmos.String()

// The default behavior will retrieve the bech32 address from the cache, ignoring the bech32 prefix change.
s.Require().Equal(osmoAddrBech32, cosmosAddrBech32)
s.Require().True(strings.HasPrefix(osmoAddrBech32, "osmo"))
s.Require().True(strings.HasPrefix(cosmosAddrBech32, "osmo"))
}

// Test that the bech32 prefix is respected when the address cache is disabled.
// This causes AccAddress.String() to print out the expected prefixes if the config is changed between bech32 lookups.
// See https://github.com/cosmos/cosmos-sdk/issues/15317.
func (s *addressTestSuite) TestAddrCacheDisabled() {
types.SetAddrCacheEnabled(false)

// Use a random key
pubBz := make([]byte, ed25519.PubKeySize)
pub := &ed25519.PubKey{Key: pubBz}
rand.Read(pub.Key)

// Set SDK bech32 prefixes to 'osmo'
prefix := "osmo"
conf := types.GetConfig()
conf.SetBech32PrefixForAccount(prefix, prefix+"pub")
conf.SetBech32PrefixForValidator(prefix+"valoper", prefix+"valoperpub")
conf.SetBech32PrefixForConsensusNode(prefix+"valcons", prefix+"valconspub")

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

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

addrCosmos := types.AccAddress(pub.Address())
cosmosAddrBech32 := addrCosmos.String()

// retrieve the bech32 address from the cache, respecting the bech32 prefix change.
s.Require().NotEqual(osmoAddrBech32, cosmosAddrBech32)
s.Require().True(strings.HasPrefix(osmoAddrBech32, "osmo"))
s.Require().True(strings.HasPrefix(cosmosAddrBech32, "cosmos"))
}

func (s *addressTestSuite) TestValAddr() {
pubBz := make([]byte, ed25519.PubKeySize)
pub := &ed25519.PubKey{Key: pubBz}
Expand Down

0 comments on commit b39cdb2

Please sign in to comment.