Merge pull request #3599 from rlan35/main

return error for invalid address parsing

accounts/address.go

@@ -6,11 +6,6 @@ import (
 	"github.com/pkg/errors"
 )
 
-// ParseAddrH is a wrapper to cast ethCommon.Address to harmony's common.Address
-func ParseAddrH(s string) common.Address {
-	return common.Address(common.ParseAddr(s))
-}
-
 // MustBech32ToAddressH is a wrapper for casting ethCommon.Address to harmony's common.Address
 func MustBech32ToAddressH(b32 string) common.Address {
 	return common.Address(common.MustBech32ToAddress(b32))

accounts/keystore/account_cache.go

@@ -263,7 +263,7 @@ func (ac *accountCache) scanAccounts() error {
 		// Parse the address.
 		key.Address = ""
 		err = json.NewDecoder(buf).Decode(&key)
-		addr := common2.ParseAddr(key.Address)
+		addr, _ := common2.ParseAddr(key.Address)
 		switch {
 		case err != nil:
 			utils.Logger().Debug().

internal/common/address.go

@@ -1,6 +1,7 @@
 package common
 
 import (
+	"bytes"
 	"crypto/ecdsa"
 	"database/sql/driver"
 	"encoding/hex"
@@ -222,13 +223,18 @@ func MustAddressToBech32(addr ethCommon.Address) string {
 }
 
 // ParseAddr parses the given address, either as bech32 or as hex.
-// The result can be 0x00..00 if the passing param is not a correct address.
-func ParseAddr(s string) ethCommon.Address {
+// Return error if the address is invalid.
+func ParseAddr(s string) (ethCommon.Address, error) {
 	if addr, err := Bech32ToAddress(s); err == nil {
-		return addr
+		return addr, nil
 	}
 	// The result can be 0x00...00 if the passing param is not a correct address.
-	return ethCommon.HexToAddress(s)
+	hex := ethCommon.HexToAddress(s)
+	emptyAddr := ethCommon.Address{}
+	if bytes.Compare(hex[:], emptyAddr[:]) == 0 {
+		return hex, errors.Errorf("invalid address: %s", s)
+	}
+	return hex, nil
 }
 
 // MustGeneratePrivateKey generates a random private key for an address. It panics on error.

internal/common/address_test.go

@@ -152,12 +152,12 @@ func TestAddressToBech32(t *testing.T) {
 
 func TestParseAddr(t *testing.T) {
 	adr := ethCommon.HexToAddress("0x15a128e599b74842bccba860311efa92991bffb5")
-	adr2 := ParseAddr("one1zksj3evekayy90xt4psrz8h6j2v3hla4qwz4ur")
+	adr2, _ := ParseAddr("one1zksj3evekayy90xt4psrz8h6j2v3hla4qwz4ur")
 	if adr.Hex() != adr2.Hex() {
 		t.Errorf("error on ParseAddr")
 	}
 	// Parsing incorrect address
-	adr3 := ParseAddr("helloworldone1zksj3evekayy90xt4psrz8h6j2v3hla4qwz4ufdfsrfasdfadfas")
+	adr3, _ := ParseAddr("helloworldone1zksj3evekayy90xt4psrz8h6j2v3hla4qwz4ufdfsrfasdfadfas")
 	if adr3.Hex() != "0x0000000000000000000000000000000000000000" {
 		t.Errorf("error on ParseAddr")
 	}

node/node_genesis.go

@@ -5,10 +5,7 @@ import (
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/ethdb"
-	"github.com/harmony-one/harmony/common/denominations"
 	"github.com/harmony-one/harmony/core"
-	common2 "github.com/harmony-one/harmony/internal/common"
-	"github.com/harmony-one/harmony/internal/genesis"
 	"github.com/harmony-one/harmony/internal/utils"
 	"github.com/harmony-one/harmony/shard"
 	"github.com/harmony-one/harmony/shard/committee"
@@ -52,14 +49,3 @@ func (node *Node) SetupGenesisBlock(db ethdb.Database, shardID uint32, myShardSt
 	// Store genesis block into db.
 	gspec.MustCommit(db)
 }
-
-// AddNodeAddressesToGenesisAlloc adds to the genesis block allocation the accounts used for network validators/nodes,
-// including the account used by the nodes of the initial beacon chain and later new nodes.
-func AddNodeAddressesToGenesisAlloc(genesisAlloc core.GenesisAlloc) {
-	for _, account := range genesis.HarmonyAccounts {
-		testBankFunds := big.NewInt(core.InitFreeFund)
-		testBankFunds = testBankFunds.Mul(testBankFunds, big.NewInt(denominations.One))
-		address := common2.ParseAddr(account.Address)
-		genesisAlloc[address] = core.GenesisAccount{Balance: testBankFunds}
-	}
-}

rpc/blockchain.go

@@ -92,7 +92,10 @@ func (s *PublicBlockchainService) getBlockOptions(opts interface{}) (*rpc_common
 func (s *PublicBlockchainService) getBalanceByBlockNumber(
 	ctx context.Context, address string, blockNum rpc.BlockNumber,
 ) (*big.Int, error) {
-	addr := internal_common.ParseAddr(address)
+	addr, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
 	balance, err := s.hmy.GetBalance(ctx, addr, blockNum)
 	if err != nil {
 		return nil, err

rpc/contract.go

@@ -64,7 +64,10 @@ func (s *PublicContractService) GetCode(
 	blockNum := blockNumber.EthBlockNumber()
 
 	// Fetch state
-	address := hmyCommon.ParseAddr(addr)
+	address, err := hmyCommon.ParseAddr(addr)
+	if err != nil {
+		return nil, err
+	}
 	state, _, err := s.hmy.StateAndHeaderByNumber(ctx, blockNum)
 	if state == nil || err != nil {
 		return nil, err
@@ -89,7 +92,10 @@ func (s *PublicContractService) GetStorageAt(
 	if state == nil || err != nil {
 		return nil, err
 	}
-	address := hmyCommon.ParseAddr(addr)
+	address, err := hmyCommon.ParseAddr(addr)
+	if err != nil {
+		return nil, err
+	}
 	res := state.GetState(address, common.HexToHash(key))
 
 	// Response output is the same for all versions

rpc/eth/rpc.go

@@ -34,7 +34,10 @@ func NewPublicEthService(hmy *hmy.Harmony, namespace string) rpc.API {
 func (s *PublicEthService) GetBalance(
 	ctx context.Context, address string, blockNr rpc.BlockNumber,
 ) (*hexutil.Big, error) {
-	addr := internal_common.ParseAddr(address)
+	addr, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
 	balance, err := s.hmy.GetBalance(ctx, addr, blockNr)
 	if err != nil {
 		return nil, err

rpc/eth/types.go

@@ -153,7 +153,10 @@ func NewReceipt(tx *types.EthTransaction, blockHash common.Hash, blockNumber, bl
 // returned. When fullTx is true the returned block contains full transaction details, otherwise it will only contain
 // transaction hashes.
 func NewBlock(b *types.Block, blockArgs *rpc_common.BlockArgs, leaderAddress string) (interface{}, error) {
-	leader := internal_common.ParseAddr(leaderAddress)
+	leader, err := internal_common.ParseAddr(leaderAddress)
+	if err != nil {
+		return nil, err
+	}
 
 	if blockArgs.FullTx {
 		return NewBlockWithFullTx(b, blockArgs, leader)

rpc/staking.go

@@ -39,7 +39,10 @@ func NewPublicStakingAPI(hmy *hmy.Harmony, version Version) rpc.API {
 func (s *PublicStakingService) getBalanceByBlockNumber(
 	ctx context.Context, address string, blockNum rpc.BlockNumber,
 ) (*big.Int, error) {
-	addr := internal_common.ParseAddr(address)
+	addr, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
 	balance, err := s.hmy.GetBalance(ctx, addr, blockNum)
 	if err != nil {
 		return nil, err
@@ -299,7 +302,11 @@ func (s *PublicStakingService) GetValidatorInformation(
 	}
 
 	// Fetch validator information
-	validatorInfo, err := s.hmy.GetValidatorInformation(internal_common.ParseAddr(address), blk)
+	addr, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
+	validatorInfo, err := s.hmy.GetValidatorInformation(addr, blk)
 	if err != nil {
 		return nil, err
 	}
@@ -328,7 +335,11 @@ func (s *PublicStakingService) GetValidatorInformationByBlockNumber(
 	}
 
 	// Fetch validator info
-	validatorInfo, err := s.hmy.GetValidatorInformation(internal_common.ParseAddr(address), blk)
+	addr, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
+	validatorInfo, err := s.hmy.GetValidatorInformation(addr, blk)
 	if err != nil {
 		return nil, err
 	}
@@ -347,7 +358,11 @@ func (s *PublicStakingService) GetValidatorSelfDelegation(
 	}
 
 	// Fetch self delegation
-	selfDelegation := s.hmy.GetValidatorSelfDelegation(internal_common.ParseAddr(address)).Uint64()
+	addr, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
+	selfDelegation := s.hmy.GetValidatorSelfDelegation(addr).Uint64()
 
 	// Format the response according to the version
 	switch s.version {
@@ -370,7 +385,11 @@ func (s *PublicStakingService) GetValidatorTotalDelegation(
 	}
 
 	// Fetch delegations & sum
-	delegations := s.hmy.GetDelegationsByValidator(internal_common.ParseAddr(address))
+	addr, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
+	delegations := s.hmy.GetDelegationsByValidator(addr)
 	totalStake := big.NewInt(0)
 	for _, delegation := range delegations {
 		totalStake.Add(totalStake, delegation.Amount)
@@ -443,7 +462,10 @@ func (s *PublicStakingService) GetDelegationsByDelegator(
 	}
 
 	// Fetch delegation
-	delegatorAddress := internal_common.ParseAddr(address)
+	delegatorAddress, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
 	validators, delegations := s.hmy.GetDelegationsByDelegator(delegatorAddress)
 
 	// Format response
@@ -492,7 +514,10 @@ func (s *PublicStakingService) GetDelegationsByDelegatorByBlockNumber(
 	}
 
 	// Fetch delegation for block number
-	delegatorAddress := internal_common.ParseAddr(address)
+	delegatorAddress, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
 	blk, err := s.hmy.BlockByNumber(ctx, blockNum)
 	if err != nil {
 		return nil, errors.Wrapf(err, "could not retrieve the blk information for blk number: %d", blockNum)
@@ -539,7 +564,10 @@ func (s *PublicStakingService) GetDelegationsByValidator(
 	}
 
 	// Fetch delegations
-	validatorAddress := internal_common.ParseAddr(address)
+	validatorAddress, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
 	delegations := s.hmy.GetDelegationsByValidator(validatorAddress)
 
 	// Format response
@@ -582,8 +610,14 @@ func (s *PublicStakingService) GetDelegationByDelegatorAndValidator(
 	}
 
 	// Fetch delegations
-	delegatorAddress := internal_common.ParseAddr(address)
-	validatorAddress := internal_common.ParseAddr(validator)
+	delegatorAddress, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
+	validatorAddress, err := internal_common.ParseAddr(validator)
+	if err != nil {
+		return nil, err
+	}
 	validators, delegations := s.hmy.GetDelegationsByDelegator(delegatorAddress)
 
 	// Format response
@@ -625,7 +659,10 @@ func (s *PublicStakingService) GetAvailableRedelegationBalance(
 
 	currEpoch := s.hmy.BlockChain.CurrentHeader().Epoch()
 
-	delegatorAddr := internal_common.ParseAddr(address)
+	delegatorAddr, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
 	_, delegations := s.hmy.GetDelegationsByDelegator(delegatorAddr)
 
 	redelegationTotal := big.NewInt(0)

rpc/transaction.go

@@ -55,7 +55,10 @@ func (s *PublicTransactionService) GetAccountNonce(
 	blockNum := blockNumber.EthBlockNumber()
 
 	// Response output is the same for all versions
-	addr := internal_common.ParseAddr(address)
+	addr, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return 0, err
+	}
 	return s.hmy.GetAccountNonce(ctx, addr, blockNum)
 }
 
@@ -68,7 +71,10 @@ func (s *PublicTransactionService) GetTransactionCount(
 ) (response interface{}, err error) {
 	// Process arguments based on version
 	blockNum := blockNumber.EthBlockNumber()
-	address := internal_common.ParseAddr(addr)
+	address, err := internal_common.ParseAddr(addr)
+	if err != nil {
+		return nil, err
+	}
 
 	// Fetch transaction count
 	var nonce uint64
@@ -110,7 +116,10 @@ func (s *PublicTransactionService) GetTransactionsCount(
 ) (count uint64, err error) {
 	if !strings.HasPrefix(address, "one1") {
 		// Handle hex address
-		addr := internal_common.ParseAddr(address)
+		addr, err := internal_common.ParseAddr(address)
+		if err != nil {
+			return 0, err
+		}
 		address, err = internal_common.AddressToBech32(addr)
 		if err != nil {
 			return 0, err
@@ -127,7 +136,10 @@ func (s *PublicTransactionService) GetStakingTransactionsCount(
 ) (count uint64, err error) {
 	if !strings.HasPrefix(address, "one1") {
 		// Handle hex address
-		addr := internal_common.ParseAddr(address)
+		addr, err := internal_common.ParseAddr(address)
+		if err != nil {
+			return 0, err
+		}
 		address, err = internal_common.AddressToBech32(addr)
 		if err != nil {
 			return 0, err
@@ -250,7 +262,10 @@ func (s *PublicTransactionService) GetTransactionsHistory(
 	if strings.HasPrefix(args.Address, "one1") {
 		address = args.Address
 	} else {
-		addr := internal_common.ParseAddr(args.Address)
+		addr, err := internal_common.ParseAddr(args.Address)
+		if err != nil {
+			return nil, err
+		}
 		address, err = internal_common.AddressToBech32(addr)
 		if err != nil {
 			return nil, err
@@ -295,7 +310,10 @@ func (s *PublicTransactionService) GetStakingTransactionsHistory(
 	if strings.HasPrefix(args.Address, "one1") {
 		address = args.Address
 	} else {
-		addr := internal_common.ParseAddr(args.Address)
+		addr, err := internal_common.ParseAddr(args.Address)
+		if err != nil {
+			return nil, err
+		}
 		address, err = internal_common.AddressToBech32(addr)
 		if err != nil {
 			utils.Logger().Debug().

rpc/v1/legacy.go

@@ -35,7 +35,10 @@ func NewPublicLegacyAPI(hmy *hmy.Harmony, namespace string) rpc.API {
 func (s *PublicLegacyService) GetBalance(
 	ctx context.Context, address string, blockNr rpc.BlockNumber,
 ) (*hexutil.Big, error) {
-	addr := internal_common.ParseAddr(address)
+	addr, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
 	balance, err := s.hmy.GetBalance(ctx, addr, blockNr)
 	if err != nil {
 		return nil, err

rpc/v2/legacy.go

@@ -34,7 +34,10 @@ func NewPublicLegacyAPI(hmy *hmy.Harmony, namespace string) rpc.API {
 func (s *PublicLegacyService) GetBalance(
 	ctx context.Context, address string,
 ) (*big.Int, error) {
-	addr := internal_common.ParseAddr(address)
+	addr, err := internal_common.ParseAddr(address)
+	if err != nil {
+		return nil, err
+	}
 	balance, err := s.hmy.GetBalance(ctx, addr, rpc.BlockNumber(-1))
 	if err != nil {
 		return nil, err

shard/committee/assignment.go

@@ -257,7 +257,7 @@ var (
 )
 
 // This is the shard state computation logic before staking epoch.
-func preStakingEnabledCommittee(s shardingconfig.Instance) *shard.State {
+func preStakingEnabledCommittee(s shardingconfig.Instance) (*shard.State, error) {
 	shardNum := int(s.NumShards())
 	shardHarmonyNodes := s.NumHarmonyOperatedNodesPerShard()
 	shardSize := s.NumNodesPerShard()
@@ -274,8 +274,12 @@ func preStakingEnabledCommittee(s shardingconfig.Instance) *shard.State {
 			pubKey := bls.SerializedPublicKey{}
 			pubKey.FromLibBLSPublicKey(pub)
 			// TODO: directly read address for bls too
+			addr, err := common2.ParseAddr(hmyAccounts[index].Address)
+			if err != nil {
+				return nil, err
+			}
 			curNodeID := shard.Slot{
-				common2.ParseAddr(hmyAccounts[index].Address),
+				addr,
 				pubKey,
 				nil,
 			}
@@ -289,8 +293,12 @@ func preStakingEnabledCommittee(s shardingconfig.Instance) *shard.State {
 			pubKey := bls.SerializedPublicKey{}
 			pubKey.FromLibBLSPublicKey(pub)
 			// TODO: directly read address for bls too
+			addr, err := common2.ParseAddr(fnAccounts[index].Address)
+			if err != nil {
+				return nil, err
+			}
 			curNodeID := shard.Slot{
-				common2.ParseAddr(fnAccounts[index].Address),
+				addr,
 				pubKey,
 				nil,
 			}
@@ -298,7 +306,7 @@ func preStakingEnabledCommittee(s shardingconfig.Instance) *shard.State {
 		}
 		shardState.Shards = append(shardState.Shards, com)
 	}
-	return shardState
+	return shardState, nil
 }
 
 func eposStakedCommittee(
@@ -322,8 +330,13 @@ func eposStakedCommittee(
 			if err := pubKey.FromLibBLSPublicKey(pub); err != nil {
 				return nil, err
 			}
+
+			addr, err := common2.ParseAddr(hAccounts[index].Address)
+			if err != nil {
+				return nil, err
+			}
 			shardState.Shards[i].Slots = append(shardState.Shards[i].Slots, shard.Slot{
-				common2.ParseAddr(hAccounts[index].Address),
+				addr,
 				pubKey,
 				nil,
 			})
@@ -380,7 +393,7 @@ func (def partialStakingEnabled) Compute(
 	instance := shard.Schedule.InstanceForEpoch(epoch)
 	if preStaking {
 		// Pre-staking shard state doesn't need to set epoch (backward compatible)
-		return preStakingEnabledCommittee(instance), nil
+		return preStakingEnabledCommittee(instance)
 	}
 	// Sanity check, can't compute against epochs in past
 	if e := stakerReader.CurrentHeader().Epoch(); epoch.Cmp(e) == -1 {