Merge pull request #284 from gzliudan/fix-issue-280

fix candidates bugs in eth api

XDCxlending/lendingstate/common.go

@@ -2,16 +2,15 @@ package lendingstate
 
 import (
 	"encoding/json"
-	"github.com/XinFinOrg/XDPoSChain/crypto"
 	"math/big"
 	"time"
 
 	"github.com/XinFinOrg/XDPoSChain/common"
+	"github.com/XinFinOrg/XDPoSChain/crypto"
 )
 
 var (
-	EmptyAddress = "xdc0000000000000000000000000000000000000000"
+	EmptyRoot = common.HexToHash("56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421")
-	EmptyRoot    = common.HexToHash("56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421")
 )
 
 var EmptyHash = common.Hash{}

XDCxlending/lendingstate/lendingitem.go

@@ -2,14 +2,15 @@ package lendingstate
 
 import (
 	"fmt"
+	"math/big"
+	"strconv"
+	"time"
+
 	"github.com/XinFinOrg/XDPoSChain/common"
 	"github.com/XinFinOrg/XDPoSChain/core/state"
 	"github.com/XinFinOrg/XDPoSChain/core/types"
 	"github.com/XinFinOrg/XDPoSChain/crypto/sha3"
 	"github.com/globalsign/mgo/bson"
-	"math/big"
-	"strconv"
-	"time"
 )
 
 const (
@@ -243,7 +244,7 @@ func (l *LendingItem) VerifyLendingSide() error {
 }
 
 func (l *LendingItem) VerifyCollateral(state *state.StateDB) error {
-	if l.CollateralToken.String() == EmptyAddress || l.CollateralToken.String() == l.LendingToken.String() {
+	if l.CollateralToken.IsZero() || l.CollateralToken == l.LendingToken {
 		return fmt.Errorf("invalid collateral %s", l.CollateralToken.Hex())
 	}
 	validCollateral := false
@@ -329,7 +330,7 @@ func (l *LendingItem) EncodedSide() *big.Int {
 	return big.NewInt(1)
 }
 
-//verify signatures
+// verify signatures
 func (l *LendingItem) VerifyLendingSignature() error {
 	V := big.NewInt(int64(l.Signature.V))
 	R := l.Signature.R.Big()

XDCxlending/order_processor.go

@@ -3,6 +3,8 @@ package XDCxlending
 import (
 	"encoding/json"
 	"fmt"
+	"math/big"
+
 	"github.com/XinFinOrg/XDPoSChain/XDCx/tradingstate"
 	"github.com/XinFinOrg/XDPoSChain/XDCxlending/lendingstate"
 	"github.com/XinFinOrg/XDPoSChain/common"
@@ -10,7 +12,6 @@ import (
 	"github.com/XinFinOrg/XDPoSChain/core/state"
 	"github.com/XinFinOrg/XDPoSChain/core/types"
 	"github.com/XinFinOrg/XDPoSChain/log"
-	"math/big"
 )
 
 func (l *Lending) CommitOrder(header *types.Header, coinbase common.Address, chain consensus.ChainContext, statedb *state.StateDB, lendingStateDB *lendingstate.LendingStateDB, tradingStateDb *tradingstate.TradingStateDB, lendingOrderBook common.Hash, order *lendingstate.LendingItem) ([]*lendingstate.LendingTrade, []*lendingstate.LendingItem, error) {
@@ -262,10 +263,9 @@ func (l *Lending) processOrderList(header *types.Header, coinbase common.Address
 			collateralToken = oldestOrder.CollateralToken
 			borrowFee = lendingstate.GetFee(statedb, oldestOrder.Relayer)
 		}
-		if collateralToken.String() == lendingstate.EmptyAddress {
+		if collateralToken.IsZero() {
 			return nil, nil, nil, fmt.Errorf("empty collateral")
 		}
-		collateralPrice := common.BasePrice
 		depositRate, liquidationRate, recallRate := lendingstate.GetCollateralDetail(statedb, collateralToken)
 		if depositRate == nil || depositRate.Sign() <= 0 {
 			return nil, nil, nil, fmt.Errorf("invalid depositRate %v", depositRate)
@@ -953,11 +953,11 @@ func (l *Lending) GetMediumTradePriceBeforeEpoch(chain consensus.ChainContext, s
 	return nil, nil
 }
 
-//LendToken and CollateralToken must meet at least one of following conditions
+// LendToken and CollateralToken must meet at least one of following conditions
-//- Have direct pair in XDCX: lendToken/CollateralToken or CollateralToken/LendToken
+// - Have direct pair in XDCX: lendToken/CollateralToken or CollateralToken/LendToken
-//- Have pairs with XDC:
+// - Have pairs with XDC:
-//-  lendToken/XDC and CollateralToken/XDC
+// -  lendToken/XDC and CollateralToken/XDC
-//-  XDC/lendToken and XDC/CollateralToken
+// -  XDC/lendToken and XDC/CollateralToken
 func (l *Lending) GetCollateralPrices(header *types.Header, chain consensus.ChainContext, statedb *state.StateDB, tradingStateDb *tradingstate.TradingStateDB, collateralToken common.Address, lendingToken common.Address) (*big.Int, *big.Int, error) {
 	// lendTokenXDCPrice: price of ticker lendToken/XDC
 	// collateralXDCPrice: price of ticker collateralToken/XDC

common/types.go

@@ -73,6 +73,9 @@ func BigToHash(b *big.Int) Hash  { return BytesToHash(b.Bytes()) }
 func Uint64ToHash(b uint64) Hash { return BytesToHash(new(big.Int).SetUint64(b).Bytes()) }
 func HexToHash(s string) Hash    { return BytesToHash(FromHex(s)) }
 
+// IsZero returns if a Hash is empty
+func (h Hash) IsZero() bool { return h == Hash{} }
+
 // Get the string representation of the underlying hash
 func (h Hash) Str() string   { return string(h[:]) }
 func (h Hash) Bytes() []byte { return h[:] }
@@ -190,6 +193,9 @@ func IsHexAddress(s string) bool {
 	return len(s) == 2*AddressLength && isHex(s)
 }
 
+// IsZero returns if a address is empty
+func (a Address) IsZero() bool { return a == Address{} }
+
 // Get the string representation of the underlying address
 func (a Address) Str() string   { return string(a[:]) }
 func (a Address) Bytes() []byte { return a[:] }

core/blockchain.go

@@ -2554,8 +2554,8 @@ func (bc *BlockChain) UpdateM1() error {
 		if err != nil {
 			return err
 		}
-		//TODO: smart contract shouldn't return "0x0000000000000000000000000000000000000000"
+		// TODO: smart contract shouldn't return "0x0000000000000000000000000000000000000000"
-		if candidate.String() != "xdc0000000000000000000000000000000000000000" {
+		if !candidate.IsZero() {
 			ms = append(ms, utils.Masternode{Address: candidate, Stake: v})
 		}
 	}

core/lending_pool.go

@@ -431,7 +431,7 @@ func (pool *LendingPool) validateNewLending(cloneStateDb *state.StateDB, cloneLe
 		return ErrInvalidLendingType
 	}
 	if tx.Side() == lendingstate.Borrowing {
-		if tx.CollateralToken().String() == lendingstate.EmptyAddress || tx.CollateralToken().String() == tx.LendingToken().String() {
+		if tx.CollateralToken().IsZero() || tx.CollateralToken() == tx.LendingToken() {
 			return ErrInvalidLendingCollateral
 		}
 		validCollateral := false
@@ -541,7 +541,7 @@ func (pool *LendingPool) validateBalance(cloneStateDb *state.StateDB, cloneLendi
 	// collateralPrice = BTC/USD (eg: 8000 USD)
 	// lendTokenXDCPrice: price of lendingToken in XDC quote
 	var lendTokenXDCPrice, collateralPrice, collateralTokenDecimal *big.Int
-	if collateralToken.String() != lendingstate.EmptyAddress {
+	if !collateralToken.IsZero() {
 		collateralTokenDecimal, err = XDCXServ.GetTokenDecimal(pool.chain, cloneStateDb, collateralToken)
 		if err != nil {
 			return fmt.Errorf("validateOrder: failed to get collateralTokenDecimal. err: %v", err)

core/state/statedb_utils.go

@@ -93,16 +93,17 @@ func GetCandidates(statedb *StateDB) []common.Address {
 	slot := slotValidatorMapping["candidates"]
 	slotHash := common.BigToHash(new(big.Int).SetUint64(slot))
 	arrLength := statedb.GetState(common.HexToAddress(common.MasternodeVotingSMC), slotHash)
-	keys := []common.Hash{}
+	count := arrLength.Big().Uint64()
-	for i := uint64(0); i < arrLength.Big().Uint64(); i++ {
+	rets := make([]common.Address, 0, count)
+
+	for i := uint64(0); i < count; i++ {
 		key := GetLocDynamicArrAtElement(slotHash, i, 1)
-		keys = append(keys, key)
-	}
-	rets := []common.Address{}
-	for _, key := range keys {
 		ret := statedb.GetState(common.HexToAddress(common.MasternodeVotingSMC), key)
-		rets = append(rets, common.HexToAddress(ret.Hex()))
+		if !ret.IsZero() {
+			rets = append(rets, common.HexToAddress(ret.Hex()))
+		}
 	}
+
 	return rets
 }
 

eth/hooks/engine_v1_hooks.go

@@ -238,9 +238,7 @@ func AttachConsensusV1Hooks(adaptor *XDPoS.XDPoS, bc *core.BlockChain, chainConf
 			if err != nil {
 				return nil, err
 			}
-			if address.String() != "xdc0000000000000000000000000000000000000000" {
+			candidates = append(candidates, utils.Masternode{Address: address, Stake: v})
-				candidates = append(candidates, utils.Masternode{Address: address, Stake: v})
-			}
 		}
 		// sort candidates by stake descending
 		sort.Slice(candidates, func(i, j int) bool {

internal/ethapi/api.go

@@ -754,11 +754,10 @@ func (s *PublicBlockChainAPI) GetCandidateStatus(ctx context.Context, coinbaseAd
 			return result, err
 		}
 		candidatesAddresses := state.GetCandidates(statedb)
+		candidates = make([]utils.Masternode, 0, len(candidatesAddresses))
 		for _, address := range candidatesAddresses {
 			v := state.GetCandidateCap(statedb, address)
-			if address.String() != "xdc0000000000000000000000000000000000000000" {
+			candidates = append(candidates, utils.Masternode{Address: address, Stake: v})
-				candidates = append(candidates, utils.Masternode{Address: address, Stake: v})
-			}
 		}
 	}
 	if err != nil || len(candidates) == 0 {
@@ -766,6 +765,7 @@ func (s *PublicBlockChainAPI) GetCandidateStatus(ctx context.Context, coinbaseAd
 		result[fieldSuccess] = false
 		return result, err
 	}
+
 	var maxMasternodes int
 	if s.b.ChainConfig().IsTIPIncreaseMasternodes(block.Number()) {
 		maxMasternodes = common.MaxMasternodesV2
@@ -773,24 +773,19 @@ func (s *PublicBlockChainAPI) GetCandidateStatus(ctx context.Context, coinbaseAd
 		maxMasternodes = common.MaxMasternodes
 	}
 
-	isTopCandidate := false
+	// check penalties from checkpoint headers and modify status of a node to SLASHED if it's in top maxMasternodes candidates.
-	// check penalties from checkpoint headers and modify status of a node to SLASHED if it's in top 150 candidates
+	// if it's SLASHED but it's out of top maxMasternodes, the status should be still PROPOSED.
-	// if it's SLASHED but it's out of top 150, the status should be still PROPOSED
+	isCandidate := false
 	for i := 0; i < len(candidates); i++ {
 		if coinbaseAddress == candidates[i].Address {
-			if i < maxMasternodes {
+			isCandidate = true
-				isTopCandidate = true
-			}
 			result[fieldStatus] = statusProposed
 			result[fieldCapacity] = candidates[i].Stake
 			break
 		}
 	}
-	if !isTopCandidate {
-		return result, nil
-	}
 
-	// Second, Find candidates that have masternode status
+	// Get masternode list
 	if engine, ok := s.b.GetEngine().(*XDPoS.XDPoS); ok {
 		masternodes = engine.GetMasternodesFromCheckpointHeader(header)
 		if len(masternodes) == 0 {
@@ -801,15 +796,30 @@ func (s *PublicBlockChainAPI) GetCandidateStatus(ctx context.Context, coinbaseAd
 	} else {
 		log.Error("Undefined XDPoS consensus engine")
 	}
-	// Set masternode status
+
+	// Set to statusMasternode if it is masternode
 	for _, masternode := range masternodes {
 		if coinbaseAddress == masternode {
 			result[fieldStatus] = statusMasternode
+			if !isCandidate {
+				result[fieldCapacity] = -1
+				log.Warn("Find non-candidate masternode", "masternode", masternode.String(), "checkpointNumber", checkpointNumber, "epoch", epoch, "epochNumber", epochNumber)
+			}
 			return result, nil
 		}
 	}
 
-	// Third, Get penalties list
+	if !isCandidate || len(masternodes) >= maxMasternodes {
+		return result, nil
+	}
+
+	if len(candidates) > maxMasternodes {
+		sort.Slice(candidates, func(i, j int) bool {
+			return candidates[i].Stake.Cmp(candidates[j].Stake) > 0
+		})
+	}
+
+	// Get penalties list
 	penalties = append(penalties, header.Penalties...)
 	// check last 5 epochs to find penalize masternodes
 	for i := 1; i <= common.LimitPenaltyEpoch; i++ {
@@ -827,12 +837,22 @@ func (s *PublicBlockChainAPI) GetCandidateStatus(ctx context.Context, coinbaseAd
 	penaltyList = common.ExtractAddressFromBytes(penalties)
 
 	// map slashing status
-	for _, pen := range penaltyList {
+	total := len(masternodes)
-		if coinbaseAddress == pen {
+	for _, candidate := range candidates {
-			result[fieldStatus] = statusSlashed
+		for _, pen := range penaltyList {
-			return result, nil
+			if candidate.Address == pen {
+				if coinbaseAddress == pen {
+					result[fieldStatus] = statusSlashed
+					return result, nil
+				}
+				total++
+				if total >= maxMasternodes {
+					return result, nil
+				}
+			}
 		}
 	}
+
 	return result, nil
 }
 
@@ -853,7 +873,6 @@ func (s *PublicBlockChainAPI) GetCandidates(ctx context.Context, epoch rpc.Epoch
 		fieldSuccess: true,
 	}
 	epochConfig := s.b.ChainConfig().XDPoS.Epoch
-	candidatesStatusMap := map[string]map[string]interface{}{}
 
 	checkpointNumber, epochNumber = s.GetPreviousCheckpointFromEpoch(ctx, epoch)
 	result[fieldEpoch] = epochNumber.Int64()
@@ -880,11 +899,10 @@ func (s *PublicBlockChainAPI) GetCandidates(ctx context.Context, epoch rpc.Epoch
 			return result, err
 		}
 		candidatesAddresses := state.GetCandidates(statedb)
+		candidates = make([]utils.Masternode, 0, len(candidatesAddresses))
 		for _, address := range candidatesAddresses {
 			v := state.GetCandidateCap(statedb, address)
-			if address.String() != "xdc0000000000000000000000000000000000000000" {
+			candidates = append(candidates, utils.Masternode{Address: address, Stake: v})
-				candidates = append(candidates, utils.Masternode{Address: address, Stake: v})
-			}
 		}
 	}
 
@@ -893,15 +911,8 @@ func (s *PublicBlockChainAPI) GetCandidates(ctx context.Context, epoch rpc.Epoch
 		result[fieldSuccess] = false
 		return result, err
 	}
-	// First, set all candidate to propose
-	for _, candidate := range candidates {
-		candidatesStatusMap[candidate.Address.String()] = map[string]interface{}{
-			fieldStatus:   statusProposed,
-			fieldCapacity: candidate.Stake,
-		}
-	}
 
-	// Second, Find candidates that have masternode status
+	// Find candidates that have masternode status
 	if engine, ok := s.b.GetEngine().(*XDPoS.XDPoS); ok {
 		masternodes = engine.GetMasternodesFromCheckpointHeader(header)
 		if len(masternodes) == 0 {
@@ -912,14 +923,49 @@ func (s *PublicBlockChainAPI) GetCandidates(ctx context.Context, epoch rpc.Epoch
 	} else {
 		log.Error("Undefined XDPoS consensus engine")
 	}
-	// Set masternode status
+
-	for _, masternode := range masternodes {
+	// Set all candidate to statusProposed
-		if candidatesStatusMap[masternode.String()] != nil {
+	candidatesStatusMap := make(map[string]map[string]interface{}, len(candidates))
-			candidatesStatusMap[masternode.String()][fieldStatus] = statusMasternode
+	for _, candidate := range candidates {
+		candidatesStatusMap[candidate.Address.String()] = map[string]interface{}{
+			fieldStatus:   statusProposed,
+			fieldCapacity: candidate.Stake,
 		}
 	}
 
-	// Third, Get penalties list
+	// Set masternodes to statusMasternode
+	for _, masternode := range masternodes {
+		key := masternode.String()
+		if candidatesStatusMap[key] != nil {
+			candidatesStatusMap[key][fieldStatus] = statusMasternode
+		} else {
+			candidatesStatusMap[key] = map[string]interface{}{
+				fieldStatus:   statusMasternode,
+				fieldCapacity: -1,
+			}
+			log.Warn("Masternode is not candidate", "masternode", key, "checkpointNumber", checkpointNumber, "epoch", epoch, "epochNumber", epochNumber)
+		}
+	}
+
+	var maxMasternodes int
+	if s.b.ChainConfig().IsTIPIncreaseMasternodes(block.Number()) {
+		maxMasternodes = common.MaxMasternodesV2
+	} else {
+		maxMasternodes = common.MaxMasternodes
+	}
+
+	if len(masternodes) >= maxMasternodes {
+		result[fieldCandidates] = candidatesStatusMap
+		return result, nil
+	}
+
+	if len(candidates) > maxMasternodes {
+		sort.Slice(candidates, func(i, j int) bool {
+			return candidates[i].Stake.Cmp(candidates[j].Stake) > 0
+		})
+	}
+
+	// Get penalties list
 	penalties = append(penalties, header.Penalties...)
 	// check last 5 epochs to find penalize masternodes
 	for i := 1; i <= common.LimitPenaltyEpoch; i++ {
@@ -941,20 +987,19 @@ func (s *PublicBlockChainAPI) GetCandidates(ctx context.Context, epoch rpc.Epoch
 	}
 	penaltyList = common.ExtractAddressFromBytes(penalties)
 
-	var topCandidates []utils.Masternode
+	// check penalties from checkpoint headers and modify status of a node to SLASHED if it's in top maxMasternodes candidates.
-	if len(candidates) > common.MaxMasternodes {
+	// if it's SLASHED but it's out of top maxMasternodes, the status should be still PROPOSED.
-		topCandidates = candidates[:common.MaxMasternodes]
+	total := len(masternodes)
-	} else {
+	for _, candidate := range candidates {
-		topCandidates = candidates
+		for _, pen := range penaltyList {
-	}
+			if candidate.Address == pen {
-	// check penalties from checkpoint headers and modify status of a node to SLASHED if it's in top 150 candidates
-	// if it's SLASHED but it's out of top 150, the status should be still PROPOSED
-	for _, pen := range penaltyList {
-		for _, candidate := range topCandidates {
-			if candidate.Address == pen && candidatesStatusMap[pen.String()] != nil {
 				candidatesStatusMap[pen.String()][fieldStatus] = statusSlashed
+				total++
+				if total >= maxMasternodes {
+					result[fieldCandidates] = candidatesStatusMap
+					return result, nil
+				}
 			}
-			penalties = append(penalties, block.Penalties()...)
 		}
 	}
 
@@ -966,28 +1011,23 @@ func (s *PublicBlockChainAPI) GetCandidates(ctx context.Context, epoch rpc.Epoch
 // GetPreviousCheckpointFromEpoch returns header of the previous checkpoint
 func (s *PublicBlockChainAPI) GetPreviousCheckpointFromEpoch(ctx context.Context, epochNum rpc.EpochNumber) (rpc.BlockNumber, rpc.EpochNumber) {
 	var checkpointNumber uint64
+	epoch := s.b.ChainConfig().XDPoS.Epoch
 
-	if engine, ok := s.b.GetEngine().(*XDPoS.XDPoS); ok {
+	if epochNum == rpc.LatestEpochNumber {
-		currentCheckpointNumber, epochNumber, err := engine.GetCurrentEpochSwitchBlock(s.chainReader, s.b.CurrentBlock().Number())
+		blockNumer := s.b.CurrentBlock().Number().Uint64()
-		if err != nil {
+		diff := blockNumer % epoch
-			log.Error("[GetPreviousCheckpointFromEpoch] Error while trying to get current epoch switch block information", "Block", s.b.CurrentBlock(), "Error", err)
+		// checkpoint number
+		checkpointNumber = blockNumer - diff
+		epochNum = rpc.EpochNumber(checkpointNumber / epoch)
+		if diff > 0 {
+			epochNum += 1
 		}
-		if epochNum == rpc.LatestEpochNumber {
+	} else if epochNum < 2 {
-			checkpointNumber = currentCheckpointNumber
+		checkpointNumber = 0
-			epochNum = rpc.EpochNumber(epochNumber)
-		} else if epochNum < 2 {
-			checkpointNumber = 0
-		} else {
-			blockNumberBeforeCurrentEpochSwitch := currentCheckpointNumber - 1
-			checkpointNumber, _, err = engine.GetCurrentEpochSwitchBlock(s.chainReader, big.NewInt(int64(blockNumberBeforeCurrentEpochSwitch)))
-			if err != nil {
-				log.Error("[GetPreviousCheckpointFromEpoch] Error while trying to get last epoch switch block information", "Number", blockNumberBeforeCurrentEpochSwitch, "Error", err)
-			}
-		}
-		return rpc.BlockNumber(checkpointNumber), epochNum
 	} else {
-		panic("[GetPreviousCheckpointFromEpoch] Error while trying to get XDPoS consensus engine")
+		checkpointNumber = epoch * (uint64(epochNum) - 1)
 	}
+	return rpc.BlockNumber(checkpointNumber), epochNum
 }
 
 // getCandidatesFromSmartContract returns all candidates with their capacities at the current time
@@ -1009,23 +1049,19 @@ func (s *PublicBlockChainAPI) getCandidatesFromSmartContract() ([]utils.Masterno
 		return []utils.Masternode{}, err
 	}
 
-	var candidatesWithStakeInfo []utils.Masternode
+	candidatesWithStakeInfo := make([]utils.Masternode, 0, len(candidates))
 
 	for _, candidate := range candidates {
-		v, err := validator.GetCandidateCap(opts, candidate)
+		if !candidate.IsZero() {
-		if err != nil {
+			v, err := validator.GetCandidateCap(opts, candidate)
-			return []utils.Masternode{}, err
+			if err != nil {
-		}
+				return []utils.Masternode{}, err
-		if candidate.String() != "xdc0000000000000000000000000000000000000000" {
+			}
+
 			candidatesWithStakeInfo = append(candidatesWithStakeInfo, utils.Masternode{Address: candidate, Stake: v})
 		}
-
-		if len(candidatesWithStakeInfo) > 0 {
-			sort.Slice(candidatesWithStakeInfo, func(i, j int) bool {
-				return candidatesWithStakeInfo[i].Stake.Cmp(candidatesWithStakeInfo[j].Stake) >= 0
-			})
-		}
 	}
+
 	return candidatesWithStakeInfo, nil
 }