hanld quote behavior in deterministic order with sorted keys, add com…

…ments to explain why map iteration is safe

x/oracle/keeper/feedermanagement/aggregator.go

@@ -139,6 +139,7 @@ func (rv *recordsValidators) Equals(rv2 *recordsValidators) bool {
 	if len(rv.records) != len(rv2.records) {
 		return false
 	}
+	// safe to range map, map compare
 	for k, v := range rv.records {
 		if v2, ok := rv2.records[k]; !ok || !v.Equals(v2) {
 			return false
@@ -160,12 +161,14 @@ func (rv *recordsValidators) Cpy() *recordsValidators {
 	var finalPrices map[string]*PriceResult
 	if len(rv.finalPrices) > 0 {
 		finalPrices = make(map[string]*PriceResult)
+		// safe to range map, map copy
 		for v, p := range rv.finalPrices {
 			price := *p
 			finalPrices[v] = &price
 		}
 	}
 	records := make(map[string]*priceValidator)
+	// safe to range map, map copy
 	for v, pv := range rv.records {
 		records[v] = pv.Cpy()
 	}
@@ -220,6 +223,7 @@ func (rv *recordsValidators) GetFinalPrice(algo AggAlgorithm) (*PriceResult, boo
 	}
 	if prices, ok := rv.GetFinalPriceForValidators(algo); ok {
 		keySlice := make([]string, 0, len(prices))
+		// safe to range map, this is used to generate a sorted keySlice
 		for validator := range prices {
 			keySlice = append(keySlice, validator)
 		}
@@ -292,6 +296,7 @@ func (rdss *recordsDSs) Equals(rdss2 *recordsDSs) bool {
 	if len(rdss.dsMap) != len(rdss2.dsMap) {
 		return false
 	}
+	// safe to range map, map compare
 	for k, v := range rdss.dsMap {
 		if v2, ok := rdss2.dsMap[k]; !ok || !v.Equals(v2) {
 			return false
@@ -306,6 +311,7 @@ func (rdss *recordsDSs) Cpy() *recordsDSs {
 		return nil
 	}
 	dsMap := make(map[int64]*recordsDS)
+	// safe to range map, map copy
 	for id, r := range rdss.dsMap {
 		dsMap[id] = r.Cpy()
 	}
@@ -345,6 +351,7 @@ func (rdss *recordsDSs) GetFinalPriceForSourceID(sourceID int64) (*PriceResult,
 
 func (rdss *recordsDSs) GetFinalPriceForSources() (map[int64]*PriceResult, bool) {
 	ret := make(map[int64]*PriceResult)
+	// safe to range map, the result is a map of 'all or none'
 	for sourceID, rds := range rdss.dsMap {
 		if finalPrice, ok := rds.GetFinalPrice(rdss.t); ok {
 			ret[sourceID] = finalPrice
@@ -419,6 +426,7 @@ func (rds *recordsDS) Cpy() *recordsDS {
 		finalPrice = &tmp
 	}
 	validators := make(map[string]struct{})
+	// safe to range map, map copy
 	for v := range rds.validators {
 		validators[v] = struct{}{}
 	}

x/oracle/keeper/feedermanagement/caches.go

@@ -23,6 +23,7 @@ func (c *caches) CpyForSimulation() *caches {
 	ret.msg = &msg
 	ret.params = &params
 	validators := make(map[string]*big.Int)
+	// safe to range map, map copy
 	for v, p := range c.validators.validators {
 		validators[v] = new(big.Int).Set(p)
 	}
@@ -160,6 +161,7 @@ func (cv *cacheValidator) Equals(cv2 *cacheValidator) bool {
 	if len(cv.validators) != len(cv2.validators) {
 		return false
 	}
+	// safe to range map, map compare
 	for k, v := range cv.validators {
 		if v2, ok := cv2.validators[k]; !ok {
 			return false
@@ -171,6 +173,7 @@ func (cv *cacheValidator) Equals(cv2 *cacheValidator) bool {
 }
 
 func (cv *cacheValidator) add(validators map[string]*big.Int) {
+	// safe to range map, check and update all KVs with another map
 	for operator, newPower := range validators {
 		if power, ok := cv.validators[operator]; ok {
 			if newPower.Cmp(zeroBig) == 0 {
@@ -207,6 +210,7 @@ func (cv *cacheValidator) slice() []string {
 		return nil
 	}
 	validators := make([]string, 0, cv.size())
+	// safe to range map, this range is used to generate a sorted slice
 	for validator := range cv.validators {
 		validators = append(validators, validator)
 	}
@@ -268,6 +272,7 @@ func (c *caches) Read(i any) bool {
 		if item == nil {
 			return false
 		}
+		// safe to range map, map copy
 		for addr, power := range c.validators.validators {
 			item[addr] = power
 		}
@@ -318,6 +323,7 @@ func (c *caches) GetTotalPower() (totalPower *big.Int) {
 	if c.validators == nil {
 		return
 	}
+	// safe to renage map, the order does not impact the result
 	for _, power := range c.validators.validators {
 		totalPower.Add(totalPower, power)
 	}

x/oracle/keeper/feedermanagement/feedermanager.go

@@ -181,7 +181,7 @@ func (f *FeederManager) prepareRounds(ctx sdk.Context) []int64 {
 // forceSeal: 1. params has some modifications related to quoting. 2.validatorSet changed
 // resetSlashing: params has some modifications related to oracle_slashing
 // func (f *FeederManager) updateAndCommitCaches(ctx sdk.Context) (forceSeal, resetSlashing bool, prevValidators, addedValidators []string) {
-func (f *FeederManager) updateAndCommitCaches(ctx sdk.Context) (addedValidators []string) {
+func (f *FeederManager) updateAndCommitCaches(ctx sdk.Context) (activeValidators []string) {
 	// update params in caches
 	if f.paramsUpdated {
 		paramsOld := &oracletypes.Params{}
@@ -201,14 +201,14 @@ func (f *FeederManager) updateAndCommitCaches(ctx sdk.Context) (addedValidators
 	if len(validatorUpdates) > 0 {
 		f.SetValidatorsUpdated()
 		f.SetForceSeal()
-		addedValidators = make([]string, 0)
+		activeValidators = make([]string, 0)
 		validatorMap := make(map[string]*big.Int)
 		for _, vu := range validatorUpdates {
 			pubKey, _ := cryptocodec.FromTmProtoPublicKey(vu.PubKey)
 			validatorStr := sdk.ConsAddress(pubKey.Address()).String()
 			validatorMap[validatorStr] = big.NewInt(vu.Power)
 			if vu.Power > 0 {
-				addedValidators = append(addedValidators, validatorStr)
+				activeValidators = append(activeValidators, validatorStr)
 			}
 		}
 		// update validator set information in cache
@@ -220,10 +220,11 @@ func (f *FeederManager) updateAndCommitCaches(ctx sdk.Context) (addedValidators
 	if vUpdated || pUpdated {
 		f.k.Logger(ctx).Info("update caches", "validatorUpdates", vUpdated, "paramsUpdated", pUpdated)
 	}
-	return addedValidators
+	return activeValidators
 }
 
 func (f *FeederManager) commitRoundsInRecovery() {
+	// safe to range map directly, this is just used to update memory state, we don't update state in recovery mode
 	for _, r := range f.rounds {
 		if r.Committable() {
 			r.FinalPrice()
@@ -240,6 +241,8 @@ func (f *FeederManager) commitRounds(ctx sdk.Context) {
 	logger := f.k.Logger(ctx)
 	height := ctx.BlockHeight()
 	successFeederIDs := make([]string, 0)
+	// it's safe to range map directly since the sate update is independent for each feederID, however we use sortedFeederIDs to keep the order of logs
+	// this can be replaced by map iteration directly when better performance is needed
 	for _, feederID := range f.sortedFeederIDs {
 		r := f.rounds[feederID]
 		if r.Committable() {
@@ -284,7 +287,7 @@ func (f *FeederManager) commitRounds(ctx sdk.Context) {
 func (f *FeederManager) handleQuotingMisBehaviorInRecovery(ctx sdk.Context) {
 	height := ctx.BlockHeight()
 	logger := f.k.Logger(ctx)
-
+	// it's safe to range map directly, no state in kvStore will be updated in recovery mode, only memory state is updated
 	for _, r := range f.rounds {
 		if r.IsQuotingWindowEnd(height) && r.a != nil {
 			validators := f.cs.GetValidators()
@@ -309,7 +312,12 @@ func (f *FeederManager) handleQuotingMisBehavior(ctx sdk.Context) {
 	height := ctx.BlockHeight()
 	logger := f.k.Logger(ctx)
 
-	for _, r := range f.rounds {
+	// it's safe to range map directly, each state update is independent for each feederID
+	// state to be updated: {validatorReportInfo, validatorMissedRoundBitArray, signInfo, assets} of individual validator
+	// we use sortedFeederIDs to keep the order of logs
+	// this can be replaced by map iteration directly when better performance is needed
+	for _, feederID := range f.sortedFeederIDs {
+		r := f.rounds[feederID]
 		if r.IsQuotingWindowEnd(height) {
 			if _, found := r.FinalPrice(); !found {
 				r.closeQuotingWindow()
@@ -455,13 +463,15 @@ func (f *FeederManager) handleQuotingMisBehavior(ctx sdk.Context) {
 
 func (f *FeederManager) setCommittableState(ctx sdk.Context) {
 	if f.forceSeal {
+		// safe to range map. update memory state only, the result would be the same in any order
 		for _, r := range f.rounds {
 			if r.status == roundStatusOpen {
 				r.status = roundStatusCommittable
 			}
 		}
 	} else {
 		height := ctx.BlockHeight()
+		// safe to range map. update memory state only, the result would be the same in any order
 		for _, r := range f.rounds {
 			if r.IsQuotingWindowEnd(height) && r.status == roundStatusOpen {
 				r.status = roundStatusCommittable
@@ -478,6 +488,7 @@ func (f *FeederManager) updateRoundsParamsAndAddNewRounds(ctx sdk.Context) {
 		params := &oracletypes.Params{}
 		f.cs.Read(params)
 		existsFeederIDs := make(map[int64]struct{})
+		// safe to range map. update memory state only, the result would be the same in any order
 		for _, r := range f.rounds {
 			r.UpdateParams(params.TokenFeeders[r.feederID], int64(params.MaxNonce))
 			existsFeederIDs[r.feederID] = struct{}{}
@@ -502,11 +513,13 @@ func (f *FeederManager) updateRoundsParamsAndAddNewRounds(ctx sdk.Context) {
 func (f *FeederManager) removeExpiredRounds(ctx sdk.Context) {
 	height := ctx.BlockHeight()
 	expiredFeederIDs := make([]int64, 0)
+	// safe to range map, we generate the slice, the content of elements would be the same, order does not matter
 	for _, r := range f.rounds {
 		if r.endBlock > 0 && r.endBlock <= height {
 			expiredFeederIDs = append(expiredFeederIDs, r.feederID)
 		}
 	}
+	// the order does not matter when remove item from slice as RemoveNonceWithFeederIDForAll does
 	for _, feederID := range expiredFeederIDs {
 		if r := f.rounds[feederID]; r.status != roundStatusClosed {
 			r.closeQuotingWindow()
@@ -669,6 +682,7 @@ func (f *FeederManager) getCheckTx() *FeederManager {
 
 	// rounds
 	rounds := make(map[int64]*round)
+	// safe to range map, map copy
 	for id, r := range fCheckTx.rounds {
 		rounds[id] = r.CopyForCheckTx()
 	}
@@ -687,8 +701,9 @@ func (f *FeederManager) updateCheckTx() {
 	ret := *f
 	ret.fCheckTx = nil
 
-	// rounds
 	rounds := make(map[int64]*round)
+
+	// safe to range map, map copy
 	for id, r := range f.rounds {
 		rounds[id] = r.CopyForCheckTx()
 	}
@@ -832,6 +847,7 @@ func (f *FeederManager) Equals(fm *FeederManager) bool {
 	if len(f.rounds) != len(fm.rounds) {
 		return false
 	}
+	// safe to range map, compare map
 	for id, r := range f.rounds {
 		if r2, ok := fm.rounds[id]; !ok {
 			return false

x/oracle/keeper/feedermanagement/prices.go

@@ -95,6 +95,7 @@ func (pv *priceValidator) Cpy() *priceValidator {
 		finalPrice = &tmp
 	}
 	priceSources := make(map[int64]*priceSource)
+	// safe to range map, map copy
 	for id, ps := range pv.priceSources {
 		priceSources[id] = ps.Cpy()
 	}
@@ -119,6 +120,7 @@ func (pv *priceValidator) Equals(pv2 *priceValidator) bool {
 	if len(pv.priceSources) != len(pv2.priceSources) {
 		return false
 	}
+	// safe to range map, map compare
 	for id, ps := range pv.priceSources {
 		ps2, ok := pv2.priceSources[id]
 		if !ok || !ps.Equals(ps2) {
@@ -163,6 +165,7 @@ func (pv *priceValidator) TryAddPriceSources(pSs []*priceSource) (updated map[in
 }
 
 func (pv *priceValidator) ApplyAddedPriceSources(psMap map[int64]*priceSource) {
+	// safe to range map, set all k-v to antoher map
 	for id, ps := range psMap {
 		pv.priceSources[id] = ps
 	}
@@ -177,6 +180,7 @@ func (pv *priceValidator) GetFinalPrice(algo AggAlgorithm) (*PriceResult, bool)
 		return nil, false
 	}
 	keySlice := make([]int64, 0, len(pv.priceSources))
+	// safe to range map, the map is iteration to genrate sorted key slice
 	for sourceID := range pv.priceSources {
 		keySlice = append(keySlice, sourceID)
 	}
@@ -257,6 +261,7 @@ func (ps *priceSource) Cpy() *priceSource {
 	}
 	// deterministic, sourceID
 	detIDs := make(map[string]struct{})
+	// safe to range map, map copy
 	for detID := range ps.detIDs {
 		detIDs[detID] = struct{}{}
 	}
@@ -340,6 +345,7 @@ func (p *PricePower) Equals(p2 *PricePower) bool {
 	if len(p.Validators) != len(p2.Validators) {
 		return false
 	}
+	// safe to range map, map compare
 	for v := range p.Validators {
 		if _, ok := p2.Validators[v]; !ok {
 			return false
@@ -351,6 +357,7 @@ func (p *PricePower) Equals(p2 *PricePower) bool {
 func (p *PricePower) Cpy() *PricePower {
 	price := *p.Price
 	validators := make(map[string]struct{})
+	// safe to range map, map copy
 	for v := range p.Validators {
 		validators[v] = struct{}{}
 	}