Skip to content

Commit 2cf6593

Browse files
facundomedicavladjdk
facundomedica
and
vladjdk
authored
feat: allow PostTxProcessing to run on failures and persist data (cosmos#479)
* feat: allow PostProcessingTx to run on failures and persist data * linf fix * add check for hooks --------- Co-authored-by: Vlad J <[email protected]>
1 parent 1f22f19 commit 2cf6593

File tree

3 files changed

+230
-29
lines changed

3 files changed

+230
-29
lines changed

‎tests/integration/x/vm/test_state_transition.go‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package vm
22

33
import (
4+
"errors"
45
"fmt"
56
"math"
67
"math/big"
78

89
"github.com/ethereum/go-ethereum/common"
910
"github.com/ethereum/go-ethereum/core"
11+
ethtypes "github.com/ethereum/go-ethereum/core/types"
1012
gethtypes "github.com/ethereum/go-ethereum/core/types"
1113
"github.com/ethereum/go-ethereum/params"
1214

@@ -624,6 +626,173 @@ func (s *KeeperTestSuite) TestApplyTransaction() {
624626
}
625627
}
626628

629+
type testHooks struct {
630+
postProcessing func(ctx sdk.Context, sender common.Address, msg core.Message, receipt *ethtypes.Receipt) error
631+
}
632+
633+
func (h *testHooks) PostTxProcessing(ctx sdk.Context, sender common.Address, msg core.Message, receipt *ethtypes.Receipt) error {
634+
return h.postProcessing(ctx, sender, msg, receipt)
635+
}
636+
637+
func (s *KeeperTestSuite) TestApplyTransactionWithTxPostProcessing() {
638+
s.EnableFeemarket = true
639+
defer func() { s.EnableFeemarket = false }()
640+
641+
testCases := []struct {
642+
name string
643+
setup func(s *KeeperTestSuite)
644+
do func(s *KeeperTestSuite)
645+
after func(s *KeeperTestSuite)
646+
}{
647+
{
648+
"pass - evm tx succeeds, post processing is called, the balance is changed",
649+
func(s *KeeperTestSuite) {
650+
s.Network.App.GetEVMKeeper().SetHooks(
651+
keeper.NewMultiEvmHooks(
652+
&testHooks{
653+
postProcessing: func(ctx sdk.Context, sender common.Address, msg core.Message, receipt *ethtypes.Receipt) error {
654+
return nil
655+
},
656+
},
657+
),
658+
)
659+
},
660+
func(s *KeeperTestSuite) {
661+
senderBefore := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Keyring.GetAccAddr(0), "aatom").Amount
662+
recipientBefore := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Keyring.GetAccAddr(1), "aatom").Amount
663+
664+
// Generate a transfer tx message
665+
sender := s.Keyring.GetKey(0)
666+
recipient := s.Keyring.GetAddr(1)
667+
transferAmt := big.NewInt(100)
668+
669+
tx, err := s.Factory.GenerateSignedEthTx(sender.Priv, types.EvmTxArgs{
670+
To: &recipient,
671+
Amount: transferAmt,
672+
})
673+
s.Require().NoError(err)
674+
675+
ethMsg := tx.GetMsgs()[0].(*types.MsgEthereumTx)
676+
res, err := s.Network.App.GetEVMKeeper().ApplyTransaction(s.Network.GetContext(), ethMsg.AsTransaction())
677+
s.Require().NoError(err)
678+
s.Require().False(res.Failed())
679+
680+
senderAfter := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Keyring.GetAccAddr(0), "aatom").Amount
681+
recipientAfter := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Keyring.GetAccAddr(1), "aatom").Amount
682+
s.Require().Equal(senderBefore.Sub(sdkmath.NewIntFromBigInt(transferAmt)), senderAfter)
683+
s.Require().Equal(recipientBefore.Add(sdkmath.NewIntFromBigInt(transferAmt)), recipientAfter)
684+
},
685+
func(s *KeeperTestSuite) {},
686+
},
687+
{
688+
"pass - evm tx succeeds, post processing is called but fails, the balance is unchanged",
689+
func(s *KeeperTestSuite) {
690+
s.Network.App.GetEVMKeeper().SetHooks(
691+
keeper.NewMultiEvmHooks(
692+
&testHooks{
693+
postProcessing: func(ctx sdk.Context, sender common.Address, msg core.Message, receipt *ethtypes.Receipt) error {
694+
return errors.New("post processing failed :(")
695+
},
696+
},
697+
),
698+
)
699+
},
700+
func(s *KeeperTestSuite) {
701+
senderBefore := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Keyring.GetAccAddr(0), "aatom").Amount
702+
recipientBefore := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Keyring.GetAccAddr(1), "aatom").Amount
703+
704+
// Generate a transfer tx message
705+
sender := s.Keyring.GetKey(0)
706+
recipient := s.Keyring.GetAddr(1)
707+
transferAmt := big.NewInt(100)
708+
709+
tx, err := s.Factory.GenerateSignedEthTx(sender.Priv, types.EvmTxArgs{
710+
To: &recipient,
711+
Amount: transferAmt,
712+
})
713+
s.Require().NoError(err)
714+
715+
ethMsg := tx.GetMsgs()[0].(*types.MsgEthereumTx)
716+
res, err := s.Network.App.GetEVMKeeper().ApplyTransaction(s.Network.GetContext(), ethMsg.AsTransaction())
717+
s.Require().NoError(err)
718+
s.Require().True(res.Failed())
719+
720+
senderAfter := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Keyring.GetAccAddr(0), "aatom").Amount
721+
recipientAfter := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Keyring.GetAccAddr(1), "aatom").Amount
722+
s.Require().Equal(senderBefore, senderAfter)
723+
s.Require().Equal(recipientBefore, recipientAfter)
724+
},
725+
func(s *KeeperTestSuite) {},
726+
},
727+
{
728+
"evm tx fails, post processing is called and persisted, the balance is not changed",
729+
func(s *KeeperTestSuite) {
730+
s.Network.App.GetEVMKeeper().SetHooks(
731+
keeper.NewMultiEvmHooks(
732+
&testHooks{
733+
postProcessing: func(ctx sdk.Context, sender common.Address, msg core.Message, receipt *ethtypes.Receipt) error {
734+
return s.Network.App.GetMintKeeper().MintCoins(
735+
ctx, sdk.NewCoins(sdk.NewCoin("arandomcoin", sdkmath.NewInt(100))),
736+
)
737+
},
738+
},
739+
),
740+
)
741+
},
742+
func(s *KeeperTestSuite) {
743+
senderBefore := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Keyring.GetAccAddr(0), "aatom").Amount
744+
recipientBefore := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Keyring.GetAccAddr(1), "aatom").Amount
745+
746+
// Generate a transfer tx message
747+
sender := s.Keyring.GetKey(0)
748+
recipient := s.Keyring.GetAddr(1)
749+
transferAmt := senderBefore.Add(sdkmath.NewInt(100)) // transfer more than the balance
750+
751+
tx, err := s.Factory.GenerateSignedEthTx(sender.Priv, types.EvmTxArgs{
752+
To: &recipient,
753+
Amount: transferAmt.BigInt(),
754+
})
755+
s.Require().NoError(err)
756+
757+
ethMsg := tx.GetMsgs()[0].(*types.MsgEthereumTx)
758+
res, err := s.Network.App.GetEVMKeeper().ApplyTransaction(s.Network.GetContext(), ethMsg.AsTransaction())
759+
s.Require().NoError(err)
760+
s.Require().True(res.Failed())
761+
762+
senderAfter := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Keyring.GetAccAddr(0), "aatom").Amount
763+
recipientAfter := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Keyring.GetAccAddr(1), "aatom").Amount
764+
s.Require().Equal(senderBefore, senderAfter)
765+
s.Require().Equal(recipientBefore, recipientAfter)
766+
},
767+
func(s *KeeperTestSuite) {
768+
// check if the mint module has "arandomcoin" in its balance, it was minted in the post processing, proving that the post processing was called
769+
// and that it can persist state even when the tx fails
770+
balance := s.Network.App.GetBankKeeper().GetBalance(s.Network.GetContext(), s.Network.App.GetAccountKeeper().GetModuleAddress("mint"), "arandomcoin")
771+
s.Require().Equal(sdkmath.NewInt(100), balance.Amount)
772+
},
773+
},
774+
}
775+
776+
for _, tc := range testCases {
777+
s.Run(fmt.Sprintf("Case %s", tc.name), func() {
778+
s.SetupTest()
779+
780+
tc.setup(s)
781+
782+
// set bounded cosmos block gas limit
783+
ctx := s.Network.GetContext().WithBlockGasMeter(storetypes.NewGasMeter(1e6))
784+
err := s.Network.App.GetBankKeeper().MintCoins(ctx, "mint", sdk.NewCoins(sdk.NewCoin("aatom", sdkmath.NewInt(3e18))))
785+
s.Require().NoError(err)
786+
err = s.Network.App.GetBankKeeper().SendCoinsFromModuleToModule(ctx, "mint", "fee_collector", sdk.NewCoins(sdk.NewCoin("aatom", sdkmath.NewInt(3e18))))
787+
s.Require().NoError(err)
788+
789+
tc.do(s)
790+
791+
tc.after(s)
792+
})
793+
}
794+
}
795+
627796
func (s *KeeperTestSuite) TestApplyMessage() {
628797
s.EnableFeemarket = true
629798
defer func() { s.EnableFeemarket = false }()

‎x/vm/keeper/keeper.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,11 @@ func (k *Keeper) PostTxProcessing(
217217
return k.hooks.PostTxProcessing(ctx, sender, msg, receipt)
218218
}
219219

220+
// HasHooks returns true if hooks are set
221+
func (k *Keeper) HasHooks() bool {
222+
return k.hooks != nil
223+
}
224+
220225
// ----------------------------------------------------------------------------
221226
// Log
222227
// ----------------------------------------------------------------------------

‎x/vm/keeper/state_transition.go‎

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -193,36 +193,54 @@ func (k *Keeper) ApplyTransaction(ctx sdk.Context, tx *ethtypes.Transaction) (*t
193193
ethLogs := types.LogsToEthereum(res.Logs)
194194
_, bloomReceipt := k.initializeBloomFromLogs(ctx, ethLogs)
195195

196-
if !res.Failed() {
197-
var contractAddr common.Address
198-
if msg.To == nil {
199-
contractAddr = crypto.CreateAddress(msg.From, msg.Nonce)
200-
}
201-
202-
receipt := &ethtypes.Receipt{
203-
Type: tx.Type(),
204-
PostState: nil,
205-
CumulativeGasUsed: calculateCumulativeGasFromEthResponse(ctx.GasMeter(), res),
206-
Bloom: bloomReceipt,
207-
Logs: ethLogs,
208-
TxHash: txConfig.TxHash,
209-
ContractAddress: contractAddr,
210-
GasUsed: res.GasUsed,
211-
BlockHash: txConfig.BlockHash,
212-
BlockNumber: big.NewInt(ctx.BlockHeight()),
213-
TransactionIndex: txConfig.TxIndex,
214-
}
196+
var contractAddr common.Address
197+
if msg.To == nil {
198+
contractAddr = crypto.CreateAddress(msg.From, msg.Nonce)
199+
}
200+
201+
receipt := &ethtypes.Receipt{
202+
Type: tx.Type(),
203+
PostState: nil,
204+
CumulativeGasUsed: calculateCumulativeGasFromEthResponse(ctx.GasMeter(), res),
205+
Bloom: bloomReceipt,
206+
Logs: ethLogs,
207+
TxHash: txConfig.TxHash,
208+
ContractAddress: contractAddr,
209+
GasUsed: res.GasUsed,
210+
BlockHash: txConfig.BlockHash,
211+
BlockNumber: big.NewInt(ctx.BlockHeight()),
212+
TransactionIndex: txConfig.TxIndex,
213+
}
214+
215+
if res.Failed() {
216+
receipt.Status = ethtypes.ReceiptStatusFailed
217+
218+
// If the tx failed we discard the old context and create a new one, so
219+
// PostTxProcessing can persist data even if the tx fails.
220+
tmpCtx, commitFn = ctx.CacheContext()
221+
} else {
222+
receipt.Status = ethtypes.ReceiptStatusSuccessful
223+
}
215224

216-
signerAddr, err := signer.Sender(tx)
217-
if err != nil {
218-
return nil, errorsmod.Wrap(err, "failed to extract sender address from ethereum transaction")
219-
}
225+
signerAddr, err := signer.Sender(tx)
226+
if err != nil {
227+
return nil, errorsmod.Wrap(err, "failed to extract sender address from ethereum transaction")
228+
}
220229

221-
eventsLen := len(tmpCtx.EventManager().Events())
230+
eventsLen := len(tmpCtx.EventManager().Events())
222231

232+
// Only call PostTxProcessing if there are hooks set, to avoid calling commitFn unnecessarily
233+
if !k.HasHooks() {
234+
// If there are no hooks, we can commit the state immediately if the tx is successful
235+
if commitFn != nil && !res.Failed() {
236+
commitFn()
237+
}
238+
} else {
223239
// Note: PostTxProcessing hooks currently do not charge for gas
224-
// and function similar to EndBlockers in abci, but for EVM transactions
225-
if err = k.PostTxProcessing(tmpCtx, signerAddr, *msg, receipt); err != nil {
240+
// and function similar to EndBlockers in abci, but for EVM transactions.
241+
// It will persist data even if the tx fails.
242+
err = k.PostTxProcessing(tmpCtx, signerAddr, *msg, receipt)
243+
if err != nil {
226244
// If hooks returns an error, revert the whole tx.
227245
res.VmError = errorsmod.Wrap(err, "failed to execute post transaction processing").Error()
228246
k.Logger(ctx).Error("tx post processing failed", "error", err)
@@ -231,11 +249,20 @@ func (k *Keeper) ApplyTransaction(ctx sdk.Context, tx *ethtypes.Transaction) (*t
231249
res.Logs = nil
232250
receipt.Logs = nil
233251
receipt.Bloom = ethtypes.Bloom{} // Clear bloom filter
234-
} else if commitFn != nil {
235-
commitFn()
252+
} else {
253+
if commitFn != nil {
254+
commitFn()
255+
}
236256

237257
// Since the post-processing can alter the log, we need to update the result
238-
res.Logs = types.NewLogsFromEth(receipt.Logs)
258+
if res.Failed() {
259+
res.Logs = nil
260+
receipt.Logs = nil
261+
receipt.Bloom = ethtypes.Bloom{}
262+
} else {
263+
res.Logs = types.NewLogsFromEth(receipt.Logs)
264+
}
265+
239266
events := tmpCtx.EventManager().Events()
240267
if len(events) > eventsLen {
241268
ctx.EventManager().EmitEvents(events[eventsLen:])

0 commit comments

Comments
 (0)