Skip to content

Commit a3d722c

Browse files
lizhefengzjshen14
lizhefeng
authored and
zjshen14
committed
IOTEX-28 Validate recipient address in actpool and blockvalidator (#75)
* IOTEX-28 Validate recipient address in actpool and blockvalidator * Add tests for address check in actpool and blockvalidator
1 parent 8e06627 commit a3d722c

File tree

7 files changed

+174
-59
lines changed

7 files changed

+174
-59
lines changed

‎actpool/actpool.go

Lines changed: 55 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -309,44 +309,50 @@ func (ap *actPool) GetActionByHash(hash hash.Hash32B) (*iproto.ActionPb, error)
309309
func (ap *actPool) validateTsf(tsf *action.Transfer) error {
310310
// Reject coinbase transfer
311311
if tsf.IsCoinbase {
312-
logger.Error().Msg("Error when validating transfer")
312+
logger.Error().Msg("Error when validating whether transfer is coinbase")
313313
return errors.Wrapf(ErrTransfer, "coinbase transfer")
314314
}
315315
// Reject oversized transfer
316316
if tsf.TotalSize() > TransferSizeLimit {
317-
logger.Error().Msg("Error when validating transfer")
317+
logger.Error().Msg("Error when validating transfer's data size")
318318
return errors.Wrapf(ErrActPool, "oversized data")
319319
}
320320
// Reject transfer of negative amount
321321
if tsf.Amount.Sign() < 0 {
322-
logger.Error().Msg("Error when validating transfer")
322+
logger.Error().Msg("Error when validating transfer's amount")
323323
return errors.Wrapf(ErrBalance, "negative value")
324324
}
325+
325326
// check if sender's address is valid
326-
_, err := iotxaddress.GetPubkeyHash(tsf.Sender)
327-
if err != nil {
328-
return errors.Wrap(err, "error when getting the pubkey hash")
327+
if _, err := iotxaddress.GetPubkeyHash(tsf.Sender); err != nil {
328+
logger.Error().Msg("Error when validating transfer sender's address")
329+
return errors.Wrapf(err, "error when validating sender's address %s", tsf.Sender)
330+
}
331+
// check if recipient's address is valid
332+
if _, err := iotxaddress.GetPubkeyHash(tsf.Recipient); err != nil {
333+
logger.Error().Msg("Error when validating transfer recipient's address")
334+
return errors.Wrapf(err, "error when validating recipient's address %s", tsf.Recipient)
329335
}
330336

331337
sender, err := iotxaddress.GetAddressByPubkey(iotxaddress.IsTestnet, iotxaddress.ChainID, tsf.SenderPublicKey)
332338
if err != nil {
333-
logger.Error().Err(err).Msg("Error when validating transfer")
339+
logger.Error().Err(err).Msg("Error when validating transfer sender's public key")
334340
return errors.Wrapf(err, "invalid address")
335341
}
336342
// Verify transfer using sender's public key
337343
if err := tsf.Verify(sender); err != nil {
338-
logger.Error().Err(err).Msg("Error when validating transfer")
344+
logger.Error().Err(err).Msg("Error when validating transfer's signature")
339345
return errors.Wrapf(err, "failed to verify Transfer signature")
340346
}
341347
// Reject transfer if nonce is too low
342348
confirmedNonce, err := ap.bc.Nonce(tsf.Sender)
343349
if err != nil {
344-
logger.Error().Err(err).Msg("Error when validating transfer")
350+
logger.Error().Err(err).Msg("Error when validating transfer's nonce")
345351
return errors.Wrapf(err, "invalid nonce value")
346352
}
347353
pendingNonce := confirmedNonce + 1
348354
if pendingNonce > tsf.Nonce {
349-
logger.Error().Msg("Error when validating transfer")
355+
logger.Error().Msg("Error when validating transfer's nonce")
350356
return errors.Wrapf(ErrNonce, "nonce too low")
351357
}
352358
return nil
@@ -365,33 +371,42 @@ func (ap *actPool) validateExecution(exec *action.Execution) error {
365371
}
366372
// Reject execution of negative amount
367373
if exec.Amount.Sign() < 0 {
368-
logger.Error().Msg("Error when validating execution amount")
374+
logger.Error().Msg("Error when validating execution's amount")
369375
return errors.Wrapf(ErrBalance, "negative value")
370376
}
377+
371378
// check if executor's address is valid
372379
if _, err := iotxaddress.GetPubkeyHash(exec.Executor); err != nil {
373-
return errors.Wrap(err, "error when getting the pubkey hash")
380+
logger.Error().Msg("Error when validating executor's address")
381+
return errors.Wrapf(err, "error when validating executor's address %s", exec.Executor)
382+
}
383+
// check if contract's address is valid
384+
if exec.Contract != action.EmptyAddress {
385+
if _, err := iotxaddress.GetPubkeyHash(exec.Contract); err != nil {
386+
logger.Error().Msg("Error when validating contract's address")
387+
return errors.Wrapf(err, "error when validating contract's address %s", exec.Contract)
388+
}
374389
}
375390

376391
executor, err := iotxaddress.GetAddressByPubkey(iotxaddress.IsTestnet, iotxaddress.ChainID, exec.ExecutorPubKey)
377392
if err != nil {
378-
logger.Error().Err(err).Msg("Error when validating execution")
393+
logger.Error().Err(err).Msg("Error when validating executor's public key")
379394
return errors.Wrapf(err, "invalid address")
380395
}
381396
// Verify transfer using executor's public key
382397
if err := exec.Verify(executor); err != nil {
383-
logger.Error().Err(err).Msg("Error when validating execution")
398+
logger.Error().Err(err).Msg("Error when validating execution's signature")
384399
return errors.Wrapf(err, "failed to verify Execution signature")
385400
}
386401
// Reject transfer if nonce is too low
387402
confirmedNonce, err := ap.bc.Nonce(executor.RawAddress)
388403
if err != nil {
389-
logger.Error().Err(err).Msg("Error when validating execution")
404+
logger.Error().Err(err).Msg("Error when validating execution's nonce")
390405
return errors.Wrapf(err, "invalid nonce value")
391406
}
392407
pendingNonce := confirmedNonce + 1
393408
if pendingNonce > exec.Nonce {
394-
logger.Error().Msg("Error when validating execution")
409+
logger.Error().Msg("Error when validating execution's nonce")
395410
return errors.Wrapf(ErrNonce, "nonce too low")
396411
}
397412
return nil
@@ -401,57 +416,66 @@ func (ap *actPool) validateExecution(exec *action.Execution) error {
401416
func (ap *actPool) validateVote(vote *action.Vote) error {
402417
// Reject oversized vote
403418
if vote.TotalSize() > VoteSizeLimit {
404-
logger.Error().Msg("Error when validating vote")
419+
logger.Error().Msg("Error when validating vote's data size")
405420
return errors.Wrapf(ErrActPool, "oversized data")
406421
}
407422
selfPublicKey, err := vote.SelfPublicKey()
408423
if err != nil {
409-
return err
424+
logger.Error().Err(err).Msg("Error when validating voter's public key")
425+
return errors.Wrapf(err, "failed to get voter's public key")
426+
}
427+
428+
// check if voter's address is valid
429+
if _, err := iotxaddress.GetPubkeyHash(vote.GetVote().VoterAddress); err != nil {
430+
logger.Error().Err(err).Msg("Error when validating voter's address")
431+
return errors.Wrapf(err, "error when validating voter's address %s", vote.GetVote().VoterAddress)
432+
}
433+
// check if votee's address is valid
434+
if vote.GetVote().VoteeAddress != action.EmptyAddress {
435+
if _, err := iotxaddress.GetPubkeyHash(vote.GetVote().VoteeAddress); err != nil {
436+
logger.Error().Err(err).Msg("Error when validating votee's address")
437+
return errors.Wrapf(err, "error when validating votee's address %s", vote.GetVote().VoteeAddress)
438+
}
410439
}
440+
411441
voter, err := iotxaddress.GetAddressByPubkey(iotxaddress.IsTestnet, iotxaddress.ChainID, selfPublicKey)
412442
if err != nil {
413-
logger.Error().Err(err).Msg("Error when validating vote")
414-
return errors.Wrapf(err, "invalid voter address")
443+
logger.Error().Err(err).Msg("Error when validating voter's public key")
444+
return errors.Wrapf(err, "invalid address")
415445
}
416446
// Verify vote using voter's public key
417447
if err := vote.Verify(voter); err != nil {
418-
logger.Error().Err(err).Msg("Error when validating vote")
448+
logger.Error().Err(err).Msg("Error when validating vote's signature")
419449
return errors.Wrapf(err, "failed to verify Vote signature")
420450
}
421451

422452
// Reject vote if nonce is too low
423453
confirmedNonce, err := ap.bc.Nonce(voter.RawAddress)
424454
if err != nil {
425-
logger.Error().Err(err).Msg("Error when validating vote")
455+
logger.Error().Err(err).Msg("Error when validating vote's nonce")
426456
return errors.Wrapf(err, "invalid nonce value")
427457
}
428458
pbVote := vote.GetVote()
429459
if pbVote.VoteeAddress != "" {
430460
// Reject vote if votee is not a candidate
431-
if err != nil {
432-
logger.Error().
433-
Err(err).Hex("voter", pbVote.SelfPubkey[:]).Str("votee", pbVote.VoteeAddress).
434-
Msg("Error when validating vote")
435-
return errors.Wrapf(err, "invalid votee public key: %s", pbVote.VoteeAddress)
436-
}
437461
voteeState, err := ap.bc.StateByAddr(pbVote.VoteeAddress)
438462
if err != nil {
439463
logger.Error().Err(err).
440464
Hex("voter", pbVote.SelfPubkey[:]).Str("votee", pbVote.VoteeAddress).
441-
Msg("Error when validating vote")
465+
Msg("Error when validating votee's state")
442466
return errors.Wrapf(err, "cannot find votee's state: %s", pbVote.VoteeAddress)
443467
}
444468
if voter.RawAddress != pbVote.VoteeAddress && !voteeState.IsCandidate {
445469
logger.Error().Err(ErrVotee).
446470
Hex("voter", pbVote.SelfPubkey[:]).Str("votee", pbVote.VoteeAddress).
447-
Msg("Error when validating vote")
471+
Msg("Error when validating votee's state")
448472
return errors.Wrapf(ErrVotee, "votee has not self-nominated: %s", pbVote.VoteeAddress)
449473
}
450474
}
451475

452476
pendingNonce := confirmedNonce + 1
453477
if pendingNonce > vote.Nonce {
454-
logger.Error().Msg("Error when validating vote")
478+
logger.Error().Msg("Error when validating vote's nonce")
455479
return errors.Wrapf(ErrNonce, "nonce too low")
456480
}
457481
return nil

‎actpool/actpool_test.go

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ package actpool
99
import (
1010
"fmt"
1111
"math/big"
12+
"strings"
1213
"testing"
1314

1415
"github.com/golang/mock/gomock"
@@ -61,26 +62,31 @@ func TestActPool_validateTsf(t *testing.T) {
6162
require.Nil(err)
6263
ap, ok := Ap.(*actPool)
6364
require.True(ok)
64-
// Case I: Coinbase Transfer
65+
// Case I: Coinbase transfer
6566
coinbaseTsf := action.Transfer{IsCoinbase: true}
6667
err = ap.validateTsf(&coinbaseTsf)
6768
require.Equal(ErrTransfer, errors.Cause(err))
68-
// Case II: Oversized Data
69+
// Case II: Oversized data
6970
tmpPayload := [32769]byte{}
7071
payload := tmpPayload[:]
7172
tsf := action.Transfer{Payload: payload}
7273
err = ap.validateTsf(&tsf)
7374
require.Equal(ErrActPool, errors.Cause(err))
74-
// Case III: Negative Amount
75+
// Case III: Negative amount
7576
tsf = action.Transfer{Amount: big.NewInt(-100)}
7677
err = ap.validateTsf(&tsf)
77-
require.NotNil(ErrBalance, errors.Cause(err))
78-
// Case IV: Signature Verification Fails
78+
require.Equal(ErrBalance, errors.Cause(err))
79+
// Case IV: Invalid address
80+
tsf = action.Transfer{Sender: addr1.RawAddress, Recipient: "io1qyqsyqcyq5narhapakcsrhksfajfcpl24us3xp38zwvsep", Amount: big.NewInt(1)}
81+
err = ap.validateTsf(&tsf)
82+
require.Error(err)
83+
require.True(strings.Contains(err.Error(), "error when validating recipient's address"))
84+
// Case V: Signature verification fails
7985
unsignedTsf, err := action.NewTransfer(uint64(1), big.NewInt(1), addr1.RawAddress, addr1.RawAddress)
8086
require.NoError(err)
8187
err = ap.validateTsf(unsignedTsf)
8288
require.Equal(action.ErrTransferError, errors.Cause(err))
83-
// Case V: Nonce is too low
89+
// Case VI: Nonce is too low
8490
prevTsf, _ := signedTransfer(addr1, addr1, uint64(1), big.NewInt(50))
8591
err = ap.AddTsf(prevTsf)
8692
require.NoError(err)
@@ -105,7 +111,7 @@ func TestActPool_validateVote(t *testing.T) {
105111
require.Nil(err)
106112
ap, ok := Ap.(*actPool)
107113
require.True(ok)
108-
// Case I: Oversized Data
114+
// Case I: Oversized data
109115
tmpSelfPubKey := [32769]byte{}
110116
selfPubKey := tmpSelfPubKey[:]
111117
vote := action.Vote{
@@ -118,13 +124,39 @@ func TestActPool_validateVote(t *testing.T) {
118124
}
119125
err = ap.validateVote(&vote)
120126
require.Equal(ErrActPool, errors.Cause(err))
121-
// Case II: Signature Verification Fails
127+
// Case II: Invalid voter's public key
128+
vote = action.Vote{
129+
ActionPb: &iproto.ActionPb{
130+
Action: &iproto.ActionPb_Vote{
131+
Vote: &iproto.VotePb{},
132+
},
133+
},
134+
}
135+
err = ap.validateVote(&vote)
136+
require.Error(err)
137+
require.True(strings.Contains(err.Error(), "failed to get voter's public key"))
138+
// Case III: Invalid address
139+
vote = action.Vote{
140+
ActionPb: &iproto.ActionPb{
141+
Action: &iproto.ActionPb_Vote{
142+
Vote: &iproto.VotePb{
143+
SelfPubkey: addr1.PublicKey[:],
144+
VoterAddress: addr1.RawAddress,
145+
VoteeAddress: "123",
146+
},
147+
},
148+
},
149+
}
150+
err = ap.validateVote(&vote)
151+
require.Error(err)
152+
require.True(strings.Contains(err.Error(), "error when validating votee's address"))
153+
// Case IV: Signature verification fails
122154
unsignedVote, err := action.NewVote(1, addr1.RawAddress, addr2.RawAddress)
123155
unsignedVote.GetVote().SelfPubkey = addr1.PublicKey[:]
124156
require.NoError(err)
125157
err = ap.validateVote(unsignedVote)
126158
require.Equal(action.ErrVoteError, errors.Cause(err))
127-
// Case III: Nonce is too low
159+
// Case V: Nonce is too low
128160
prevTsf, _ := signedTransfer(addr1, addr1, uint64(1), big.NewInt(50))
129161
err = ap.AddTsf(prevTsf)
130162
require.NoError(err)
@@ -134,7 +166,7 @@ func TestActPool_validateVote(t *testing.T) {
134166
nVote, _ := signedVote(addr1, addr1, uint64(1))
135167
err = ap.validateVote(nVote)
136168
require.Equal(ErrNonce, errors.Cause(err))
137-
// Case IV: Votee is not a candidate
169+
// Case VI: Votee is not a candidate
138170
vote2, _ := signedVote(addr1, addr2, uint64(2))
139171
err = ap.validateVote(vote2)
140172
require.Equal(ErrVotee, errors.Cause(err))

‎actpool/actQueue.go renamed to ‎actpool/actqueue.go

File renamed without changes.

‎actpool/actQueue_test.go renamed to ‎actpool/actqueue_test.go

File renamed without changes.

‎blockchain/block_test.go

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ func TestWrongCoinbaseTsf(t *testing.T) {
367367
err = val.Validate(blk, 2, hash)
368368
require.Error(err)
369369
require.True(
370-
strings.Contains(err.Error(), "Wrong number of coinbase transfers"),
370+
strings.Contains(err.Error(), "wrong number of coinbase transfers"),
371371
)
372372

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

383383
// no transfer
@@ -387,6 +387,33 @@ func TestWrongCoinbaseTsf(t *testing.T) {
387387
err = val.Validate(blk, 2, hash)
388388
require.Error(err)
389389
require.True(
390-
strings.Contains(err.Error(), "Wrong number of coinbase transfers"),
390+
strings.Contains(err.Error(), "wrong number of coinbase transfers"),
391391
)
392392
}
393+
394+
func TestWrongAddress(t *testing.T) {
395+
val := validator{}
396+
invalidRecipient := "io1qyqsyqcyq5narhapakcsrhksfajfcpl24us3xp38zwvsep"
397+
tsf, err := action.NewTransfer(1, big.NewInt(1), ta.Addrinfo["producer"].RawAddress, invalidRecipient)
398+
require.NoError(t, err)
399+
blk1 := NewBlock(1, 3, hash.ZeroHash32B, clock.New(), []*action.Transfer{tsf}, nil, nil)
400+
err = val.verifyActions(blk1)
401+
require.Error(t, err)
402+
require.True(t, strings.Contains(err.Error(), "failed to validate transfer recipient's address"))
403+
404+
invalidVotee := "ioaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
405+
vote, err := action.NewVote(1, ta.Addrinfo["producer"].RawAddress, invalidVotee)
406+
require.NoError(t, err)
407+
blk2 := NewBlock(1, 3, hash.ZeroHash32B, clock.New(), nil, []*action.Vote{vote}, nil)
408+
err = val.verifyActions(blk2)
409+
require.Error(t, err)
410+
require.True(t, strings.Contains(err.Error(), "failed to validate votee's address"))
411+
412+
invalidContract := "123"
413+
execution, err := action.NewExecution(ta.Addrinfo["producer"].RawAddress, invalidContract, 1, big.NewInt(1), uint64(100000), uint64(10), []byte{})
414+
require.NoError(t, err)
415+
blk3 := NewBlock(1, 3, hash.ZeroHash32B, clock.New(), nil, nil, []*action.Execution{execution})
416+
err = val.verifyActions(blk3)
417+
require.Error(t, err)
418+
require.True(t, strings.Contains(err.Error(), "failed to validate contract's address"))
419+
}

‎blockchain/blockchain_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ func TestBlockchain_MintNewDummyBlock(t *testing.T) {
436436
err = val.Validate(blk, 0, tipHash)
437437
require.Error(err)
438438
require.True(
439-
strings.Contains(err.Error(), "Fail to verify block's signature"),
439+
strings.Contains(err.Error(), "failed to verify block's signature"),
440440
)
441441
}
442442

0 commit comments

Comments
 (0)