fix: Municipal Inflation: cache redesign atomic + simtest (#166)

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
 

x/bank/simulation/genesis.go

@@ -12,6 +12,16 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/bank/types"
 )
 
+var (
+	testSupplyVal, _                = sdk.NewIntFromString("1000000000000000000")
+	AdditionalTestBalancePerAccount = 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
@@ -41,9 +51,11 @@ 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,
 		})
 	}
 
@@ -66,7 +78,11 @@ 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))
+	for _, b := range AdditionalTestBalancePerAccount {
+		supply.Add(sdk.NewCoin(b.Denom, b.Amount.MulRaw(numAccs)))
+	}
 
 	bankGenesis := types.GenesisState{
 		Params: types.Params{

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,13 @@ 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())
+
+	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())
 }
 
 // TestRandomizedGenState tests abnormal scenarios of applying RandomizedGenState.

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,18 +12,23 @@ 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
+}
+
+// MunicipalInflationCache Cache optimised for concurrent reading performance.
+// *NO* support for concurrent writing operations.
+type MunicipalInflationCache struct {
+	internal atomic.Value
 }
 
 // GMunicipalInflationCache Thread safety:
 // 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* 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,
@@ -32,41 +37,48 @@ 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. 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).
+// 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.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)
+	if val := cache.internal.Load(); val == nil || val.(*MunicipalInflationCacheInternal).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]
+	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 {
-	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
+	val := cache.internal.Load()
+	if val == nil {
+		return &[]*types.MunicipalInflationPair{}
+	}
+
+	current := val.(*MunicipalInflationCacheInternal)
+	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 +101,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
 }

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)

x/mint/simulation/genesis.go

@@ -7,6 +7,8 @@ import (
 	"fmt"
 	"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"
@@ -28,6 +30,35 @@ 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.AdditionalTestBalancePerAccount))
+	for i := 0; i < len(simulation.AdditionalTestBalancePerAccount); i++ {
+		coins[i] = &simulation.AdditionalTestBalancePerAccount[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)
+
+		// 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]
+
+		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 +79,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 {

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.