Check signing threshold and set EPoS status only at last epoch (#2533)

* Check signing threshold and set EPoS status only at last epoch

* base on status if validator signing enough blocks

* Fix lint

hmy/api_backend.go

@@ -356,13 +356,10 @@ func (b *APIBackend) GetValidatorInformation(
 		return defaultReply, nil
 	}
 
-	computed, err := availability.ComputeCurrentSigning(
+	computed := availability.ComputeCurrentSigning(
-		snapshot, wrapper, shard.Schedule.BlocksPerEpoch(),
+		snapshot, wrapper,
 	)
-
+	computed.BlocksLeftInEpoch = shard.Schedule.BlocksPerEpoch() - computed.ToSign.Uint64()
-	if err != nil {
-		return defaultReply, nil
-	}
 
 	stats, err := b.hmy.BlockChain().ReadValidatorStats(addr)
 	if err != nil {
@@ -402,8 +399,9 @@ func (b *APIBackend) GetTotalStakingSnapshot() *big.Int {
 	}
 	stakes := big.NewInt(0)
 	for i := range candidates {
+		snapshot, _ := b.hmy.BlockChain().ReadValidatorSnapshot(candidates[i])
 		validator, _ := b.hmy.BlockChain().ReadValidatorInformation(candidates[i])
-		if !staking.IsEligibleForEPoSAuction(validator) {
+		if !committee.IsEligibleForEPoSAuction(snapshot, validator) {
 			continue
 		}
 		for i := range validator.Delegations {

internal/chain/engine.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"encoding/binary"
 
+	"github.com/harmony-one/harmony/staking/availability"
+
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/rlp"
@@ -266,7 +268,7 @@ func (e *engineImpl) Finalize(
 	isNewEpoch := len(header.ShardState()) > 0
 	inStakingEra := chain.Config().IsStaking(header.Epoch())
 
-	// Process Undelegations and set LastEpochInCommittee
+	// Process Undelegations, set LastEpochInCommittee and set EPoS status
 	if isBeaconChain && isNewEpoch && inStakingEra {
 		if err := payoutUndelegations(chain, header, state); err != nil {
 			return nil, nil, err
@@ -275,6 +277,18 @@ func (e *engineImpl) Finalize(
 		if err := setLastEpochInCommittee(header, state); err != nil {
 			return nil, nil, err
 		}
+
+		curShardState, err := chain.ReadShardState(chain.CurrentBlock().Epoch())
+		if err != nil {
+			return nil, nil, err
+		}
+		for _, addr := range curShardState.StakedValidators().Addrs {
+			if err := availability.ComputeAndMutateEPOSStatus(
+				chain, state, addr,
+			); err != nil {
+				return nil, nil, err
+			}
+		}
 	}
 
 	// Apply slashes

internal/chain/reward.go

@@ -5,6 +5,8 @@ import (
 	"math/big"
 	"sort"
 
+	"github.com/harmony-one/harmony/internal/ctxerror"
+
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/rlp"
 	"github.com/harmony-one/harmony/block"
@@ -24,7 +26,24 @@ import (
 func ballotResultBeaconchain(
 	bc engine.ChainReader, header *block.Header,
 ) (shard.SlotList, shard.SlotList, shard.SlotList, error) {
-	return availability.BallotResult(bc, header, shard.BeaconChainShardID)
+	// TODO ek – retrieving by parent number (blockNum - 1) doesn't work,
+	//  while it is okay with hash.  Sounds like DB inconsistency.
+	//  Figure out why.
+	parentHeader := bc.GetHeaderByHash(header.ParentHash())
+	if parentHeader == nil {
+		return nil, nil, nil, ctxerror.New(
+			"cannot find parent block header in DB",
+			"parentHash", header.ParentHash(),
+		)
+	}
+	parentShardState, err := bc.ReadShardState(parentHeader.Epoch())
+	if err != nil {
+		return nil, nil, nil, ctxerror.New(
+			"cannot read shard state", "epoch", parentHeader.Epoch(),
+		).WithCause(err)
+	}
+
+	return availability.BallotResult(parentHeader, header, parentShardState, shard.BeaconChainShardID)
 }
 
 var (
@@ -276,14 +295,28 @@ func AccumulateRewards(
 	}
 
 	// Before staking
+	// TODO ek – retrieving by parent number (blockNum - 1) doesn't work,
+	//  while it is okay with hash.  Sounds like DB inconsistency.
+	//  Figure out why.
 	parentHeader := bc.GetHeaderByHash(header.ParentHash())
+	if parentHeader == nil {
+		return network.EmptyPayout, ctxerror.New(
+			"cannot find parent block header in DB",
+			"parentHash", header.ParentHash(),
+		)
+	}
 	if parentHeader.Number().Cmp(common.Big0) == 0 {
 		// Parent is an epoch block,
 		// which is not signed in the usual manner therefore rewards nothing.
 		return network.EmptyPayout, nil
 	}
-
+	parentShardState, err := bc.ReadShardState(parentHeader.Epoch())
-	_, signers, _, err := availability.BallotResult(bc, header, header.ShardID())
+	if err != nil {
+		return nil, ctxerror.New(
+			"cannot read shard state", "epoch", parentHeader.Epoch(),
+		).WithCause(err)
+	}
+	_, signers, _, err := availability.BallotResult(parentHeader, header, parentShardState, header.ShardID())
 
 	if err != nil {
 		return network.EmptyPayout, err

node/node_handler.go

@@ -494,10 +494,11 @@ func (node *Node) PostConsensusProcessing(
 				if err != nil {
 					return
 				}
-				computed, err :=
+				computed := availability.ComputeCurrentSigning(
-					availability.ComputeCurrentSigning(
+					snapshot, wrapper,
-						snapshot, wrapper, shard.Schedule.BlocksPerEpoch(),
+				)
-					)
+				computed.BlocksLeftInEpoch = shard.Schedule.BlocksPerEpoch() - computed.ToSign.Uint64()
+
 				if err != nil && computed.IsBelowThreshold {
 					url := h.Availability.OnDroppedBelowThreshold
 					go func() {

shard/committee/assignment.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"math/big"
 
+	"github.com/harmony-one/harmony/staking/availability"
+
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/harmony-one/bls/ffi/go/bls"
 	"github.com/harmony-one/harmony/block"
@@ -119,7 +121,13 @@ func prepareOrders(
 		if err != nil {
 			return nil, err
 		}
-		if !staking.IsEligibleForEPoSAuction(validator) {
+		snapshot, err := stakedReader.ReadValidatorSnapshot(
+			candidates[i],
+		)
+		if err != nil {
+			return nil, err
+		}
+		if !IsEligibleForEPoSAuction(snapshot, validator) {
 			continue
 		}
 
@@ -160,6 +168,28 @@ func prepareOrders(
 	return essentials, nil
 }
 
+// IsEligibleForEPoSAuction ..
+func IsEligibleForEPoSAuction(snapshot, validator *staking.ValidatorWrapper) bool {
+	if snapshot.Counters.NumBlocksToSign.Cmp(validator.Counters.NumBlocksToSign) != 0 {
+		// validator was in last epoch's committee
+		// validator with below-threshold signing activity won't be considered for next epoch
+		// and their status will be turned to inactive in FinalizeNewBlock
+		computed := availability.ComputeCurrentSigning(snapshot, validator)
+		if computed.IsBelowThreshold {
+			return false
+		}
+	}
+	// For validators who were not in last epoch's committee
+	// or for those who were and signed enough blocks,
+	// the decision is based on the status
+	switch validator.Status {
+	case effective.Active:
+		return true
+	default:
+		return false
+	}
+}
+
 // ChainReader is a subset of Engine.ChainReader, just enough to do assignment
 type ChainReader interface {
 	// ReadShardState retrieves sharding state given the epoch number.

staking/availability/measure.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/harmony-one/harmony/block"
-	engine "github.com/harmony-one/harmony/consensus/engine"
 	"github.com/harmony-one/harmony/core/state"
 	bls2 "github.com/harmony-one/harmony/crypto/bls"
 	"github.com/harmony-one/harmony/internal/ctxerror"
@@ -22,7 +21,6 @@ var (
 	errValidatorEpochDeviation = errors.New(
 		"validator snapshot epoch not exactly one epoch behind",
 	)
-	errNegativeSign = errors.New("impossible period of signing")
 	// ErrDivByZero ..
 	ErrDivByZero = errors.New("toSign of availability cannot be 0, mistake in protocol")
 )
@@ -71,25 +69,8 @@ func BlockSigners(
 // BallotResult returns
 // (parentCommittee.Slots, payable, missing, err)
 func BallotResult(
-	bc engine.ChainReader, header *block.Header, shardID uint32,
+	parentHeader, header *block.Header, parentShardState *shard.State, shardID uint32,
 ) (shard.SlotList, shard.SlotList, shard.SlotList, error) {
-	// TODO ek – retrieving by parent number (blockNum - 1) doesn't work,
-	//  while it is okay with hash.  Sounds like DB inconsistency.
-	//  Figure out why.
-	parentHeader := bc.GetHeaderByHash(header.ParentHash())
-	if parentHeader == nil {
-		return nil, nil, nil, ctxerror.New(
-			"cannot find parent block header in DB",
-			"parentHash", header.ParentHash(),
-		)
-	}
-	parentShardState, err := bc.ReadShardState(parentHeader.Epoch())
-	if err != nil {
-		return nil, nil, nil, ctxerror.New(
-			"cannot read shard state", "epoch", parentHeader.Epoch(),
-		).WithCause(err)
-	}
-
 	parentCommittee, err := parentShardState.FindCommitteeByID(shardID)
 
 	if err != nil {
@@ -117,7 +98,6 @@ func bumpCount(
 	signers []signerKind,
 	stakedAddrSet map[common.Address]struct{},
 ) error {
-	blocksPerEpoch := shard.Schedule.BlocksPerEpoch()
 	for _, subset := range signers {
 		for i := range subset.committee {
 			addr := subset.committee[i].EcdsaAddress
@@ -143,12 +123,6 @@ func bumpCount(
 				)
 			}
 
-			if err := computeAndMutateEPOSStatus(
-				bc, state, wrapper, blocksPerEpoch,
-			); err != nil {
-				return err
-			}
-
 			if err := state.UpdateValidatorWrapper(
 				addr, wrapper,
 			); err != nil {
@@ -183,8 +157,7 @@ type Reader interface {
 // ComputeCurrentSigning returns (signed, toSign, quotient, error)
 func ComputeCurrentSigning(
 	snapshot, wrapper *staking.ValidatorWrapper,
-	blocksPerEpoch uint64,
+) *staking.Computed {
-) (*staking.Computed, error) {
 	statsNow, snapSigned, snapToSign :=
 		wrapper.Counters,
 		snapshot.Counters.NumBlocksSigned,
@@ -193,37 +166,32 @@ func ComputeCurrentSigning(
 	signed, toSign :=
 		new(big.Int).Sub(statsNow.NumBlocksSigned, snapSigned),
 		new(big.Int).Sub(statsNow.NumBlocksToSign, snapToSign)
-	leftToGo := blocksPerEpoch - toSign.Uint64()
 
 	computed := staking.NewComputed(
-		signed, toSign, leftToGo, numeric.ZeroDec(), true,
+		signed, toSign, 0, numeric.ZeroDec(), true,
 	)
 
 	if toSign.Cmp(common.Big0) == 0 {
 		utils.Logger().Info().
 			Msg("toSign is 0, perhaps did not receive crosslink proving signing")
-		return computed, nil
+		return computed
 	}
 
 	if signed.Sign() == -1 {
-		return nil, errors.Wrapf(
+		// Shouldn't happen
-			errNegativeSign, "diff for signed period wrong: stat %s, snapshot %s",
+		utils.Logger().Error().Msg("negative number of signed blocks")
-			statsNow.NumBlocksSigned.String(), snapSigned.String(),
-		)
 	}
 
 	if toSign.Sign() == -1 {
-		return nil, errors.Wrapf(
+		// Shouldn't happen
-			errNegativeSign, "diff for toSign period wrong: stat %s, snapshot %s",
+		utils.Logger().Error().Msg("negative number of blocks to sign")
-			statsNow.NumBlocksToSign.String(), snapToSign.String(),
-		)
 	}
 
 	s1, s2 :=
 		numeric.NewDecFromBigInt(signed), numeric.NewDecFromBigInt(toSign)
 	computed.Percentage = s1.Quo(s2)
 	computed.IsBelowThreshold = IsBelowSigningThreshold(computed.Percentage)
-	return computed, nil
+	return computed
 }
 
 // IsBelowSigningThreshold ..
@@ -231,30 +199,30 @@ func IsBelowSigningThreshold(quotient numeric.Dec) bool {
 	return quotient.LTE(measure)
 }
 
-// computeAndMutateEPOSStatus sets the validator to
+// ComputeAndMutateEPOSStatus sets the validator to
 // inactive and thereby keeping it out of
 // consideration in the pool of validators for
 // whenever committee selection happens in future, the
 // signing threshold is 66%
-func computeAndMutateEPOSStatus(
+func ComputeAndMutateEPOSStatus(
 	bc Reader,
 	state *state.DB,
-	wrapper *staking.ValidatorWrapper,
+	addr common.Address,
-	blocksPerEpoch uint64,
 ) error {
 	utils.Logger().Info().Msg("begin compute for availability")
 
-	snapshot, err := bc.ReadValidatorSnapshot(wrapper.Address)
+	wrapper, err := state.ValidatorWrapper(addr)
 	if err != nil {
 		return err
 	}
 
-	computed, err := ComputeCurrentSigning(snapshot, wrapper, blocksPerEpoch)
+	snapshot, err := bc.ReadValidatorSnapshot(wrapper.Address)
-
 	if err != nil {
 		return err
 	}
 
+	computed := ComputeCurrentSigning(snapshot, wrapper)
+
 	utils.Logger().Info().
 		Str("signed", computed.Signed.String()).
 		Str("to-sign", computed.ToSign.String()).
@@ -271,11 +239,14 @@ func computeAndMutateEPOSStatus(
 			Str("threshold", measure.String()).
 			Msg("validator failed availability threshold, set to inactive")
 	default:
-		// TODO we need to take care of the situation when a validator
+		// Default is no-op so validator who wants to leave the committee can actually leave.
-		// wants to stop validating, but if they turns their validator
+	}
-		// to inactive and his node is still running,
+
-		// then the status will be turned back to active automatically.
+	if err := state.UpdateValidatorWrapper(
-		wrapper.Status = effective.Active
+		addr, wrapper,
+	); err != nil {
+		const msg = "[ComputeAndMutateEPOSStatus] failed update validator info"
+		return ctxerror.New(msg).WithCause(err)
 	}
 
 	return nil

staking/types/validator.go

@@ -563,16 +563,6 @@ func UpdateValidatorFromEditMsg(validator *Validator, edit *EditValidator) error
 	return nil
 }
 
-// IsEligibleForEPoSAuction ..
-func IsEligibleForEPoSAuction(validator *ValidatorWrapper) bool {
-	switch validator.Status {
-	case effective.Active:
-		return true
-	default:
-		return false
-	}
-}
-
 // String returns a human readable string representation of a validator.
 func (v Validator) String() string {
 	s, _ := json.Marshal(v)