Merge pull request #6110 from multiversx/MX-15351-backwards-compat-is…

…sues

FIX: Possible backwards incompatibilities fixes

common/constants.go

@@ -1011,7 +1011,6 @@ const (
 	StakingV4Step1Flag                                 core.EnableEpochFlag = "StakingV4Step1Flag"
 	StakingV4Step2Flag                                 core.EnableEpochFlag = "StakingV4Step2Flag"
 	StakingV4Step3Flag                                 core.EnableEpochFlag = "StakingV4Step3Flag"
-	StakingQueueFlag                                   core.EnableEpochFlag = "StakingQueueFlag"
 	StakingV4StartedFlag                               core.EnableEpochFlag = "StakingV4StartedFlag"
 	AlwaysMergeContextsInEEIFlag                       core.EnableEpochFlag = "AlwaysMergeContextsInEEIFlag"
 	// all new flags must be added to createAllFlagsMap method, as part of enableEpochsHandler allFlagsDefined

common/enablers/enableEpochsHandler.go

@@ -713,12 +713,6 @@ func (handler *enableEpochsHandler) createAllFlagsMap() {
 			},
 			activationEpoch: handler.enableEpochsConfig.StakingV4Step3EnableEpoch,
 		},
-		common.StakingQueueFlag: {
-			isActiveInEpoch: func(epoch uint32) bool {
-				return epoch < handler.enableEpochsConfig.StakingV4Step1EnableEpoch
-			},
-			activationEpoch: handler.enableEpochsConfig.StakingV4Step1EnableEpoch,
-		},
 		common.StakingV4StartedFlag: {
 			isActiveInEpoch: func(epoch uint32) bool {
 				return epoch >= handler.enableEpochsConfig.StakingV4Step1EnableEpoch

common/enablers/enableEpochsHandler_test.go

@@ -192,13 +192,6 @@ func TestEnableEpochsHandler_IsFlagEnabled(t *testing.T) {
 	handler.EpochConfirmed(cfg.SetGuardianEnableEpoch+1, 0)
 	require.True(t, handler.IsFlagEnabled(common.SetGuardianFlag))
 
-	handler.EpochConfirmed(cfg.StakingV4Step1EnableEpoch-1, 0)
-	require.True(t, handler.IsFlagEnabled(common.StakingQueueFlag))
-	handler.EpochConfirmed(cfg.StakingV4Step1EnableEpoch, 0)
-	require.False(t, handler.IsFlagEnabled(common.StakingQueueFlag))
-	handler.EpochConfirmed(cfg.StakingV4Step1EnableEpoch+1, 0)
-	require.False(t, handler.IsFlagEnabled(common.StakingQueueFlag))
-
 	handler.EpochConfirmed(cfg.StakingV4Step1EnableEpoch-1, 0)
 	require.False(t, handler.IsFlagEnabled(common.StakingV4StartedFlag))
 	handler.EpochConfirmed(cfg.StakingV4Step1EnableEpoch, 0)
@@ -318,7 +311,6 @@ func TestEnableEpochsHandler_IsFlagEnabled(t *testing.T) {
 	require.False(t, handler.IsFlagEnabled(common.StakingV4Step1Flag))
 	require.True(t, handler.IsFlagEnabled(common.StakingV4Step2Flag))
 	require.True(t, handler.IsFlagEnabled(common.StakingV4Step3Flag))
-	require.False(t, handler.IsFlagEnabled(common.StakingQueueFlag))
 	require.True(t, handler.IsFlagEnabled(common.StakingV4StartedFlag))
 	require.True(t, handler.IsFlagEnabled(common.AlwaysMergeContextsInEEIFlag))
 }
@@ -434,7 +426,6 @@ func TestEnableEpochsHandler_GetActivationEpoch(t *testing.T) {
 	require.Equal(t, cfg.StakingV4Step1EnableEpoch, handler.GetActivationEpoch(common.StakingV4Step1Flag))
 	require.Equal(t, cfg.StakingV4Step2EnableEpoch, handler.GetActivationEpoch(common.StakingV4Step2Flag))
 	require.Equal(t, cfg.StakingV4Step3EnableEpoch, handler.GetActivationEpoch(common.StakingV4Step3Flag))
-	require.Equal(t, cfg.StakingV4Step1EnableEpoch, handler.GetActivationEpoch(common.StakingQueueFlag))
 	require.Equal(t, cfg.StakingV4Step1EnableEpoch, handler.GetActivationEpoch(common.StakingV4StartedFlag))
 	require.Equal(t, cfg.AlwaysMergeContextsInEEIEnableEpoch, handler.GetActivationEpoch(common.AlwaysMergeContextsInEEIFlag))
 }

epochStart/metachain/legacySystemSCs.go

@@ -207,7 +207,7 @@ func (s *legacySystemSCProcessor) processLegacy(
 			return err
 		}
 
-		if s.enableEpochsHandler.IsFlagEnabled(common.StakingQueueFlag) {
+		if !s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag) {
 			err = s.stakeNodesFromQueue(validatorsInfoMap, numUnStaked, nonce, common.NewList)
 			if err != nil {
 				return err
@@ -292,8 +292,7 @@ func (s *legacySystemSCProcessor) unStakeNodesWithNotEnoughFunds(
 		stakingV4Enabled := s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag)
 		validatorLeaving := validatorInfo.ShallowClone()
 		validatorLeaving.SetListAndIndex(string(common.LeavingList), validatorLeaving.GetIndex(), stakingV4Enabled)
-		err = s.replaceValidators(validatorInfo, validatorLeaving, validatorsInfoMap)
-		log.LogIfError(err)
+		s.replaceValidators(validatorInfo, validatorLeaving, validatorsInfoMap)
 	}
 
 	err = s.updateDelegationContracts(mapOwnersKeys)
@@ -430,16 +429,28 @@ func (s *legacySystemSCProcessor) fillStakingDataForNonEligible(validatorsInfoMa
 		}
 
 		if deleteCalled {
-			err := validatorsInfoMap.SetValidatorsInShard(shId, newList)
-			if err != nil {
-				return err
-			}
+			s.setValidatorsInShard(validatorsInfoMap, shId, newList)
 		}
 	}
 
 	return nil
 }
 
+func (s *legacySystemSCProcessor) setValidatorsInShard(
+	validatorsInfoMap state.ShardValidatorsInfoMapHandler,
+	shardID uint32,
+	validators []state.ValidatorInfoHandler,
+) {
+	err := validatorsInfoMap.SetValidatorsInShard(shardID, validators)
+	if err == nil {
+		return
+	}
+
+	// this should never happen, but replace them anyway, as in old legacy code
+	log.Error("legacySystemSCProcessor.setValidatorsInShard", "error", err)
+	validatorsInfoMap.SetValidatorsInShardUnsafe(shardID, validators)
+}
+
 func (s *legacySystemSCProcessor) prepareStakingDataForEligibleNodes(validatorsInfoMap state.ShardValidatorsInfoMapHandler) error {
 	eligibleNodes, err := getEligibleNodeKeys(validatorsInfoMap)
 	if err != nil {
@@ -587,14 +598,21 @@ func (s *legacySystemSCProcessor) updateMaxNodes(validatorsInfoMap state.ShardVa
 		return err
 	}
 
-	if s.enableEpochsHandler.IsFlagEnabled(common.StakingQueueFlag) {
-		sw.Start("stakeNodesFromQueue")
-		err = s.stakeNodesFromQueue(validatorsInfoMap, maxNumberOfNodes-prevMaxNumberOfNodes, nonce, common.NewList)
-		sw.Stop("stakeNodesFromQueue")
-		if err != nil {
-			return err
-		}
+	if s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag) {
+		return nil
+	}
+
+	if maxNumberOfNodes < prevMaxNumberOfNodes {
+		return epochStart.ErrInvalidMaxNumberOfNodes
+	}
+
+	sw.Start("stakeNodesFromQueue")
+	err = s.stakeNodesFromQueue(validatorsInfoMap, maxNumberOfNodes-prevMaxNumberOfNodes, nonce, common.NewList)
+	sw.Stop("stakeNodesFromQueue")
+	if err != nil {
+		return err
 	}
+
 	return nil
 }
 
@@ -752,8 +770,7 @@ func (s *legacySystemSCProcessor) stakingToValidatorStatistics(
 	}
 
 	newValidatorInfo := s.validatorInfoCreator.PeerAccountToValidatorInfo(account)
-	err = s.replaceValidators(jailedValidator, newValidatorInfo, validatorsInfoMap)
-	log.LogIfError(err)
+	s.replaceValidators(jailedValidator, newValidatorInfo, validatorsInfoMap)
 
 	return blsPubKey, nil
 }
@@ -762,14 +779,21 @@ func (s *legacySystemSCProcessor) replaceValidators(
 	old state.ValidatorInfoHandler,
 	new state.ValidatorInfoHandler,
 	validatorsInfoMap state.ShardValidatorsInfoMapHandler,
-) error {
-	stakingV4Enabled := s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag)
-	if stakingV4Enabled {
-		return validatorsInfoMap.Replace(old, new)
+) {
+	// legacy code
+	if !s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag) {
+		_ = validatorsInfoMap.ReplaceValidatorByKey(old.GetPublicKey(), new, old.GetShardId())
+		return
 	}
 
-	_ = validatorsInfoMap.ReplaceValidatorByKey(old.GetPublicKey(), new, old.GetShardId())
-	return nil
+	// try with new code which does extra validity checks.
+	// if this also fails, do legacy code
+	if err := validatorsInfoMap.Replace(old, new); err != nil {
+		log.Error("legacySystemSCProcessor.replaceValidators", "error", err)
+
+		replaced := validatorsInfoMap.ReplaceValidatorByKey(old.GetPublicKey(), new, old.GetShardId())
+		log.Debug("legacySystemSCProcessor.replaceValidators", "old", old.GetPublicKey(), "new", new.GetPublicKey(), "was replace successful", replaced)
+	}
 }
 
 func isValidator(validator state.ValidatorInfoHandler) bool {
@@ -1219,11 +1243,6 @@ func (s *legacySystemSCProcessor) addNewlyStakedNodesToValidatorTrie(
 			return err
 		}
 
-		err = peerAcc.SetBLSPublicKey(blsKey)
-		if err != nil {
-			return err
-		}
-
 		peerAcc.SetListAndIndex(peerAcc.GetShardId(), string(list), uint32(nonce), s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag))
 		peerAcc.SetTempRating(s.startRating)
 		peerAcc.SetUnStakedEpoch(common.DefaultUnstakedEpoch)
@@ -1244,22 +1263,32 @@ func (s *legacySystemSCProcessor) addNewlyStakedNodesToValidatorTrie(
 			AccumulatedFees: big.NewInt(0),
 		}
 
-		existingValidator := validatorsInfoMap.GetValidator(validatorInfo.GetPublicKey())
-		// This fix is not be backwards incompatible
-		if !check.IfNil(existingValidator) && s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag) {
-			err = validatorsInfoMap.Delete(existingValidator)
-			if err != nil {
-				return err
-			}
+		err = s.addNewValidator(validatorsInfoMap, validatorInfo)
+		if err != nil {
+			return err
 		}
+	}
+
+	return nil
+}
+
+func (s *legacySystemSCProcessor) addNewValidator(
+	validatorsInfoMap state.ShardValidatorsInfoMapHandler,
+	validatorInfo state.ValidatorInfoHandler,
+) error {
+	if !s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag) {
+		return validatorsInfoMap.Add(validatorInfo)
+	}
 
-		err = validatorsInfoMap.Add(validatorInfo)
+	existingValidator := validatorsInfoMap.GetValidator(validatorInfo.GetPublicKey())
+	if !check.IfNil(existingValidator) {
+		err := validatorsInfoMap.Delete(existingValidator)
 		if err != nil {
 			return err
 		}
 	}
 
-	return nil
+	return validatorsInfoMap.Add(validatorInfo)
 }
 
 func (s *legacySystemSCProcessor) initESDT() error {
@@ -1277,7 +1306,7 @@ func (s *legacySystemSCProcessor) extractConfigFromESDTContract() ([][]byte, err
 			CallerAddr:  s.endOfEpochCallerAddress,
 			Arguments:   [][]byte{},
 			CallValue:   big.NewInt(0),
-			GasProvided: math.MaxUint64,
+			GasProvided: math.MaxInt64,
 		},
 		Function:      "getContractConfig",
 		RecipientAddr: vm.ESDTSCAddress,

epochStart/metachain/systemSCs.go

@@ -78,7 +78,6 @@ func NewSystemSCProcessor(args ArgsNewEpochStartSystemSCProcessing) (*systemSCPr
 		common.SaveJailedAlwaysFlag,
 		common.StakingV4Step1Flag,
 		common.StakingV4Step2Flag,
-		common.StakingQueueFlag,
 		common.StakingV4StartedFlag,
 		common.DelegationSmartContractFlagInSpecificEpochOnly,
 		common.GovernanceFlagInSpecificEpochOnly,

epochStart/metachain/systemSCs_test.go

@@ -2326,6 +2326,10 @@ func TestSystemSCProcessor_LegacyEpochConfirmedCorrectMaxNumNodesAfterNodeRestar
 	args.EpochNotifier.CheckEpoch(&block.Header{Epoch: 6, Nonce: 6})
 	require.True(t, s.flagChangeMaxNodesEnabled.IsSet())
 	err = s.processLegacy(validatorsInfoMap, 6, 6)
+	require.Equal(t, epochStart.ErrInvalidMaxNumberOfNodes, err)
+
+	args.EnableEpochsHandler.(*enableEpochsHandlerMock.EnableEpochsHandlerStub).AddActiveFlags(common.StakingV4StartedFlag)
+	err = s.processLegacy(validatorsInfoMap, 6, 6)
 	require.Nil(t, err)
 	require.Equal(t, nodesConfigEpoch6.MaxNumNodes, s.maxNodes)
 

state/interface.go

@@ -297,6 +297,7 @@ type ShardValidatorsInfoMapHandler interface {
 	Replace(old ValidatorInfoHandler, new ValidatorInfoHandler) error
 	ReplaceValidatorByKey(oldBlsKey []byte, new ValidatorInfoHandler, shardID uint32) bool
 	SetValidatorsInShard(shardID uint32, validators []ValidatorInfoHandler) error
+	SetValidatorsInShardUnsafe(shardID uint32, validators []ValidatorInfoHandler)
 }
 
 // ValidatorInfoHandler defines which data shall a validator info hold.

state/validatorsInfoMap.go

@@ -161,6 +161,14 @@ func (vi *shardValidatorsInfoMap) SetValidatorsInShard(shardID uint32, validator
 	return nil
 }
 
+// SetValidatorsInShardUnsafe resets all validators saved in a specific shard with the provided ones.
+// It does not check that provided validators are in the same shard as provided shard id.
+func (vi *shardValidatorsInfoMap) SetValidatorsInShardUnsafe(shardID uint32, validators []ValidatorInfoHandler) {
+	vi.mutex.Lock()
+	vi.valInfoMap[shardID] = validators
+	vi.mutex.Unlock()
+}
+
 // Delete will delete the provided validator from the internally stored map, if found.
 // The validators slice at the corresponding shardID key will be re-sliced, without reordering
 func (vi *shardValidatorsInfoMap) Delete(validator ValidatorInfoHandler) error {