fix: accaddr cachefix (#15433)

Co-authored-by: Julien Robert <julien@rbrt.fr>
Co-authored-by: Marko <marbar3778@yahoo.com>
This commit is contained in:
KyleMoser
2023-03-31 12:12:33 +00:00
committed by GitHub
co-authored by Julien Robert Marko
parent a9546bfad0
commit cdf6471e51
3 changed files with 117 additions and 16 deletions
+1
View File
@@ -159,6 +159,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Bug Fixes
* (types) [#15433](https://github.com/cosmos/cosmos-sdk/pull/15433) Allow disabling of account address caches (for printing bech32 account addresses).
* (x/auth) [#15059](https://github.com/cosmos/cosmos-sdk/pull/15059) `ante.CountSubKeys` returns 0 when passing a nil `Pubkey`.
* (x/capability) [#15030](https://github.com/cosmos/cosmos-sdk/pull/15030) Prevent `x/capability` from consuming `GasMeter` gas during `InitMemStore`
* (types/coin) [#14739](https://github.com/cosmos/cosmos-sdk/pull/14739) Deprecate the method `Coin.IsEqual` in favour of `Coin.Equal`. The difference between the two methods is that the first one results in a panic when denoms are not equal. This panic lead to unexpected behavior
+45 -16
View File
@@ -8,6 +8,7 @@ import (
"fmt"
"strings"
"sync"
"sync/atomic"
"github.com/hashicorp/golang-lru/simplelru"
"sigs.k8s.io/yaml"
@@ -85,6 +86,8 @@ var (
consAddrCache *simplelru.LRU
valAddrMu sync.Mutex
valAddrCache *simplelru.LRU
isCachingEnabled atomic.Bool
)
// sentinel errors
@@ -94,6 +97,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 {
@@ -107,6 +112,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
@@ -287,11 +302,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)
}
@@ -437,11 +456,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)
}
@@ -592,11 +615,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)
}
@@ -676,6 +703,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
View File
@@ -126,6 +126,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}