troubleshooting engine_test issues

consensus/engine/consensus_engine.go

@@ -151,5 +151,5 @@ type Engine interface {
 		receipts []*types.Receipt, outcxs []*types.CXReceipt,
 		incxs []*types.CXReceiptsProof, stks staking.StakingTransactions,
 		doubleSigners slash.Records, sigsReady chan bool, viewID func() uint64,
-	) (*types.Block, reward.Reader, error)
+	) (*types.Block, map[common.Address][]common.Address, reward.Reader, error)
 }

core/blockchain.go

@@ -100,6 +100,10 @@ type BlockChain interface {
 	// Rollback is designed to remove a chain of links from the database that aren't
 	// certain enough to be valid.
 	Rollback(chain []common.Hash) error
+	// WriteBlockWithoutState writes only the block and its metadata to the database,
+	// but does not write any state. This is used to construct competing side forks
+	// up to the point where they exceed the canonical total difficulty.
+
 	// GetMaxGarbageCollectedBlockNumber ..
 	GetMaxGarbageCollectedBlockNumber() int64
 	// InsertChain attempts to insert the given batch of blocks in to the canonical
@@ -274,6 +278,7 @@ type BlockChain interface {
 	UpdateStakingMetaData(
 		batch rawdb.DatabaseWriter, block *types.Block,
 		stakeMsgs []types2.StakeMsg,
+		delegationsToRemove map[common.Address][]common.Address,
 		state *state.DB, epoch, newEpoch *big.Int,
 	) (newValidators []common.Address, err error)
 	// ReadBlockRewardAccumulator must only be called on beaconchain
@@ -314,6 +319,7 @@ type BlockChain interface {
 		receipts []*types.Receipt,
 		cxReceipts []*types.CXReceipt,
 		stakeMsgs []types2.StakeMsg,
+		delegationsToRemove map[common.Address][]common.Address,
 		payout reward.Reader,
 		state *state.DB,
 	) (status WriteStatus, err error)

core/blockchain_impl.go

@@ -534,7 +534,7 @@ func (bc *BlockChainImpl) validateNewBlock(block *types.Block) error {
 	// NOTE Order of mutating state here matters.
 	// Process block using the parent state as reference point.
 	// Do not read cache from processor.
-	receipts, cxReceipts, _, _, usedGas, _, _, err := bc.processor.Process(
+	receipts, cxReceipts, _, _, _, usedGas, _, _, err := bc.processor.Process(
 		block, state, bc.vmConfig, false,
 	)
 	if err != nil {
@@ -1198,6 +1198,7 @@ func (bc *BlockChainImpl) writeBlockWithState(
 	block *types.Block, receipts []*types.Receipt,
 	cxReceipts []*types.CXReceipt,
 	stakeMsgs []staking.StakeMsg,
+	delegationsToRemove map[common.Address][]common.Address,
 	paid reward.Reader,
 	state *state.DB,
 ) (status WriteStatus, err error) {
@@ -1294,7 +1295,7 @@ func (bc *BlockChainImpl) writeBlockWithState(
 	// Write offchain data
 	if status, err := bc.CommitOffChainData(
 		batch, block, receipts,
-		cxReceipts, stakeMsgs,
+		cxReceipts, stakeMsgs, delegationsToRemove,
 		paid, state,
 	); err != nil {
 		return status, err
@@ -1547,7 +1548,7 @@ func (bc *BlockChainImpl) insertChain(chain types.Blocks, verifyHeaders bool) (i
 		}
 		// Process block using the parent state as reference point.
 		substart := time.Now()
-		receipts, cxReceipts, stakeMsgs, logs, usedGas, payout, newState, err := bc.processor.Process(
+		receipts, cxReceipts, stakeMsgs, delegationsToRemove, logs, usedGas, payout, newState, err := bc.processor.Process(
 			block, state, vmConfig, true,
 		)
 		state = newState // update state in case the new state is cached.
@@ -1584,7 +1585,7 @@ func (bc *BlockChainImpl) insertChain(chain types.Blocks, verifyHeaders bool) (i
 		// Write the block to the chain and get the status.
 		substart = time.Now()
 		status, err := bc.writeBlockWithState(
-			block, receipts, cxReceipts, stakeMsgs, payout, state,
+			block, receipts, cxReceipts, stakeMsgs, delegationsToRemove, payout, state,
 		)
 		if err != nil {
 			return i, events, coalescedLogs, err
@@ -2687,6 +2688,7 @@ func (bc *BlockChainImpl) writeDelegationsByDelegator(
 func (bc *BlockChainImpl) UpdateStakingMetaData(
 	batch rawdb.DatabaseWriter, block *types.Block,
 	stakeMsgs []staking.StakeMsg,
+	delegationsToRemove map[common.Address][]common.Address,
 	state *state.DB, epoch, newEpoch *big.Int,
 ) (newValidators []common.Address, err error) {
 	newValidators, newDelegations, err := bc.prepareStakingMetaData(block, stakeMsgs, state)
@@ -2741,6 +2743,13 @@ func (bc *BlockChainImpl) UpdateStakingMetaData(
 		if err := bc.writeDelegationsByDelegator(batch, addr, delegations); err != nil {
 			return newValidators, err
 		}
+
+		for delegatorAddress, validatorAddresses := range delegationsToRemove {
+			if err := bc.RemoveDelegationsFromDelegator(batch, delegatorAddress, validatorAddresses); err != nil {
+				return newValidators, err
+			}
+		}
+
 	}
 	return newValidators, nil
 }
@@ -2771,7 +2780,7 @@ func (bc *BlockChainImpl) prepareStakingMetaData(
 				return nil, nil, err
 			}
 		} else {
-			panic("Only *staking.Delegate stakeMsgs are supported at the moment")
+			return nil, nil, errors.New("Only *staking.Delegate stakeMsgs are supported at the moment")
 		}
 	}
 	for _, txn := range block.StakingTransactions() {
@@ -2899,7 +2908,7 @@ func (bc *BlockChainImpl) addDelegationIndex(
 		}
 	}
 
-	// Found the delegation from state and add the delegation index
+	// Find the delegation from state and add the delegation index (the position in validator)
 	// Note this should read from the state of current block in concern
 	wrapper, err := state.ValidatorWrapper(validatorAddress, true, false)
 	if err != nil {
@@ -2915,6 +2924,11 @@ func (bc *BlockChainImpl) addDelegationIndex(
 				Index:            uint64(i),
 				BlockNum:         blockNum,
 			})
+
+			// wrapper.Delegations will not have another delegator
+			// with the same address, so we are done
+			break
+
 		}
 	}
 	return delegations, nil

core/blockchain_stub.go

@@ -124,7 +124,7 @@ func (a Stub) WriteBlockWithoutState(block *types.Block, td *big.Int) (err error
 	return errors.Errorf("method WriteBlockWithoutState not implemented for %s", a.Name)
 }
 
-func (a Stub) WriteBlockWithState(block *types.Block, receipts []*types.Receipt, cxReceipts []*types.CXReceipt, stakeMsgs []staking.StakeMsg, paid reward.Reader, state *state.DB) (status WriteStatus, err error) {
+func (a Stub) WriteBlockWithState(block *types.Block, receipts []*types.Receipt, cxReceipts []*types.CXReceipt, stakeMsgs []staking.StakeMsg, delegationsToRemove map[common.Address][]common.Address, paid reward.Reader, state *state.DB) (status WriteStatus, err error) {
 	return 0, errors.Errorf("method WriteBlockWithState not implemented for %s", a.Name)
 }
 
@@ -384,7 +384,7 @@ func (a Stub) ReadDelegationsByDelegatorAt(delegator common.Address, blockNum *b
 	return nil, errors.Errorf("method ReadDelegationsByDelegatorAt not implemented for %s", a.Name)
 }
 
-func (a Stub) UpdateStakingMetaData(batch rawdb.DatabaseWriter, block *types.Block, stakeMsgs []staking.StakeMsg, state *state.DB, epoch, newEpoch *big.Int) (newValidators []common.Address, err error) {
+func (a Stub) UpdateStakingMetaData(batch rawdb.DatabaseWriter, block *types.Block, stakeMsgs []staking.StakeMsg, delegationsToRemove map[common.Address][]common.Address, state *state.DB, epoch, newEpoch *big.Int) (newValidators []common.Address, err error) {
 	return nil, errors.Errorf("method UpdateStakingMetaData not implemented for %s", a.Name)
 }
 
@@ -423,7 +423,7 @@ func (a Stub) IsEnablePruneBeaconChainFeature() bool {
 	return false
 }
 
-func (a Stub) CommitOffChainData(batch rawdb.DatabaseWriter, block *types.Block, receipts []*types.Receipt, cxReceipts []*types.CXReceipt, stakeMsgs []staking.StakeMsg, payout reward.Reader, state *state.DB) (status WriteStatus, err error) {
+func (a Stub) CommitOffChainData(batch rawdb.DatabaseWriter, block *types.Block, receipts []*types.Receipt, cxReceipts []*types.CXReceipt, stakeMsgs []staking.StakeMsg, delegationsToRemove map[common.Address][]common.Address, payout reward.Reader, state *state.DB) (status WriteStatus, err error) {
 	return 0, errors.Errorf("method CommitOffChainData not implemented for %s", a.Name)
 }
 

core/chain_makers.go

@@ -14,16 +14,17 @@
 // You should have received a copy of the GNU Lesser General Public License
 // along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
 
-package chain
+package core
 
 import (
 	"fmt"
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/ethdb"
 	"github.com/ethereum/go-ethereum/trie"
-	"github.com/harmony-one/harmony/core"
+	"github.com/harmony-one/harmony/internal/params"
 
 	"github.com/harmony-one/harmony/block"
 	blockfactory "github.com/harmony-one/harmony/block/factory"
@@ -32,7 +33,7 @@ import (
 	"github.com/harmony-one/harmony/core/state/snapshot"
 	"github.com/harmony-one/harmony/core/types"
 	"github.com/harmony-one/harmony/core/vm"
-	"github.com/harmony-one/harmony/internal/params"
+
 	"github.com/harmony-one/harmony/shard"
 	staking "github.com/harmony-one/harmony/staking/types"
 )
@@ -104,7 +105,7 @@ func (b *BlockGen) AddTxWithChain(bc core.BlockChain, tx *types.Transaction) {
 	b.statedb.Prepare(tx.Hash(), common.Hash{}, len(b.txs))
 	coinbase := b.header.Coinbase()
 	gasUsed := b.header.GasUsed()
-	receipt, _, _, _, err := core.ApplyTransaction(bc, &coinbase, b.gasPool, b.statedb, b.header, tx, &gasUsed, vm.Config{})
+	receipt, _, _, _, err := ApplyTransaction(bc, &coinbase, b.gasPool, b.statedb, b.header, tx, &gasUsed, vm.Config{})
 	b.header.SetGasUsed(gasUsed)
 	b.header.SetCoinbase(coinbase)
 	if err != nil {
@@ -185,7 +186,7 @@ func GenerateChain(
 
 		if b.engine != nil {
 			// Finalize and seal the block
-			block, _, err := b.engine.Finalize(
+			block, _, _, err := b.engine.Finalize(
 				chainreader, nil, b.header, statedb, b.txs, b.receipts, nil, nil, nil, nil, nil, func() uint64 { return 0 },
 			)
 			if err != nil {
@@ -229,7 +230,7 @@ func makeHeader(chain consensus_engine.ChainReader, parent *block.Header, state
 		Root(state.IntermediateRoot(chain.Config().IsS3(parent.Epoch()))).
 		ParentHash(parent.Hash()).
 		Coinbase(parent.Coinbase()).
-		GasLimit(core.CalcGasLimit(parent, parent.GasLimit(), parent.GasLimit())).
+		GasLimit(core.CalcGasLimit(parent.GasLimit(), parent.GasLimit())).
 		Number(new(big.Int).Add(parent.Number(), common.Big1)).
 		Time(time).
 		Header()

core/evm.go

@@ -243,7 +243,7 @@ func DelegateFn(ref *block.Header, chain ChainContext) vm.DelegateFunc {
 func UndelegateFn(ref *block.Header, chain ChainContext) vm.UndelegateFunc {
 	// moved from state_transition.go to here, with some modifications
 	return func(db vm.StateDB, rosettaTracer vm.RosettaTracer, undelegate *stakingTypes.Undelegate) error {
-		wrapper, err := VerifyAndUndelegateFromMsg(db, ref.Epoch(), undelegate)
+		wrapper, err := VerifyAndUndelegateFromMsg(db, ref.Epoch(), chain.Config(), undelegate)
 		if err != nil {
 			return err
 		}

core/evm_test.go

@@ -32,7 +32,7 @@ func getTestEnvironment(testBankKey ecdsa.PrivateKey) (*BlockChainImpl, *state.D
 	var (
 		testBankAddress = crypto.PubkeyToAddress(testBankKey.PublicKey)
 		testBankFunds   = new(big.Int).Mul(big.NewInt(denominations.One), big.NewInt(40000))
-		chainConfig     = params.TestChainConfig
+		chainConfig     = params.LocalnetChainConfig
 		blockFactory    = blockfactory.ForTest
 		database        = rawdb.NewMemoryDatabase()
 		gspec           = Genesis{
@@ -50,7 +50,7 @@ func getTestEnvironment(testBankKey ecdsa.PrivateKey) (*BlockChainImpl, *state.D
 	chain, _ := NewBlockChain(database, nil, nil, cacheConfig, gspec.Config, engine, vm.Config{})
 	db, _ := chain.StateAt(genesis.Root())
 
-	// make a fake block header (use epoch 1 so that locked tokens can be tested)
+	// make a fake block header
 	header := blockFactory.NewHeader(common.Big0)
 
 	return chain, db, header, database
@@ -119,6 +119,51 @@ func TestEVMStaking(t *testing.T) {
 		t.Errorf("Got error %v in Undelegate", err)
 	}
 
+	// undelegate test - epoch 3 to test NoNilDelegationsEpoch case
+	delegatorKey, _ := crypto.GenerateKey()
+	ctx3 := NewEVMContext(msg, blockfactory.ForTest.NewHeader(common.Big3), chain, nil)
+	delegate = sampleDelegate(*delegatorKey) // 1000 ONE
+	db.AddBalance(delegate.DelegatorAddress, delegate.Amount)
+	delegate.ValidatorAddress = wrapper.Address
+	err = ctx.Delegate(db, nil, &delegate)
+	if err != nil {
+		t.Errorf("Got error %v in Delegate for new delegator", err)
+	}
+	undelegate = sampleUndelegate(*delegatorKey)
+	// try undelegating such that remaining < minimum (100 ONE)
+	undelegate.ValidatorAddress = wrapper.Address
+	undelegate.Amount = new(big.Int).Mul(big.NewInt(denominations.One), big.NewInt(901))
+	err = ctx3.Undelegate(db, nil, &undelegate)
+	if err == nil {
+		t.Errorf("Got no error in Undelegate for new delegator")
+	} else {
+		if err.Error() != "Minimum: 100000000000000000000, Remaining: 99000000000000000000: remaining delegation must be 0 or >= 100 ONE" {
+			t.Errorf("Got error %v but expected %v", err, staking.ErrUndelegationRemaining)
+		}
+	}
+	// now undelegate such that remaining == minimum (100 ONE)
+	undelegate.Amount = new(big.Int).Mul(big.NewInt(denominations.One), big.NewInt(900))
+	err = ctx3.Undelegate(db, nil, &undelegate)
+	if err != nil {
+		t.Errorf("Got error %v in Undelegate for new delegator", err)
+	}
+	// remaining < 100 ONE after remaining = minimum
+	undelegate.Amount = new(big.Int).Mul(big.NewInt(denominations.One), big.NewInt(1))
+	err = ctx3.Undelegate(db, nil, &undelegate)
+	if err == nil {
+		t.Errorf("Got no error in Undelegate for new delegator")
+	} else {
+		if err.Error() != "Minimum: 100000000000000000000, Remaining: 99000000000000000000: remaining delegation must be 0 or >= 100 ONE" {
+			t.Errorf("Got error %v but expected %v", err, staking.ErrUndelegationRemaining)
+		}
+	}
+	// remaining == 0
+	undelegate.Amount = new(big.Int).Mul(big.NewInt(denominations.One), big.NewInt(100))
+	err = ctx3.Undelegate(db, nil, &undelegate)
+	if err != nil {
+		t.Errorf("Got error %v in Undelegate for new delegator", err)
+	}
+
 	// collectRewards test
 	collectRewards := sampleCollectRewards(*key)
 	// add block rewards to make sure there are some to collect

core/offchain.go

@@ -28,6 +28,7 @@ func (bc *BlockChainImpl) CommitOffChainData(
 	receipts []*types.Receipt,
 	cxReceipts []*types.CXReceipt,
 	stakeMsgs []staking.StakeMsg,
+	delegationsToRemove map[common.Address][]common.Address,
 	payout reward.Reader,
 	state *state.DB,
 ) (status WriteStatus, err error) {
@@ -118,7 +119,7 @@ func (bc *BlockChainImpl) CommitOffChainData(
 
 	// Do bookkeeping for new staking txns
 	newVals, err := bc.UpdateStakingMetaData(
-		batch, block, stakeMsgs, state, epoch, nextBlockEpoch,
+		batch, block, stakeMsgs, delegationsToRemove, state, epoch, nextBlockEpoch,
 	)
 	if err != nil {
 		utils.Logger().Err(err).Msg("UpdateStakingMetaData failed")
@@ -327,3 +328,33 @@ func (bc *BlockChainImpl) getNextBlockEpoch(header *block.Header) (*big.Int, err
 	}
 	return nextBlockEpoch, nil
 }
+
+func (bc *BlockChainImpl) RemoveDelegationsFromDelegator(
+	batch rawdb.DatabaseWriter,
+	delegatorAddress common.Address,
+	validatorAddresses []common.Address,
+) error {
+	delegationIndexes, err := bc.ReadDelegationsByDelegator(delegatorAddress)
+	if err != nil {
+		return err
+	}
+	finalDelegationIndexes := delegationIndexes[:0]
+	for _, validatorAddress := range validatorAddresses {
+		// TODO: can this be sped up from O(vd) to something shorter?
+		for _, delegationIndex := range delegationIndexes {
+			if bytes.Equal(
+				validatorAddress.Bytes(),
+				delegationIndex.ValidatorAddress.Bytes(),
+			) {
+				// do nothing
+				break
+			}
+			finalDelegationIndexes = append(
+				finalDelegationIndexes,
+				delegationIndex,
+			)
+		}
+	}
+	bc.writeDelegationsByDelegator(batch, delegatorAddress, finalDelegationIndexes)
+	return nil
+}

core/preimages.go

@@ -226,7 +226,7 @@ func GeneratePreimages(chain BlockChain, start, end uint64) error {
 			return fmt.Errorf("block %d not found", i)
 		}
 		stateAt, _ := chain.StateAt(block.Root())
-		_, _, _, _, _, _, endingState, errProcess = chain.Processor().Process(block, startingState, *chain.GetVMConfig(), false)
+		_, _, _, _, _, _, _, endingState, errProcess = chain.Processor().Process(block, startingState, *chain.GetVMConfig(), false)
 		if errProcess != nil {
 			return fmt.Errorf("error executing block #%d: %s", i, errProcess)
 		}

core/staking_verifier.go

@@ -367,7 +367,7 @@ func VerifyAndDelegateFromMsg(
 //
 // Note that this function never updates the stateDB, it only reads from stateDB.
 func VerifyAndUndelegateFromMsg(
-	stateDB vm.StateDB, epoch *big.Int, msg *staking.Undelegate,
+	stateDB vm.StateDB, epoch *big.Int, chainConfig *params.ChainConfig, msg *staking.Undelegate,
 ) (*staking.ValidatorWrapper, error) {
 	if stateDB == nil {
 		return nil, errStateDBIsMissing
@@ -389,10 +389,15 @@ func VerifyAndUndelegateFromMsg(
 		return nil, err
 	}
 
+	var minimumRemainingDelegation *big.Int
+	if chainConfig.IsNoNilDelegations(epoch) {
+		minimumRemainingDelegation = minimumDelegationV2 // 100 ONE
+	}
+
 	for i := range wrapper.Delegations {
 		delegation := &wrapper.Delegations[i]
 		if bytes.Equal(delegation.DelegatorAddress.Bytes(), msg.DelegatorAddress.Bytes()) {
-			if err := delegation.Undelegate(epoch, msg.Amount); err != nil {
+			if err := delegation.Undelegate(epoch, msg.Amount, minimumRemainingDelegation); err != nil {
 				return nil, err
 			}
 			if err := wrapper.SanityCheck(); err != nil {

core/staking_verifier_test.go

@@ -1196,6 +1196,7 @@ func TestVerifyAndUndelegateFromMsg(t *testing.T) {
 
 		expVWrapper           staking.ValidatorWrapper
 		expErr                error
+		noNilDelegationsEpoch *big.Int
 	}{
 		{
 			// 0: Undelegate at delegation with an entry already exist at the same epoch.
@@ -1362,9 +1363,68 @@ func TestVerifyAndUndelegateFromMsg(t *testing.T) {
 
 			expErr: errNoDelegationToUndelegate,
 		},
+
+		{
+			// 12: Undelegate with NoNilDelegationsEpoch set
+			// such that remaining < minimum
+			sdb:   makeDefaultStateForUndelegate(t), // delegatorAddr has 15k ones delegated
+			epoch: big.NewInt(defaultEpoch),
+			msg: func() staking.Undelegate {
+				msg := defaultMsgUndelegate()
+				msg.Amount = new(big.Int).Mul(oneBig, big.NewInt(14901))
+				return msg
+			}(),
+			expVWrapper:           makeDefaultSnapVWrapperForUndelegate(t),
+			expErr:                staking.ErrUndelegationRemaining,
+			noNilDelegationsEpoch: big.NewInt(defaultEpoch),
+		},
+		{
+			// 13: Undelegate with NoNilDelegationsEpoch set
+			// such that remaining == minimum
+			// delegatorAddr has 15k ones delegated and 5k in Undelegations at defaultEpoch
+			sdb:   makeDefaultStateForUndelegate(t),
+			epoch: big.NewInt(defaultEpoch),
+			msg: func() staking.Undelegate {
+				msg := defaultMsgUndelegate()
+				msg.Amount = new(big.Int).Mul(oneBig, big.NewInt(14900))
+				return msg
+			}(),
+			expVWrapper: func(t *testing.T) staking.ValidatorWrapper {
+				w := makeDefaultSnapVWrapperForUndelegate(t)
+				w.Delegations[1].Amount = new(big.Int).Mul(oneBig, big.NewInt(100))
+				w.Delegations[1].Undelegations[0].Amount = new(big.Int).Mul(oneBig, big.NewInt(19900))
+				return w
+			}(t),
+			noNilDelegationsEpoch: big.NewInt(defaultEpoch),
+		},
+		{
+			// 14: Undelegate with NoNilDelegationsEpoch set
+			// such that remaining == 0
+			// delegatorAddr has 15k ones delegated and 5k in Undelegations at defaultEpoch
+			sdb:   makeDefaultStateForUndelegate(t),
+			epoch: big.NewInt(defaultEpoch),
+			msg: func() staking.Undelegate {
+				msg := defaultMsgUndelegate()
+				msg.Amount = fifteenKOnes
+				return msg
+			}(),
+			expVWrapper: func(t *testing.T) staking.ValidatorWrapper {
+				w := makeDefaultSnapVWrapperForUndelegate(t)
+				w.Delegations[1].Amount = common.Big0
+				w.Delegations[1].Undelegations[0].Amount = twentyKOnes
+				return w
+			}(t),
+			noNilDelegationsEpoch: big.NewInt(defaultEpoch),
+		},
 	}
 	for i, test := range tests {
-		w, err := VerifyAndUndelegateFromMsg(test.sdb, test.epoch, &test.msg)
+		config := &params.ChainConfig{}
+		if test.noNilDelegationsEpoch != nil {
+			config.NoNilDelegationsEpoch = test.noNilDelegationsEpoch
+		} else {
+			config.NoNilDelegationsEpoch = big.NewInt(10000000) // EpochTBD
+		}
+		w, err := VerifyAndUndelegateFromMsg(test.sdb, test.epoch, config, &test.msg)
 
 		if assErr := assertError(err, test.expErr); assErr != nil {
 			t.Errorf("Test %v: %v", i, assErr)
@@ -1383,7 +1443,7 @@ func makeDefaultSnapVWrapperForUndelegate(t *testing.T) staking.ValidatorWrapper
 	w := makeVWrapperByIndex(validatorIndex)
 
 	newDelegation := staking.NewDelegation(delegatorAddr, new(big.Int).Set(twentyKOnes))
-	if err := newDelegation.Undelegate(big.NewInt(defaultEpoch), fiveKOnes); err != nil {
+	if err := newDelegation.Undelegate(big.NewInt(defaultEpoch), fiveKOnes, nil); err != nil {
 		t.Fatal(err)
 	}
 	w.Delegations = append(w.Delegations, newDelegation)

core/state/statedb.go

@@ -18,6 +18,7 @@
 package state
 
 import (
+	"bytes"
 	"fmt"
 	"math/big"
 	"sort"
@@ -1234,7 +1235,7 @@ func (db *DB) ValidatorWrapper(
 ) (*stk.ValidatorWrapper, error) {
 	// if cannot revert and ask for a copy
 	if sendOriginal && copyDelegations {
-		panic("'Cannot revert' must not expect copy of delegations")
+		return nil, errors.New("'sendOriginal' must not expect copy of delegations")
 	}
 
 	// Read cache first
@@ -1368,6 +1369,7 @@ func (db *DB) AddReward(
 	snapshot *stk.ValidatorWrapper,
 	reward *big.Int,
 	shareLookup map[common.Address]numeric.Dec,
+	nilDelegationsRemoved bool,
 ) error {
 	if reward.Cmp(common.Big0) == 0 {
 		utils.Logger().Info().RawJSON("validator", []byte(snapshot.String())).
@@ -1375,6 +1377,10 @@ func (db *DB) AddReward(
 		return nil
 	}
 
+	if len(snapshot.Delegations) != len(shareLookup) {
+		return errors.New("[AddReward] Snapshot and shareLookup mismatch")
+	}
+
 	curValidator, err := db.ValidatorWrapper(snapshot.Address, true, false)
 	if err != nil {
 		return errors.Wrapf(err, "failed to distribute rewards: validator does not exist")
@@ -1399,8 +1405,14 @@ func (db *DB) AddReward(
 		rewardPool.Sub(rewardPool, commissionInt)
 	}
 
+	// no sanity check for length of delegations between curValidator and snapshot
+	// if a delegation happens, len(curValidator) > len(snapshot)
+	// if it doesn't happen, curValidator == snapshot
+	// if there are only removals, curValidator < snapshot
+
 	// Payout each delegator's reward pro-rata
 	totalRewardForDelegators := big.NewInt(0).Set(rewardPool)
+	if !nilDelegationsRemoved {
 		for i := range snapshot.Delegations {
 			delegation := snapshot.Delegations[i]
 			percentage, ok := shareLookup[delegation.DelegatorAddress]
@@ -1411,12 +1423,67 @@ func (db *DB) AddReward(
 
 			rewardInt := percentage.MulInt(totalRewardForDelegators).RoundInt()
 			curDelegation := curValidator.Delegations[i]
+			// should we check that curDelegation.DelegatorAddress == delegation.DelegatorAddress ?
+			// wasn't there originally so I leave it for now
 			curDelegation.Reward.Add(curDelegation.Reward, rewardInt)
 			rewardPool.Sub(rewardPool, rewardInt)
 		}
+	} else {
+		// iterate simply over snapshot delegations
+		// those added later to curValidator are new delegations which do not receive rewards immediately
+		// those removed from curValidator are stale delegations which do not receive rewards anyway
+		offset := 0
+		for i := 0; i < len(snapshot.Delegations); i++ {
+			delegationFromSnapshot := snapshot.Delegations[i]
+			percentage, ok := shareLookup[delegationFromSnapshot.DelegatorAddress]
+			if !ok {
+				return errors.Wrapf(err, "missing delegation shares for reward distribution")
+			}
+			if percentage.IsZero() { // stale delegation
+
+				offset++
+				continue
+			}
+			// try to find in wrapper
+			// this is O(N) even though order is not guaranteed
+			// for example, snapshot is A / B / C / D / E where C is a stale delegation
+			// wrapper then becomes A / B / D / E
+			// if C then delegates to this validator, the wrapper becomes A / B / D / E / C
+			// (1) snapshot = A / B / C / D / E and wrapper  = A / B / D / E / C (remove and re-add)
+			// for i in [0, 1] j = i - 0 (offset) works well
+			// for i = 2, offset becomes 1
+			// for i in [3, 4] use j = i - 1 (offset)
+			// other cases are
+			// (1) snapshot = A / B / C / D and wrapper = A / C / D (just remove B)
+			// (2) snapshot = A / B / C / D and wrapper = A / C / D / E (remove B and add E)
+			// (3) snapshot = A / B / C / D and wrapper = A / B / C / D / E (just add E)
+			// (4) snapshot and wrapper equal (no effort needed)
+			//		 even if a stale delegation is removed from the end and re-added this works
+			// (5) snapshot = A / B / C / D / E and wrapper = A / B / D / E / C / F (remove and re-add + add)
+
+			found := false
+			for j := i - offset; j < len(curValidator.Delegations) && !found; j++ {
+				delegationFromWrapper := curValidator.Delegations[j]
+				if bytes.Equal(
+					delegationFromWrapper.DelegatorAddress.Bytes(),
+					delegationFromSnapshot.DelegatorAddress.Bytes(),
+				) {
+					found = true
+					rewardInt := percentage.MulInt(totalRewardForDelegators).RoundInt()
+					delegationFromWrapper.Reward.Add(delegationFromWrapper.Reward, rewardInt)
+					rewardPool.Sub(rewardPool, rewardInt)
+				}
+			}
+			// delegation in snapshot with non zero reward but not in wrapper
+			if !found {
+				return errors.New("Non-zero reward found in snapshot but delegation missing in wrapper")
+			}
+		}
+	}
 
 	// The last remaining bit belongs to the validator (remember the validator's self delegation is
-	// always at index 0)
+	// always at index 0). We do not allow validator deletions (yet?) so a validator's
+	// self stake is never deleted even if otherwise stale
 	if rewardPool.Cmp(common.Big0) > 0 {
 		curValidator.Delegations[0].Reward.Add(curValidator.Delegations[0].Reward, rewardPool)
 	}

core/state_processor.go

@@ -71,6 +71,7 @@ type ProcessorResult struct {
 	UsedGas             uint64
 	Reward              reward.Reader
 	State               *state.DB
+	DelegationsToRemove map[common.Address][]common.Address
 }
 
 // NewStateProcessor initialises a new StateProcessor.
@@ -104,6 +105,7 @@ func (p *StateProcessor) Process(
 	block *types.Block, statedb *state.DB, cfg vm.Config, readCache bool,
 ) (
 	types.Receipts, types.CXReceipts, []staking.StakeMsg,
+	map[common.Address][]common.Address,
 	[]*types.Log, UsedGas, reward.Reader, *state.DB, error,
 ) {
 	cacheKey := block.Hash()
@@ -113,7 +115,7 @@ func (p *StateProcessor) Process(
 			// Only the successful results are cached in case for retry.
 			result := cached.(*ProcessorResult)
 			utils.Logger().Info().Str("block num", block.Number().String()).Msg("result cache hit.")
-			return result.Receipts, result.CxReceipts, result.StakeMsgs, result.Logs, result.UsedGas, result.Reward, result.State, nil
+			return result.Receipts, result.CxReceipts, result.StakeMsgs, result.DelegationsToRemove, result.Logs, result.UsedGas, result.Reward, result.State, nil
 		}
 	}
 
@@ -130,7 +132,7 @@ func (p *StateProcessor) Process(
 
 	beneficiary, err := p.bc.GetECDSAFromCoinbase(header)
 	if err != nil {
-		return nil, nil, nil, nil, 0, nil, statedb, err
+		return nil, nil, nil, nil, nil, 0, nil, statedb, err
 	}
 
 	processTxsAndStxs := true
@@ -141,7 +143,7 @@ func (p *StateProcessor) Process(
 			processTxsAndStxs = false
 		}
 		if !errors.Is(err, ErrNoMigrationRequired) && !errors.Is(err, ErrNoMigrationPossible) {
-			return nil, nil, nil, nil, 0, nil, statedb, err
+			return nil, nil, nil, nil, nil, 0, nil, statedb, err
 		}
 	} else {
 		if cxReceipt != nil {
@@ -160,7 +162,7 @@ func (p *StateProcessor) Process(
 				p.bc, &beneficiary, gp, statedb, header, tx, usedGas, cfg,
 			)
 			if err != nil {
-				return nil, nil, nil, nil, 0, nil, statedb, err
+				return nil, nil, nil, nil, nil, 0, nil, statedb, err
 			}
 			receipts = append(receipts, receipt)
 			if cxReceipt != nil {
@@ -183,7 +185,7 @@ func (p *StateProcessor) Process(
 				p.bc, &beneficiary, gp, statedb, header, tx, usedGas, cfg,
 			)
 			if err != nil {
-				return nil, nil, nil, nil, 0, nil, statedb, err
+				return nil, nil, nil, nil, nil, 0, nil, statedb, err
 			}
 			receipts = append(receipts, receipt)
 			allLogs = append(allLogs, receipt.Logs...)
@@ -196,7 +198,7 @@ func (p *StateProcessor) Process(
 		if err := ApplyIncomingReceipt(
 			p.bc.Config(), statedb, header, cx,
 		); err != nil {
-			return nil, nil,
+			return nil, nil, nil,
 				nil, nil, 0, nil, statedb, errors.New("[Process] Cannot apply incoming receipts")
 		}
 	}
@@ -204,14 +206,13 @@ func (p *StateProcessor) Process(
 	slashes := slash.Records{}
 	if s := header.Slashes(); len(s) > 0 {
 		if err := rlp.DecodeBytes(s, &slashes); err != nil {
-			return nil, nil, nil, nil, 0, nil, statedb, errors.New(
-				"[Process] Cannot finalize block",
-			)
+			return nil, nil, nil, nil, nil, 0, nil, statedb, errors.Wrap(err,
+				"[Process] Cannot finalize block")
 		}
 	}
 
 	if err := MayShardReduction(p.bc, statedb, header); err != nil {
-		return nil, nil, nil, nil, 0, nil, statedb, err
+		return nil, nil, nil, nil, nil, 0, nil, statedb, err
 	}
 
 	// Finalize the block, applying any consensus engine specific extras (e.g. block rewards)
@@ -220,14 +221,14 @@ func (p *StateProcessor) Process(
 		// Block processing don't need to block on reward computation as in block proposal
 		sigsReady <- true
 	}()
-	_, payout, err := p.bc.Engine().Finalize(
+	_, delegationsToRemove, payout, err := p.bc.Engine().Finalize(
 		p.bc,
 		p.beacon,
 		header, statedb, block.Transactions(),
 		receipts, outcxs, incxs, block.StakingTransactions(), slashes, sigsReady, func() uint64 { return header.ViewID().Uint64() },
 	)
 	if err != nil {
-		return nil, nil, nil, nil, 0, nil, statedb, errors.WithMessage(err, "[Process] Cannot finalize block")
+		return nil, nil, nil, nil, nil, 0, nil, statedb, errors.WithMessage(err, "[Process] Cannot finalize block")
 	}
 
 	result := &ProcessorResult{
@@ -238,9 +239,10 @@ func (p *StateProcessor) Process(
 		UsedGas:             *usedGas,
 		Reward:              payout,
 		State:               statedb,
+		DelegationsToRemove: delegationsToRemove,
 	}
 	p.resultCache.Add(cacheKey, result)
-	return receipts, outcxs, blockStakeMsgs, allLogs, *usedGas, payout, statedb, nil
+	return receipts, outcxs, blockStakeMsgs, delegationsToRemove, allLogs, *usedGas, payout, statedb, nil
 }
 
 // CacheProcessorResult caches the process result on the cache key.

core/tx_pool.go

@@ -933,7 +933,7 @@ func (pool *TxPool) validateStakingTx(tx *staking.StakingTransaction) error {
 			return errors.WithMessagef(ErrInvalidSender, "staking transaction sender is %s", b32)
 		}
 
-		_, err = VerifyAndUndelegateFromMsg(pool.currentState, pool.pendingEpoch(), stkMsg)
+		_, err = VerifyAndUndelegateFromMsg(pool.currentState, pool.pendingEpoch(), pool.chainconfig, stkMsg)
 		return err
 	case staking.DirectiveCollectRewards:
 		msg, err := staking.RLPDecodeStakeMsg(tx.Data(), staking.DirectiveCollectRewards)

core/types.go

@@ -17,6 +17,7 @@
 package core
 
 import (
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/harmony-one/harmony/consensus/reward"
 	"github.com/harmony-one/harmony/core/state"
 	"github.com/harmony-one/harmony/core/types"
@@ -55,6 +56,7 @@ type Validator interface {
 type Processor interface {
 	Process(block *types.Block, statedb *state.DB, cfg vm.Config, readCache bool) (
 		types.Receipts, types.CXReceipts, []stakingTypes.StakeMsg,
+		map[common.Address][]common.Address,
 		[]*types.Log, uint64, reward.Reader, *state.DB, error,
 	)
 	CacheProcessorResult(cacheKey interface{}, result *ProcessorResult)

core/vm/interface.go

@@ -49,8 +49,7 @@ type StateDB interface {
 	UnsetValidatorFlag(common.Address)
 	IsValidator(common.Address) bool
 	GetValidatorFirstElectionEpoch(addr common.Address) *big.Int
-	AddReward(*staking.ValidatorWrapper, *big.Int, map[common.Address]numeric.Dec) error
-
+	AddReward(*staking.ValidatorWrapper, *big.Int, map[common.Address]numeric.Dec, bool) error
 	AddRefund(uint64)
 	SubRefund(uint64)
 	GetRefund() uint64

go.mod

@@ -68,7 +68,6 @@ require (
 
 require (
 	github.com/c2h5oh/datasize v0.0.0-20220606134207-859f65c6625b
-	github.com/grafana/pyroscope-go v1.0.4
 	github.com/holiman/bloomfilter/v2 v2.0.3
 	github.com/ledgerwatch/erigon-lib v0.0.0-20230607152933-42c9c28cac68
 	github.com/ledgerwatch/log/v3 v3.8.0
@@ -148,7 +147,6 @@ require (
 	github.com/google/pprof v0.0.0-20230405160723-4a4c7d95572b // indirect
 	github.com/google/uuid v1.3.0 // indirect
 	github.com/gostaticanalysis/analysisutil v0.0.0-20190318220348-4088753ea4d3 // indirect
-	github.com/grafana/pyroscope-go/godeltaprof v0.1.4 // indirect
 	github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-multierror v1.1.1 // indirect

go.sum

@@ -632,10 +632,6 @@ github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWm
 github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
 github.com/gostaticanalysis/analysisutil v0.0.0-20190318220348-4088753ea4d3 h1:JVnpOZS+qxli+rgVl98ILOXVNbW+kb5wcxeGx8ShUIw=
 github.com/gostaticanalysis/analysisutil v0.0.0-20190318220348-4088753ea4d3/go.mod h1:eEOZF4jCKGi+aprrirO9e7WKB3beBRtWgqGunKl6pKE=
-github.com/grafana/pyroscope-go v1.0.4 h1:oyQX0BOkL+iARXzHuCdIF5TQ7/sRSel1YFViMHC7Bm0=
-github.com/grafana/pyroscope-go v1.0.4/go.mod h1:0d7ftwSMBV/Awm7CCiYmHQEG8Y44Ma3YSjt+nWcWztY=
-github.com/grafana/pyroscope-go/godeltaprof v0.1.4 h1:mDsJ3ngul7UfrHibGQpV66PbZ3q1T8glz/tK3bQKKEk=
-github.com/grafana/pyroscope-go/godeltaprof v0.1.4/go.mod h1:1HSPtjU8vLG0jE9JrTdzjgFqdJ/VgN7fvxBNq3luJko=
 github.com/graph-gophers/graphql-go v1.3.0/go.mod h1:9CQHMSxwO4MprSdzoIEobiHpoLtHm77vfxsvsIN5Vuc=
 github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA=
 github.com/grpc-ecosystem/go-grpc-middleware v1.0.0/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs=

hmy/downloader/adapter_test.go

@@ -160,8 +160,8 @@ func (e *dummyEngine) Finalize(
 	receipts []*types.Receipt, outcxs []*types.CXReceipt,
 	incxs []*types.CXReceiptsProof, stks staking.StakingTransactions,
 	doubleSigners slash.Records, sigsReady chan bool, viewID func() uint64,
-) (*types.Block, reward.Reader, error) {
-	return nil, nil, nil
+) (*types.Block, map[common.Address][]common.Address, reward.Reader, error) {
+	return nil, nil, nil, nil
 }
 
 type testInsertHelper struct {

hmy/tracer.go

@@ -281,7 +281,7 @@ func (hmy *Harmony) TraceChain(ctx context.Context, start, end *types.Block, con
 				traced += uint64(len(txs))
 			}
 			// Generate the next state snapshot fast without tracing
-			_, _, _, _, _, _, _, err := hmy.BlockChain.Processor().Process(block, statedb, vm.Config{}, false)
+			_, _, _, _, _, _, _, _, err := hmy.BlockChain.Processor().Process(block, statedb, vm.Config{}, false)
 			if err != nil {
 				failed = err
 				break
@@ -676,7 +676,7 @@ func (hmy *Harmony) ComputeStateDB(block *types.Block, reexec uint64) (*state.DB
 		if block = hmy.BlockChain.GetBlockByNumber(block.NumberU64() + 1); block == nil {
 			return nil, fmt.Errorf("block #%d not found", block.NumberU64()+1)
 		}
-		_, _, _, _, _, _, _, err := hmy.BlockChain.Processor().Process(block, statedb, vm.Config{}, false)
+		_, _, _, _, _, _, _, _, err := hmy.BlockChain.Processor().Process(block, statedb, vm.Config{}, false)
 		if err != nil {
 			return nil, fmt.Errorf("processing block %d failed: %v", block.NumberU64(), err)
 		}

internal/chain/engine.go

@@ -269,7 +269,7 @@ func (e *engineImpl) Finalize(
 	receipts []*types.Receipt, outcxs []*types.CXReceipt,
 	incxs []*types.CXReceiptsProof, stks staking.StakingTransactions,
 	doubleSigners slash.Records, sigsReady chan bool, viewID func() uint64,
-) (*types.Block, reward.Reader, error) {
+) (*types.Block, map[common.Address][]common.Address, reward.Reader, error) {
 
 	isBeaconChain := header.ShardID() == shard.BeaconChainShardID
 	inStakingEra := chain.Config().IsStaking(header.Epoch())
@@ -279,22 +279,22 @@ func (e *engineImpl) Finalize(
 	if IsCommitteeSelectionBlock(chain, header) {
 		startTime := time.Now()
 		if err := payoutUndelegations(chain, header, state); err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
-		utils.Logger().Debug().Int64("elapsed time", time.Now().Sub(startTime).Milliseconds()).Msg("PayoutUndelegations")
+		utils.Logger().Debug().Int64("elapsed time", time.Since(startTime).Milliseconds()).Msg("PayoutUndelegations")
 
 		// Needs to be after payoutUndelegations because payoutUndelegations
 		// depends on the old LastEpochInCommittee
 
 		startTime = time.Now()
 		if err := setElectionEpochAndMinFee(chain, header, state, chain.Config()); err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
-		utils.Logger().Debug().Int64("elapsed time", time.Now().Sub(startTime).Milliseconds()).Msg("SetElectionEpochAndMinFee")
+		utils.Logger().Debug().Int64("elapsed time", time.Since(startTime).Milliseconds()).Msg("SetElectionEpochAndMinFee")
 
 		curShardState, err := chain.ReadShardState(chain.CurrentBlock().Epoch())
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
 		startTime = time.Now()
 		// Needs to be before AccumulateRewardsAndCountSigs because
@@ -305,7 +305,7 @@ func (e *engineImpl) Finalize(
 			if err := availability.ComputeAndMutateEPOSStatus(
 				chain, state, addr,
 			); err != nil {
-				return nil, nil, err
+				return nil, nil, nil, err
 			}
 		}
 		utils.Logger().Debug().Int64("elapsed time", time.Now().Sub(startTime).Milliseconds()).Msg("ComputeAndMutateEPOSStatus")
@@ -317,16 +317,16 @@ func (e *engineImpl) Finalize(
 		chain, state, header, beacon, sigsReady,
 	)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 
 	// Apply slashes
 	if isBeaconChain && inStakingEra && len(doubleSigners) > 0 {
 		if err := applySlashes(chain, header, state, doubleSigners); err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
 	} else if len(doubleSigners) > 0 {
-		return nil, nil, errors.New("slashes proposed in non-beacon chain or non-staking epoch")
+		return nil, nil, nil, errors.New("slashes proposed in non-beacon chain or non-staking epoch")
 	}
 
 	// ViewID setting needs to happen after commig sig reward logic for pipelining reason.
@@ -350,9 +350,36 @@ func (e *engineImpl) Finalize(
 			remainderOne,
 		)
 	}
+
+	// **** clear stale delegations
+	// must be done after slashing and paying out rewards
+	// to avoid conflicts with snapshots
+	// any nil (un)delegations are eligible to be removed
+	// bulk pruning happens at the first block of the NoNilDelegationsEpoch + 1
+	// and each epoch thereafter
+	delegationsToRemove := make(map[common.Address][]common.Address, 0)
+	if shouldPruneStaleStakingData(chain, header) {
+		startTime := time.Now()
+		// this will modify wrappers in-situ, which will be used by UpdateValidatorSnapshots
+		// which occurs in the next epoch at the second to last block
+		err = pruneStaleStakingData(chain, header, state, delegationsToRemove)
+		if err != nil {
+			utils.Logger().Error().AnErr("err", err).
+				Uint64("blockNum", header.Number().Uint64()).
+				Uint64("epoch", header.Epoch().Uint64()).
+				Msg("pruneStaleStakingData error")
+			return nil, nil, nil, err
+		}
+		utils.Logger().Info().
+			Int64("elapsed time", time.Since(startTime).Milliseconds()).
+			Uint64("blockNum", header.Number().Uint64()).
+			Uint64("epoch", header.Epoch().Uint64()).
+			Msg("pruneStaleStakingData")
+	}
+
 	// Finalize the state root
 	header.SetRoot(state.IntermediateRoot(chain.Config().IsS3(header.Epoch())))
-	return types.NewBlock(header, txs, receipts, outcxs, incxs, stks), payout, nil
+	return types.NewBlock(header, txs, receipts, outcxs, incxs, stks), delegationsToRemove, payout, nil
 }
 
 // Withdraw unlocked tokens to the delegators' accounts
@@ -409,6 +436,86 @@ func IsCommitteeSelectionBlock(chain engine.ChainReader, header *block.Header) b
 	return isBeaconChain && header.IsLastBlockInEpoch() && inPreStakingEra
 }
 
+// shouldPruneStaleStakingData checks that all of the following are true
+// (1) we are in the beacon chain
+// (2) it is the first block of the epoch
+// (3) the chain is hard forked to no nil delegations epoch
+func shouldPruneStaleStakingData(
+	chain engine.ChainReader,
+	header *block.Header,
+) bool {
+	firstBlockInEpoch := false
+	// if not first epoch
+	if header.Epoch().Cmp(common.Big0) > 0 {
+		// calculate the last block of prior epoch
+		targetEpoch := new(big.Int).Sub(header.Epoch(), common.Big1)
+		lastBlockNumber := shard.Schedule.EpochLastBlock(targetEpoch.Uint64())
+		// add 1 to it
+		firstBlockInEpoch = header.Number().Uint64() == lastBlockNumber+1
+	} else {
+		// otherwise gensis block
+		firstBlockInEpoch = header.Number().Cmp(common.Big0) == 0
+	}
+	return header.ShardID() == shard.BeaconChainShardID &&
+		firstBlockInEpoch &&
+		chain.Config().IsNoNilDelegations(header.Epoch())
+}
+
+// pruneStaleStakingData prunes any stale staking data
+// must be called only if shouldPruneStaleStakingData is true
+// here and not in staking package to avoid import cycle for state
+func pruneStaleStakingData(
+	chain engine.ChainReader,
+	header *block.Header,
+	state *state.DB,
+	delegationsToRemove map[common.Address][]common.Address,
+) error {
+	validators, err := chain.ReadValidatorList()
+	if err != nil {
+		return err
+	}
+	for _, validator := range validators {
+		wrapper, err := state.ValidatorWrapper(validator, true, false)
+		if err != nil {
+			return errors.New(
+				"[pruneStaleStakingData] failed to get validator from state to finalize",
+			)
+		}
+		delegationsFinal := wrapper.Delegations[:0]
+		for i := range wrapper.Delegations {
+			delegation := &wrapper.Delegations[i]
+			shouldRemove := i != 0 && // never remove the (inactive) validator
+				len(delegation.Undelegations) == 0 &&
+				delegation.Amount.Cmp(common.Big0) == 0 &&
+				delegation.Reward.Cmp(common.Big0) == 0
+			if !shouldRemove {
+				// append it to final delegations
+				delegationsFinal = append(
+					delegationsFinal,
+					*delegation,
+				)
+			} else {
+				// in this delegator's delegationIndexes, remove the one which has this validatorAddress
+				// since a validatorAddress is enough information to uniquely identify the delegationIndex
+				delegationsToRemove[delegation.DelegatorAddress] = append(
+					delegationsToRemove[delegation.DelegatorAddress],
+					wrapper.Address,
+				)
+			}
+		}
+		if len(wrapper.Delegations) != len(delegationsFinal) {
+			utils.Logger().Info().
+				Str("ValidatorAddress", wrapper.Address.Hex()).
+				Uint64("epoch", header.Epoch().Uint64()).
+				Int("count", len(wrapper.Delegations)-len(delegationsFinal)).
+				Msg("pruneStaleStakingData pruned count")
+		}
+		// now re-assign the delegations
+		wrapper.Delegations = delegationsFinal
+	}
+	return nil
+}
+
 func setElectionEpochAndMinFee(chain engine.ChainReader, header *block.Header, state *state.DB, config *params.ChainConfig) error {
 	newShardState, err := header.GetShardState()
 	if err != nil {

internal/chain/engine_test.go

@@ -1,29 +1,38 @@
 package chain
 
 import (
+	"bytes"
 	"fmt"
 	"math/big"
 	"testing"
 
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/trie"
 	bls_core "github.com/harmony-one/bls/ffi/go/bls"
 	"github.com/harmony-one/harmony/block"
 	blockfactory "github.com/harmony-one/harmony/block/factory"
+	"github.com/harmony-one/harmony/common/denominations"
 	"github.com/harmony-one/harmony/consensus/engine"
 	consensus_sig "github.com/harmony-one/harmony/consensus/signature"
+
 	"github.com/harmony-one/harmony/crypto/bls"
+	"github.com/harmony-one/harmony/crypto/hash"
+
 	"github.com/harmony-one/harmony/numeric"
 	"github.com/harmony-one/harmony/shard"
 	"github.com/harmony-one/harmony/staking/effective"
 	"github.com/harmony-one/harmony/staking/slash"
 	staking "github.com/harmony-one/harmony/staking/types"
 	types2 "github.com/harmony-one/harmony/staking/types"
+	staketest "github.com/harmony-one/harmony/staking/types/test"
 
-	"github.com/ethereum/go-ethereum/common"
+	"github.com/harmony-one/harmony/core"
 	"github.com/harmony-one/harmony/core/rawdb"
 	"github.com/harmony-one/harmony/core/state"
 	"github.com/harmony-one/harmony/core/state/snapshot"
 	"github.com/harmony-one/harmony/core/types"
+	"github.com/harmony-one/harmony/core/vm"
+
 	"github.com/harmony-one/harmony/internal/params"
 )
 
@@ -35,6 +44,10 @@ var (
 	thousandKOnes = new(big.Int).Mul(big.NewInt(1000000), bigOne)
 )
 
+type fakeReader struct {
+	core.FakeChainReader
+}
+
 const (
 	// validator creation parameters
 	doubleSignShardID     = 0
@@ -305,6 +318,90 @@ func makeFakeBlockChain() *fakeBlockChain {
 	}
 }
 
+func makeTestAddr(item interface{}) common.Address {
+	s := fmt.Sprintf("harmony-one-%v", item)
+	return common.BytesToAddress([]byte(s))
+}
+
+var (
+	validator1 = makeTestAddr("validator1")
+	validator2 = makeTestAddr("validator2")
+	delegator1 = makeTestAddr("delegator1")
+	delegator2 = makeTestAddr("delegator2")
+	delegator3 = makeTestAddr("delegator3")
+)
+
+var (
+	defaultDesc = staking.Description{
+		Name:            "SuperHero",
+		Identity:        "YouWouldNotKnow",
+		Website:         "Secret Website",
+		SecurityContact: "LicenseToKill",
+		Details:         "blah blah blah",
+	}
+
+	defaultCommissionRates = staking.CommissionRates{
+		Rate:          numeric.NewDecWithPrec(1, 1),
+		MaxRate:       numeric.NewDecWithPrec(9, 1),
+		MaxChangeRate: numeric.NewDecWithPrec(5, 1),
+	}
+)
+
+func (cr *fakeReader) ReadValidatorList() ([]common.Address, error) {
+	return []common.Address{validator1, validator2}, nil
+}
+
+func getDatabase() *state.DB {
+	database := rawdb.NewMemoryDatabase()
+	gspec := core.Genesis{Factory: blockfactory.ForTest}
+	genesis := gspec.MustCommit(database)
+	chain, _ := core.NewBlockChain(database, nil, nil, nil, nil, nil, vm.Config{}) //doublecheck this
+	db, _ := chain.StateAt(genesis.Root())
+	return db
+}
+
+func generateBLSKeyAndSig() (bls.SerializedPublicKey, bls.SerializedSignature) {
+	blsPriv := bls.RandPrivateKey()
+	blsPub := blsPriv.GetPublicKey()
+	msgHash := hash.Keccak256([]byte(staking.BLSVerificationStr))
+	sig := blsPriv.SignHash(msgHash)
+
+	var shardPub bls.SerializedPublicKey
+	copy(shardPub[:], blsPub.Serialize())
+
+	var shardSig bls.SerializedSignature
+	copy(shardSig[:], sig.Serialize())
+
+	return shardPub, shardSig
+}
+
+func sampleWrapper(address common.Address) *staking.ValidatorWrapper {
+	pub, _ := generateBLSKeyAndSig()
+	v := staking.Validator{
+		Address:              address,
+		SlotPubKeys:          []bls.SerializedPublicKey{pub},
+		LastEpochInCommittee: new(big.Int),
+		MinSelfDelegation:    staketest.DefaultMinSelfDel,
+		MaxTotalDelegation:   staketest.DefaultMaxTotalDel,
+		Commission: staking.Commission{
+			CommissionRates: defaultCommissionRates,
+			UpdateHeight:    big.NewInt(100),
+		},
+		Description:    defaultDesc,
+		CreationHeight: big.NewInt(100),
+	}
+	// ds := staking.Delegations{
+	//   staking.NewDelegation(address, big.NewInt(0)),
+	// }
+	w := &staking.ValidatorWrapper{
+		Validator:   v,
+		BlockReward: big.NewInt(0),
+	}
+	w.Counters.NumBlocksSigned = common.Big0
+	w.Counters.NumBlocksToSign = common.Big0
+	return w
+}
+
 func makeBlockForTest(epoch int64, index int) *types.Block {
 	h := blockfactory.NewTestHeader()
 
@@ -395,3 +492,103 @@ func makeVoteData(kp blsKeyPair, block *types.Block) slash.Vote {
 		Signature:       kp.Sign(block),
 	}
 }
+
+func TestPruneStaleStakingData(t *testing.T) {
+	blockFactory := blockfactory.ForTest
+	header := blockFactory.NewHeader(common.Big0) // epoch
+
+	chain := fakeReader{core.FakeChainReader{InternalConfig: params.LocalnetChainConfig}}
+	db := getDatabase()
+	// now make the two wrappers and store them
+	wrapper := sampleWrapper(validator1)
+	wrapper.Status = effective.Inactive
+	wrapper.Delegations = staking.Delegations{
+		staking.NewDelegation(wrapper.Address, big.NewInt(0)),
+		staking.NewDelegation(delegator1, big.NewInt(0)),
+		staking.NewDelegation(delegator2, big.NewInt(0)),
+		staking.NewDelegation(delegator3, new(big.Int).Mul(big.NewInt(denominations.One), big.NewInt(100))),
+	}
+	if err := wrapper.Delegations[3].Undelegate(
+		big.NewInt(2), new(big.Int).Mul(big.NewInt(denominations.One), big.NewInt(100)), nil,
+	); err != nil {
+		t.Fatalf("Got error %v", err)
+	}
+	if wrapper.Delegations[3].Amount.Cmp(common.Big0) != 0 {
+		t.Fatalf("Expected 0 delegation but got %v", wrapper.Delegations[3].Amount)
+	}
+	if err := db.UpdateValidatorWrapper(wrapper.Address, wrapper); err != nil {
+		t.Fatalf("Got error %v", err)
+	}
+	wrapper = sampleWrapper(validator2)
+	wrapper.Status = effective.Active
+	wrapper.Delegations = staking.Delegations{
+		staking.NewDelegation(wrapper.Address, new(big.Int).Mul(big.NewInt(denominations.One), big.NewInt(10000))),
+		staking.NewDelegation(delegator1, new(big.Int).Mul(big.NewInt(denominations.One), big.NewInt(100))),
+		staking.NewDelegation(delegator2, big.NewInt(0)),
+		staking.NewDelegation(delegator3, big.NewInt(0)),
+		staking.NewDelegation(validator1, big.NewInt(0)),
+	}
+	wrapper.Delegations[3].Reward = common.Big257
+	if err := db.UpdateValidatorWrapper(wrapper.Address, wrapper); err != nil {
+		t.Fatalf("Got error %v", err)
+	}
+	// we expect
+	// (1) validator1 to show up with validator2 only (and not validator1 where the delegation is 0)
+	// (2) delegator1 to show up with validator1 only (validator2 has amount)
+	// (3) delegator2 to show up with both validator1 and validator2
+	// (4) delegator3 to show up with neither validator1 (undelegation) nor validator2 (reward)
+	delegationsToRemove := make(map[common.Address][]common.Address, 0)
+	if err := pruneStaleStakingData(&chain, header, db, delegationsToRemove); err != nil {
+		t.Fatalf("Got error %v", err)
+	}
+	if toRemove, ok := delegationsToRemove[validator1]; ok {
+		if len(toRemove) != 1 {
+			t.Errorf("Unexpected # of removals for validator1 %d", len(toRemove))
+		}
+		if len(toRemove) > 0 {
+			for _, validatorAddress := range toRemove {
+				if bytes.Equal(validatorAddress.Bytes(), validator1.Bytes()) {
+					t.Errorf("Found validator1 being removed from validator1's delegations")
+				}
+			}
+		}
+	}
+	if toRemove, ok := delegationsToRemove[delegator1]; ok {
+		if len(toRemove) != 1 {
+			t.Errorf("Unexpected # of removals for delegator1 %d", len(toRemove))
+		}
+		if len(toRemove) > 0 {
+			for _, validatorAddress := range toRemove {
+				if !bytes.Equal(validatorAddress.Bytes(), validator1.Bytes()) {
+					t.Errorf("Unexpected removal for delegator1; validator1 %s, validator2 %s, validatorAddress %s",
+						validator1.Hex(),
+						validator2.Hex(),
+						validatorAddress.Hex(),
+					)
+				}
+			}
+		}
+	}
+	if toRemove, ok := delegationsToRemove[delegator2]; ok {
+		if len(toRemove) != 2 {
+			t.Errorf("Unexpected # of removals for delegator2 %d", len(toRemove))
+		}
+		if len(toRemove) > 0 {
+			for _, validatorAddress := range toRemove {
+				if !(bytes.Equal(validatorAddress.Bytes(), validator1.Bytes()) ||
+					bytes.Equal(validatorAddress.Bytes(), validator2.Bytes())) {
+					t.Errorf("Unexpected removal for delegator2; validator1 %s, validator2 %s, validatorAddress %s",
+						validator1.Hex(),
+						validator2.Hex(),
+						validatorAddress.Hex(),
+					)
+				}
+			}
+		}
+	}
+	if toRemove, ok := delegationsToRemove[delegator3]; ok {
+		if len(toRemove) != 0 {
+			t.Errorf("Unexpected # of removals for delegator3 %d", len(toRemove))
+		}
+	}
+}

internal/chain/reward.go

@@ -270,7 +270,8 @@ func AccumulateRewardsAndCountSigs(
 
 	// Handle rewards on pre-aggregated rewards era.
 	if !bc.Config().IsAggregatedRewardEpoch(header.Epoch()) {
-		return distributeRewardBeforeAggregateEpoch(bc, state, header, beaconChain, defaultReward, sigsReady)
+		reader, err := distributeRewardBeforeAggregateEpoch(bc, state, header, beaconChain, defaultReward, sigsReady)
+		return numeric.ZeroDec(), reader, err
 	}
 
 	// Aggregated Rewards Era: Rewards are aggregated every 64 blocks.
@@ -285,7 +286,8 @@ func AccumulateRewardsAndCountSigs(
 		return numeric.ZeroDec(), network.EmptyPayout, nil
 	}
 
-	return distributeRewardAfterAggregateEpoch(bc, state, header, beaconChain, defaultReward)
+	_, reader, err := distributeRewardAfterAggregateEpoch(bc, state, header, beaconChain, defaultReward)
+	return numeric.ZeroDec(), reader, err
 }
 
 func waitForCommitSigs(sigsReady chan bool) error {
@@ -405,21 +407,39 @@ func distributeRewardAfterAggregateEpoch(bc engine.ChainReader, state *state.DB,
 		if err != nil {
 			return numeric.ZeroDec(), network.EmptyPayout, err
 		}
-		if err := state.AddReward(snapshot.Validator, due, shares); err != nil {
+		if err := state.AddReward(
+			snapshot.Validator,
+			due,
+			shares,
+			// epoch prior to no nil delegations
+			// block 32767 -> snapshot saved, rewards paid
+			// in no nil delegations epoch
+			// block 1 -> rewards not paid (unless schedule changes),
+			// and delegations pruned afterwards
+			bc.Config().IsNoNilDelegations(header.Epoch()),
+		); err != nil {
 			return numeric.ZeroDec(), network.EmptyPayout, err
 		}
 	}
-	utils.Logger().Debug().Int64("elapsed time", time.Now().Sub(startTimeLocal).Milliseconds()).Msg("After Chain Reward (AddReward)")
-	utils.Logger().Debug().Int64("elapsed time", time.Now().Sub(startTime).Milliseconds()).Msg("After Chain Reward")
+
+	utils.Logger().Debug().Int64("elapsed time", time.Since(startTimeLocal).Milliseconds()).Msg("After Chain Reward (AddReward)")
+	utils.Logger().Debug().Int64("elapsed time", time.Since(startTime).Milliseconds()).Msg("After Chain Reward")
 
 	// remainingReward needs to be multipled with the number of crosslinks across all shards
+
 	return remainingReward.MulInt(big.NewInt(int64(len(allCrossLinks)))), network.NewStakingEraRewardForRound(
 		newRewards, payouts,
 	), nil
 }
 
-func distributeRewardBeforeAggregateEpoch(bc engine.ChainReader, state *state.DB, header *block.Header, beaconChain engine.ChainReader,
-	defaultReward numeric.Dec, sigsReady chan bool) (numeric.Dec, reward.Reader, error) {
+func distributeRewardBeforeAggregateEpoch(
+	bc engine.ChainReader,
+	state *state.DB,
+	header *block.Header,
+	beaconChain engine.ChainReader,
+	defaultReward numeric.Dec,
+	sigsReady chan bool,
+) (reward.Reader, error) {
 	newRewards, payouts :=
 		big.NewInt(0), []reward.Payout{}
 
@@ -429,9 +449,9 @@ func distributeRewardBeforeAggregateEpoch(bc engine.ChainReader, state *state.DB
 		startTime := time.Now()
 		crossLinks := types.CrossLinks{}
 		if err := rlp.DecodeBytes(cxLinks, &crossLinks); err != nil {
-			return numeric.ZeroDec(), network.EmptyPayout, err
+			return network.EmptyPayout, err
 		}
-		utils.Logger().Debug().Int64("elapsed time", time.Now().Sub(startTime).Milliseconds()).Msg("Decode Cross Links")
+		utils.Logger().Debug().Int64("elapsed time", time.Since(startTime).Milliseconds()).Msg("Decode Cross Links")
 
 		startTime = time.Now()
 		for i := range crossLinks {
@@ -439,7 +459,7 @@ func distributeRewardBeforeAggregateEpoch(bc engine.ChainReader, state *state.DB
 			payables, _, err := processOneCrossLink(bc, state, cxLink, defaultReward, i)
 
 			if err != nil {
-				return numeric.ZeroDec(), network.EmptyPayout, err
+				return network.EmptyPayout, err
 			}
 
 			allPayables = append(allPayables, payables...)
@@ -473,17 +493,17 @@ func distributeRewardBeforeAggregateEpoch(bc engine.ChainReader, state *state.DB
 					payable.EcdsaAddress,
 				)
 				if err != nil {
-					return numeric.ZeroDec(), network.EmptyPayout, err
+					return network.EmptyPayout, err
 				}
 				due := resultsHandle[bucket][payThem].payout
 				newRewards.Add(newRewards, due)
 
 				shares, err := lookupDelegatorShares(snapshot)
 				if err != nil {
-					return numeric.ZeroDec(), network.EmptyPayout, err
+					return network.EmptyPayout, err
 				}
-				if err := state.AddReward(snapshot.Validator, due, shares); err != nil {
-					return numeric.ZeroDec(), network.EmptyPayout, err
+				if err := state.AddReward(snapshot.Validator, due, shares, bc.Config().IsNoNilDelegations(header.Epoch())); err != nil {
+					return network.EmptyPayout, err
 				}
 				payouts = append(payouts, reward.Payout{
 					Addr:        payable.EcdsaAddress,
@@ -492,21 +512,21 @@ func distributeRewardBeforeAggregateEpoch(bc engine.ChainReader, state *state.DB
 				})
 			}
 		}
-		utils.Logger().Debug().Int64("elapsed time", time.Now().Sub(startTimeLocal).Milliseconds()).Msg("Shard Chain Reward (AddReward)")
-		utils.Logger().Debug().Int64("elapsed time", time.Now().Sub(startTime).Milliseconds()).Msg("Shard Chain Reward")
+		utils.Logger().Debug().Int64("elapsed time", time.Since(startTimeLocal).Milliseconds()).Msg("Shard Chain Reward (AddReward)")
+		utils.Logger().Debug().Int64("elapsed time", time.Since(startTime).Milliseconds()).Msg("Shard Chain Reward")
 	}
 
 	// Block here until the commit sigs are ready or timeout.
 	// sigsReady signal indicates that the commit sigs are already populated in the header object.
 	if err := waitForCommitSigs(sigsReady); err != nil {
-		return numeric.ZeroDec(), network.EmptyPayout, err
+		return network.EmptyPayout, err
 	}
 
 	startTime := time.Now()
 	// Take care of my own beacon chain committee, _ is missing, for slashing
 	parentE, members, payable, missing, err := ballotResultBeaconchain(beaconChain, header)
 	if err != nil {
-		return numeric.ZeroDec(), network.EmptyPayout, errors.Wrapf(err, "shard 0 block %d reward error with bitmap %x", header.Number(), header.LastCommitBitmap())
+		return network.EmptyPayout, errors.Wrapf(err, "shard 0 block %d reward error with bitmap %x", header.Number(), header.LastCommitBitmap())
 	}
 	subComm := shard.Committee{ShardID: shard.BeaconChainShardID, Slots: members}
 
@@ -517,13 +537,13 @@ func distributeRewardBeforeAggregateEpoch(bc engine.ChainReader, state *state.DB
 		payable,
 		missing,
 	); err != nil {
-		return numeric.ZeroDec(), network.EmptyPayout, err
+		return network.EmptyPayout, err
 	}
 	votingPower, err := lookupVotingPower(
 		parentE, &subComm,
 	)
 	if err != nil {
-		return numeric.ZeroDec(), network.EmptyPayout, err
+		return network.EmptyPayout, err
 	}
 
 	allSignersShare := numeric.ZeroDec()
@@ -540,7 +560,7 @@ func distributeRewardBeforeAggregateEpoch(bc engine.ChainReader, state *state.DB
 		if !voter.IsHarmonyNode {
 			snapshot, err := bc.ReadValidatorSnapshot(voter.EarningAccount)
 			if err != nil {
-				return numeric.ZeroDec(), network.EmptyPayout, err
+				return network.EmptyPayout, err
 			}
 			due := defaultReward.Mul(
 				voter.OverallPercent.Quo(allSignersShare),
@@ -549,10 +569,10 @@ func distributeRewardBeforeAggregateEpoch(bc engine.ChainReader, state *state.DB
 
 			shares, err := lookupDelegatorShares(snapshot)
 			if err != nil {
-				return numeric.ZeroDec(), network.EmptyPayout, err
+				return network.EmptyPayout, err
 			}
-			if err := state.AddReward(snapshot.Validator, due, shares); err != nil {
-				return numeric.ZeroDec(), network.EmptyPayout, err
+			if err := state.AddReward(snapshot.Validator, due, shares, bc.Config().IsNoNilDelegations(header.Epoch())); err != nil {
+				return network.EmptyPayout, err
 			}
 			payouts = append(payouts, reward.Payout{
 				Addr:        voter.EarningAccount,
@@ -561,9 +581,9 @@ func distributeRewardBeforeAggregateEpoch(bc engine.ChainReader, state *state.DB
 			})
 		}
 	}
-	utils.Logger().Debug().Int64("elapsed time", time.Now().Sub(startTime).Milliseconds()).Msg("Beacon Chain Reward")
+	utils.Logger().Debug().Int64("elapsed time", time.Since(startTime).Milliseconds()).Msg("Beacon Chain Reward")
 
-	return numeric.ZeroDec(), network.NewStakingEraRewardForRound(
+	return network.NewStakingEraRewardForRound(
 		newRewards, payouts,
 	), nil
 }

internal/params/config.go

@@ -74,6 +74,7 @@ var (
 		FeeCollectEpoch:                       big.NewInt(1535), // 2023-07-20 05:51:07+00:00
 		ValidatorCodeFixEpoch:                 big.NewInt(1535), // 2023-07-20 05:51:07+00:00
 		HIP30Epoch:                            big.NewInt(1673), // 2023-11-02 17:30:00+00:00
+		NoNilDelegationsEpoch:                 EpochTBD,
 		BlockGas30MEpoch:                      big.NewInt(1673), // 2023-11-02 17:30:00+00:00
 		MaxRateEpoch:                          EpochTBD,
 	}
@@ -118,6 +119,7 @@ var (
 		FeeCollectEpoch:                       big.NewInt(1296), // 2023-04-28 07:14:20+00:00
 		ValidatorCodeFixEpoch:                 big.NewInt(1296), // 2023-04-28 07:14:20+00:00
 		HIP30Epoch:                            big.NewInt(2176), // 2023-10-12 10:00:00+00:00
+		NoNilDelegationsEpoch:                 EpochTBD,
 		BlockGas30MEpoch:                      big.NewInt(2176), // 2023-10-12 10:00:00+00:00
 		MaxRateEpoch:                          EpochTBD,
 	}
@@ -162,6 +164,7 @@ var (
 		FeeCollectEpoch:                       EpochTBD,
 		ValidatorCodeFixEpoch:                 EpochTBD,
 		HIP30Epoch:                            EpochTBD,
+		NoNilDelegationsEpoch:                 EpochTBD,
 		BlockGas30MEpoch:                      big.NewInt(0),
 		MaxRateEpoch:                          EpochTBD,
 	}
@@ -208,6 +211,7 @@ var (
 		ValidatorCodeFixEpoch:                 big.NewInt(5),
 		HIP30Epoch:                            big.NewInt(7),
 		BlockGas30MEpoch:                      big.NewInt(7),
+		NoNilDelegationsEpoch:                 EpochTBD,
 		MaxRateEpoch:                          EpochTBD,
 	}
 
@@ -252,6 +256,7 @@ var (
 		LeaderRotationExternalValidatorsEpoch: EpochTBD,
 		ValidatorCodeFixEpoch:                 EpochTBD,
 		HIP30Epoch:                            EpochTBD,
+		NoNilDelegationsEpoch:                 big.NewInt(2),
 		BlockGas30MEpoch:                      big.NewInt(0),
 		MaxRateEpoch:                          EpochTBD,
 	}
@@ -296,6 +301,7 @@ var (
 		FeeCollectEpoch:                       big.NewInt(2),
 		ValidatorCodeFixEpoch:                 big.NewInt(2),
 		HIP30Epoch:                            EpochTBD,
+		NoNilDelegationsEpoch:                 big.NewInt(2),
 		BlockGas30MEpoch:                      big.NewInt(0),
 		MaxRateEpoch:                          EpochTBD,
 	}
@@ -342,7 +348,8 @@ var (
 		big.NewInt(0),                      // FeeCollectEpoch
 		big.NewInt(0),                      // ValidatorCodeFixEpoch
 		big.NewInt(0),                      // BlockGas30M
-		big.NewInt(0),                      // BlockGas30M
+		big.NewInt(0),                      // HIP30Epoch
+		big.NewInt(0),                      // NoNilDelegationsEpoch
 		big.NewInt(0),                      // MaxRateEpoch
 	}
 
@@ -388,6 +395,7 @@ var (
 		big.NewInt(0),        // FeeCollectEpoch
 		big.NewInt(0),        // ValidatorCodeFixEpoch
 		big.NewInt(0),        // HIP30Epoch
+		big.NewInt(0),        // NoNilDelegationsEpoch
 		big.NewInt(0),        // BlockGas30M
 		big.NewInt(0),        // MaxRateEpoch
 	}
@@ -529,6 +537,9 @@ type ChainConfig struct {
 	// AllowlistEpoch is the first epoch to support allowlist of HIP18
 	AllowlistEpoch *big.Int
 
+	// The first epoch at the end of which stale delegations are removed
+	NoNilDelegationsEpoch *big.Int `json:"no-nil-delegations-epoch,omitempty"`
+
 	LeaderRotationInternalValidatorsEpoch *big.Int `json:"leader-rotation-internal-validators,omitempty"`
 
 	LeaderRotationExternalValidatorsEpoch *big.Int `json:"leader-rotation-external-validators,omitempty"`
@@ -562,7 +573,19 @@ type ChainConfig struct {
 
 // String implements the fmt.Stringer interface.
 func (c *ChainConfig) String() string {
-	return fmt.Sprintf("{ChainID: %v EthCompatibleChainID: %v EIP155: %v CrossTx: %v Staking: %v CrossLink: %v ReceiptLog: %v SHA3Epoch: %v StakingPrecompileEpoch: %v ChainIdFixEpoch: %v CrossShardXferPrecompileEpoch: %v}",
+	// use string1 + string2 here instead of concatening in the end
+	return fmt.Sprintf("{ChainID: %v "+
+		"EthCompatibleChainID: %v "+
+		"EIP155: %v "+
+		"CrossTx: %v "+
+		"Staking: %v "+
+		"CrossLink: %v "+
+		"ReceiptLog: %v "+
+		"SHA3Epoch: %v "+
+		"StakingPrecompileEpoch: %v "+
+		"ChainIdFixEpoch: %v "+
+		"CrossShardXferPrecompileEpoch: %v "+
+		"NoNilDelegationsEpoch: %v}",
 		c.ChainID,
 		c.EthCompatibleChainID,
 		c.EIP155Epoch,
@@ -574,6 +597,7 @@ func (c *ChainConfig) String() string {
 		c.StakingPrecompileEpoch,
 		c.ChainIdFixEpoch,
 		c.CrossShardXferPrecompileEpoch,
+		c.NoNilDelegationsEpoch,
 	)
 }
 
@@ -767,12 +791,18 @@ func (c *ChainConfig) IsHIP6And8Epoch(epoch *big.Int) bool {
 	return isForked(c.HIP6And8Epoch, epoch)
 }
 
-// IsStakingPrecompileEpoch determines whether staking
+// IsStakingPrecompile determines whether staking
 // precompiles are available in the EVM
 func (c *ChainConfig) IsStakingPrecompile(epoch *big.Int) bool {
 	return isForked(c.StakingPrecompileEpoch, epoch)
 }
 
+// IsNoNilDelegations determines whether to clear
+// nil delegations regularly (and of course also once)
+func (c *ChainConfig) IsNoNilDelegations(epoch *big.Int) bool {
+	return isForked(c.NoNilDelegationsEpoch, epoch)
+}
+
 // IsCrossShardXferPrecompile determines whether the
 // Cross Shard Transfer Precompile is available in the EVM
 func (c *ChainConfig) IsCrossShardXferPrecompile(epoch *big.Int) bool {
@@ -882,6 +912,7 @@ type Rules struct {
 	// eip-155 chain id fix
 	IsChainIdFix bool
 	IsValidatorCodeFix bool
+	IsNoNilDelegations bool
 }
 
 // Rules ensures c's ChainID is not nil.
@@ -907,5 +938,6 @@ func (c *ChainConfig) Rules(epoch *big.Int) Rules {
 		IsCrossShardXferPrecompile: c.IsCrossShardXferPrecompile(epoch),
 		IsChainIdFix:               c.IsChainIdFix(epoch),
 		IsValidatorCodeFix:         c.IsValidatorCodeFix(epoch),
+		IsNoNilDelegations:         c.IsNoNilDelegations(epoch),
 	}
 }

node/worker/worker.go

@@ -46,6 +46,7 @@ type environment struct {
 	incxs               []*types.CXReceiptsProof // cross shard receipts and its proof (desitinatin shard)
 	slashes             slash.Records
 	stakeMsgs           []staking.StakeMsg
+	delegationsToRemove map[common.Address][]common.Address
 }
 
 func (env *environment) CurrentHeader() *block.Header {
@@ -414,6 +415,7 @@ func (w *Worker) GetCurrentResult() *core.ProcessorResult {
 		Reward:              w.current.reward,
 		State:               w.current.state,
 		StakeMsgs:           w.current.stakeMsgs,
+		DelegationsToRemove: w.current.delegationsToRemove,
 	}
 }
 
@@ -615,7 +617,7 @@ func (w *Worker) FinalizeNewBlock(
 		}
 	}()
 
-	block, payout, err := w.chain.Engine().Finalize(
+	block, delegationsToRemove, payout, err := w.chain.Engine().Finalize(
 		w.chain,
 		w.beacon,
 		copyHeader, state, w.current.txs, w.current.receipts,
@@ -626,6 +628,7 @@ func (w *Worker) FinalizeNewBlock(
 		return nil, errors.Wrapf(err, "cannot finalize block")
 	}
 	w.current.reward = payout
+	w.current.delegationsToRemove = delegationsToRemove
 	return block, nil
 }
 

staking/types/delegation.go

@@ -2,18 +2,21 @@ package types
 
 import (
 	"encoding/json"
-	"errors"
+	//"errors"
+	"fmt"
 	"math/big"
 	"sort"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/harmony-one/harmony/crypto/hash"
 	common2 "github.com/harmony-one/harmony/internal/common"
+	"github.com/pkg/errors"
 )
 
 var (
 	errInsufficientBalance   = errors.New("insufficient balance to undelegate")
 	errInvalidAmount         = errors.New("invalid amount, must be positive")
+	ErrUndelegationRemaining = errors.New("remaining delegation must be 0 or >= 100 ONE")
 )
 
 const (
@@ -120,13 +123,25 @@ func NewDelegation(delegatorAddr common.Address,
 }
 
 // Undelegate - append entry to the undelegation
-func (d *Delegation) Undelegate(epoch *big.Int, amt *big.Int) error {
+func (d *Delegation) Undelegate(
+	epoch *big.Int,
+	amt *big.Int,
+	minimumRemainingDelegation *big.Int,
+) error {
 	if amt.Sign() <= 0 {
 		return errInvalidAmount
 	}
 	if d.Amount.Cmp(amt) < 0 {
 		return errInsufficientBalance
 	}
+	if minimumRemainingDelegation != nil {
+		finalAmount := big.NewInt(0).Sub(d.Amount, amt)
+		if finalAmount.Cmp(minimumRemainingDelegation) < 0 && finalAmount.Cmp(common.Big0) != 0 {
+			return errors.Wrapf(ErrUndelegationRemaining,
+				fmt.Sprintf("Minimum: %d, Remaining: %d", minimumRemainingDelegation, finalAmount),
+			)
+		}
+	}
 	d.Amount.Sub(d.Amount, amt)
 
 	exist := false

staking/types/delegation_test.go

@@ -19,7 +19,7 @@ var (
 func TestUndelegate(t *testing.T) {
 	epoch1 := big.NewInt(10)
 	amount1 := big.NewInt(1000)
-	delegation.Undelegate(epoch1, amount1)
+	delegation.Undelegate(epoch1, amount1, nil)
 
 	// check the undelegation's Amount
 	if delegation.Undelegations[0].Amount.Cmp(amount1) != 0 {
@@ -32,7 +32,7 @@ func TestUndelegate(t *testing.T) {
 
 	epoch2 := big.NewInt(12)
 	amount2 := big.NewInt(2000)
-	delegation.Undelegate(epoch2, amount2)
+	delegation.Undelegate(epoch2, amount2, nil)
 
 	// check the number of undelegations
 	if len(delegation.Undelegations) != 2 {
@@ -54,7 +54,7 @@ func TestDeleteEntry(t *testing.T) {
 	// Undelegations[]: 1000, 2000, 3000
 	epoch3 := big.NewInt(15)
 	amount3 := big.NewInt(3000)
-	delegation.Undelegate(epoch3, amount3)
+	delegation.Undelegate(epoch3, amount3, nil)
 
 	// delete the second undelegation entry
 	// Undelegations[]: 1000, 3000
@@ -73,7 +73,7 @@ func TestUnlockedLastEpochInCommittee(t *testing.T) {
 
 	epoch4 := big.NewInt(21)
 	amount4 := big.NewInt(4000)
-	delegation.Undelegate(epoch4, amount4)
+	delegation.Undelegate(epoch4, amount4, nil)
 
 	result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 7, false)
 	if result.Cmp(big.NewInt(8000)) != 0 {
@@ -88,7 +88,7 @@ func TestUnlockedLastEpochInCommitteeFail(t *testing.T) {
 
 	epoch4 := big.NewInt(21)
 	amount4 := big.NewInt(4000)
-	delegation.Undelegate(epoch4, amount4)
+	delegation.Undelegate(epoch4, amount4, nil)
 
 	result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 7, false)
 	if result.Cmp(big.NewInt(0)) != 0 {
@@ -102,7 +102,7 @@ func TestUnlockedFullPeriod(t *testing.T) {
 
 	epoch5 := big.NewInt(27)
 	amount5 := big.NewInt(4000)
-	delegation.Undelegate(epoch5, amount5)
+	delegation.Undelegate(epoch5, amount5, nil)
 
 	result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 7, false)
 	if result.Cmp(big.NewInt(4000)) != 0 {
@@ -116,7 +116,7 @@ func TestQuickUnlock(t *testing.T) {
 
 	epoch7 := big.NewInt(44)
 	amount7 := big.NewInt(4000)
-	delegation.Undelegate(epoch7, amount7)
+	delegation.Undelegate(epoch7, amount7, nil)
 
 	result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 0, false)
 	if result.Cmp(big.NewInt(4000)) != 0 {
@@ -131,7 +131,7 @@ func TestUnlockedFullPeriodFail(t *testing.T) {
 
 	epoch5 := big.NewInt(28)
 	amount5 := big.NewInt(4000)
-	delegation.Undelegate(epoch5, amount5)
+	delegation.Undelegate(epoch5, amount5, nil)
 
 	result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 7, false)
 	if result.Cmp(big.NewInt(0)) != 0 {
@@ -145,7 +145,7 @@ func TestUnlockedPremature(t *testing.T) {
 
 	epoch6 := big.NewInt(42)
 	amount6 := big.NewInt(4000)
-	delegation.Undelegate(epoch6, amount6)
+	delegation.Undelegate(epoch6, amount6, nil)
 
 	result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 7, false)
 	if result.Cmp(big.NewInt(0)) != 0 {
@@ -159,7 +159,7 @@ func TestNoEarlyUnlock(t *testing.T) {
 
 	epoch4 := big.NewInt(21)
 	amount4 := big.NewInt(4000)
-	delegation.Undelegate(epoch4, amount4)
+	delegation.Undelegate(epoch4, amount4, nil)
 
 	result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 7, true)
 	if result.Cmp(big.NewInt(0)) != 0 {

test/chain/reward/main.go

@@ -139,7 +139,7 @@ func main() {
 	fmt.Printf("Time required to calc percentage %d delegations: %f seconds\n", len(validator.Delegations), endTime.Sub(startTime).Seconds())
 
 	startTime = time.Now()
-	statedb.AddReward(validator, big.NewInt(1000), shares)
+	statedb.AddReward(validator, big.NewInt(1000), shares, false)
 	endTime = time.Now()
 	fmt.Printf("Time required to reward a validator with %d delegations: %f seconds\n", len(validator.Delegations), endTime.Sub(startTime).Seconds())