Fix edge case empty waiting list

integrationTests/vm/staking/stakingV4_test.go

@@ -901,3 +901,284 @@ func TestStakingV4_JailAndUnJailNodes(t *testing.T) {
 	requireSameSliceDifferentOrder(t, queue, currNodesConfig.auction)
 	require.Empty(t, node.NodesConfig.queue)
 }
+
+func TestStakingV4_DifferentEdgeCasesWithNotEnoughNodesInWaitingShouldSendShuffledToToWaiting(t *testing.T) {
+	pubKeys := generateAddresses(0, 20)
+
+	owner1 := "owner1"
+	owner1Stats := &OwnerStats{
+		EligibleBlsKeys: map[uint32][][]byte{
+			core.MetachainShardId: pubKeys[:4],
+			0:                     pubKeys[4:8],
+		},
+		WaitingBlsKeys: map[uint32][][]byte{
+			core.MetachainShardId: pubKeys[8:9],
+			0:                     pubKeys[9:10],
+		},
+		TotalStake: big.NewInt(20 * nodePrice),
+	}
+
+	cfg := &InitialNodesConfig{
+		MetaConsensusGroupSize:        2,
+		ShardConsensusGroupSize:       2,
+		MinNumberOfEligibleShardNodes: 4,
+		MinNumberOfEligibleMetaNodes:  4,
+		NumOfShards:                   1,
+		Owners: map[string]*OwnerStats{
+			owner1: owner1Stats,
+		},
+		MaxNodesChangeConfig: []config.MaxNodesChangeConfig{
+			{
+				EpochEnable:            0,
+				MaxNumNodes:            12,
+				NodesToShufflePerShard: 1,
+			},
+			{
+				EpochEnable:            stakingV4Step3EnableEpoch, // epoch 3
+				MaxNumNodes:            10,
+				NodesToShufflePerShard: 1,
+			},
+			{
+				EpochEnable:            6,
+				MaxNumNodes:            12,
+				NodesToShufflePerShard: 1,
+			},
+		},
+	}
+	node := NewTestMetaProcessorWithCustomNodes(cfg)
+	node.EpochStartTrigger.SetRoundsPerEpoch(4)
+
+	// 1. Check initial config is correct
+	currNodesConfig := node.NodesConfig
+	require.Len(t, getAllPubKeys(currNodesConfig.eligible), 8)
+	require.Len(t, getAllPubKeys(currNodesConfig.waiting), 2)
+	require.Len(t, currNodesConfig.eligible[core.MetachainShardId], 4)
+	require.Len(t, currNodesConfig.waiting[core.MetachainShardId], 1)
+	require.Len(t, currNodesConfig.eligible[0], 4)
+	require.Len(t, currNodesConfig.waiting[0], 1)
+	require.Empty(t, currNodesConfig.shuffledOut)
+	require.Empty(t, currNodesConfig.auction)
+
+	prevNodesConfig := currNodesConfig
+	epoch := uint32(0)
+
+	// During these 9 epochs, we will always have:
+	// - 10 activeNodes (8 eligible + 2 waiting)
+	// - 1 node to shuffle out per shard
+	// Meanwhile, maxNumNodes changes from 12-10-12
+	// Since activeNodes <= maxNumNodes, shuffled out nodes will always be sent directly to waiting list,
+	// instead of auction(there is no reason to send them to auction, they will be selected anyway)
+	for epoch < 9 {
+		node.Process(t, 5)
+
+		currNodesConfig = node.NodesConfig
+		require.Len(t, getAllPubKeys(currNodesConfig.eligible), 8)
+		require.Len(t, getAllPubKeys(currNodesConfig.waiting), 2)
+		require.Len(t, currNodesConfig.eligible[core.MetachainShardId], 4)
+		require.Len(t, currNodesConfig.waiting[core.MetachainShardId], 1)
+		require.Len(t, currNodesConfig.eligible[0], 4)
+		require.Len(t, currNodesConfig.waiting[0], 1)
+		require.Empty(t, currNodesConfig.shuffledOut)
+		require.Empty(t, currNodesConfig.auction)
+
+		// Shuffled nodes from previous eligible are sent to waiting and previous waiting list nodes are replacing shuffled nodes
+		requireSliceContainsNumOfElements(t, getAllPubKeys(currNodesConfig.eligible), getAllPubKeys(prevNodesConfig.waiting), 2)
+		requireSliceContainsNumOfElements(t, getAllPubKeys(currNodesConfig.eligible), getAllPubKeys(prevNodesConfig.eligible), 6)
+		requireSliceContainsNumOfElements(t, getAllPubKeys(currNodesConfig.waiting), getAllPubKeys(prevNodesConfig.eligible), 2)
+
+		prevNodesConfig = currNodesConfig
+		epoch++
+	}
+
+	require.Equal(t, epoch, node.EpochStartTrigger.Epoch())
+
+	// Epoch = 9 with:
+	// - activeNodes = 10
+	// - maxNumNodes = 12
+	// Owner2 stakes 2 nodes, which should be initially sent to auction list
+	owner2Nodes := pubKeys[10:12]
+	node.ProcessStake(t, map[string]*NodesRegisterData{
+		"owner2": {
+			BLSKeys:    owner2Nodes,
+			TotalStake: big.NewInt(5 * nodePrice),
+		},
+	})
+	currNodesConfig = node.NodesConfig
+	require.Len(t, getAllPubKeys(currNodesConfig.eligible), 8)
+	require.Len(t, getAllPubKeys(currNodesConfig.waiting), 2)
+	require.Len(t, currNodesConfig.eligible[core.MetachainShardId], 4)
+	require.Len(t, currNodesConfig.waiting[core.MetachainShardId], 1)
+	require.Len(t, currNodesConfig.eligible[0], 4)
+	require.Len(t, currNodesConfig.waiting[0], 1)
+	require.Empty(t, currNodesConfig.shuffledOut)
+	requireSameSliceDifferentOrder(t, currNodesConfig.auction, owner2Nodes)
+
+	// Epoch = 10 with:
+	// - activeNodes = 12
+	// - maxNumNodes = 12
+	// Owner2's new nodes are selected from auction and distributed to waiting list
+	node.Process(t, 5)
+	currNodesConfig = node.NodesConfig
+	require.Len(t, getAllPubKeys(currNodesConfig.eligible), 8)
+	require.Len(t, getAllPubKeys(currNodesConfig.waiting), 4)
+	require.Len(t, currNodesConfig.eligible[core.MetachainShardId], 4)
+	require.Len(t, currNodesConfig.waiting[core.MetachainShardId], 2)
+	require.Len(t, currNodesConfig.eligible[0], 4)
+	require.Len(t, currNodesConfig.waiting[0], 2)
+	require.Empty(t, currNodesConfig.shuffledOut)
+	require.Empty(t, currNodesConfig.auction)
+
+	// Shuffled nodes from previous eligible are sent to waiting and previous waiting list nodes are replacing shuffled nodes
+	requireSliceContainsNumOfElements(t, getAllPubKeys(currNodesConfig.eligible), getAllPubKeys(prevNodesConfig.waiting), 2)
+	requireSliceContainsNumOfElements(t, getAllPubKeys(currNodesConfig.eligible), getAllPubKeys(prevNodesConfig.eligible), 6)
+	requireSliceContainsNumOfElements(t, getAllPubKeys(currNodesConfig.waiting), getAllPubKeys(prevNodesConfig.eligible), 2)
+	requireSliceContains(t, getAllPubKeys(currNodesConfig.waiting), owner2Nodes)
+
+	// During epochs 10-13, we will have:
+	// - activeNodes = 12
+	// - maxNumNodes = 12
+	// Since activeNodes == maxNumNodes, shuffled out nodes will always be sent directly to waiting list, instead of auction
+	epoch = 10
+	require.Equal(t, epoch, node.EpochStartTrigger.Epoch())
+	prevNodesConfig = currNodesConfig
+	for epoch < 13 {
+		node.Process(t, 5)
+
+		currNodesConfig = node.NodesConfig
+		require.Len(t, getAllPubKeys(currNodesConfig.eligible), 8)
+		require.Len(t, getAllPubKeys(currNodesConfig.waiting), 4)
+		require.Len(t, currNodesConfig.eligible[core.MetachainShardId], 4)
+		require.Len(t, currNodesConfig.waiting[core.MetachainShardId], 2)
+		require.Len(t, currNodesConfig.eligible[0], 4)
+		require.Len(t, currNodesConfig.waiting[0], 2)
+		require.Empty(t, currNodesConfig.shuffledOut)
+		require.Empty(t, currNodesConfig.auction)
+
+		// Shuffled nodes from previous eligible are sent to waiting and previous waiting list nodes are replacing shuffled nodes
+		requireSliceContainsNumOfElements(t, getAllPubKeys(currNodesConfig.eligible), getAllPubKeys(prevNodesConfig.waiting), 2)
+		requireSliceContainsNumOfElements(t, getAllPubKeys(currNodesConfig.eligible), getAllPubKeys(prevNodesConfig.eligible), 6)
+		requireSliceContainsNumOfElements(t, getAllPubKeys(currNodesConfig.waiting), getAllPubKeys(prevNodesConfig.eligible), 2)
+
+		prevNodesConfig = currNodesConfig
+		epoch++
+	}
+
+	// Epoch = 13 with:
+	// - activeNodes = 12
+	// - maxNumNodes = 12
+	// Owner3 stakes 2 nodes, which should be initially sent to auction list
+	owner3Nodes := pubKeys[12:14]
+	node.ProcessStake(t, map[string]*NodesRegisterData{
+		"owner3": {
+			BLSKeys:    owner3Nodes,
+			TotalStake: big.NewInt(5 * nodePrice),
+		},
+	})
+	currNodesConfig = node.NodesConfig
+	require.Len(t, getAllPubKeys(currNodesConfig.eligible), 8)
+	require.Len(t, getAllPubKeys(currNodesConfig.waiting), 4)
+	require.Len(t, currNodesConfig.eligible[core.MetachainShardId], 4)
+	require.Len(t, currNodesConfig.waiting[core.MetachainShardId], 2)
+	require.Len(t, currNodesConfig.eligible[0], 4)
+	require.Len(t, currNodesConfig.waiting[0], 2)
+	require.Empty(t, currNodesConfig.shuffledOut)
+	requireSameSliceDifferentOrder(t, currNodesConfig.auction, owner3Nodes)
+
+	// During epochs 14-18, we will have:
+	// - activeNodes = 14
+	// - maxNumNodes = 12
+	// Since activeNodes > maxNumNodes, shuffled out nodes (2) will be sent to auction list
+	node.Process(t, 5)
+	prevNodesConfig = node.NodesConfig
+	epoch = 14
+	require.Equal(t, epoch, node.EpochStartTrigger.Epoch())
+	for epoch < 18 {
+
+		require.Len(t, getAllPubKeys(currNodesConfig.eligible), 8)
+		require.Len(t, getAllPubKeys(currNodesConfig.waiting), 4)
+		require.Len(t, currNodesConfig.eligible[core.MetachainShardId], 4)
+		require.Len(t, currNodesConfig.waiting[core.MetachainShardId], 2)
+		require.Len(t, currNodesConfig.eligible[0], 4)
+		require.Len(t, currNodesConfig.waiting[0], 2)
+		require.Len(t, currNodesConfig.auction, 2)
+
+		node.Process(t, 5)
+
+		currNodesConfig = node.NodesConfig
+		// Nodes which are now in eligible are from previous waiting list
+		requireSliceContainsNumOfElements(t, getAllPubKeys(currNodesConfig.eligible), getAllPubKeys(prevNodesConfig.waiting), 2)
+
+		// New auction list does not contain nodes from previous auction list, since all of them have been distributed to waiting
+		requireSliceContainsNumOfElements(t, currNodesConfig.auction, prevNodesConfig.auction, 0)
+
+		// All shuffled out are from previous eligible config
+		requireMapContains(t, prevNodesConfig.eligible, getAllPubKeys(currNodesConfig.shuffledOut))
+
+		// All shuffled out are now in auction
+		requireSliceContains(t, currNodesConfig.auction, getAllPubKeys(currNodesConfig.shuffledOut))
+
+		// All nodes which have been selected from previous auction list are now in waiting
+		requireSliceContains(t, getAllPubKeys(currNodesConfig.waiting), prevNodesConfig.auction)
+
+		prevNodesConfig = currNodesConfig
+		epoch++
+	}
+
+	// Epoch = 18, with:
+	// - activeNodes = 14
+	// - maxNumNodes = 12
+	// Owner3 unStakes one of his nodes
+	node.ProcessUnStake(t, map[string][][]byte{
+		"owner3": {owner3Nodes[0]},
+	})
+
+	// Epoch = 19, with:
+	// - activeNodes = 13
+	// - maxNumNodes = 12
+	// Owner3's unStaked node is now leaving
+	node.Process(t, 5)
+	currNodesConfig = node.NodesConfig
+	require.Len(t, currNodesConfig.leaving, 1)
+	requireMapContains(t, currNodesConfig.leaving, [][]byte{owner3Nodes[0]})
+
+	epoch = 19
+	require.Equal(t, epoch, node.EpochStartTrigger.Epoch())
+	prevNodesConfig = node.NodesConfig
+	require.Equal(t, epoch, node.EpochStartTrigger.Epoch())
+
+	// During epochs 19-23, we will have:
+	// - activeNodes = 13
+	// - maxNumNodes = 12
+	// Since activeNodes > maxNumNodes:
+	// - shuffled out nodes (2) will be sent to auction list
+	// - waiting lists will be unbalanced (3 in total: 1 + 2 per shard)
+	// - no node will spend extra epochs in eligible/waiting, since waiting lists will always be refilled
+	for epoch < 23 {
+		require.Len(t, getAllPubKeys(currNodesConfig.eligible), 8)
+		require.Len(t, getAllPubKeys(currNodesConfig.waiting), 3)
+		require.Len(t, currNodesConfig.eligible[core.MetachainShardId], 4)
+		require.Len(t, currNodesConfig.eligible[0], 4)
+		require.Len(t, currNodesConfig.auction, 2)
+
+		node.Process(t, 5)
+
+		currNodesConfig = node.NodesConfig
+		// Nodes which are now in eligible are from previous waiting list
+		requireSliceContainsNumOfElements(t, getAllPubKeys(currNodesConfig.eligible), getAllPubKeys(prevNodesConfig.waiting), 2)
+
+		// New auction list does not contain nodes from previous auction list, since all of them have been distributed to waiting
+		requireSliceContainsNumOfElements(t, currNodesConfig.auction, prevNodesConfig.auction, 0)
+
+		// All shuffled out are from previous eligible config
+		requireMapContains(t, prevNodesConfig.eligible, getAllPubKeys(currNodesConfig.shuffledOut))
+
+		// All shuffled out are now in auction
+		requireSliceContains(t, currNodesConfig.auction, getAllPubKeys(currNodesConfig.shuffledOut))
+
+		// All nodes which have been selected from previous auction list are now in waiting
+		requireSliceContains(t, getAllPubKeys(currNodesConfig.waiting), prevNodesConfig.auction)
+
+		prevNodesConfig = currNodesConfig
+		epoch++
+	}
+}

sharding/nodesCoordinator/hashValidatorShuffler.go

@@ -41,6 +41,7 @@ type shuffleNodesArg struct {
 	nodesPerShard           uint32
 	nbShards                uint32
 	maxNodesToSwapPerShard  uint32
+	maxNumNodes             uint32
 	flagBalanceWaitingLists bool
 	flagStakingV4Step2      bool
 	flagStakingV4Step3      bool
@@ -195,6 +196,7 @@ func (rhs *randHashShuffler) UpdateNodeLists(args ArgsUpdateNodes) (*ResUpdateNo
 		flagBalanceWaitingLists: rhs.flagBalanceWaitingLists.IsSet(),
 		flagStakingV4Step2:      rhs.flagStakingV4Step2.IsSet(),
 		flagStakingV4Step3:      rhs.flagStakingV4Step3.IsSet(),
+		maxNumNodes:             rhs.activeNodesConfig.MaxNumNodes,
 	})
 }
 
@@ -283,6 +285,30 @@ func shuffleNodes(arg shuffleNodesArg) (*ResUpdateNodes, error) {
 
 	shuffledOutMap, newEligible := shuffleOutNodes(newEligible, numToRemove, arg.randomness)
 
+	numShuffled := getNumPubKeys(shuffledOutMap)
+	numNewEligible := getNumPubKeys(newEligible)
+	numNewWaiting := getNumPubKeys(newWaiting)
+
+	numSelectedAuction := uint32(len(arg.auction))
+	totalNewWaiting := numNewWaiting + numSelectedAuction
+
+	totalNodes := totalNewWaiting + numNewEligible + numShuffled
+	maxNumNodes := arg.maxNumNodes
+
+	distributeShuffledToWaitingInStakingV4 := false
+	if totalNodes <= maxNumNodes {
+		log.Warn("num of total nodes in waiting is too low after shuffling; will distribute "+
+			"shuffled out nodes directly to waiting and skip sending them to auction",
+			"numShuffled", numShuffled,
+			"numNewEligible", numNewEligible,
+			"numSelectedAuction", numSelectedAuction,
+			"totalNewWaiting", totalNewWaiting,
+			"totalNodes", totalNodes,
+			"maxNumNodes", maxNumNodes)
+
+		distributeShuffledToWaitingInStakingV4 = arg.flagStakingV4Step2
+	}
+
 	err = moveMaxNumNodesToMap(newEligible, newWaiting, arg.nodesMeta, arg.nodesPerShard)
 	if err != nil {
 		log.Warn("moveNodesToMap failed", "error", err)
@@ -294,12 +320,27 @@ func shuffleNodes(arg shuffleNodesArg) (*ResUpdateNodes, error) {
 	}
 
 	if arg.flagStakingV4Step3 {
+		log.Debug("distributing selected nodes from auction to waiting",
+			"num auction nodes", len(arg.auction), "num waiting nodes", numNewWaiting)
+
 		// Distribute selected validators from AUCTION -> WAITING
 		err = distributeValidators(newWaiting, arg.auction, arg.randomness, false)
 		if err != nil {
 			log.Warn("distributeValidators auction list failed", "error", err)
 		}
 	}
+
+	if distributeShuffledToWaitingInStakingV4 {
+		log.Debug("distributing shuffled out nodes to waiting in staking V4",
+			"num shuffled nodes", numShuffled, "num waiting nodes", numNewWaiting)
+
+		// Distribute validators from SHUFFLED OUT -> WAITING
+		err = arg.distributor.DistributeValidators(newWaiting, shuffledOutMap, arg.randomness, arg.flagBalanceWaitingLists)
+		if err != nil {
+			log.Warn("distributeValidators shuffledOut failed", "error", err)
+		}
+	}
+
 	if !arg.flagStakingV4Step2 {
 		// Distribute validators from SHUFFLED OUT -> WAITING
 		err = arg.distributor.DistributeValidators(newWaiting, shuffledOutMap, arg.randomness, arg.flagBalanceWaitingLists)
@@ -614,6 +655,16 @@ func moveNodesToMap(destination map[uint32][]Validator, source map[uint32][]Vali
 	return nil
 }
 
+func getNumPubKeys(shardValidatorsMap map[uint32][]Validator) uint32 {
+	numPubKeys := uint32(0)
+
+	for _, validatorsInShard := range shardValidatorsMap {
+		numPubKeys += uint32(len(validatorsInShard))
+	}
+
+	return numPubKeys
+}
+
 // moveMaxNumNodesToMap moves the validators in the source list to the corresponding destination list
 // but adding just enough nodes so that at most the number of nodes is kept in the destination list
 // The parameter maxNodesToMove is a limiting factor and should limit the number of nodes

vm/systemSmartContracts/delegation.go

@@ -2883,22 +2883,6 @@ func (d *delegation) executeStakeAndUpdateStatus(
 	return vmcommon.Ok
 }
 
-func (d *delegation) getConfigStatusAndGlobalFund() (*DelegationConfig, *DelegationContractStatus, *GlobalFundData, error) {
-	dConfig, err := d.getDelegationContractConfig()
-	if err != nil {
-		return nil, nil, nil, err
-	}
-	globalFund, err := d.getGlobalFundData()
-	if err != nil {
-		return nil, nil, nil, err
-	}
-	dStatus, err := d.getDelegationStatus()
-	if err != nil {
-		return nil, nil, nil, err
-	}
-	return dConfig, dStatus, globalFund, nil
-}
-
 func (d *delegation) executeOnValidatorSC(address []byte, function string, args [][]byte, value *big.Int) (*vmcommon.VMOutput, error) {
 	validatorCall := function
 	for _, key := range args {