From 2f15cd5da8a81ac05a41bbb916c96cb224731dde Mon Sep 17 00:00:00 2001 From: Rongjian Lan Date: Wed, 18 Mar 2020 18:28:12 -0700 Subject: [PATCH] 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 | 12 +++-- internal/chain/engine.go | 16 ++++++- internal/chain/reward.go | 39 +++++++++++++++-- node/node_handler.go | 9 ++-- shard/committee/assignment.go | 32 +++++++++++++- staking/availability/measure.go | 77 ++++++++++----------------------- staking/types/validator.go | 10 ----- 7 files changed, 116 insertions(+), 79 deletions(-) diff --git a/hmy/api_backend.go b/hmy/api_backend.go index 89728f892..941124de4 100644 --- a/hmy/api_backend.go +++ b/hmy/api_backend.go @@ -356,13 +356,10 @@ func (b *APIBackend) GetValidatorInformation( return defaultReply, nil } - computed, err := availability.ComputeCurrentSigning( - snapshot, wrapper, shard.Schedule.BlocksPerEpoch(), + computed := availability.ComputeCurrentSigning( + snapshot, wrapper, ) - - if err != nil { - return defaultReply, nil - } + computed.BlocksLeftInEpoch = shard.Schedule.BlocksPerEpoch() - computed.ToSign.Uint64() 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 { diff --git a/internal/chain/engine.go b/internal/chain/engine.go index 1e76bb24e..1d2c0ef8e 100644 --- a/internal/chain/engine.go +++ b/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 diff --git a/internal/chain/reward.go b/internal/chain/reward.go index 644ed38b7..9fe567193 100644 --- a/internal/chain/reward.go +++ b/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 } - - _, signers, _, err := availability.BallotResult(bc, header, header.ShardID()) + parentShardState, err := bc.ReadShardState(parentHeader.Epoch()) + 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 diff --git a/node/node_handler.go b/node/node_handler.go index 637c842bf..ddb66842e 100644 --- a/node/node_handler.go +++ b/node/node_handler.go @@ -494,10 +494,11 @@ func (node *Node) PostConsensusProcessing( if err != nil { return } - computed, err := - availability.ComputeCurrentSigning( - snapshot, wrapper, shard.Schedule.BlocksPerEpoch(), - ) + computed := availability.ComputeCurrentSigning( + snapshot, wrapper, + ) + computed.BlocksLeftInEpoch = shard.Schedule.BlocksPerEpoch() - computed.ToSign.Uint64() + if err != nil && computed.IsBelowThreshold { url := h.Availability.OnDroppedBelowThreshold go func() { diff --git a/shard/committee/assignment.go b/shard/committee/assignment.go index c7ee08358..cae3278de 100644 --- a/shard/committee/assignment.go +++ b/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. diff --git a/staking/availability/measure.go b/staking/availability/measure.go index 7d08bcd6d..fa642d934 100644 --- a/staking/availability/measure.go +++ b/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, error) { +) *staking.Computed { 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( - errNegativeSign, "diff for signed period wrong: stat %s, snapshot %s", - statsNow.NumBlocksSigned.String(), snapSigned.String(), - ) + // Shouldn't happen + utils.Logger().Error().Msg("negative number of signed blocks") } if toSign.Sign() == -1 { - return nil, errors.Wrapf( - errNegativeSign, "diff for toSign period wrong: stat %s, snapshot %s", - statsNow.NumBlocksToSign.String(), snapToSign.String(), - ) + // Shouldn't happen + utils.Logger().Error().Msg("negative number of blocks to sign") } 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, - blocksPerEpoch uint64, + addr common.Address, ) 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 - // 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. - wrapper.Status = effective.Active + // Default is no-op so validator who wants to leave the committee can actually leave. + } + + if err := state.UpdateValidatorWrapper( + addr, wrapper, + ); err != nil { + const msg = "[ComputeAndMutateEPOSStatus] failed update validator info" + return ctxerror.New(msg).WithCause(err) } return nil diff --git a/staking/types/validator.go b/staking/types/validator.go index 92410fbbf..dc6b6f11c 100644 --- a/staking/types/validator.go +++ b/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)