From 887ebabb361bdb9ec5e803f46ce3bc7b1ef14134 Mon Sep 17 00:00:00 2001 From: RobertKwiatkowski <101144854+RobertKwiatkowski@users.noreply.github.com> Date: Wed, 16 Mar 2022 04:26:54 +0100 Subject: [PATCH] [action] move Verify to SealedEnvelope (#3197) --- action/action.go | 28 -------------------------- action/action_test.go | 8 ++++---- action/candidateregister_test.go | 2 +- action/candidateupdate_test.go | 2 +- action/execution_test.go | 4 ++-- action/protocol/generic_validator.go | 2 +- action/rlp_tx_test.go | 2 +- action/sealedenvelope.go | 28 ++++++++++++++++++++++++++ action/stake_changecandidate_test.go | 2 +- action/stake_transferownership_test.go | 2 +- action/stakeadddeposit_test.go | 2 +- action/stakecreate_test.go | 2 +- action/stakereclaim_test.go | 4 ++-- action/stakerestake_test.go | 2 +- action/transfer_test.go | 6 +++--- 15 files changed, 48 insertions(+), 48 deletions(-) diff --git a/action/action.go b/action/action.go index 265f46ec1a..a76204e39e 100644 --- a/action/action.go +++ b/action/action.go @@ -7,15 +7,11 @@ package action import ( - "encoding/hex" "math" "math/big" "github.com/iotexproject/go-pkgs/crypto" "github.com/pkg/errors" - "go.uber.org/zap" - - "github.com/iotexproject/iotex-core/pkg/log" ) // Action is the action can be Executed in protocols. The method is added to avoid mistakenly used empty interface as action. @@ -78,30 +74,6 @@ func AssembleSealedEnvelope(act Envelope, pk crypto.PublicKey, sig []byte) Seale return sealed } -// Verify verifies the action using sender's public key -func Verify(sealed SealedEnvelope) error { - if sealed.SrcPubkey() == nil { - return errors.New("empty public key") - } - // Reject action with insufficient gas limit - intrinsicGas, err := sealed.IntrinsicGas() - if intrinsicGas > sealed.GasLimit() || err != nil { - return ErrIntrinsicGas - } - - h, err := sealed.envelopeHash() - if err != nil { - return errors.Wrap(err, "failed to generate envelope hash") - } - if !sealed.SrcPubkey().Verify(h[:], sealed.Signature()) { - log.L().Info("failed to verify action hash", - zap.String("hash", hex.EncodeToString(h[:])), - zap.String("signature", hex.EncodeToString(sealed.Signature()))) - return ErrInvalidSender - } - return nil -} - // ClassifyActions classfies actions func ClassifyActions(actions []SealedEnvelope) ([]*Transfer, []*Execution) { tsfs := make([]*Transfer, 0) diff --git a/action/action_test.go b/action/action_test.go index d74faff7b1..84ec344aa4 100644 --- a/action/action_test.go +++ b/action/action_test.go @@ -33,7 +33,7 @@ func TestActionProtoAndVerify(t *testing.T) { selp, err := Sign(elp, identityset.PrivateKey(28)) require.NoError(err) - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) nselp := &SealedEnvelope{} require.NoError(nselp.LoadProto(selp.Proto())) @@ -55,7 +55,7 @@ func TestActionProtoAndVerify(t *testing.T) { selp.srcPubkey = nil - require.EqualError(Verify(selp), "empty public key") + require.EqualError(selp.Verify(), "empty public key") }) t.Run("gas limit too low", func(t *testing.T) { bd := &EnvelopeBuilder{} @@ -66,7 +66,7 @@ func TestActionProtoAndVerify(t *testing.T) { selp, err := Sign(elp, identityset.PrivateKey(28)) require.NoError(err) - require.Equal(ErrIntrinsicGas, errors.Cause(Verify(selp))) + require.Equal(ErrIntrinsicGas, errors.Cause(selp.Verify())) }) t.Run("invalid signature", func(t *testing.T) { bd := &EnvelopeBuilder{} @@ -77,7 +77,7 @@ func TestActionProtoAndVerify(t *testing.T) { selp, err := Sign(elp, identityset.PrivateKey(28)) require.NoError(err) selp.signature = []byte("invalid signature") - require.Equal(ErrInvalidSender, errors.Cause(Verify(selp))) + require.Equal(ErrInvalidSender, errors.Cause(selp.Verify())) }) } diff --git a/action/candidateregister_test.go b/action/candidateregister_test.go index 573098abcd..0e2a2cadc4 100644 --- a/action/candidateregister_test.go +++ b/action/candidateregister_test.go @@ -122,7 +122,7 @@ func TestCandidateRegister(t *testing.T) { require.NoError(err) require.Equal(test.SelpHash, hex.EncodeToString(hash[:])) // verify signature - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) } } diff --git a/action/candidateupdate_test.go b/action/candidateupdate_test.go index 6b3d6b4f34..f8bfd7c2f1 100644 --- a/action/candidateupdate_test.go +++ b/action/candidateupdate_test.go @@ -76,7 +76,7 @@ func TestCandidateUpdateSignVerify(t *testing.T) { require.NoError(err) require.Equal("ca1a28f0e9a58ffc67037cc75066dbdd8e024aa2b2e416e4d6ce16c3d86282e5", hex.EncodeToString(hash[:])) // verify signature - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) } func TestCandidateUpdateABIEncodeAndDecode(t *testing.T) { diff --git a/action/execution_test.go b/action/execution_test.go index b7ad22601b..df44a355b5 100644 --- a/action/execution_test.go +++ b/action/execution_test.go @@ -38,7 +38,7 @@ func TestExecutionSignVerify(t *testing.T) { require.True(ok) w := AssembleSealedEnvelope(elp, executorKey.PublicKey(), []byte("lol")) - require.Error(Verify(w)) + require.Error(w.Verify()) ex2, ok := w.Envelope.Action().(*Execution) require.True(ok) require.Equal(ex, ex2) @@ -50,7 +50,7 @@ func TestExecutionSignVerify(t *testing.T) { require.NotNil(selp) // verify signature - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) } func TestExecutionSanityCheck(t *testing.T) { diff --git a/action/protocol/generic_validator.go b/action/protocol/generic_validator.go index afd1975d5a..55a48f9f93 100644 --- a/action/protocol/generic_validator.go +++ b/action/protocol/generic_validator.go @@ -38,7 +38,7 @@ func NewGenericValidator(sr StateReader, accountState AccountState) *GenericVali // Validate validates a generic action func (v *GenericValidator) Validate(ctx context.Context, selp action.SealedEnvelope) error { // Verify action using action sender's public key - if err := action.Verify(selp); err != nil { + if err := selp.Verify(); err != nil { return err } caller := selp.SrcPubkey().Address() diff --git a/action/rlp_tx_test.go b/action/rlp_tx_test.go index d9edc918db..3b4d391086 100644 --- a/action/rlp_tx_test.go +++ b/action/rlp_tx_test.go @@ -332,7 +332,7 @@ func TestRlpDecodeVerify(t *testing.T) { require.NoError(err) require.True(bytes.Equal(rawHash[:], raw[:])) require.NotEqual(raw, h) - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) } } diff --git a/action/sealedenvelope.go b/action/sealedenvelope.go index db2b913ba7..5726c61b07 100644 --- a/action/sealedenvelope.go +++ b/action/sealedenvelope.go @@ -1,14 +1,18 @@ package action import ( + "encoding/hex" + "github.com/iotexproject/go-pkgs/crypto" "github.com/iotexproject/go-pkgs/hash" "github.com/iotexproject/iotex-address/address" "github.com/iotexproject/iotex-proto/golang/iotextypes" "github.com/pkg/errors" + "go.uber.org/zap" "google.golang.org/protobuf/proto" "github.com/iotexproject/iotex-core/config" + "github.com/iotexproject/iotex-core/pkg/log" "github.com/iotexproject/iotex-core/pkg/util/byteutil" ) @@ -179,3 +183,27 @@ func wrapStakingActionIntoExecution(ab AbstractAction, toAddr []byte, pb proto.M data: data, }, nil } + +// Verify verifies the action using sender's public key +func (sealed *SealedEnvelope) Verify() error { + if sealed.SrcPubkey() == nil { + return errors.New("empty public key") + } + // Reject action with insufficient gas limit + intrinsicGas, err := sealed.IntrinsicGas() + if intrinsicGas > sealed.GasLimit() || err != nil { + return ErrIntrinsicGas + } + + h, err := sealed.envelopeHash() + if err != nil { + return errors.Wrap(err, "failed to generate envelope hash") + } + if !sealed.SrcPubkey().Verify(h[:], sealed.Signature()) { + log.L().Info("failed to verify action hash", + zap.String("hash", hex.EncodeToString(h[:])), + zap.String("signature", hex.EncodeToString(sealed.Signature()))) + return ErrInvalidSender + } + return nil +} diff --git a/action/stake_changecandidate_test.go b/action/stake_changecandidate_test.go index b14c8c239c..d043e5fca4 100644 --- a/action/stake_changecandidate_test.go +++ b/action/stake_changecandidate_test.go @@ -75,7 +75,7 @@ func TestChangeCandidateSignVerify(t *testing.T) { require.NoError(err) require.Equal("186526b5b9fe74e25beb52c83c41780a69108160bef2ddaf3bffb9f1f1e5e73a", hex.EncodeToString(hash[:])) // verify signature - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) } func TestChangeCandidateABIEncodeAndDecode(t *testing.T) { diff --git a/action/stake_transferownership_test.go b/action/stake_transferownership_test.go index 82c70a4abc..ad9b3be268 100644 --- a/action/stake_transferownership_test.go +++ b/action/stake_transferownership_test.go @@ -61,7 +61,7 @@ func TestStakingTransferSignVerify(t *testing.T) { require.NoError(err) require.Equal("74b2e1d6a09ba5d1298fa422d5850991ae516865077282196295a38f93c78b85", hex.EncodeToString(hash[:])) // verify signature - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) } func TestStakingTransferABIEncodeAndDecode(t *testing.T) { diff --git a/action/stakeadddeposit_test.go b/action/stakeadddeposit_test.go index 06d116d2b6..edd0f0a1da 100644 --- a/action/stakeadddeposit_test.go +++ b/action/stakeadddeposit_test.go @@ -111,7 +111,7 @@ func TestDeposit(t *testing.T) { require.NoError(err) require.Equal(test.SelpHash, hex.EncodeToString(hash[:])) // verify signature - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) } } diff --git a/action/stakecreate_test.go b/action/stakecreate_test.go index 9d3ef99013..0724b358e5 100644 --- a/action/stakecreate_test.go +++ b/action/stakecreate_test.go @@ -118,7 +118,7 @@ func TestCreateStake(t *testing.T) { require.NoError(err) require.Equal(test.SelpHash, hex.EncodeToString(hash[:])) // verify signature - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) } } diff --git a/action/stakereclaim_test.go b/action/stakereclaim_test.go index 9e43f93cb7..b1bf1fe0ab 100644 --- a/action/stakereclaim_test.go +++ b/action/stakereclaim_test.go @@ -79,7 +79,7 @@ func TestUnstakeSignVerify(t *testing.T) { require.NoError(err) require.Equal("bed58b64a6c4e959eca60a86f0b2149ce0e1dd527ac5fd26aef725ebf7c22a7d", hex.EncodeToString(hash[:])) // verify signature - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) } func TestUnstakeABIEncodeAndDecode(t *testing.T) { @@ -148,7 +148,7 @@ func TestWithdrawSignVerify(t *testing.T) { require.NoError(err) require.Equal("28049348cf34f1aa927caa250e7a1b08778c44efaf73b565b6fa9abe843871b4", hex.EncodeToString(hash[:])) // verify signature - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) } func TestWithdrawABIEncodeAndDecode(t *testing.T) { diff --git a/action/stakerestake_test.go b/action/stakerestake_test.go index 925dcde8fa..6b619d212d 100644 --- a/action/stakerestake_test.go +++ b/action/stakerestake_test.go @@ -74,7 +74,7 @@ func TestRestakeSignVerify(t *testing.T) { require.NoError(err) require.Equal("8816e8f784a1fce40b54d1cd172bb6976fd9552f1570c73d1d9fcdc5635424a9", hex.EncodeToString(hash[:])) // verify signature - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) } func TestRestakeABIEncodeAndDecode(t *testing.T) { diff --git a/action/transfer_test.go b/action/transfer_test.go index 244be3fb60..113064a721 100644 --- a/action/transfer_test.go +++ b/action/transfer_test.go @@ -34,7 +34,7 @@ func TestTransferSignVerify(t *testing.T) { require.True(ok) w := AssembleSealedEnvelope(elp, senderKey.PublicKey(), []byte("lol")) - require.Error(Verify(w)) + require.Error(w.Verify()) tsf2, ok := w.Envelope.Action().(*Transfer) require.True(ok) require.Equal(tsf, tsf2) @@ -46,7 +46,7 @@ func TestTransferSignVerify(t *testing.T) { require.NotNil(selp) // verify signature - require.NoError(Verify(selp)) + require.NoError(selp.Verify()) } func TestTransfer(t *testing.T) { @@ -67,7 +67,7 @@ func TestTransfer(t *testing.T) { require.True(ok) w := AssembleSealedEnvelope(elp, senderKey.PublicKey(), []byte("lol")) - require.Error(Verify(w)) + require.Error(w.Verify()) require.NoError(err) require.Equal("10", tsf.Amount().Text(10))