Skip to content

Commit

Permalink
IOTEX-28 Validate recipient address in actpool and blockvalidator (#75)
Browse files Browse the repository at this point in the history 
* IOTEX-28 Validate recipient address in actpool and blockvalidator

* Add tests for address check in actpool and blockvalidator
  • Loading branch information
@lizhefeng @zjshen14
lizhefeng authored and zjshen14 committed Sep 12, 2018
1 parent 8e06627 commit a3d722c
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 59 deletions.
86 changes: 55 additions & 31 deletions actpool/actpool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,44 +309,50 @@ func (ap *actPool) GetActionByHash(hash hash.Hash32B) (*iproto.ActionPb, error)
func (ap *actPool) validateTsf(tsf *action.Transfer) error {
// Reject coinbase transfer
if tsf.IsCoinbase {
logger.Error().Msg("Error when validating transfer")
logger.Error().Msg("Error when validating whether transfer is coinbase")
return errors.Wrapf(ErrTransfer, "coinbase transfer")
}
// Reject oversized transfer
if tsf.TotalSize() > TransferSizeLimit {
logger.Error().Msg("Error when validating transfer")
logger.Error().Msg("Error when validating transfer's data size")
return errors.Wrapf(ErrActPool, "oversized data")
}
// Reject transfer of negative amount
if tsf.Amount.Sign() < 0 {
logger.Error().Msg("Error when validating transfer")
logger.Error().Msg("Error when validating transfer's amount")
return errors.Wrapf(ErrBalance, "negative value")
}

// check if sender's address is valid
_, err := iotxaddress.GetPubkeyHash(tsf.Sender)
if err != nil {
return errors.Wrap(err, "error when getting the pubkey hash")
if _, err := iotxaddress.GetPubkeyHash(tsf.Sender); err != nil {
logger.Error().Msg("Error when validating transfer sender's address")
return errors.Wrapf(err, "error when validating sender's address %s", tsf.Sender)
}
// check if recipient's address is valid
if _, err := iotxaddress.GetPubkeyHash(tsf.Recipient); err != nil {
logger.Error().Msg("Error when validating transfer recipient's address")
return errors.Wrapf(err, "error when validating recipient's address %s", tsf.Recipient)
}

sender, err := iotxaddress.GetAddressByPubkey(iotxaddress.IsTestnet, iotxaddress.ChainID, tsf.SenderPublicKey)
if err != nil {
logger.Error().Err(err).Msg("Error when validating transfer")
logger.Error().Err(err).Msg("Error when validating transfer sender's public key")
return errors.Wrapf(err, "invalid address")
}
// Verify transfer using sender's public key
if err := tsf.Verify(sender); err != nil {
logger.Error().Err(err).Msg("Error when validating transfer")
logger.Error().Err(err).Msg("Error when validating transfer's signature")
return errors.Wrapf(err, "failed to verify Transfer signature")
}
// Reject transfer if nonce is too low
confirmedNonce, err := ap.bc.Nonce(tsf.Sender)
if err != nil {
logger.Error().Err(err).Msg("Error when validating transfer")
logger.Error().Err(err).Msg("Error when validating transfer's nonce")
return errors.Wrapf(err, "invalid nonce value")
}
pendingNonce := confirmedNonce + 1
if pendingNonce > tsf.Nonce {
logger.Error().Msg("Error when validating transfer")
logger.Error().Msg("Error when validating transfer's nonce")
return errors.Wrapf(ErrNonce, "nonce too low")
}
return nil
Expand All @@ -365,33 +371,42 @@ func (ap *actPool) validateExecution(exec *action.Execution) error {
}
// Reject execution of negative amount
if exec.Amount.Sign() < 0 {
logger.Error().Msg("Error when validating execution amount")
logger.Error().Msg("Error when validating execution's amount")
return errors.Wrapf(ErrBalance, "negative value")
}

// check if executor's address is valid
if _, err := iotxaddress.GetPubkeyHash(exec.Executor); err != nil {
return errors.Wrap(err, "error when getting the pubkey hash")
logger.Error().Msg("Error when validating executor's address")
return errors.Wrapf(err, "error when validating executor's address %s", exec.Executor)
}
// check if contract's address is valid
if exec.Contract != action.EmptyAddress {
if _, err := iotxaddress.GetPubkeyHash(exec.Contract); err != nil {
logger.Error().Msg("Error when validating contract's address")
return errors.Wrapf(err, "error when validating contract's address %s", exec.Contract)
}
}

executor, err := iotxaddress.GetAddressByPubkey(iotxaddress.IsTestnet, iotxaddress.ChainID, exec.ExecutorPubKey)
if err != nil {
logger.Error().Err(err).Msg("Error when validating execution")
logger.Error().Err(err).Msg("Error when validating executor's public key")
return errors.Wrapf(err, "invalid address")
}
// Verify transfer using executor's public key
if err := exec.Verify(executor); err != nil {
logger.Error().Err(err).Msg("Error when validating execution")
logger.Error().Err(err).Msg("Error when validating execution's signature")
return errors.Wrapf(err, "failed to verify Execution signature")
}
// Reject transfer if nonce is too low
confirmedNonce, err := ap.bc.Nonce(executor.RawAddress)
if err != nil {
logger.Error().Err(err).Msg("Error when validating execution")
logger.Error().Err(err).Msg("Error when validating execution's nonce")
return errors.Wrapf(err, "invalid nonce value")
}
pendingNonce := confirmedNonce + 1
if pendingNonce > exec.Nonce {
logger.Error().Msg("Error when validating execution")
logger.Error().Msg("Error when validating execution's nonce")
return errors.Wrapf(ErrNonce, "nonce too low")
}
return nil
Expand All @@ -401,57 +416,66 @@ func (ap *actPool) validateExecution(exec *action.Execution) error {
func (ap *actPool) validateVote(vote *action.Vote) error {
// Reject oversized vote
if vote.TotalSize() > VoteSizeLimit {
logger.Error().Msg("Error when validating vote")
logger.Error().Msg("Error when validating vote's data size")
return errors.Wrapf(ErrActPool, "oversized data")
}
selfPublicKey, err := vote.SelfPublicKey()
if err != nil {
return err
logger.Error().Err(err).Msg("Error when validating voter's public key")
return errors.Wrapf(err, "failed to get voter's public key")
}

// check if voter's address is valid
if _, err := iotxaddress.GetPubkeyHash(vote.GetVote().VoterAddress); err != nil {
logger.Error().Err(err).Msg("Error when validating voter's address")
return errors.Wrapf(err, "error when validating voter's address %s", vote.GetVote().VoterAddress)
}
// check if votee's address is valid
if vote.GetVote().VoteeAddress != action.EmptyAddress {
if _, err := iotxaddress.GetPubkeyHash(vote.GetVote().VoteeAddress); err != nil {
logger.Error().Err(err).Msg("Error when validating votee's address")
return errors.Wrapf(err, "error when validating votee's address %s", vote.GetVote().VoteeAddress)
}
}

voter, err := iotxaddress.GetAddressByPubkey(iotxaddress.IsTestnet, iotxaddress.ChainID, selfPublicKey)
if err != nil {
logger.Error().Err(err).Msg("Error when validating vote")
return errors.Wrapf(err, "invalid voter address")
logger.Error().Err(err).Msg("Error when validating voter's public key")
return errors.Wrapf(err, "invalid address")
}
// Verify vote using voter's public key
if err := vote.Verify(voter); err != nil {
logger.Error().Err(err).Msg("Error when validating vote")
logger.Error().Err(err).Msg("Error when validating vote's signature")
return errors.Wrapf(err, "failed to verify Vote signature")
}

// Reject vote if nonce is too low
confirmedNonce, err := ap.bc.Nonce(voter.RawAddress)
if err != nil {
logger.Error().Err(err).Msg("Error when validating vote")
logger.Error().Err(err).Msg("Error when validating vote's nonce")
return errors.Wrapf(err, "invalid nonce value")
}
pbVote := vote.GetVote()
if pbVote.VoteeAddress != "" {
// Reject vote if votee is not a candidate
if err != nil {
logger.Error().
Err(err).Hex("voter", pbVote.SelfPubkey[:]).Str("votee", pbVote.VoteeAddress).
Msg("Error when validating vote")
return errors.Wrapf(err, "invalid votee public key: %s", pbVote.VoteeAddress)
}
voteeState, err := ap.bc.StateByAddr(pbVote.VoteeAddress)
if err != nil {
logger.Error().Err(err).
Hex("voter", pbVote.SelfPubkey[:]).Str("votee", pbVote.VoteeAddress).
Msg("Error when validating vote")
Msg("Error when validating votee's state")
return errors.Wrapf(err, "cannot find votee's state: %s", pbVote.VoteeAddress)
}
if voter.RawAddress != pbVote.VoteeAddress && !voteeState.IsCandidate {
logger.Error().Err(ErrVotee).
Hex("voter", pbVote.SelfPubkey[:]).Str("votee", pbVote.VoteeAddress).
Msg("Error when validating vote")
Msg("Error when validating votee's state")
return errors.Wrapf(ErrVotee, "votee has not self-nominated: %s", pbVote.VoteeAddress)
}
}

pendingNonce := confirmedNonce + 1
if pendingNonce > vote.Nonce {
logger.Error().Msg("Error when validating vote")
logger.Error().Msg("Error when validating vote's nonce")
return errors.Wrapf(ErrNonce, "nonce too low")
}
return nil
Expand Down
52 changes: 42 additions & 10 deletions actpool/actpool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ package actpool
import (
"fmt"
"math/big"
"strings"
"testing"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -61,26 +62,31 @@ func TestActPool_validateTsf(t *testing.T) {
require.Nil(err)
ap, ok := Ap.(*actPool)
require.True(ok)
// Case I: Coinbase Transfer
// Case I: Coinbase transfer
coinbaseTsf := action.Transfer{IsCoinbase: true}
err = ap.validateTsf(&coinbaseTsf)
require.Equal(ErrTransfer, errors.Cause(err))
// Case II: Oversized Data
// Case II: Oversized data
tmpPayload := [32769]byte{}
payload := tmpPayload[:]
tsf := action.Transfer{Payload: payload}
err = ap.validateTsf(&tsf)
require.Equal(ErrActPool, errors.Cause(err))
// Case III: Negative Amount
// Case III: Negative amount
tsf = action.Transfer{Amount: big.NewInt(-100)}
err = ap.validateTsf(&tsf)
require.NotNil(ErrBalance, errors.Cause(err))
// Case IV: Signature Verification Fails
require.Equal(ErrBalance, errors.Cause(err))
// Case IV: Invalid address
tsf = action.Transfer{Sender: addr1.RawAddress, Recipient: "io1qyqsyqcyq5narhapakcsrhksfajfcpl24us3xp38zwvsep", Amount: big.NewInt(1)}
err = ap.validateTsf(&tsf)
require.Error(err)
require.True(strings.Contains(err.Error(), "error when validating recipient's address"))
// Case V: Signature verification fails
unsignedTsf, err := action.NewTransfer(uint64(1), big.NewInt(1), addr1.RawAddress, addr1.RawAddress)
require.NoError(err)
err = ap.validateTsf(unsignedTsf)
require.Equal(action.ErrTransferError, errors.Cause(err))
// Case V: Nonce is too low
// Case VI: Nonce is too low
prevTsf, _ := signedTransfer(addr1, addr1, uint64(1), big.NewInt(50))
err = ap.AddTsf(prevTsf)
require.NoError(err)
Expand All @@ -105,7 +111,7 @@ func TestActPool_validateVote(t *testing.T) {
require.Nil(err)
ap, ok := Ap.(*actPool)
require.True(ok)
// Case I: Oversized Data
// Case I: Oversized data
tmpSelfPubKey := [32769]byte{}
selfPubKey := tmpSelfPubKey[:]
vote := action.Vote{
Expand All @@ -118,13 +124,39 @@ func TestActPool_validateVote(t *testing.T) {
}
err = ap.validateVote(&vote)
require.Equal(ErrActPool, errors.Cause(err))
// Case II: Signature Verification Fails
// Case II: Invalid voter's public key
vote = action.Vote{
ActionPb: &iproto.ActionPb{
Action: &iproto.ActionPb_Vote{
Vote: &iproto.VotePb{},
},
},
}
err = ap.validateVote(&vote)
require.Error(err)
require.True(strings.Contains(err.Error(), "failed to get voter's public key"))
// Case III: Invalid address
vote = action.Vote{
ActionPb: &iproto.ActionPb{
Action: &iproto.ActionPb_Vote{
Vote: &iproto.VotePb{
SelfPubkey: addr1.PublicKey[:],
VoterAddress: addr1.RawAddress,
VoteeAddress: "123",
},
},
},
}
err = ap.validateVote(&vote)
require.Error(err)
require.True(strings.Contains(err.Error(), "error when validating votee's address"))
// Case IV: Signature verification fails
unsignedVote, err := action.NewVote(1, addr1.RawAddress, addr2.RawAddress)
unsignedVote.GetVote().SelfPubkey = addr1.PublicKey[:]
require.NoError(err)
err = ap.validateVote(unsignedVote)
require.Equal(action.ErrVoteError, errors.Cause(err))
// Case III: Nonce is too low
// Case V: Nonce is too low
prevTsf, _ := signedTransfer(addr1, addr1, uint64(1), big.NewInt(50))
err = ap.AddTsf(prevTsf)
require.NoError(err)
Expand All @@ -134,7 +166,7 @@ func TestActPool_validateVote(t *testing.T) {
nVote, _ := signedVote(addr1, addr1, uint64(1))
err = ap.validateVote(nVote)
require.Equal(ErrNonce, errors.Cause(err))
// Case IV: Votee is not a candidate
// Case VI: Votee is not a candidate
vote2, _ := signedVote(addr1, addr2, uint64(2))
err = ap.validateVote(vote2)
require.Equal(ErrVotee, errors.Cause(err))
Expand Down
0 actpool/actQueue.go → actpool/actqueue.go
View file
File renamed without changes.
0 actpool/actQueue_test.go → actpool/actqueue_test.go
View file
File renamed without changes.
33 changes: 30 additions & 3 deletions blockchain/block_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ func TestWrongCoinbaseTsf(t *testing.T) {
err = val.Validate(blk, 2, hash)
require.Error(err)
require.True(
strings.Contains(err.Error(), "Wrong number of coinbase transfers"),
strings.Contains(err.Error(), "wrong number of coinbase transfers"),
)

// extra coinbase transfer
Expand All @@ -377,7 +377,7 @@ func TestWrongCoinbaseTsf(t *testing.T) {
err = val.Validate(blk, 2, hash)
require.Error(err)
require.True(
strings.Contains(err.Error(), "Wrong number of coinbase transfers"),
strings.Contains(err.Error(), "wrong number of coinbase transfers"),
)

// no transfer
Expand All @@ -387,6 +387,33 @@ func TestWrongCoinbaseTsf(t *testing.T) {
err = val.Validate(blk, 2, hash)
require.Error(err)
require.True(
strings.Contains(err.Error(), "Wrong number of coinbase transfers"),
strings.Contains(err.Error(), "wrong number of coinbase transfers"),
)
}

func TestWrongAddress(t *testing.T) {
val := validator{}
invalidRecipient := "io1qyqsyqcyq5narhapakcsrhksfajfcpl24us3xp38zwvsep"
tsf, err := action.NewTransfer(1, big.NewInt(1), ta.Addrinfo["producer"].RawAddress, invalidRecipient)
require.NoError(t, err)
blk1 := NewBlock(1, 3, hash.ZeroHash32B, clock.New(), []*action.Transfer{tsf}, nil, nil)
err = val.verifyActions(blk1)
require.Error(t, err)
require.True(t, strings.Contains(err.Error(), "failed to validate transfer recipient's address"))

invalidVotee := "ioaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
vote, err := action.NewVote(1, ta.Addrinfo["producer"].RawAddress, invalidVotee)
require.NoError(t, err)
blk2 := NewBlock(1, 3, hash.ZeroHash32B, clock.New(), nil, []*action.Vote{vote}, nil)
err = val.verifyActions(blk2)
require.Error(t, err)
require.True(t, strings.Contains(err.Error(), "failed to validate votee's address"))

invalidContract := "123"
execution, err := action.NewExecution(ta.Addrinfo["producer"].RawAddress, invalidContract, 1, big.NewInt(1), uint64(100000), uint64(10), []byte{})
require.NoError(t, err)
blk3 := NewBlock(1, 3, hash.ZeroHash32B, clock.New(), nil, nil, []*action.Execution{execution})
err = val.verifyActions(blk3)
require.Error(t, err)
require.True(t, strings.Contains(err.Error(), "failed to validate contract's address"))
}
2 changes: 1 addition & 1 deletion blockchain/blockchain_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ func TestBlockchain_MintNewDummyBlock(t *testing.T) {
err = val.Validate(blk, 0, tipHash)
require.Error(err)
require.True(
strings.Contains(err.Error(), "Fail to verify block's signature"),
strings.Contains(err.Error(), "failed to verify block's signature"),
)
}

Expand Down
Loading

0 comments on commit a3d722c

Please sign in to comment.