[reward] Abstract out block-reward, use rewarder, check result correct (#1820)

* [reward] Factor out interface for block-reward

* [reward] Use factored out block rewarder which is actually same object as quorum.Decider

* [quorum] Fix type error because of silly internal/common.Address vs common.Address of ethereum

* [testing] Somehow this port being in tandem with previously used one fixes a build timing issue

consensus/consensus_leader_msg_test.go

@@ -46,7 +46,7 @@ func TestConstructAnnounceMessage(test *testing.T) {
 func TestConstructPreparedMessage(test *testing.T) {
 	leaderPriKey := bls.RandPrivateKey()
 	leaderPubKey := leaderPriKey.GetPublicKey()
-	leader := p2p.Peer{IP: "127.0.0.1", Port: "6000", ConsensusPubKey: leaderPubKey}
+	leader := p2p.Peer{IP: "127.0.0.1", Port: "19999", ConsensusPubKey: leaderPubKey}
 
 	validatorPriKey := bls.RandPrivateKey()
 	validatorPubKey := leaderPriKey.GetPublicKey()

consensus/engine/consensus_engine.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/harmony-one/harmony/block"
+	"github.com/harmony-one/harmony/consensus/reward"
 	"github.com/harmony-one/harmony/core/state"
 	"github.com/harmony-one/harmony/core/types"
 	"github.com/harmony-one/harmony/internal/params"
@@ -73,6 +74,12 @@ type Engine interface {
 	// rules of a particular engine. The changes are executed inline.
 	Prepare(chain ChainReader, header *block.Header) error
 
+	// Rewarder handles the distribution of block rewards
+	Rewarder() reward.Distributor
+
+	// SetRewarder assigns the Distributor used in block reward
+	SetRewarder(reward.Distributor)
+
 	// Finalize runs any post-transaction state modifications (e.g. block rewards)
 	// and assembles the final block.
 	// Note: The block header and state database might be updated to reflect any

consensus/quorum/one-node-one-vote.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/harmony-one/bls/ffi/go/bls"
-	common2 "github.com/harmony-one/harmony/internal/common"
 	"github.com/harmony-one/harmony/internal/utils"
 	// "github.com/harmony-one/harmony/staking/effective"
 )
@@ -20,10 +19,6 @@ func (v *uniformVoteWeight) Policy() Policy {
 	return SuperMajorityVote
 }
 
-// func (v *uniformVoteWeight) SetShardIDProvider(p func() (uint32, error)) {
-// 	v.p = p
-// }
-
 // IsQuorumAchieved ..
 func (v *uniformVoteWeight) IsQuorumAchieved(p Phase) bool {
 	r := v.SignersCount(p) >= v.QuorumThreshold().Int64()
@@ -59,7 +54,7 @@ func (v *uniformVoteWeight) ToggleActive(*bls.PublicKey) bool {
 func (v *uniformVoteWeight) Award(
 	// Here hook is the callback which gets the amount the earner is due in just reward
 	// up to the hook to do side-effects like write the statedb
-	Pie *big.Int, earners []common2.Address, hook func(earner common.Address, due *big.Int),
+	Pie *big.Int, earners []common.Address, hook func(earner common.Address, due *big.Int),
 ) *big.Int {
 	payout := big.NewInt(0)
 	last := big.NewInt(0)

consensus/quorum/one-node-staked-vote.go

@@ -3,8 +3,8 @@ package quorum
 import (
 	"math/big"
 
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/harmony-one/bls/ffi/go/bls"
-	"github.com/harmony-one/harmony/internal/common"
 	"github.com/harmony-one/harmony/numeric"
 	"github.com/harmony-one/harmony/shard"
 )

consensus/reward/rewarder.go

@@ -0,0 +1,16 @@
+package reward
+
+import (
+	"math/big"
+
+	"github.com/ethereum/go-ethereum/common"
+)
+
+// Distributor ..
+type Distributor interface {
+	Award(
+		Pie *big.Int,
+		earners []common.Address,
+		hook func(earner common.Address, due *big.Int),
+	) (payout *big.Int)
+}

internal/chain/engine.go

@@ -8,6 +8,7 @@ import (
 	"github.com/harmony-one/bls/ffi/go/bls"
 	"github.com/harmony-one/harmony/block"
 	"github.com/harmony-one/harmony/consensus/engine"
+	"github.com/harmony-one/harmony/consensus/reward"
 	"github.com/harmony-one/harmony/core/state"
 	"github.com/harmony-one/harmony/core/types"
 	"github.com/harmony-one/harmony/internal/ctxerror"
@@ -19,10 +20,22 @@ import (
 	"golang.org/x/crypto/sha3"
 )
 
-type engineImpl struct{}
+type engineImpl struct {
+	d reward.Distributor
+}
 
 // Engine is an algorithm-agnostic consensus engine.
-var Engine = &engineImpl{}
+var Engine = &engineImpl{nil}
+
+// Rewarder handles the distribution of block rewards
+func (e *engineImpl) Rewarder() reward.Distributor {
+	return e.d
+}
+
+// SetRewarder ..
+func (e *engineImpl) SetRewarder(d reward.Distributor) {
+	e.d = d
+}
 
 // SealHash returns the hash of a block prior to it being sealed.
 func (e *engineImpl) SealHash(header *block.Header) (hash common.Hash) {
@@ -156,7 +169,7 @@ func (e *engineImpl) Finalize(
 	incxs []*types.CXReceiptsProof, stks []*staking.StakingTransaction) (*types.Block, error) {
 	// Accumulate any block and uncle rewards and commit the final state root
 	// Header seems complete, assemble into a block and return
-	if err := AccumulateRewards(chain, state, header); err != nil {
+	if err := AccumulateRewards(chain, state, header, e.Rewarder()); err != nil {
 		return nil, ctxerror.New("cannot pay block reward").WithCause(err)
 	}
 	header.SetRoot(state.IntermediateRoot(chain.Config().IsS3(header.Epoch())))

internal/chain/reward.go

@@ -8,21 +8,26 @@ import (
 	"github.com/harmony-one/harmony/block"
 	"github.com/harmony-one/harmony/common/denominations"
 	"github.com/harmony-one/harmony/consensus/engine"
+	"github.com/harmony-one/harmony/consensus/reward"
 	"github.com/harmony-one/harmony/core/state"
 	bls2 "github.com/harmony-one/harmony/crypto/bls"
 	common2 "github.com/harmony-one/harmony/internal/common"
 	"github.com/harmony-one/harmony/internal/ctxerror"
 	"github.com/harmony-one/harmony/internal/utils"
+	"github.com/pkg/errors"
 )
 
-// BlockReward is the block reward, to be split evenly among block signers.
-var BlockReward = new(big.Int).Mul(big.NewInt(24), big.NewInt(denominations.One))
+var (
+	// BlockReward is the block reward, to be split evenly among block signers.
+	BlockReward                  = new(big.Int).Mul(big.NewInt(24), big.NewInt(denominations.One))
+	errPayoutNotEqualBlockReward = errors.New("total payout not equal to blockreward")
+)
 
 // AccumulateRewards credits the coinbase of the given block with the mining
 // reward. The total reward consists of the static block reward and rewards for
 // included uncles. The coinbase of each uncle block is also rewarded.
 func AccumulateRewards(
-	bc engine.ChainReader, state *state.DB, header *block.Header,
+	bc engine.ChainReader, state *state.DB, header *block.Header, rewarder reward.Distributor,
 ) error {
 	blockNum := header.Number().Uint64()
 	if blockNum == 0 {
@@ -72,9 +77,9 @@ func AccumulateRewards(
 	if err := mask.SetMask(header.LastCommitBitmap()); err != nil {
 		return ctxerror.New("cannot set group sig mask bits").WithCause(err)
 	}
-	totalAmount := big.NewInt(0)
-	var accounts []common.Address
-	signers := []string{}
+
+	accounts := []common.Address{}
+
 	for idx, member := range parentCommittee.NodeList {
 		if signed, err := mask.IndexEnabled(idx); err != nil {
 			return ctxerror.New("cannot check for committer bit",
@@ -85,19 +90,33 @@ func AccumulateRewards(
 		}
 	}
 
-	numAccounts := big.NewInt(int64(len(accounts)))
-	last := new(big.Int)
-	for i, account := range accounts {
-		cur := new(big.Int)
-		cur.Mul(BlockReward, big.NewInt(int64(i+1))).Div(cur, numAccounts)
-		diff := new(big.Int).Sub(cur, last)
-		signers = append(signers, common2.MustAddressToBech32(account))
-		state.AddBalance(account, diff)
-		totalAmount = new(big.Int).Add(totalAmount, diff)
-		last = cur
+	type t struct {
+		common.Address
+		*big.Int
 	}
+	signers := []string{}
+	payable := []t{}
+
+	totalAmount := rewarder.Award(
+		BlockReward, accounts, func(receipient common.Address, amount *big.Int) {
+			signers = append(signers, common2.MustAddressToBech32(receipient))
+			payable = append(payable, t{receipient, amount})
+		})
+
+	if totalAmount.Cmp(BlockReward) != 0 {
+		utils.Logger().Error().
+			Int64("block-reward", BlockReward.Int64()).
+			Int64("total-amount-paid-out", totalAmount.Int64()).
+			Msg("Total paid out was not equal to block-reward")
+		return errors.Wrapf(errPayoutNotEqualBlockReward, "payout "+totalAmount.String())
+	}
+
+	for i := range payable {
+		state.AddBalance(payable[i].Address, payable[i].Int)
+	}
+
 	header.Logger(utils.Logger()).Debug().
-		Str("NumAccounts", numAccounts.String()).
+		Int("NumAccounts", len(accounts)).
 		Str("TotalAmount", totalAmount.String()).
 		Strs("Signers", signers).
 		Msg("[Block Reward] Successfully paid out block reward")

node/node.go

@@ -17,6 +17,7 @@ import (
 	"github.com/harmony-one/harmony/api/service/syncing/downloader"
 	"github.com/harmony-one/harmony/block"
 	"github.com/harmony-one/harmony/consensus"
+	"github.com/harmony-one/harmony/consensus/reward"
 	"github.com/harmony-one/harmony/contracts"
 	"github.com/harmony-one/harmony/core"
 	"github.com/harmony-one/harmony/core/types"
@@ -514,6 +515,7 @@ func New(host p2p.Host, consensusObj *consensus.Consensus, chainDBFactory shardc
 		node.pendingTransactions = make(map[common.Hash]*types.Transaction)
 		node.pendingStakingTransactions = make(map[common.Hash]*staking.StakingTransaction)
 		node.Consensus.VerifiedNewBlock = make(chan *types.Block)
+		chain.Engine.SetRewarder(node.Consensus.Decider.(reward.Distributor))
 		// the sequence number is the next block number to be added in consensus protocol, which is always one more than current chain header block
 		node.Consensus.SetBlockNum(blockchain.CurrentBlock().NumberU64() + 1)