fix block proposal ordering; other offchain commits change (#2761)

* fix block proposal ordering; other offchain commits change

* remove div by 0 checks

* Fix imports

consensus/consensus.go

@@ -2,7 +2,6 @@ package consensus
 
 import (
 	"fmt"
-	"math/big"
 	"sync"
 	"time"
 
@@ -129,8 +128,6 @@ type Consensus struct {
 	syncNotReadyChan chan struct{}
 	// If true, this consensus will not propose view change.
 	disableViewChange bool
-	// last node block reward for metrics
-	lastBlockReward *big.Int
 	// Have a dedicated reader thread pull from this chan, like in node
 	SlashChan chan slash.Record
 }
@@ -167,11 +164,6 @@ func (consensus *Consensus) VdfSeedSize() int {
 	return int(consensus.Decider.ParticipantsCount()) * 2 / 3
 }
 
-// GetBlockReward returns last node block reward
-func (consensus *Consensus) GetBlockReward() *big.Int {
-	return consensus.lastBlockReward
-}
-
 // GetLeaderPrivateKey returns leader private key if node is the leader
 func (consensus *Consensus) GetLeaderPrivateKey(leaderKey *bls.PublicKey) (*bls.SecretKey, error) {
 	for i, key := range consensus.PubKey.PublicKey {
@@ -229,7 +221,6 @@ func New(
 	consensus.SlashChan = make(chan slash.Record)
 	consensus.commitFinishChan = make(chan uint64)
 	consensus.ReadySignal = make(chan struct{})
-	consensus.lastBlockReward = common.Big0
 	// channel for receiving newly generated VDF
 	consensus.RndChannel = make(chan [vdfAndSeedSize]byte)
 	return &consensus, nil

core/blockchain.go

@@ -51,7 +51,6 @@ import (
 	"github.com/harmony-one/harmony/shard"
 	"github.com/harmony-one/harmony/shard/committee"
 	"github.com/harmony-one/harmony/staking/apr"
-	"github.com/harmony-one/harmony/staking/availability"
 	"github.com/harmony-one/harmony/staking/effective"
 	"github.com/harmony-one/harmony/staking/slash"
 	staking "github.com/harmony-one/harmony/staking/types"
@@ -2214,7 +2213,6 @@ func (bc *BlockChain) ReadValidatorSnapshot(
 		}
 		return &v, nil
 	}
-
 	return rawdb.ReadValidatorSnapshot(bc.db, addr, epoch)
 }
 
@@ -2232,6 +2230,7 @@ func (bc *BlockChain) writeValidatorSnapshots(
 		}
 		validators = append(validators, validator)
 	}
+
 	// Batch write the current data as snapshot
 	for i := range validators {
 		if err := rawdb.WriteValidatorSnapshot(batch, validators[i], epoch); err != nil {
@@ -2353,22 +2352,16 @@ func (bc *BlockChain) UpdateValidatorVotingPower(
 				utils.Logger().Debug().Err(err).Msg("issue with compute of apr")
 			}
 
-			snapshot, err := bc.ReadValidatorSnapshotAtEpoch(
-				currentEpochSuperCommittee.Epoch, wrapper.Address,
-			)
-
 			if err != nil {
 				return nil, err
 			}
 
-			computed := availability.ComputeCurrentSigning(snapshot, wrapper)
-
 			if _, wasBooted := bootedFromSuperCommittee[wrapper.Address]; wasBooted {
 				stats.BootedStatus = effective.LostEPoSAuction
 			}
 
-			if computed.IsBelowThreshold {
-				stats.BootedStatus = effective.InsufficientUptimeDuringEpoch
+			if wrapper.Status == effective.Inactive {
+				stats.BootedStatus = effective.TurnedInactiveOrInsufficientUptime
 			}
 
 			if slash.IsBanned(wrapper) {
@@ -2381,23 +2374,6 @@ func (bc *BlockChain) UpdateValidatorVotingPower(
 	return validatorStats, nil
 }
 
-// deleteValidatorSnapshots deletes the snapshot staking information of given validator address
-// TODO: delete validator snapshots from X epochs ago
-// NOTE Use when needed but don't compile at all until then
-// func (bc *BlockChain) deleteValidatorSnapshots(addrs []common.Address) error {
-// 	batch := bc.db.NewBatch()
-// 	for i := range addrs {
-// 		rawdb.DeleteValidatorSnapshot(batch, addrs[i], bc.CurrentBlock().Epoch())
-// 	}
-// 	if err := batch.Write(); err != nil {
-// 		return err
-// 	}
-// 	for i := range addrs {
-// 		bc.validatorCache.Remove("validator-snapshot-" + string(addrs[i].Bytes()))
-// 	}
-// 	return nil
-// }
-
 // UpdateValidatorSnapshots updates the content snapshot of all validators
 // Note: this should only be called within the blockchain insert process.
 func (bc *BlockChain) UpdateValidatorSnapshots(
@@ -2411,11 +2387,6 @@ func (bc *BlockChain) UpdateValidatorSnapshots(
 	}
 
 	allValidators = append(allValidators, newValidators...)
-	// TODO: enable this once we allow validator to delete itself.
-	//err = bc.deleteValidatorSnapshots(allValidators)
-	//if err != nil {
-	//	return err
-	//}
 
 	return bc.writeValidatorSnapshots(batch, allValidators, epoch, state)
 }
@@ -2515,7 +2486,7 @@ func (bc *BlockChain) writeDelegationsByDelegator(
 // Note: this should only be called within the blockchain insert process.
 func (bc *BlockChain) UpdateStakingMetaData(
 	batch rawdb.DatabaseWriter, txns staking.StakingTransactions,
-	state *state.DB, epoch *big.Int, isNewEpoch bool,
+	state *state.DB, epoch, newEpoch *big.Int,
 ) (newValidators []common.Address, err error) {
 	newValidators, newDelegations, err := bc.prepareStakingMetaData(txns, state)
 	if err != nil {
@@ -2547,8 +2518,7 @@ func (bc *BlockChain) UpdateStakingMetaData(
 			}
 			// For validator created at exactly the last block of an epoch, we should create the snapshot
 			// for next epoch too.
-			if isNewEpoch {
-				newEpoch := new(big.Int).Add(epoch, common.Big1)
+			if newEpoch.Cmp(epoch) > 0 {
 				if err := rawdb.WriteValidatorSnapshot(batch, validator, newEpoch); err != nil {
 					return newValidators, err
 				}
@@ -2646,7 +2616,7 @@ func (bc *BlockChain) prepareStakingMetaData(
 // ReadBlockRewardAccumulator must only be called on beaconchain
 func (bc *BlockChain) ReadBlockRewardAccumulator(number uint64) (*big.Int, error) {
 	if !bc.chainConfig.IsStaking(shard.Schedule.CalcEpochNumber(number)) {
-		return common.Big0, nil
+		return big.NewInt(0), nil
 	}
 	if cached, ok := bc.blockAccumulatorCache.Get(number); ok {
 		return cached.(*big.Int), nil

core/offchain.go

@@ -92,26 +92,17 @@ func (bc *BlockChain) CommitOffChainData(
 	//	}
 	//}
 
-	// Do bookkeeping for new staking txns
-	newVals, err := bc.UpdateStakingMetaData(
-		batch, block.StakingTransactions(), state, epoch, isNewEpoch,
-	)
-	if err != nil {
-		utils.Logger().Err(err).Msg("UpdateStakingMetaData failed")
-		return NonStatTy, err
-	}
-
+	newEpoch := new(big.Int).Add(header.Epoch(), common.Big1)
 	// Shard State and Validator Update
 	if isNewEpoch {
 		// Write shard state for the new epoch
-		epoch := new(big.Int).Add(header.Epoch(), common.Big1)
 		shardState, err := block.Header().GetShardState()
 		if err == nil && shardState.Epoch != nil && bc.chainConfig.IsStaking(shardState.Epoch) {
 			// After staking, the epoch will be decided by the epoch in the shard state.
-			epoch = new(big.Int).Set(shardState.Epoch)
+			newEpoch = new(big.Int).Set(shardState.Epoch)
 		}
 
-		newShardState, err := bc.WriteShardStateBytes(batch, epoch, header.ShardState())
+		newShardState, err := bc.WriteShardStateBytes(batch, newEpoch, header.ShardState())
 		if err != nil {
 			header.Logger(utils.Logger()).Warn().Err(err).Msg("cannot store shard state")
 			return NonStatTy, err
@@ -125,12 +116,20 @@ func (bc *BlockChain) CommitOffChainData(
 		}
 	}
 
-	// Snapshot for all validators at the second to last block
+	// Do bookkeeping for new staking txns
+	newVals, err := bc.UpdateStakingMetaData(
+		batch, block.StakingTransactions(), state, epoch, newEpoch,
+	)
+	if err != nil {
+		utils.Logger().Err(err).Msg("UpdateStakingMetaData failed")
+		return NonStatTy, err
+	}
+
+	// Snapshot for all validators for new epoch at the second to last block
 	// This snapshot of the state is consistent with the state used for election
 	if isBeaconChain && shard.Schedule.IsLastBlock(header.Number().Uint64()+1) {
 		// Update snapshots for all validators
-		epoch := new(big.Int).Add(header.Epoch(), common.Big1)
-		if err := bc.UpdateValidatorSnapshots(batch, epoch, state, newVals); err != nil {
+		if err := bc.UpdateValidatorSnapshots(batch, newEpoch, state, newVals); err != nil {
 			return NonStatTy, err
 		}
 	}
@@ -187,13 +186,12 @@ func (bc *BlockChain) CommitOffChainData(
 		for i, c := uint32(0), shard.Schedule.InstanceForEpoch(
 			epoch,
 		).NumShards(); i < c; i++ {
-			if err := bc.LastContinuousCrossLink(batch, i); err != nil {
-				utils.Logger().Info().
-					Err(err).Msg("could not batch process last continuous crosslink")
-			}
+			bc.LastContinuousCrossLink(batch, i)
 		}
 	}
 
+	// BELOW ARE NON-MISSION-CRITICAL COMMITS
+
 	// Update voting power of validators for all shards
 	tempValidatorStats := map[common.Address]*staking.ValidatorStats{}
 	if isNewEpoch && isBeaconChain {

core/staking_verifier.go

@@ -5,6 +5,7 @@ import (
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/common"
+
 	"github.com/harmony-one/harmony/common/denominations"
 	"github.com/harmony-one/harmony/core/vm"
 	common2 "github.com/harmony-one/harmony/internal/common"

hmy/api_backend.go

@@ -356,7 +356,9 @@ func (b *APIBackend) GetValidatorInformation(
 	}
 
 	now := block.Epoch()
-	inCommittee := now.Cmp(wrapper.LastEpochInCommittee) == 0
+	// At the last block of epoch, block epoch is e while val.LastEpochInCommittee
+	// is already updated to e+1. So need the >= check rather than ==
+	inCommittee := wrapper.LastEpochInCommittee.Cmp(now) >= 0
 	defaultReply := &staking.ValidatorRPCEnchanced{
 		CurrentlyInCommittee: inCommittee,
 		Wrapper:              *wrapper,

internal/chain/engine.go

@@ -257,25 +257,20 @@ func (e *engineImpl) Finalize(
 	incxs []*types.CXReceiptsProof, stks staking.StakingTransactions,
 	doubleSigners slash.Records,
 ) (*types.Block, reward.Reader, error) {
-	// Accumulate block rewards and commit the final state root
-	// Header seems complete, assemble into a block and return
-	payout, err := AccumulateRewards(
-		chain, state, header, e.Beaconchain(),
-	)
-	if err != nil {
-		return nil, nil, ctxerror.New("cannot pay block reward").WithCause(err)
-	}
 
 	isBeaconChain := header.ShardID() == shard.BeaconChainShardID
 	isNewEpoch := len(header.ShardState()) > 0
 	inStakingEra := chain.Config().IsStaking(header.Epoch())
 
 	// Process Undelegations, set LastEpochInCommittee and set EPoS status
+	// Needs to be before AccumulateRewardsAndCountSigs
 	if isBeaconChain && isNewEpoch && inStakingEra {
 		if err := payoutUndelegations(chain, header, state); err != nil {
 			return nil, nil, err
 		}
 
+		// Needs to be after payoutUndelegations because payoutUndelegations
+		// depends on the old LastEpochInCommittee
 		if err := setLastEpochInCommittee(header, state); err != nil {
 			return nil, nil, err
 		}
@@ -284,6 +279,10 @@ func (e *engineImpl) Finalize(
 		if err != nil {
 			return nil, nil, err
 		}
+		// Needs to be before AccumulateRewardsAndCountSigs because
+		// ComputeAndMutateEPOSStatus depends on the signing counts that's
+		// consistent with the counts when the new shardState was proposed.
+		// Refer to committee.IsEligibleForEPoSAuction()
 		for _, addr := range curShardState.StakedValidators().Addrs {
 			if err := availability.ComputeAndMutateEPOSStatus(
 				chain, state, addr,
@@ -293,6 +292,15 @@ func (e *engineImpl) Finalize(
 		}
 	}
 
+	// Accumulate block rewards and commit the final state root
+	// Header seems complete, assemble into a block and return
+	payout, err := AccumulateRewardsAndCountSigs(
+		chain, state, header, e.Beaconchain(),
+	)
+	if err != nil {
+		return nil, nil, ctxerror.New("cannot pay block reward").WithCause(err)
+	}
+
 	// Apply slashes
 	if isBeaconChain && inStakingEra && len(doubleSigners) > 0 {
 		if err := applySlashes(chain, header, state, doubleSigners); err != nil {

internal/chain/reward.go

@@ -74,9 +74,10 @@ func lookupVotingPower(
 	return results.(*votepower.Roster), nil
 }
 
-// AccumulateRewards credits the coinbase of the given block with the mining
+// AccumulateRewardsAndCountSigs credits the coinbase of the given block with the mining
 // reward. The total reward consists of the static block reward
-func AccumulateRewards(
+// This func also do IncrementValidatorSigningCounts for validators
+func AccumulateRewardsAndCountSigs(
 	bc engine.ChainReader, state *state.DB,
 	header *block.Header, beaconChain engine.ChainReader,
 ) (reward.Reader, error) {

staking/effective/eligible.go

@@ -82,8 +82,8 @@ const (
 	NotBooted BootedStatus = iota
 	// LostEPoSAuction ..
 	LostEPoSAuction
-	// InsufficientUptimeDuringEpoch ..
-	InsufficientUptimeDuringEpoch
+	// TurnedInactiveOrInsufficientUptime ..
+	TurnedInactiveOrInsufficientUptime
 	// BannedForDoubleSigning ..
 	BannedForDoubleSigning
 )
@@ -92,8 +92,8 @@ func (r BootedStatus) String() string {
 	switch r {
 	case LostEPoSAuction:
 		return "lost epos auction"
-	case InsufficientUptimeDuringEpoch:
-		return "bad uptime"
+	case TurnedInactiveOrInsufficientUptime:
+		return "manually turned inactive or insufficient uptime"
 	case BannedForDoubleSigning:
 		return doubleSigningBanned
 	default:

staking/network/reward.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"math/big"
 
-	"github.com/ethereum/go-ethereum/common"
 	"github.com/harmony-one/harmony/common/denominations"
 	"github.com/harmony-one/harmony/consensus/engine"
 	"github.com/harmony-one/harmony/consensus/reward"
@@ -31,7 +30,7 @@ var (
 		"total payout not equal to blockreward",
 	)
 	// NoReward ..
-	NoReward = common.Big0
+	NoReward = big.NewInt(0)
 	// EmptyPayout ..
 	EmptyPayout = noReward{}
 )
@@ -46,7 +45,7 @@ type noReward struct{ ignoreMissing }
 
 func (noReward) ReadRoundResult() *reward.CompletedRound {
 	return &reward.CompletedRound{
-		Total:            common.Big0,
+		Total:            big.NewInt(0),
 		BeaconchainAward: []reward.Payout{},
 		ShardChainAward:  []reward.Payout{},
 	}