Fix block rewards; Add staking signer check; Allow 0 MaxTotalDelegation (#1887)

* Fix block rewards; Add staking signer check; Allow 0 MaxTotalDelegation

* Fix lint

core/state_transition.go

@@ -32,6 +32,7 @@ import (
 )
 
 var (
+	errInvalidSigner               = errors.New("invalid signer for staking transaction")
 	errInsufficientBalanceForGas   = errors.New("insufficient balance to pay for gas")
 	errInsufficientBalanceForStake = errors.New("insufficient balance to stake")
 	errValidatorExist              = errors.New("staking validator already exists")
@@ -302,6 +303,9 @@ func (st *StateTransition) StakingTransitionDb() (usedGas uint64, err error) {
 	switch msg.Type() {
 	case types.StakeNewVal:
 		stkMsg := &staking.CreateValidator{}
+		if msg.From() != stkMsg.ValidatorAddress {
+			return 0, errInvalidSigner
+		}
 		if err = rlp.DecodeBytes(msg.Data(), stkMsg); err != nil {
 			return 0, err
 		}
@@ -309,6 +313,9 @@ func (st *StateTransition) StakingTransitionDb() (usedGas uint64, err error) {
 
 	case types.StakeEditVal:
 		stkMsg := &staking.EditValidator{}
+		if msg.From() != stkMsg.ValidatorAddress {
+			return 0, errInvalidSigner
+		}
 		if err = rlp.DecodeBytes(msg.Data(), stkMsg); err != nil {
 			return 0, err
 		}
@@ -316,6 +323,9 @@ func (st *StateTransition) StakingTransitionDb() (usedGas uint64, err error) {
 
 	case types.Delegate:
 		stkMsg := &staking.Delegate{}
+		if msg.From() != stkMsg.DelegatorAddress {
+			return 0, errInvalidSigner
+		}
 		if err = rlp.DecodeBytes(msg.Data(), stkMsg); err != nil {
 			return 0, err
 		}
@@ -323,12 +333,18 @@ func (st *StateTransition) StakingTransitionDb() (usedGas uint64, err error) {
 
 	case types.Undelegate:
 		stkMsg := &staking.Undelegate{}
+		if msg.From() != stkMsg.DelegatorAddress {
+			return 0, errInvalidSigner
+		}
 		if err = rlp.DecodeBytes(msg.Data(), stkMsg); err != nil {
 			return 0, err
 		}
 		err = st.applyUndelegateTx(stkMsg)
 	case types.CollectRewards:
 		stkMsg := &staking.CollectRewards{}
+		if msg.From() != stkMsg.DelegatorAddress {
+			return 0, errInvalidSigner
+		}
 		if err = rlp.DecodeBytes(msg.Data(), stkMsg); err != nil {
 			return 0, err
 		}

internal/chain/reward.go

@@ -3,7 +3,8 @@ package chain
 import (
 	"math/big"
 	"sort"
-	"sync"
+
+	"github.com/harmony-one/harmony/shard/committee"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/rlp"
@@ -220,90 +221,71 @@ func AccumulateRewards(
 				return err
 			}
 
-			w := sync.WaitGroup{}
-
 			type slotPayable struct {
 				effective numeric.Dec
 				payee     common.Address
 				bucket    int
 				index     int
-				oops      error
 			}
 
-			payable := make(chan slotPayable)
+			allPayables := []slotPayable{}
-
-			slotError := func(err error, receive chan slotPayable) {
-				s := slotPayable{}
-				s.oops = err
-				go func() {
-					receive <- s
-				}()
-			}
 
 			for i := range crossLinks {
-				w.Add(1)
-
-				go func(i int) {
-					defer w.Done()
-					cxLink := crossLinks[i]
-					subCommittee := shard.State{}
-					if err := rlp.DecodeBytes(
-						cxLink.ChainHeader.ShardState(), &subCommittee,
-					); err != nil {
-						slotError(err, payable)
-						return
-					}
 
-					subComm := subCommittee.FindCommitteeByID(cxLink.ShardID())
+				cxLink := crossLinks[i]
-					// _ are the missing signers, later for slashing
-					payableSigners, _, err := blockSigners(cxLink.Header(), subComm)
-					votingPower := votepower.Compute(subComm.Slots)
 
-					if err != nil {
+				shardState, err := bc.ReadShardState(cxLink.ChainHeader.Epoch())
-						slotError(err, payable)
+				if !bc.Config().IsStaking(cxLink.Header().Epoch()) {
-						return
+					shardState, err = committee.WithStakingEnabled.Compute(cxLink.ChainHeader.Epoch(), bc)
-					}
+				}
+
+				if err != nil {
+					// TEMP HACK: IGNORE THE ERROR as THERE IS NO WAY TO VERIFY THE SIG OF FIRST BLOCK OF SHARD FIRST TIME ENTERING STAKING, NO WAY TO FIND THE LAST COMMITEE AS THERE IS GAP
+					// TODO: FIX THIS WITH NEW CROSSLINK FORMAT
+					continue
+				}
 
-					for member := range payableSigners {
+				subComm := shardState.FindCommitteeByID(cxLink.ShardID())
-						voter := votingPower.Voters[payableSigners[member].BlsPublicKey]
+				// _ are the missing signers, later for slashing
-						if !voter.IsHarmonyNode {
+				payableSigners, _, err := blockSigners(cxLink.Header(), subComm)
-							due := defaultReward.Mul(
+
-								voter.EffectivePercent.Quo(votepower.StakersShare),
+				if err != nil {
-							)
+					// TEMP HACK: IGNORE THE ERROR as THERE IS NO WAY TO VERIFY THE SIG OF FIRST BLOCK OF SHARD FIRST TIME ENTERING STAKING, NO WAY TO FIND THE LAST COMMITEE AS THERE IS GAP
-							to := voter.EarningAccount
+					// TODO: FIX THIS WITH NEW CROSSLINK FORMAT
-							go func(signersDue numeric.Dec, addr common.Address, j int) {
+					continue
-								payable <- slotPayable{
+				}
-									effective: signersDue,
+
-									payee:     addr,
+				votingPower := votepower.Compute(payableSigners)
-									bucket:    i,
+				for j := range payableSigners {
-									index:     j,
+					voter := votingPower.Voters[payableSigners[j].BlsPublicKey]
-									oops:      nil,
+					if !voter.IsHarmonyNode && !voter.EffectivePercent.IsZero() {
-								}
+						due := defaultReward.Mul(
-							}(due, to, member)
+							voter.EffectivePercent.Quo(votepower.StakersShare),
-						}
+						)
+						to := voter.EarningAccount
+						allPayables = append(allPayables, slotPayable{
+							effective: due,
+							payee:     to,
+							bucket:    i,
+							index:     j,
+						})
 					}
-				}(i)
+				}
+
 			}
 
-			w.Wait()
 			resultsHandle := make([][]slotPayable, len(crossLinks))
 			for i := range resultsHandle {
 				resultsHandle[i] = []slotPayable{}
 			}
 
-			for payThem := range payable {
+			for _, payThem := range allPayables {
 				bucket := payThem.bucket
 				resultsHandle[bucket] = append(resultsHandle[bucket], payThem)
 			}
 
 			// Check if any errors and sort each bucket to enforce order
 			for bucket := range resultsHandle {
-				for payThem := range resultsHandle[bucket] {
-					if err := resultsHandle[bucket][payThem].oops; err != nil {
-						return err
-					}
-				}
-
 				sort.SliceStable(resultsHandle[bucket],
 					func(i, j int) bool {
 						return resultsHandle[bucket][i].index < resultsHandle[bucket][j].index

staking/types/validator.go

@@ -65,7 +65,7 @@ type Validator struct {
 	UnbondingHeight *big.Int `json:"unbonding_height" yaml:"unbonding_height"`
 	// validator's self declared minimum self delegation
 	MinSelfDelegation *big.Int `json:"min_self_delegation" yaml:"min_self_delegation"`
-	// maximum total delgation allowed
+	// maximum total delegation allowed
 	MaxTotalDelegation *big.Int `json:"max_total_delegation" yaml:"max_total_delegation"`
 	// Is the validator active in the validating process or not
 	Active bool `json:"active" yaml:"active"`
@@ -102,21 +102,24 @@ func (w *ValidatorWrapper) SanityCheck() error {
 		return errMinSelfDelegationTooSmall
 	}
 
-	// MaxTotalDelegation must not be less than MinSelfDelegation
-	if w.Validator.MaxTotalDelegation.Cmp(w.Validator.MinSelfDelegation) < 0 {
-		return errInvalidMaxTotalDelegation
-	}
-
 	selfDelegation := w.Delegations[0].Amount
 	// Self delegation must be >= MinSelfDelegation
 	if selfDelegation.Cmp(w.Validator.MinSelfDelegation) < 0 {
 		return errInvalidSelfDelegation
 	}
 
-	totalDelegation := w.TotalDelegation()
+	// Only enforce rules on MaxTotalDelegation is it's > 0; 0 means no limit for max total delegation
-	// Total delegation must be <= MaxTotalDelegation
+	if w.Validator.MaxTotalDelegation != nil && w.Validator.MaxTotalDelegation.Cmp(big.NewInt(0)) > 0 {
-	if totalDelegation.Cmp(w.Validator.MaxTotalDelegation) > 0 {
+		// MaxTotalDelegation must not be less than MinSelfDelegation
-		return errInvalidTotalDelegation
+		if w.Validator.MaxTotalDelegation.Cmp(w.Validator.MinSelfDelegation) < 0 {
+			return errInvalidMaxTotalDelegation
+		}
+
+		totalDelegation := w.TotalDelegation()
+		// Total delegation must be <= MaxTotalDelegation
+		if totalDelegation.Cmp(w.Validator.MaxTotalDelegation) > 0 {
+			return errInvalidTotalDelegation
+		}
 	}
 
 	hundredPercent := numeric.NewDec(1)