From 7aafc7ded55834de6cae05d1d495af2a5948a816 Mon Sep 17 00:00:00 2001 From: Peter Bukva Date: Thu, 10 Aug 2023 00:35:37 +0100 Subject: [PATCH 1/6] [WIP] Municipal Inflation: Add randomised simulation generation --- x/bank/simulation/genesis.go | 18 ++++++++++++++++- x/bank/simulation/genesis_test.go | 4 +++- x/mint/simulation/genesis.go | 33 ++++++++++++++++++++++++++++++- x/mint/types/minter.go | 6 ++++-- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/x/bank/simulation/genesis.go b/x/bank/simulation/genesis.go index 9031d03364..de45fe2ab9 100644 --- a/x/bank/simulation/genesis.go +++ b/x/bank/simulation/genesis.go @@ -12,6 +12,16 @@ import ( "github.com/cosmos/cosmos-sdk/x/bank/types" ) +var ( + testSupplyVal, _ = sdk.NewIntFromString("1000000000000000000") + AdditionalTestSupply = sdk.NewCoins( + sdk.NewCoin("denom0", testSupplyVal), + sdk.NewCoin("denom1", testSupplyVal), + sdk.NewCoin("denom2", testSupplyVal), + sdk.NewCoin("denom3", testSupplyVal), + ) +) + // RandomGenesisDefaultSendParam computes randomized allow all send transfers param for the bank module func RandomGenesisDefaultSendParam(r *rand.Rand) bool { // 90% chance of transfers being enable or P(a) = 0.9 for success @@ -47,6 +57,11 @@ func RandomGenesisBalances(simState *module.SimulationState) []types.Balance { }) } + if len(genesisBalances) > 0 { + nb := genesisBalances[0].Coins.Add(AdditionalTestSupply...) + genesisBalances[0].Coins = nb + } + return genesisBalances } @@ -66,7 +81,8 @@ func RandomizedGenState(simState *module.SimulationState) { numAccs := int64(len(simState.Accounts)) totalSupply := sdk.NewInt(simState.InitialStake * (numAccs + simState.NumBonded)) - supply := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, totalSupply)) + + supply := AdditionalTestSupply.Add(sdk.NewCoin(sdk.DefaultBondDenom, totalSupply)) bankGenesis := types.GenesisState{ Params: types.Params{ diff --git a/x/bank/simulation/genesis_test.go b/x/bank/simulation/genesis_test.go index fc31ca38e9..a3f7ff385a 100644 --- a/x/bank/simulation/genesis_test.go +++ b/x/bank/simulation/genesis_test.go @@ -2,6 +2,7 @@ package simulation_test import ( "encoding/json" + sdk "github.com/cosmos/cosmos-sdk/types" "math/rand" "testing" @@ -43,7 +44,8 @@ func TestRandomizedGenState(t *testing.T) { require.Len(t, bankGenesis.Balances, 3) require.Equal(t, "cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r", bankGenesis.Balances[2].GetAddress().String()) require.Equal(t, "1000stake", bankGenesis.Balances[2].GetCoins().String()) - require.Equal(t, "6000stake", bankGenesis.Supply.String()) + expectedSupply := simulation.AdditionalTestSupply.Add(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(6000))) + require.Equal(t, expectedSupply.String(), bankGenesis.Supply.String()) } // TestRandomizedGenState tests abnormal scenarios of applying RandomizedGenState. diff --git a/x/mint/simulation/genesis.go b/x/mint/simulation/genesis.go index ce1fa89d96..783cc1d886 100644 --- a/x/mint/simulation/genesis.go +++ b/x/mint/simulation/genesis.go @@ -5,6 +5,7 @@ package simulation import ( "encoding/json" "fmt" + "github.com/cosmos/cosmos-sdk/x/bank/simulation" "math/rand" sdk "github.com/cosmos/cosmos-sdk/types" @@ -28,6 +29,34 @@ func GenInflationRate(r *rand.Rand) sdk.Dec { return sdk.NewDecWithPrec(int64(r.Intn(99)), 2) } +// GenMunicipalInflation randomized Municipal Inflation configuration +func GenMunicipalInflation(simState *module.SimulationState) []*types.MunicipalInflationPair { + r := simState.Rand + + coins := make([]*sdk.Coin, len(simulation.AdditionalTestSupply)) + for i := 0; i < len(simulation.AdditionalTestSupply); i++ { + coins[i] = &simulation.AdditionalTestSupply[i] + } + + len_ := r.Intn(len(coins) + 1) + municipalInflation := make([]*types.MunicipalInflationPair, len_) + for i := 0; i < len_; i++ { + lenCoins := len(coins) + lastIdx := lenCoins - 1 + rndIdx := r.Intn(lenCoins) + fmt.Println(">>>>>>>>>>>>>>>", coins, "rndIdx:", rndIdx) + c := coins[rndIdx] + coins[rndIdx] = coins[lastIdx] + coins = coins[:lastIdx] + + acc := &simState.Accounts[r.Intn(len(simState.Accounts))] + infl := sdk.NewDecWithPrec(r.Int63n(201), 2) + municipalInflation[i] = &types.MunicipalInflationPair{Denom: c.Denom, Inflation: types.NewMunicipalInflation(acc.Address.String(), infl)} + } + + return municipalInflation +} + // RandomizedGenState generates a random GenesisState for mint func RandomizedGenState(simState *module.SimulationState) { // minter @@ -48,7 +77,9 @@ func RandomizedGenState(simState *module.SimulationState) { blocksPerYear := uint64(60 * 60 * 8766 / 5) params := types.NewParams(mintDenom, inflationRateChange, blocksPerYear) - mintGenesis := types.NewGenesisState(types.InitialMinter(inflation), params) + minter := types.InitialMinter(inflation) + minter.MunicipalInflation = GenMunicipalInflation(simState) + mintGenesis := types.NewGenesisState(minter, params) bz, err := json.MarshalIndent(&mintGenesis, "", " ") if err != nil { diff --git a/x/mint/types/minter.go b/x/mint/types/minter.go index 484a8d27ea..2fdc1ff6fc 100644 --- a/x/mint/types/minter.go +++ b/x/mint/types/minter.go @@ -33,13 +33,15 @@ func DefaultInitialMinter() Minter { ) } -// validate minter +// ValidateMinter validate minter func ValidateMinter(minter Minter) error { if minter.Inflation.IsNegative() { return fmt.Errorf("mint parameter AnnualInflation should be positive, is %s", minter.Inflation.String()) } - return nil + + err := ValidateMunicipalInflations(&minter.MunicipalInflation) + return err } // NextInflationRate returns the new inflation rate for the next hour. From ee475324c5950261c9977ee025518510c648a921 Mon Sep 17 00:00:00 2001 From: Peter Bukva Date: Thu, 10 Aug 2023 08:31:19 +0100 Subject: [PATCH 2/6] Redesigning cache using atomic.Pointer + fixing tests --- x/bank/simulation/genesis.go | 18 ++++---- x/bank/simulation/genesis_test.go | 7 ++- x/mint/cache/municipal_inflation_cahe.go | 59 +++++++++++------------- x/mint/genesis.go | 2 + x/mint/simulation/genesis.go | 12 +++-- 5 files changed, 50 insertions(+), 48 deletions(-) diff --git a/x/bank/simulation/genesis.go b/x/bank/simulation/genesis.go index de45fe2ab9..c1209ed4df 100644 --- a/x/bank/simulation/genesis.go +++ b/x/bank/simulation/genesis.go @@ -13,8 +13,8 @@ import ( ) var ( - testSupplyVal, _ = sdk.NewIntFromString("1000000000000000000") - AdditionalTestSupply = sdk.NewCoins( + testSupplyVal, _ = sdk.NewIntFromString("1000000000000000000") + AdditionalTestBalancePerAccount = sdk.NewCoins( sdk.NewCoin("denom0", testSupplyVal), sdk.NewCoin("denom1", testSupplyVal), sdk.NewCoin("denom2", testSupplyVal), @@ -51,17 +51,14 @@ func RandomGenesisBalances(simState *module.SimulationState) []types.Balance { genesisBalances := []types.Balance{} for _, acc := range simState.Accounts { + coins := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(simState.InitialStake))) + coins.Add(AdditionalTestBalancePerAccount...) genesisBalances = append(genesisBalances, types.Balance{ Address: acc.Address.String(), - Coins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(simState.InitialStake))), + Coins: coins, }) } - if len(genesisBalances) > 0 { - nb := genesisBalances[0].Coins.Add(AdditionalTestSupply...) - genesisBalances[0].Coins = nb - } - return genesisBalances } @@ -82,7 +79,10 @@ func RandomizedGenState(simState *module.SimulationState) { numAccs := int64(len(simState.Accounts)) totalSupply := sdk.NewInt(simState.InitialStake * (numAccs + simState.NumBonded)) - supply := AdditionalTestSupply.Add(sdk.NewCoin(sdk.DefaultBondDenom, totalSupply)) + supply := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, totalSupply)) + for _, b := range AdditionalTestBalancePerAccount { + supply.Add(sdk.NewCoin(b.Denom, b.Amount.MulRaw(numAccs))) + } bankGenesis := types.GenesisState{ Params: types.Params{ diff --git a/x/bank/simulation/genesis_test.go b/x/bank/simulation/genesis_test.go index a3f7ff385a..2632450c62 100644 --- a/x/bank/simulation/genesis_test.go +++ b/x/bank/simulation/genesis_test.go @@ -44,7 +44,12 @@ func TestRandomizedGenState(t *testing.T) { require.Len(t, bankGenesis.Balances, 3) require.Equal(t, "cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r", bankGenesis.Balances[2].GetAddress().String()) require.Equal(t, "1000stake", bankGenesis.Balances[2].GetCoins().String()) - expectedSupply := simulation.AdditionalTestSupply.Add(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(6000))) + + numAccs := int64(len(simState.Accounts)) + expectedSupply := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(6000))) + for _, b := range simulation.AdditionalTestBalancePerAccount { + expectedSupply.Add(sdk.NewCoin(b.Denom, b.Amount.MulRaw(numAccs))) + } require.Equal(t, expectedSupply.String(), bankGenesis.Supply.String()) } diff --git a/x/mint/cache/municipal_inflation_cahe.go b/x/mint/cache/municipal_inflation_cahe.go index dfc82bd15a..a52c377b3a 100644 --- a/x/mint/cache/municipal_inflation_cahe.go +++ b/x/mint/cache/municipal_inflation_cahe.go @@ -1,7 +1,7 @@ package cache import ( - "sync" + "sync/atomic" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/mint/types" @@ -12,11 +12,14 @@ type MunicipalInflationCacheItem struct { AnnualInflation *types.MunicipalInflation } -type MunicipalInflationCache struct { +type MunicipalInflationCacheInternal struct { blocksPerYear uint64 original *[]*types.MunicipalInflationPair inflations map[string]*MunicipalInflationCacheItem // {denom: inflationPerBlock} - mu sync.RWMutex +} + +type MunicipalInflationCache struct { + internal atomic.Pointer[MunicipalInflationCacheInternal] } // GMunicipalInflationCache Thread safety: @@ -32,41 +35,38 @@ type MunicipalInflationCache struct { var GMunicipalInflationCache = MunicipalInflationCache{} func (cache *MunicipalInflationCache) Refresh(inflations *[]*types.MunicipalInflationPair, blocksPerYear uint64) { - cache.mu.Lock() - defer cache.mu.Unlock() - - cache.refresh(inflations, blocksPerYear) + newCache := MunicipalInflationCacheInternal{} + newCache.refresh(inflations, blocksPerYear) + cache.internal.Store(&newCache) } +// RefreshIfNecessary +// IMPORTANT: Assuming *NO* concurrent writes, designed with emphasis for concurrent reads. This requirement +// guaranteed, since this method is called exclusively from call contexts which are never concurrent. +// This approach will guarantee the most possible effective cache operation in heavily concurrent +// read environment with minimum possible blocking for concurrent read operations, but with slight +// limitation for write operations (there should not be concurrent write operations). +// Most of the read operations are assumed to be done from RPC (querying municipal inflation). func (cache *MunicipalInflationCache) RefreshIfNecessary(inflations *[]*types.MunicipalInflationPair, blocksPerYear uint64) { - cache.mu.Lock() - defer cache.mu.Unlock() - - cache.refreshIfNecessary(inflations, blocksPerYear) -} - -func (cache *MunicipalInflationCache) IsRefreshRequired(blocksPerYear uint64) bool { - cache.mu.RLock() - defer cache.mu.RUnlock() - return cache.isRefreshRequired(blocksPerYear) + current := cache.internal.Load() + if current.isRefreshRequired(blocksPerYear) { + cache.Refresh(inflations, blocksPerYear) + } } func (cache *MunicipalInflationCache) GetInflation(denom string) (MunicipalInflationCacheItem, bool) { - cache.mu.RLock() - defer cache.mu.RUnlock() - infl, exists := cache.inflations[denom] + current := cache.internal.Load() + infl, exists := current.inflations[denom] return *infl, exists } func (cache *MunicipalInflationCache) GetOriginal() *[]*types.MunicipalInflationPair { - cache.mu.RLock() - defer cache.mu.RUnlock() - // NOTE(pb): Mutex locking might not be necessary here since we are returning by pointer - return cache.original + current := cache.internal.Load() + return current.original } // NOTE(pb): *NOT* thread safe -func (cache *MunicipalInflationCache) refresh(inflations *[]*types.MunicipalInflationPair, blocksPerYear uint64) { +func (cache *MunicipalInflationCacheInternal) refresh(inflations *[]*types.MunicipalInflationPair, blocksPerYear uint64) { if err := types.ValidateMunicipalInflations(inflations); err != nil { panic(err) } @@ -89,13 +89,6 @@ func (cache *MunicipalInflationCache) refresh(inflations *[]*types.MunicipalInfl } // NOTE(pb): *NOT* thread safe -func (cache *MunicipalInflationCache) refreshIfNecessary(inflations *[]*types.MunicipalInflationPair, blocksPerYear uint64) { - if cache.isRefreshRequired(blocksPerYear) { - cache.refresh(inflations, blocksPerYear) - } -} - -// NOTE(pb): *NOT* thread safe -func (cache *MunicipalInflationCache) isRefreshRequired(blocksPerYear uint64) bool { +func (cache *MunicipalInflationCacheInternal) isRefreshRequired(blocksPerYear uint64) bool { return cache.blocksPerYear != blocksPerYear } diff --git a/x/mint/genesis.go b/x/mint/genesis.go index be7ac61d50..6a1eb3050d 100644 --- a/x/mint/genesis.go +++ b/x/mint/genesis.go @@ -2,12 +2,14 @@ package mint import ( sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/mint/cache" "github.com/cosmos/cosmos-sdk/x/mint/keeper" "github.com/cosmos/cosmos-sdk/x/mint/types" ) // InitGenesis new mint genesis func InitGenesis(ctx sdk.Context, keeper keeper.Keeper, ak types.AccountKeeper, data *types.GenesisState) { + cache.GMunicipalInflationCache.Refresh(&data.Minter.MunicipalInflation, data.Params.BlocksPerYear) keeper.SetMinter(ctx, data.Minter) keeper.SetParams(ctx, data.Params) ak.GetModuleAccount(ctx, types.ModuleName) diff --git a/x/mint/simulation/genesis.go b/x/mint/simulation/genesis.go index 783cc1d886..b0692f6122 100644 --- a/x/mint/simulation/genesis.go +++ b/x/mint/simulation/genesis.go @@ -5,9 +5,10 @@ package simulation import ( "encoding/json" "fmt" - "github.com/cosmos/cosmos-sdk/x/bank/simulation" "math/rand" + "github.com/cosmos/cosmos-sdk/x/bank/simulation" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/cosmos-sdk/x/mint/types" @@ -33,9 +34,9 @@ func GenInflationRate(r *rand.Rand) sdk.Dec { func GenMunicipalInflation(simState *module.SimulationState) []*types.MunicipalInflationPair { r := simState.Rand - coins := make([]*sdk.Coin, len(simulation.AdditionalTestSupply)) - for i := 0; i < len(simulation.AdditionalTestSupply); i++ { - coins[i] = &simulation.AdditionalTestSupply[i] + coins := make([]*sdk.Coin, len(simulation.AdditionalTestBalancePerAccount)) + for i := 0; i < len(simulation.AdditionalTestBalancePerAccount); i++ { + coins[i] = &simulation.AdditionalTestBalancePerAccount[i] } len_ := r.Intn(len(coins) + 1) @@ -44,7 +45,8 @@ func GenMunicipalInflation(simState *module.SimulationState) []*types.MunicipalI lenCoins := len(coins) lastIdx := lenCoins - 1 rndIdx := r.Intn(lenCoins) - fmt.Println(">>>>>>>>>>>>>>>", coins, "rndIdx:", rndIdx) + + // Swapping rndIdx element with the last element in the slice and cuttig slice without last element c := coins[rndIdx] coins[rndIdx] = coins[lastIdx] coins = coins[:lastIdx] From f0412ff5b4d0b0a7c5bd868bf566645de40a5897 Mon Sep 17 00:00:00 2001 From: Peter Bukva Date: Thu, 10 Aug 2023 12:59:36 +0100 Subject: [PATCH 3/6] Using atomic.Value instead of atomic.Pointer due to go version --- Makefile | 9 ++++----- x/mint/cache/municipal_inflation_cahe.go | 21 ++++++++++++++++----- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index 531088c0c3..26a572d54d 100644 --- a/Makefile +++ b/Makefile @@ -20,11 +20,10 @@ GO_MINOR_VERSION := $(shell echo $(GO_VERSION) | cut -d. -f2) GO_MICRO_VERSION := $(shell echo $(GO_VERSION) | cut -d. -f3) SUPPORTED_GO_VERSION = 1.18.10 -IS_SUPPORTED_VERSION := $(shell expr "$(GO_VERSION)" "==" "$(SUPPORTED_GO_VERSION)") -ifeq "$(IS_SUPPORTED_VERSION)" "1" - $(info Go version is supported: $(GO_VERSION)) +ifeq ($(GO_VERSION), $(SUPPORTED_GO_VERSION)) + $(info Go version $(GO_VERSION) is supported) else - $(info WARN: Go version not supported, some tests might fail without version $(SUPPORTED_GO_VERSION)) + $(info WARN: Go version $(GO_VERSION) is not supported, some tests might fail on different version than supported $(SUPPORTED_GO_VERSION)) endif export GO111MODULE = on @@ -167,7 +166,7 @@ clean: ############################################################################### go.sum: go.mod - echo "Ensure dependencies have not been modified ..." >&2 + @echo "Ensure dependencies have not been modified ..." >&2 go mod verify go mod tidy diff --git a/x/mint/cache/municipal_inflation_cahe.go b/x/mint/cache/municipal_inflation_cahe.go index a52c377b3a..016bfcc2a6 100644 --- a/x/mint/cache/municipal_inflation_cahe.go +++ b/x/mint/cache/municipal_inflation_cahe.go @@ -1,6 +1,7 @@ package cache import ( + "sync" "sync/atomic" sdk "github.com/cosmos/cosmos-sdk/types" @@ -18,8 +19,12 @@ type MunicipalInflationCacheInternal struct { inflations map[string]*MunicipalInflationCacheItem // {denom: inflationPerBlock} } +// MunicipalInflationCache Cache optimised for concurrent reading performance. +// *NO* support for concurrent writing operations. type MunicipalInflationCache struct { - internal atomic.Pointer[MunicipalInflationCacheInternal] + internal atomic.Value + // writeGuard is present solely to synchronise writ + writeGuard sync.Mutex } // GMunicipalInflationCache Thread safety: @@ -37,6 +42,8 @@ var GMunicipalInflationCache = MunicipalInflationCache{} func (cache *MunicipalInflationCache) Refresh(inflations *[]*types.MunicipalInflationPair, blocksPerYear uint64) { newCache := MunicipalInflationCacheInternal{} newCache.refresh(inflations, blocksPerYear) + cache.writeGuard.Lock() + defer cache.writeGuard.Unlock() cache.internal.Store(&newCache) } @@ -48,20 +55,24 @@ func (cache *MunicipalInflationCache) Refresh(inflations *[]*types.MunicipalInfl // limitation for write operations (there should not be concurrent write operations). // Most of the read operations are assumed to be done from RPC (querying municipal inflation). func (cache *MunicipalInflationCache) RefreshIfNecessary(inflations *[]*types.MunicipalInflationPair, blocksPerYear uint64) { - current := cache.internal.Load() + cache.writeGuard.Lock() + defer cache.writeGuard.Unlock() + current := cache.internal.Load().(*MunicipalInflationCacheInternal) if current.isRefreshRequired(blocksPerYear) { - cache.Refresh(inflations, blocksPerYear) + newCache := MunicipalInflationCacheInternal{} + newCache.refresh(inflations, blocksPerYear) + cache.internal.Store(&newCache) } } func (cache *MunicipalInflationCache) GetInflation(denom string) (MunicipalInflationCacheItem, bool) { - current := cache.internal.Load() + current := cache.internal.Load().(*MunicipalInflationCacheInternal) infl, exists := current.inflations[denom] return *infl, exists } func (cache *MunicipalInflationCache) GetOriginal() *[]*types.MunicipalInflationPair { - current := cache.internal.Load() + current := cache.internal.Load().(*MunicipalInflationCacheInternal) return current.original } From 68557f9665a6c4ab9e936d11ac4fbc7e0f501de5 Mon Sep 17 00:00:00 2001 From: Peter Bukva Date: Thu, 10 Aug 2023 23:30:19 +0100 Subject: [PATCH 4/6] Fixing intialisation of the cache --- x/mint/cache/municipal_inflation_cahe.go | 40 +++++++++++++++++------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/x/mint/cache/municipal_inflation_cahe.go b/x/mint/cache/municipal_inflation_cahe.go index 016bfcc2a6..86d5fda005 100644 --- a/x/mint/cache/municipal_inflation_cahe.go +++ b/x/mint/cache/municipal_inflation_cahe.go @@ -31,7 +31,7 @@ type MunicipalInflationCache struct { // As the things stand now from design & impl. perspective: // 1. This global variable is supposed to be initialised(= its value set) // just *ONCE* here, at this place, -// 2. This global variable is *NOT* used anywhere else in the initialisation +// 2. This global variable shall *NOT* used anywhere else in the initialisation // context of the *global scope* - e.g. as input for initialisation of // another global variable, etc. ... // 3. All *exported* methods of `MunicipalInflationCache` type *ARE* thread safe, @@ -48,17 +48,26 @@ func (cache *MunicipalInflationCache) Refresh(inflations *[]*types.MunicipalInfl } // RefreshIfNecessary -// IMPORTANT: Assuming *NO* concurrent writes, designed with emphasis for concurrent reads. This requirement -// guaranteed, since this method is called exclusively from call contexts which are never concurrent. +// NOTE: Current implementation is using mutex to allow concurrent write operations, however, +// it is actually not necessary to implement support for concurrent write operations given the +// non-concurrent threading model in which write operation are executed from = hence no real +// necessity for mutex. The current implementation uses mutex solely for experimental & backup +// reasons, and it is going to be dropped in the following commit. +// +// IMPORTANT: (In the case without mutex) Assuming *NO* concurrent writes. This requirement is +// guaranteed given the *current* usage of this component = this method is called exclusively +// from non-concurrent call contexts. // This approach will guarantee the most possible effective cache operation in heavily concurrent -// read environment with minimum possible blocking for concurrent read operations, but with slight -// limitation for write operations (there should not be concurrent write operations). -// Most of the read operations are assumed to be done from RPC (querying municipal inflation). +// read environment = with minimum possible blocking for concurrent read operations, but with slight +// limitation for write operations (= no concurrent write operations). +// Most of the read operations are assumed to be done from RPC (querying municipal inflation), +// and since threading models of the RPC implementation is not know, the worst scenario(= heavily +// concurrent threading model) for read operation is assumed. func (cache *MunicipalInflationCache) RefreshIfNecessary(inflations *[]*types.MunicipalInflationPair, blocksPerYear uint64) { cache.writeGuard.Lock() defer cache.writeGuard.Unlock() - current := cache.internal.Load().(*MunicipalInflationCacheInternal) - if current.isRefreshRequired(blocksPerYear) { + + if val := cache.internal.Load(); val == nil || val.(*MunicipalInflationCacheInternal).isRefreshRequired(blocksPerYear) { newCache := MunicipalInflationCacheInternal{} newCache.refresh(inflations, blocksPerYear) cache.internal.Store(&newCache) @@ -66,13 +75,22 @@ func (cache *MunicipalInflationCache) RefreshIfNecessary(inflations *[]*types.Mu } func (cache *MunicipalInflationCache) GetInflation(denom string) (MunicipalInflationCacheItem, bool) { - current := cache.internal.Load().(*MunicipalInflationCacheInternal) - infl, exists := current.inflations[denom] + val := cache.internal.Load() + if val == nil { + return MunicipalInflationCacheItem{}, false + } + + infl, exists := val.(*MunicipalInflationCacheInternal).inflations[denom] return *infl, exists } func (cache *MunicipalInflationCache) GetOriginal() *[]*types.MunicipalInflationPair { - current := cache.internal.Load().(*MunicipalInflationCacheInternal) + val := cache.internal.Load() + if val == nil { + return &[]*types.MunicipalInflationPair{} + } + + current := val.(*MunicipalInflationCacheInternal) return current.original } From 72698263f6630b1d30ab6a7502d811e3aca1e1e8 Mon Sep 17 00:00:00 2001 From: Peter Bukva Date: Thu, 10 Aug 2023 23:37:50 +0100 Subject: [PATCH 5/6] Dropping mutex from the cache = concurent writes NOT supported anymore --- x/mint/cache/municipal_inflation_cahe.go | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/x/mint/cache/municipal_inflation_cahe.go b/x/mint/cache/municipal_inflation_cahe.go index 86d5fda005..1be120cd80 100644 --- a/x/mint/cache/municipal_inflation_cahe.go +++ b/x/mint/cache/municipal_inflation_cahe.go @@ -1,7 +1,6 @@ package cache import ( - "sync" "sync/atomic" sdk "github.com/cosmos/cosmos-sdk/types" @@ -23,8 +22,6 @@ type MunicipalInflationCacheInternal struct { // *NO* support for concurrent writing operations. type MunicipalInflationCache struct { internal atomic.Value - // writeGuard is present solely to synchronise writ - writeGuard sync.Mutex } // GMunicipalInflationCache Thread safety: @@ -42,21 +39,12 @@ var GMunicipalInflationCache = MunicipalInflationCache{} func (cache *MunicipalInflationCache) Refresh(inflations *[]*types.MunicipalInflationPair, blocksPerYear uint64) { newCache := MunicipalInflationCacheInternal{} newCache.refresh(inflations, blocksPerYear) - cache.writeGuard.Lock() - defer cache.writeGuard.Unlock() cache.internal.Store(&newCache) } // RefreshIfNecessary -// NOTE: Current implementation is using mutex to allow concurrent write operations, however, -// it is actually not necessary to implement support for concurrent write operations given the -// non-concurrent threading model in which write operation are executed from = hence no real -// necessity for mutex. The current implementation uses mutex solely for experimental & backup -// reasons, and it is going to be dropped in the following commit. -// -// IMPORTANT: (In the case without mutex) Assuming *NO* concurrent writes. This requirement is -// guaranteed given the *current* usage of this component = this method is called exclusively -// from non-concurrent call contexts. +// IMPORTANT: Assuming *NO* concurrent writes. This requirement is guaranteed given the *current* +// usage of this component = this method is called exclusively from non-concurrent call contexts. // This approach will guarantee the most possible effective cache operation in heavily concurrent // read environment = with minimum possible blocking for concurrent read operations, but with slight // limitation for write operations (= no concurrent write operations). @@ -64,13 +52,8 @@ func (cache *MunicipalInflationCache) Refresh(inflations *[]*types.MunicipalInfl // and since threading models of the RPC implementation is not know, the worst scenario(= heavily // concurrent threading model) for read operation is assumed. func (cache *MunicipalInflationCache) RefreshIfNecessary(inflations *[]*types.MunicipalInflationPair, blocksPerYear uint64) { - cache.writeGuard.Lock() - defer cache.writeGuard.Unlock() - if val := cache.internal.Load(); val == nil || val.(*MunicipalInflationCacheInternal).isRefreshRequired(blocksPerYear) { - newCache := MunicipalInflationCacheInternal{} - newCache.refresh(inflations, blocksPerYear) - cache.internal.Store(&newCache) + cache.Refresh(inflations, blocksPerYear) } } From b48cc8b1292091bb38e31fb41178c3d739bd8afc Mon Sep 17 00:00:00 2001 From: Peter Bukva Date: Thu, 10 Aug 2023 23:45:03 +0100 Subject: [PATCH 6/6] [Cosmetic] Comment typo fix --- x/mint/cache/municipal_inflation_cahe.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/mint/cache/municipal_inflation_cahe.go b/x/mint/cache/municipal_inflation_cahe.go index 1be120cd80..5b2695b7bc 100644 --- a/x/mint/cache/municipal_inflation_cahe.go +++ b/x/mint/cache/municipal_inflation_cahe.go @@ -28,7 +28,7 @@ type MunicipalInflationCache struct { // As the things stand now from design & impl. perspective: // 1. This global variable is supposed to be initialised(= its value set) // just *ONCE* here, at this place, -// 2. This global variable shall *NOT* used anywhere else in the initialisation +// 2. This global variable shall *NOT* be used anywhere else in the initialisation // context of the *global scope* - e.g. as input for initialisation of // another global variable, etc. ... // 3. All *exported* methods of `MunicipalInflationCache` type *ARE* thread safe,