Merge pull request #3490 from jorgemmsilva/fix/evm-event-deposit

fix: avoid edge case when deposit event gets wrongly emitted

packages/isc/sandbox_interface.go

@@ -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 {

packages/vm/core/evm/evmimpl/impl.go

@@ -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)
 	})
 
@@ -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

packages/vm/core/evm/evmtest/evm_test.go

@@ -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)
+}

packages/vm/vmimpl/internal.go

@@ -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