From 760a76735951c1d55a9f033cace5b415ec312a08 Mon Sep 17 00:00:00 2001 From: MariusC Date: Mon, 8 Apr 2024 16:14:17 +0300 Subject: [PATCH 1/4] FIX: Possible backwards incompatibilities fixes --- epochStart/metachain/legacySystemSCs.go | 28 ++++++++++++++++++------- epochStart/metachain/systemSCs_test.go | 4 ++++ state/interface.go | 1 + state/validatorsInfoMap.go | 8 +++++++ 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/epochStart/metachain/legacySystemSCs.go b/epochStart/metachain/legacySystemSCs.go index 4173a3cfc59..6b434eb209e 100644 --- a/epochStart/metachain/legacySystemSCs.go +++ b/epochStart/metachain/legacySystemSCs.go @@ -432,7 +432,7 @@ func (s *legacySystemSCProcessor) fillStakingDataForNonEligible(validatorsInfoMa } if deleteCalled { - err := validatorsInfoMap.SetValidatorsInShard(shId, newList) + err := s.setValidatorsInShard(validatorsInfoMap, shId, newList) if err != nil { return err } @@ -442,6 +442,19 @@ func (s *legacySystemSCProcessor) fillStakingDataForNonEligible(validatorsInfoMa return nil } +func (s *legacySystemSCProcessor) setValidatorsInShard( + validatorsInfoMap state.ShardValidatorsInfoMapHandler, + shardID uint32, + validators []state.ValidatorInfoHandler, +) error { + if s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag) { + return validatorsInfoMap.SetValidatorsInShard(shardID, validators) + } + + validatorsInfoMap.SetValidatorsInShardUnsafe(shardID, validators) + return nil +} + func (s *legacySystemSCProcessor) prepareStakingDataForEligibleNodes(validatorsInfoMap state.ShardValidatorsInfoMapHandler) error { eligibleNodes, err := getEligibleNodeKeys(validatorsInfoMap) if err != nil { @@ -589,6 +602,12 @@ func (s *legacySystemSCProcessor) updateMaxNodes(validatorsInfoMap state.ShardVa return err } + if !s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag) { + if maxNumberOfNodes < prevMaxNumberOfNodes { + return epochStart.ErrInvalidMaxNumberOfNodes + } + } + if s.enableEpochsHandler.IsFlagEnabled(common.StakingQueueFlag) { sw.Start("stakeNodesFromQueue") err = s.stakeNodesFromQueue(validatorsInfoMap, maxNumberOfNodes-prevMaxNumberOfNodes, nonce, common.NewList) @@ -1223,11 +1242,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) @@ -1281,7 +1295,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, diff --git a/epochStart/metachain/systemSCs_test.go b/epochStart/metachain/systemSCs_test.go index 7826c461d36..0dc9eb82b23 100644 --- a/epochStart/metachain/systemSCs_test.go +++ b/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) diff --git a/state/interface.go b/state/interface.go index 842260cff28..6a6c098ade5 100644 --- a/state/interface.go +++ b/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. diff --git a/state/validatorsInfoMap.go b/state/validatorsInfoMap.go index d72d1b17996..27100745e02 100644 --- a/state/validatorsInfoMap.go +++ b/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 { From c655353a7298c99e2d4800c4d7c02ab9c81b53bf Mon Sep 17 00:00:00 2001 From: MariusC Date: Thu, 11 Apr 2024 10:53:18 +0300 Subject: [PATCH 2/4] FEAT: Extra safety measure checks --- epochStart/metachain/legacySystemSCs.go | 40 ++++++++++++++----------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/epochStart/metachain/legacySystemSCs.go b/epochStart/metachain/legacySystemSCs.go index aa9ff71c3fe..3247cb2dff1 100644 --- a/epochStart/metachain/legacySystemSCs.go +++ b/epochStart/metachain/legacySystemSCs.go @@ -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,10 +429,7 @@ func (s *legacySystemSCProcessor) fillStakingDataForNonEligible(validatorsInfoMa } if deleteCalled { - err := s.setValidatorsInShard(validatorsInfoMap, shId, newList) - if err != nil { - return err - } + s.setValidatorsInShard(validatorsInfoMap, shId, newList) } } @@ -444,13 +440,15 @@ func (s *legacySystemSCProcessor) setValidatorsInShard( validatorsInfoMap state.ShardValidatorsInfoMapHandler, shardID uint32, validators []state.ValidatorInfoHandler, -) error { - if s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag) { - return validatorsInfoMap.SetValidatorsInShard(shardID, validators) +) { + 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) - return nil } func (s *legacySystemSCProcessor) prepareStakingDataForEligibleNodes(validatorsInfoMap state.ShardValidatorsInfoMapHandler) error { @@ -771,8 +769,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 } @@ -781,14 +778,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 { From f8151b193d479b93ff487b5df94a47dbe7a4d9eb Mon Sep 17 00:00:00 2001 From: MariusC Date: Fri, 12 Apr 2024 11:35:32 +0300 Subject: [PATCH 3/4] CLN: AddNewValidator to trie --- epochStart/metachain/legacySystemSCs.go | 28 +++++++++++++++++-------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/epochStart/metachain/legacySystemSCs.go b/epochStart/metachain/legacySystemSCs.go index 3247cb2dff1..2abe8a993fb 100644 --- a/epochStart/metachain/legacySystemSCs.go +++ b/epochStart/metachain/legacySystemSCs.go @@ -1262,22 +1262,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 } + } - err = validatorsInfoMap.Add(validatorInfo) + return nil +} + +func (s *legacySystemSCProcessor) addNewValidator( + validatorsInfoMap state.ShardValidatorsInfoMapHandler, + validatorInfo state.ValidatorInfoHandler, +) error { + if !s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag) { + return 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 { From e1d0f11464dc3bdae9b63a57861052a61ec79283 Mon Sep 17 00:00:00 2001 From: MariusC Date: Fri, 12 Apr 2024 17:31:43 +0300 Subject: [PATCH 4/4] FIX: Remove StakingQueueEnabled flag --- common/constants.go | 1 - common/enablers/enableEpochsHandler.go | 6 ----- common/enablers/enableEpochsHandler_test.go | 9 -------- epochStart/metachain/legacySystemSCs.go | 25 +++++++++++---------- epochStart/metachain/systemSCs.go | 1 - 5 files changed, 13 insertions(+), 29 deletions(-) diff --git a/common/constants.go b/common/constants.go index 0476f1aa5e5..16c77a5d147 100644 --- a/common/constants.go +++ b/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 diff --git a/common/enablers/enableEpochsHandler.go b/common/enablers/enableEpochsHandler.go index d560a432462..f64dbf99ea5 100644 --- a/common/enablers/enableEpochsHandler.go +++ b/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 diff --git a/common/enablers/enableEpochsHandler_test.go b/common/enablers/enableEpochsHandler_test.go index c91f65b805a..4155b15dfbb 100644 --- a/common/enablers/enableEpochsHandler_test.go +++ b/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)) } diff --git a/epochStart/metachain/legacySystemSCs.go b/epochStart/metachain/legacySystemSCs.go index 2abe8a993fb..677cbcb682b 100644 --- a/epochStart/metachain/legacySystemSCs.go +++ b/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 @@ -598,20 +598,21 @@ func (s *legacySystemSCProcessor) updateMaxNodes(validatorsInfoMap state.ShardVa return err } - if !s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag) { - if maxNumberOfNodes < prevMaxNumberOfNodes { - return epochStart.ErrInvalidMaxNumberOfNodes - } + if s.enableEpochsHandler.IsFlagEnabled(common.StakingV4StartedFlag) { + return nil } - 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 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 } diff --git a/epochStart/metachain/systemSCs.go b/epochStart/metachain/systemSCs.go index 229a41d5710..96cba60251b 100644 --- a/epochStart/metachain/systemSCs.go +++ b/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,