From e4effde897ba0be0caf92672bedcaac132e3f89b Mon Sep 17 00:00:00 2001 From: dustinxie Date: Wed, 28 Aug 2024 23:25:03 -0700 Subject: [PATCH] [action] remove AbstractAction from all actions --- action/actctx.go | 17 ---------- action/action.go | 6 ++-- action/builder.go | 7 +--- action/envelope.go | 33 +++++++++++++++++-- action/protocol/account/protocol.go | 5 ++- action/protocol/account/transfer.go | 10 +++--- action/protocol/generic_validator.go | 3 +- action/signedaction.go | 6 +--- action/transfer.go | 30 +++-------------- action/tx_container.go | 2 -- state/factory/factory_test.go | 39 +++++++--------------- state/factory/workingset_test.go | 16 ++------- test/mock/mock_envelope/mock_envelope.go | 42 ++++++++++++++++-------- 13 files changed, 92 insertions(+), 124 deletions(-) diff --git a/action/actctx.go b/action/actctx.go index 25f42be2bf..fffbcae0ba 100644 --- a/action/actctx.go +++ b/action/actctx.go @@ -86,23 +86,6 @@ func (act *AbstractAction) BasicActionSize() uint32 { return uint32(size) } -// SetEnvelopeContext sets the struct according to input -func (act *AbstractAction) SetEnvelopeContext(in *AbstractAction) { - if act == nil { - return - } - *act = *in - if in.gasPrice != nil { - act.gasPrice = new(big.Int).Set(in.gasPrice) - } - if in.gasTipCap != nil { - act.gasTipCap = new(big.Int).Set(in.gasTipCap) - } - if in.gasFeeCap != nil { - act.gasFeeCap = new(big.Int).Set(in.gasFeeCap) - } -} - // SanityCheck validates the variables in the action func (act *AbstractAction) SanityCheck() error { // Reject execution of negative gas price diff --git a/action/action.go b/action/action.go index 1639139116..21eae53209 100644 --- a/action/action.go +++ b/action/action.go @@ -33,9 +33,7 @@ type ( } actionPayload interface { - Cost() (*big.Int, error) IntrinsicGas() (uint64, error) - SetEnvelopeContext(*AbstractAction) SanityCheck() error } @@ -46,6 +44,10 @@ type ( hasSize interface { Size() uint32 } + + hasAmount interface { + Amount() *big.Int + } ) // Sign signs the action using sender's private key diff --git a/action/builder.go b/action/builder.go index 7521ba0c7f..3dbe84122d 100644 --- a/action/builder.go +++ b/action/builder.go @@ -146,7 +146,6 @@ func (b *EnvelopeBuilder) build() Envelope { if b.elp.payload == nil { panic("cannot build Envelope w/o a valid payload") } - b.elp.payload.SetEnvelopeContext(&b.elp.AbstractAction) return &b.elp } @@ -156,11 +155,7 @@ func (b *EnvelopeBuilder) BuildTransfer(tx *types.Transaction) (Envelope, error) return nil, ErrInvalidAct } b.setEnvelopeCommonFields(tx) - tsf, err := NewTransfer(tx.Nonce(), tx.Value(), getRecipientAddr(tx.To()), tx.Data(), tx.Gas(), tx.GasPrice()) - if err != nil { - return nil, err - } - b.elp.payload = tsf + b.elp.payload = NewTransfer(tx.Value(), getRecipientAddr(tx.To()), tx.Data()) return b.build(), nil } diff --git a/action/envelope.go b/action/envelope.go index 506e2bb1cf..754ba251b5 100644 --- a/action/envelope.go +++ b/action/envelope.go @@ -18,7 +18,7 @@ import ( type ( // Envelope defines an envelope wrapped on action with some envelope metadata. Envelope interface { - Version() uint32 + CommonAction() AbstractAction Nonce() uint64 ChainID() uint32 GasLimit() uint64 @@ -35,6 +35,7 @@ type ( LoadProto(pbAct *iotextypes.ActionCore) error SetNonce(n uint64) SetChainID(chainID uint32) + SanityCheck() error } envelope struct { @@ -43,6 +44,10 @@ type ( } ) +func (elp *envelope) CommonAction() AbstractAction { + return elp.AbstractAction +} + // Destination returns the destination address func (elp *envelope) Destination() (string, bool) { r, ok := elp.payload.(hasDestination) @@ -55,7 +60,22 @@ func (elp *envelope) Destination() (string, bool) { // Cost returns cost of actions func (elp *envelope) Cost() (*big.Int, error) { - return elp.payload.Cost() + var cost *big.Int + switch act := elp.payload.(type) { + case *Execution, *MigrateStake: + cost = new(big.Int).SetUint64(elp.GasLimit()) + default: + gas, err := act.IntrinsicGas() + if err != nil { + return nil, errors.Wrap(err, "failed to get payload's intrinsic gas") + } + cost = new(big.Int).SetUint64(gas) + } + cost.Mul(cost, elp.GasPrice()) + if amount, ok := elp.payload.(hasAmount); ok { + cost.Add(cost, amount.Amount()) + } + return cost, nil } // IntrinsicGas returns intrinsic gas of action. @@ -278,9 +298,16 @@ func (elp *envelope) LoadProto(pbAct *iotextypes.ActionCore) error { default: return errors.Errorf("no applicable action to handle proto type %T", pbAct.Action) } - elp.payload.SetEnvelopeContext(&elp.AbstractAction) return nil } // SetChainID sets the chainID value func (elp *envelope) SetChainID(chainID uint32) { elp.chainID = chainID } + +// SanityCheck does the sanity check +func (elp *envelope) SanityCheck() error { + if err := elp.payload.SanityCheck(); err != nil { + return err + } + return elp.AbstractAction.SanityCheck() +} diff --git a/action/protocol/account/protocol.go b/action/protocol/account/protocol.go index b600bad5a9..7757911f3e 100644 --- a/action/protocol/account/protocol.go +++ b/action/protocol/account/protocol.go @@ -65,9 +65,8 @@ func FindProtocol(registry *protocol.Registry) *Protocol { // Handle handles an account func (p *Protocol) Handle(ctx context.Context, elp action.Envelope, sm protocol.StateManager) (*action.Receipt, error) { - switch act := elp.Action().(type) { - case *action.Transfer: - return p.handleTransfer(ctx, act, sm) + if _, ok := elp.Action().(*action.Transfer); ok { + return p.handleTransfer(ctx, elp, sm) } return nil, nil } diff --git a/action/protocol/account/transfer.go b/action/protocol/account/transfer.go index 2672920e58..6843b4f59d 100644 --- a/action/protocol/account/transfer.go +++ b/action/protocol/account/transfer.go @@ -23,11 +23,13 @@ import ( const TransferSizeLimit = 32 * 1024 // handleTransfer handles a transfer -func (p *Protocol) handleTransfer(ctx context.Context, tsf *action.Transfer, sm protocol.StateManager) (*action.Receipt, error) { +func (p *Protocol) handleTransfer(ctx context.Context, elp action.Envelope, sm protocol.StateManager) (*action.Receipt, error) { var ( fCtx = protocol.MustGetFeatureCtx(ctx) actionCtx = protocol.MustGetActionCtx(ctx) blkCtx = protocol.MustGetBlockCtx(ctx) + common = elp.CommonAction() + tsf = elp.Action().(*action.Transfer) ) accountCreationOpts := []state.AccountCreationOption{} if fCtx.CreateLegacyNonceAccount { @@ -39,7 +41,7 @@ func (p *Protocol) handleTransfer(ctx context.Context, tsf *action.Transfer, sm return nil, errors.Wrapf(err, "failed to load or create the account of sender %s", actionCtx.Caller.String()) } - gasFee, baseFee, err := protocol.SplitGas(ctx, &tsf.AbstractAction, actionCtx.IntrinsicGas) + gasFee, baseFee, err := protocol.SplitGas(ctx, &common, actionCtx.IntrinsicGas) if err != nil { return nil, errors.Wrapf(err, "failed to split gas") } @@ -71,9 +73,9 @@ func (p *Protocol) handleTransfer(ctx context.Context, tsf *action.Transfer, sm } } - if fCtx.FixGasAndNonceUpdate || tsf.Nonce() != 0 { + if fCtx.FixGasAndNonceUpdate || common.Nonce() != 0 { // update sender Nonce - if err := sender.SetPendingNonce(tsf.Nonce() + 1); err != nil { + if err := sender.SetPendingNonce(common.Nonce() + 1); err != nil { return nil, errors.Wrapf(err, "failed to update pending nonce of sender %s", actionCtx.Caller.String()) } } diff --git a/action/protocol/generic_validator.go b/action/protocol/generic_validator.go index ef8efc2216..0b9f167571 100644 --- a/action/protocol/generic_validator.go +++ b/action/protocol/generic_validator.go @@ -85,6 +85,5 @@ func (v *GenericValidator) Validate(ctx context.Context, selp *action.SealedEnve } } } - - return selp.Action().SanityCheck() + return selp.Envelope.SanityCheck() } diff --git a/action/signedaction.go b/action/signedaction.go index 1bad27a90d..46e9852651 100644 --- a/action/signedaction.go +++ b/action/signedaction.go @@ -30,15 +30,11 @@ func WithChainID(chainID uint32) SignedActionOption { // SignedTransfer return a signed transfer func SignedTransfer(recipientAddr string, senderPriKey crypto.PrivateKey, nonce uint64, amount *big.Int, payload []byte, gasLimit uint64, gasPrice *big.Int, options ...SignedActionOption) (*SealedEnvelope, error) { - transfer, err := NewTransfer(nonce, amount, recipientAddr, payload, gasLimit, gasPrice) - if err != nil { - return nil, err - } bd := &EnvelopeBuilder{} bd = bd.SetNonce(nonce). SetGasPrice(gasPrice). SetGasLimit(gasLimit). - SetAction(transfer) + SetAction(NewTransfer(amount, recipientAddr, payload)) for _, opt := range options { opt(bd) } diff --git a/action/transfer.go b/action/transfer.go index ec26b8c53c..b91d4d4a78 100644 --- a/action/transfer.go +++ b/action/transfer.go @@ -15,7 +15,6 @@ import ( "google.golang.org/protobuf/proto" "github.com/iotexproject/iotex-core/pkg/util/byteutil" - "github.com/iotexproject/iotex-core/pkg/version" ) const ( @@ -28,13 +27,12 @@ const ( var ( _ hasDestination = (*Transfer)(nil) _ hasSize = (*Transfer)(nil) + _ hasAmount = (*Transfer)(nil) _ EthCompatibleAction = (*Transfer)(nil) ) // Transfer defines the struct of account-based transfer type Transfer struct { - AbstractAction - amount *big.Int recipient string payload []byte @@ -42,25 +40,15 @@ type Transfer struct { // NewTransfer returns a Transfer instance func NewTransfer( - nonce uint64, amount *big.Int, recipient string, payload []byte, - gasLimit uint64, - gasPrice *big.Int, -) (*Transfer, error) { +) *Transfer { return &Transfer{ - AbstractAction: AbstractAction{ - version: version.ProtocolVersion, - nonce: nonce, - gasLimit: gasLimit, - gasPrice: gasPrice, - }, recipient: recipient, amount: amount, payload: payload, - // SenderPublicKey and Signature will be populated in Sign() - }, nil + } } // Amount returns the amount @@ -135,23 +123,13 @@ func (tsf *Transfer) IntrinsicGas() (uint64, error) { return CalculateIntrinsicGas(TransferBaseIntrinsicGas, TransferPayloadGas, payloadSize) } -// Cost returns the total cost of a transfer -func (tsf *Transfer) Cost() (*big.Int, error) { - intrinsicGas, err := tsf.IntrinsicGas() - if err != nil { - return nil, errors.Wrap(err, "failed to get intrinsic gas for the transfer") - } - transferFee := big.NewInt(0).Mul(tsf.GasPrice(), big.NewInt(0).SetUint64(intrinsicGas)) - return big.NewInt(0).Add(tsf.Amount(), transferFee), nil -} - // SanityCheck validates the variables in the action func (tsf *Transfer) SanityCheck() error { // Reject transfer of negative amount if tsf.Amount().Sign() < 0 { return ErrNegativeValue } - return tsf.AbstractAction.SanityCheck() + return nil } // EthTo returns the address for converting to eth tx diff --git a/action/tx_container.go b/action/tx_container.go index 8fe071ac3f..048ee74131 100644 --- a/action/tx_container.go +++ b/action/tx_container.go @@ -126,8 +126,6 @@ func (etx *txContainer) IntrinsicGas() (uint64, error) { return gas, nil } -func (etx *txContainer) SetEnvelopeContext(*AbstractAction) {} - func (etx *txContainer) SanityCheck() error { // Reject execution of negative amount if etx.tx.Value().Sign() < 0 { diff --git a/state/factory/factory_test.go b/state/factory/factory_test.go index 2c8c32aa07..a35e228637 100644 --- a/state/factory/factory_test.go +++ b/state/factory/factory_test.go @@ -469,8 +469,7 @@ func testState(sf Factory, t *testing.T) { require.NoError(t, sf.Stop(ctx)) }() - tsf, err := action.NewTransfer(1, big.NewInt(10), identityset.Address(31).String(), nil, uint64(20000), big.NewInt(0)) - require.NoError(t, err) + tsf := action.NewTransfer(big.NewInt(10), identityset.Address(31).String(), nil) bd := &action.EnvelopeBuilder{} elp := bd.SetAction(tsf).SetGasLimit(20000).SetNonce(1).Build() selp, err := action.Sign(elp, priKeyA) @@ -533,8 +532,7 @@ func testHistoryState(sf Factory, t *testing.T, statetx, archive bool) { require.NoError(t, err) require.Equal(t, big.NewInt(100), accountA.Balance) require.Equal(t, big.NewInt(0), accountB.Balance) - tsf, err := action.NewTransfer(1, big.NewInt(10), b.String(), nil, uint64(20000), big.NewInt(0)) - require.NoError(t, err) + tsf := action.NewTransfer(big.NewInt(10), b.String(), nil) bd := &action.EnvelopeBuilder{} elp := bd.SetAction(tsf).SetGasLimit(20000).SetNonce(1).Build() selp, err := action.Sign(elp, priKeyA) @@ -617,8 +615,7 @@ func testFactoryStates(sf Factory, t *testing.T) { defer func() { require.NoError(t, sf.Stop(ctx)) }() - tsf, err := action.NewTransfer(1, big.NewInt(10), b, nil, uint64(20000), big.NewInt(0)) - require.NoError(t, err) + tsf := action.NewTransfer(big.NewInt(10), b, nil) bd := &action.EnvelopeBuilder{} elp := bd.SetAction(tsf).SetGasLimit(20000).SetNonce(1).Build() selp, err := action.Sign(elp, priKeyA) @@ -784,10 +781,9 @@ func testNonce(ctx context.Context, sf Factory, t *testing.T) { ws, err := sf.(workingSetCreator).newWorkingSet(ctx, 1) require.NoError(t, err) - tx, err := action.NewTransfer(0, big.NewInt(2), b, nil, uint64(20000), big.NewInt(0)) - require.NoError(t, err) + tx := action.NewTransfer(big.NewInt(2), b, nil) bd := &action.EnvelopeBuilder{} - elp := bd.SetAction(tx).SetNonce(0).SetGasLimit(20000).Build() + elp := bd.SetAction(tx).SetGasLimit(20000).Build() selp, err := action.Sign(elp, priKeyA) require.NoError(t, err) @@ -820,8 +816,7 @@ func testNonce(ctx context.Context, sf Factory, t *testing.T) { require.NoError(t, err) require.Equal(t, uint64(1), state.PendingNonce()) - tx, err = action.NewTransfer(1, big.NewInt(2), b, nil, uint64(20000), big.NewInt(0)) - require.NoError(t, err) + tx = action.NewTransfer(big.NewInt(2), b, nil) bd = &action.EnvelopeBuilder{} elp = bd.SetAction(tx).SetNonce(1).SetGasLimit(20000).Build() selp, err = action.Sign(elp, priKeyA) @@ -995,15 +990,13 @@ func testCommit(factory Factory, t *testing.T) { b := identityset.Address(29).String() priKeyB := identityset.PrivateKey(29) - tx1, err := action.NewTransfer(uint64(1), big.NewInt(10), b, nil, uint64(100000), big.NewInt(0)) - require.NoError(err) + tx1 := action.NewTransfer(big.NewInt(10), b, nil) bd := &action.EnvelopeBuilder{} elp := bd.SetNonce(1).SetAction(tx1).Build() selp1, err := action.Sign(elp, priKeyA) require.NoError(err) - tx2, err := action.NewTransfer(uint64(1), big.NewInt(20), a, nil, uint64(100000), big.NewInt(0)) - require.NoError(err) + tx2 := action.NewTransfer(big.NewInt(20), a, nil) bd = &action.EnvelopeBuilder{} elp = bd.SetNonce(1).SetAction(tx2).Build() selp2, err := action.Sign(elp, priKeyB) @@ -1107,8 +1100,7 @@ func testNewBlockBuilder(factory Factory, t *testing.T) { priKeyB := identityset.PrivateKey(29) accMap := make(map[string][]*action.SealedEnvelope) - tx1, err := action.NewTransfer(uint64(1), big.NewInt(10), b, nil, uint64(100000), big.NewInt(0)) - require.NoError(err) + tx1 := action.NewTransfer(big.NewInt(10), b, nil) bd := &action.EnvelopeBuilder{} elp := bd.SetNonce(1).SetAction(tx1).Build() selp1, err := action.Sign(elp, priKeyA) @@ -1120,8 +1112,7 @@ func testNewBlockBuilder(factory Factory, t *testing.T) { require.NoError(err) accMap[identityset.Address(0).String()] = []*action.SealedEnvelope{tsf0} - tx2, err := action.NewTransfer(uint64(1), big.NewInt(20), a, nil, uint64(100000), big.NewInt(0)) - require.NoError(err) + tx2 := action.NewTransfer(big.NewInt(20), a, nil) bd = &action.EnvelopeBuilder{} elp = bd.SetNonce(1).SetAction(tx2).Build() selp2, err := action.Sign(elp, priKeyB) @@ -1584,10 +1575,7 @@ func benchRunAction(sf Factory, b *testing.B) { } receiver := receiverAddr.String() nonces[senderIdx] += nonces[senderIdx] - tx, err := action.NewTransfer(nonces[senderIdx], big.NewInt(1), receiver, nil, uint64(0), big.NewInt(0)) - if err != nil { - b.Fatal(err) - } + tx := action.NewTransfer(big.NewInt(1), receiver, nil) bd := &action.EnvelopeBuilder{} elp := bd.SetNonce(nonces[senderIdx]).SetAction(tx).Build() selp := action.FakeSeal(elp, pubKeys[senderIdx]) @@ -1713,10 +1701,7 @@ func benchState(sf Factory, b *testing.B) { } receiver := receiverAddr.String() nonces[senderIdx] += nonces[senderIdx] - tx, err := action.NewTransfer(nonces[senderIdx], big.NewInt(1), receiver, nil, uint64(0), big.NewInt(0)) - if err != nil { - b.Fatal(err) - } + tx := action.NewTransfer(big.NewInt(1), receiver, nil) bd := &action.EnvelopeBuilder{} elp := bd.SetNonce(nonces[senderIdx]).SetAction(tx).Build() selp := action.FakeSeal(elp, pubKeys[senderIdx]) diff --git a/state/factory/workingset_test.go b/state/factory/workingset_test.go index ba3f0db751..75de2c8a6d 100644 --- a/state/factory/workingset_test.go +++ b/state/factory/workingset_test.go @@ -358,20 +358,10 @@ func TestWorkingSet_ValidateBlock_SystemAction(t *testing.T) { } func makeTransferAction(t *testing.T, nonce uint64) *action.SealedEnvelope { - tsf, err := action.NewTransfer( - uint64(nonce), - big.NewInt(1), - identityset.Address(29).String(), - nil, - testutil.TestGasLimit, - big.NewInt(0), - ) - require.NoError(t, err) - eb := action.EnvelopeBuilder{} - evlp := eb. + tsf := action.NewTransfer(big.NewInt(1), identityset.Address(29).String(), nil) + evlp := (&action.EnvelopeBuilder{}). SetAction(tsf). - SetGasLimit(tsf.GasLimit()). - SetGasPrice(tsf.GasPrice()). + SetGasLimit(testutil.TestGasLimit). SetNonce(nonce). SetChainID(1). SetVersion(1). diff --git a/test/mock/mock_envelope/mock_envelope.go b/test/mock/mock_envelope/mock_envelope.go index e2d67bc77e..d97a307877 100644 --- a/test/mock/mock_envelope/mock_envelope.go +++ b/test/mock/mock_envelope/mock_envelope.go @@ -65,6 +65,20 @@ func (mr *MockEnvelopeMockRecorder) ChainID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ChainID", reflect.TypeOf((*MockEnvelope)(nil).ChainID)) } +// CommonAction mocks base method. +func (m *MockEnvelope) CommonAction() action.AbstractAction { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CommonAction") + ret0, _ := ret[0].(action.AbstractAction) + return ret0 +} + +// CommonAction indicates an expected call of CommonAction. +func (mr *MockEnvelopeMockRecorder) CommonAction() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CommonAction", reflect.TypeOf((*MockEnvelope)(nil).CommonAction)) +} + // Cost mocks base method. func (m *MockEnvelope) Cost() (*big.Int, error) { m.ctrl.T.Helper() @@ -208,6 +222,20 @@ func (mr *MockEnvelopeMockRecorder) Proto() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Proto", reflect.TypeOf((*MockEnvelope)(nil).Proto)) } +// SanityCheck mocks base method. +func (m *MockEnvelope) SanityCheck() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SanityCheck") + ret0, _ := ret[0].(error) + return ret0 +} + +// SanityCheck indicates an expected call of SanityCheck. +func (mr *MockEnvelopeMockRecorder) SanityCheck() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SanityCheck", reflect.TypeOf((*MockEnvelope)(nil).SanityCheck)) +} + // SetChainID mocks base method. func (m *MockEnvelope) SetChainID(chainID uint32) { m.ctrl.T.Helper() @@ -260,17 +288,3 @@ func (mr *MockEnvelopeMockRecorder) ToEthTx(arg0, arg1 interface{}) *gomock.Call mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ToEthTx", reflect.TypeOf((*MockEnvelope)(nil).ToEthTx), arg0, arg1) } - -// Version mocks base method. -func (m *MockEnvelope) Version() uint32 { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Version") - ret0, _ := ret[0].(uint32) - return ret0 -} - -// Version indicates an expected call of Version. -func (mr *MockEnvelopeMockRecorder) Version() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Version", reflect.TypeOf((*MockEnvelope)(nil).Version)) -}