Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: avoid edge case when deposit event gets wrongly emitted #3490

Merged
merged 1 commit into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/isc/sandbox_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ type Privileged interface {
CreditToAccount(AgentID, *big.Int)
}

type CoreCallbackFunc func(contractPartition kv.KVStore, gasBurned uint64)
type CoreCallbackFunc func(contractPartition kv.KVStore, gasBurned uint64, vmError *VMError)

// RequestParameters represents parameters of the on-ledger request. The output is build from these parameters
type RequestParameters struct {
Expand Down
7 changes: 5 additions & 2 deletions packages/vm/core/evm/evmimpl/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func applyTransaction(ctx isc.Sandbox) dict.Dict {

// make sure we always store the EVM tx/receipt in the BlockchainDB, even
// if the ISC request is reverted
ctx.Privileged().OnWriteReceipt(func(evmPartition kv.KVStore, _ uint64) {
ctx.Privileged().OnWriteReceipt(func(evmPartition kv.KVStore, _ uint64, _ *isc.VMError) {
saveExecutedTx(evmPartition, chainInfo, tx, receipt)
})

Expand Down Expand Up @@ -516,7 +516,10 @@ func AddDummyTxWithTransferEvents(
return
}

ctx.Privileged().OnWriteReceipt(func(evmPartition kv.KVStore, gasBurned uint64) {
ctx.Privileged().OnWriteReceipt(func(evmPartition kv.KVStore, gasBurned uint64, vmError *isc.VMError) {
if vmError != nil {
return // do not issue deposit event if execution failed
}
receipt.GasUsed = gas.ISCGasBurnedToEVM(gasBurned, &chainInfo.GasFeePolicy.EVMGasRatio)
blockchainDB := createBlockchainDB(evmPartition, chainInfo)
receipt.CumulativeGasUsed = blockchainDB.GetPendingCumulativeGasUsed() + receipt.GasUsed
Expand Down
39 changes: 39 additions & 0 deletions packages/vm/core/evm/evmtest/evm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2800,3 +2800,42 @@ func TestDisableMagicWrap(t *testing.T) {
envWithMagicWrap := InitEVM(t, true)
require.NotNil(t, envWithMagicWrap.getCode(envWithMagicWrap.ERC20BaseTokens(nil).address))
}

func TestEVMEventOnFailedL1Deposit(t *testing.T) {
env := InitEVM(t, false)
_, ethAddr := env.Chain.NewEthereumAccountWithL2Funds()

// set gas policy to a higher price (so that it can fails when charging ISC gas)
{
feePolicy := env.Chain.GetGasFeePolicy()
feePolicy.GasPerToken.A = 1
feePolicy.GasPerToken.B = 10
err := env.setFeePolicy(*feePolicy)
require.NoError(t, err)
}
// mint an NFT and send it to the chain
issuerWallet, issuerAddress := env.solo.NewKeyPairWithFunds()
metadata := []byte("foobar")
nft, _, err := env.solo.MintNFTL1(issuerWallet, issuerAddress, metadata)
require.NoError(t, err)
ethAgentID := isc.NewEthereumAddressAgentID(env.Chain.ChainID, ethAddr)

callParams := solo.NewCallParams(accounts.Contract.Name, accounts.FuncTransferAllowanceTo.Name, accounts.ParamAgentID, codec.Encode(ethAgentID)).
AddBaseTokens(1_000_000).
WithNFT(nft).
WithAllowance(isc.NewEmptyAssets().AddNFTs(nft.ID)).
WithMaxAffordableGasBudget()

// do not include enough gas budget (but just enough to execute until the end)
_, estimatedReceipt, err := env.Chain.EstimateGasOnLedger(callParams, issuerWallet)
require.NoError(t, err)
callParams.WithGasBudget(estimatedReceipt.GasBurned - 1)

_, err = env.Chain.PostRequestSync(callParams, issuerWallet)
require.Error(t, err)
require.Contains(t, err.Error(), "gas budget exceeded")

// assert NO event is issued
logs := env.LastBlockEVMLogs()
require.Len(t, logs, 0)
}
2 changes: 1 addition & 1 deletion packages/vm/vmimpl/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func (reqctx *requestContext) writeReceiptToBlockLog(vmError *isc.VMError) *bloc
}
for _, f := range reqctx.onWriteReceipt {
reqctx.callCore(corecontracts.All[f.contract], func(s kv.KVStore) {
f.callback(s, receipt.GasBurned)
f.callback(s, receipt.GasBurned, vmError)
})
}
return receipt
Expand Down
Loading