From 4e150779d393b88eacc4e8064310e26333001dbf Mon Sep 17 00:00:00 2001 From: Rongjian Lan Date: Mon, 23 Mar 2020 22:13:50 -0700 Subject: [PATCH 1/2] Fix APR's nill header issue; Compute APR based on last epoch --- consensus/votepower/roster.go | 5 +- core/blockchain.go | 6 +- core/offchain.go | 2 +- shard/committee/assignment.go | 1 + staking/apr/compute.go | 212 ++++++++++++---------------------- 5 files changed, 80 insertions(+), 146 deletions(-) diff --git a/consensus/votepower/roster.go b/consensus/votepower/roster.go index a24825f77..3a7548f20 100644 --- a/consensus/votepower/roster.go +++ b/consensus/votepower/roster.go @@ -204,7 +204,10 @@ func Compute(subComm *shard.Committee, epoch *big.Int) (*Roster, error) { ourPercentage = ourPercentage.Add(member.OverallPercent) } - roster.Voters[staked[i].BlsPublicKey] = &member + // TODO: make sure external user's BLS key can be same as harmony's bls keys + if _, ok := roster.Voters[staked[i].BlsPublicKey]; !ok { + roster.Voters[staked[i].BlsPublicKey] = &member + } } // NOTE Enforce voting power sums to one, diff --git a/core/blockchain.go b/core/blockchain.go index 6b75284ca..9c55fc502 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -2266,6 +2266,7 @@ func (bc *BlockChain) ReadValidatorStats( // UpdateValidatorVotingPower writes the voting power for the committees func (bc *BlockChain) UpdateValidatorVotingPower( batch rawdb.DatabaseWriter, + block *types.Block, newEpochSuperCommittee, currentEpochSuperCommittee *shard.State, // NOTE Do not update this state, only read from state *state.DB, @@ -2287,7 +2288,6 @@ func (bc *BlockChain) UpdateValidatorVotingPower( } rosters[i] = roster } - blkPerEpoch := shard.Schedule.BlocksPerEpoch() networkWide := votepower.AggregateRosters(rosters) for key, value := range networkWide { stats, err := rawdb.ReadValidatorStats(bc.db, key) @@ -2304,9 +2304,9 @@ func (bc *BlockChain) UpdateValidatorVotingPower( if err != nil { return err } + aprComputed, err := apr.ComputeForValidator( - bc, newEpochSuperCommittee.Epoch, - state, wrapper, blkPerEpoch, + bc, block, wrapper, ) if err == nil && aprComputed != nil { a := *aprComputed diff --git a/core/offchain.go b/core/offchain.go index 9321e4210..9cae21996 100644 --- a/core/offchain.go +++ b/core/offchain.go @@ -133,7 +133,7 @@ func (bc *BlockChain) CommitOffChainData( header.ShardState(), ); err == nil { if err := bc.UpdateValidatorVotingPower( - batch, shardState, currentSuperCommittee, state, + batch, block, shardState, currentSuperCommittee, state, ); err != nil { utils.Logger(). Err(err). diff --git a/shard/committee/assignment.go b/shard/committee/assignment.go index 6c31aafee..e442fb566 100644 --- a/shard/committee/assignment.go +++ b/shard/committee/assignment.go @@ -292,6 +292,7 @@ func eposStakedCommittee( } } + // TODO: make sure external validator BLS key are also not duplicate to Harmony's keys completedEPoSRound, err := NewEPoSRound(stakerReader) if err != nil { diff --git a/staking/apr/compute.go b/staking/apr/compute.go index b1bb7bc34..bd242e558 100644 --- a/staking/apr/compute.go +++ b/staking/apr/compute.go @@ -1,11 +1,12 @@ package apr import ( + "github.com/harmony-one/harmony/core/types" + "github.com/harmony-one/harmony/shard" "math/big" "github.com/ethereum/go-ethereum/common" "github.com/harmony-one/harmony/block" - "github.com/harmony-one/harmony/core/state" "github.com/harmony-one/harmony/internal/params" "github.com/harmony-one/harmony/internal/utils" "github.com/harmony-one/harmony/numeric" @@ -15,6 +16,7 @@ import ( // Reader .. type Reader interface { + GetHeaderByNumber(number uint64) *block.Header Config() *params.ChainConfig GetHeaderByHash(hash common.Hash) *block.Header // GetHeader retrieves a block header from the database by hash and number. @@ -27,7 +29,7 @@ type Reader interface { } const ( - secondsInYear = int64(31_557_600) + secondsInYear = int64(31557600) ) var ( @@ -35,14 +37,13 @@ var ( ) func expectedRewardPerYear( - oneEpochAgo, twoEpochAgo *block.Header, - oneSnapshotAgo, twoSnapshotAgo *staking.ValidatorWrapper, - blocksPerEpoch uint64, + now, oneEpochAgo *block.Header, + curValidator, snapshotLastEpoch *staking.ValidatorWrapper, ) (*big.Int, error) { - oneTAgo, twoTAgo := oneEpochAgo.Time(), twoEpochAgo.Time() + timeNow, oneTAgo := now.Time(), oneEpochAgo.Time() diffTime, diffReward := - new(big.Int).Sub(twoTAgo, oneTAgo), - new(big.Int).Sub(twoSnapshotAgo.BlockReward, oneSnapshotAgo.BlockReward) + new(big.Int).Sub(timeNow, oneTAgo), + new(big.Int).Sub(curValidator.BlockReward, snapshotLastEpoch.BlockReward) // impossibility but keep sane if diffTime.Sign() == -1 { @@ -55,66 +56,15 @@ func expectedRewardPerYear( // TODO some more sanity checks of some sort? expectedValue := new(big.Int).Div(diffReward, diffTime) expectedPerYear := new(big.Int).Mul(expectedValue, oneYear) - utils.Logger().Info(). + utils.Logger().Info().Interface("now", curValidator).Interface("before", snapshotLastEpoch). Uint64("diff-reward", diffReward.Uint64()). Uint64("diff-time", diffTime.Uint64()). - Uint64("expected-value", expectedValue.Uint64()). - Uint64("expected-per-year", expectedPerYear.Uint64()). + Interface("expected-value", expectedValue). + Interface("expected-per-year", expectedPerYear). Msg("expected reward per year computed") return expectedPerYear, nil } -func pastTwoEpochHeaders( - bc Reader, -) (*block.Header, *block.Header, error) { - current := bc.CurrentHeader() - epochNow := current.Epoch() - oneEpochAgo, twoEpochAgo := - new(big.Int).Sub(epochNow, common.Big1), - new(big.Int).Sub(epochNow, common.Big2) - - bottomOut := new(big.Int).Add( - bc.Config().StakingEpoch, - common.Big3, - ) - - var oneAgoHeader, twoAgoHeader **block.Header - - for e1, e2 := false, false; ; { - current = bc.GetHeader(current.ParentHash(), current.Number().Uint64()-1) - - if current == nil { - return nil, nil, errors.New("could not go up parent") - } - - if current.Epoch().Cmp(bottomOut) == 0 { - if twoAgoHeader == nil || oneAgoHeader == nil { - return nil, nil, errors.New( - "could not find headers for apr computation", - ) - } - } - - switch { - // haven't found either epoch yet - case !e1 && !e2: - if current.Epoch().Cmp(oneEpochAgo) == 0 { - e1 = true - oneAgoHeader = ¤t - continue - } - case e1 && !e2: - if current.Epoch().Cmp(twoEpochAgo) == 0 { - e2 = true - twoAgoHeader = ¤t - break - } - } - } - - return *oneAgoHeader, *twoAgoHeader, nil -} - var ( zero = numeric.ZeroDec() ) @@ -122,85 +72,65 @@ var ( // ComputeForValidator .. func ComputeForValidator( bc Reader, - now *big.Int, - state *state.DB, + block *types.Block, validatorNow *staking.ValidatorWrapper, - blocksPerEpoch uint64, ) (*numeric.Dec, error) { - return &zero, nil - // twoEpochAgo, oneEpochAgo, zero := - // new(big.Int).Sub(now, common.Big2), - // new(big.Int).Sub(now, common.Big1), - // numeric.ZeroDec() - - // utils.Logger().Info(). - // Uint64("now", now.Uint64()). - // Uint64("two-epoch-ago", twoEpochAgo.Uint64()). - // Uint64("one-epoch-ago", oneEpochAgo.Uint64()). - // Msg("apr - begin compute for validator ") - - // twoSnapshotAgo, err := bc.ReadValidatorSnapshotAtEpoch( - // twoEpochAgo, - // validatorNow.Address, - // ) - - // if err != nil { - // return &zero, nil - // } - - // oneSnapshotAgo, err := bc.ReadValidatorSnapshotAtEpoch( - // oneEpochAgo, - // validatorNow.Address, - // ) - - // if err != nil { - // return &zero, nil - // } - - // blockNumAtTwoEpochAgo, blockNumAtOneEpochAgo := - // shard.Schedule.EpochLastBlock(twoEpochAgo.Uint64()), - // shard.Schedule.EpochLastBlock(oneEpochAgo.Uint64()) - - // headerOneEpochAgo, headerTwoEpochAgo, err := pastTwoEpochHeaders(bc) - - // // TODO Figure out why this is happening - // if headerOneEpochAgo == nil || headerTwoEpochAgo == nil || err != nil { - // utils.Logger().Debug(). - // Msgf("apr compute headers epochs ago %+v %+v %+v %+v %+v %+v", - // twoEpochAgo, oneEpochAgo, - // blockNumAtTwoEpochAgo, blockNumAtOneEpochAgo, - // headerOneEpochAgo, headerTwoEpochAgo, - // ) - // return &zero, nil - // } - - // utils.Logger().Info(). - // RawJSON("current-epoch-header", []byte(bc.CurrentHeader().String())). - // RawJSON("one-epoch-ago-header", []byte(headerOneEpochAgo.String())). - // RawJSON("two-epoch-ago-header", []byte(headerTwoEpochAgo.String())). - // Msg("headers used for apr computation") - - // estimatedRewardPerYear, err := expectedRewardPerYear( - // headerOneEpochAgo, headerTwoEpochAgo, - // oneSnapshotAgo, twoSnapshotAgo, - // blocksPerEpoch, - // ) - - // if err != nil { - // return nil, err - // } - - // if estimatedRewardPerYear.Cmp(common.Big0) == 0 { - // return &zero, nil - // } - - // total := numeric.NewDecFromBigInt(validatorNow.TotalDelegation()) - // if total.IsZero() { - // return nil, errors.New("zero total delegation will cause div by zero") - // } - - // result := numeric.NewDecFromBigInt(estimatedRewardPerYear).Quo( - // total, - // ) - // return &result, nil + oneEpochAgo, zero := + new(big.Int).Sub(block.Epoch(), common.Big1), + numeric.ZeroDec() + + utils.Logger().Info(). + Uint64("now", block.Epoch().Uint64()). + Uint64("one-epoch-ago", oneEpochAgo.Uint64()). + Msg("apr - begin compute for validator ") + + oneSnapshotAgo, err := bc.ReadValidatorSnapshotAtEpoch( + oneEpochAgo, + validatorNow.Address, + ) + + if err != nil { + return &zero, nil + } + + blockNumAtOneEpochAgo := shard.Schedule.EpochLastBlock(oneEpochAgo.Uint64()) + + headerOneEpochAgo := bc.GetHeaderByNumber(blockNumAtOneEpochAgo) + if block.Header() == nil || headerOneEpochAgo == nil || err != nil { + utils.Logger().Debug(). + Msgf("apr compute headers epochs ago %+v %+v %+v", + oneEpochAgo, + blockNumAtOneEpochAgo, + headerOneEpochAgo, + ) + return &zero, nil + } + + utils.Logger().Info(). + RawJSON("current-epoch-header", []byte(bc.CurrentHeader().String())). + RawJSON("one-epoch-ago-header", []byte(headerOneEpochAgo.String())). + Msg("headers used for apr computation") + + estimatedRewardPerYear, err := expectedRewardPerYear( + block.Header(), headerOneEpochAgo, + validatorNow, oneSnapshotAgo, + ) + + if err != nil { + return nil, err + } + + if estimatedRewardPerYear.Cmp(common.Big0) == 0 { + return &zero, nil + } + + total := numeric.NewDecFromBigInt(validatorNow.TotalDelegation()) + if total.IsZero() { + return nil, errors.New("zero total delegation will cause div by zero") + } + + result := numeric.NewDecFromBigInt(estimatedRewardPerYear).Quo( + total, + ) + return &result, nil } From ae19b26787d936152f027fca678f379098872308 Mon Sep 17 00:00:00 2001 From: Rongjian Lan Date: Tue, 24 Mar 2020 10:51:54 -0700 Subject: [PATCH 2/2] Fix comments on log/errors --- consensus/votepower/roster.go | 3 +++ staking/apr/compute.go | 10 +++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/consensus/votepower/roster.go b/consensus/votepower/roster.go index 3a7548f20..48f60fa46 100644 --- a/consensus/votepower/roster.go +++ b/consensus/votepower/roster.go @@ -3,6 +3,7 @@ package votepower import ( "encoding/hex" "encoding/json" + "github.com/harmony-one/harmony/internal/utils" "math/big" "sort" @@ -207,6 +208,8 @@ func Compute(subComm *shard.Committee, epoch *big.Int) (*Roster, error) { // TODO: make sure external user's BLS key can be same as harmony's bls keys if _, ok := roster.Voters[staked[i].BlsPublicKey]; !ok { roster.Voters[staked[i].BlsPublicKey] = &member + } else { + utils.Logger().Debug().Str("blsKey", staked[i].BlsPublicKey.Hex()).Msg("Duplicate BLS key found") } } diff --git a/staking/apr/compute.go b/staking/apr/compute.go index bd242e558..cc4cacdc3 100644 --- a/staking/apr/compute.go +++ b/staking/apr/compute.go @@ -79,7 +79,7 @@ func ComputeForValidator( new(big.Int).Sub(block.Epoch(), common.Big1), numeric.ZeroDec() - utils.Logger().Info(). + utils.Logger().Debug(). Uint64("now", block.Epoch().Uint64()). Uint64("one-epoch-ago", oneEpochAgo.Uint64()). Msg("apr - begin compute for validator ") @@ -90,23 +90,23 @@ func ComputeForValidator( ) if err != nil { - return &zero, nil + return &zero, err } blockNumAtOneEpochAgo := shard.Schedule.EpochLastBlock(oneEpochAgo.Uint64()) headerOneEpochAgo := bc.GetHeaderByNumber(blockNumAtOneEpochAgo) - if block.Header() == nil || headerOneEpochAgo == nil || err != nil { + if block.Header() == nil || headerOneEpochAgo == nil { utils.Logger().Debug(). Msgf("apr compute headers epochs ago %+v %+v %+v", oneEpochAgo, blockNumAtOneEpochAgo, headerOneEpochAgo, ) - return &zero, nil + return &zero, errors.New("can't get headers for APR computation") } - utils.Logger().Info(). + utils.Logger().Debug(). RawJSON("current-epoch-header", []byte(bc.CurrentHeader().String())). RawJSON("one-epoch-ago-header", []byte(headerOneEpochAgo.String())). Msg("headers used for apr computation")