Skip to content

Commit

Permalink
fix: out of gas on ZetaClient during onRevert (#3144)
Browse files Browse the repository at this point in the history
* add gas consumption on revert

* comment tests

* format

* fix testdapp deploy

* set no limit

* set higher gas

* decrease gas usage

* try again with 4M

* push back old values

* add tests back

* fix unit test
  • Loading branch information
lumtis authored Nov 13, 2024
1 parent 5631ad7 commit 5012d28
Show file tree
Hide file tree
Showing 10 changed files with 138 additions and 11 deletions.
2 changes: 2 additions & 0 deletions e2e/e2etests/test_deploy_contract.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func deployZEVMTestDApp(r *runner.E2ERunner) (ethcommon.Address, error) {
addr, tx, _, err := testdappv2.DeployTestDAppV2(
r.ZEVMAuth,
r.ZEVMClient,
true,
)
if err != nil {
return addr, err
Expand All @@ -64,6 +65,7 @@ func deployEVMTestDApp(r *runner.E2ERunner) (ethcommon.Address, error) {
addr, tx, _, err := testdappv2.DeployTestDAppV2(
r.EVMAuth,
r.EVMClient,
false,
)
if err != nil {
return addr, err
Expand Down
8 changes: 7 additions & 1 deletion e2e/runner/v2_setup_evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"math/big"
"time"

"github.com/ethereum/go-ethereum/accounts/abi/bind"
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/stretchr/testify/require"
erc20custodyv2 "github.com/zeta-chain/protocol-contracts/v2/pkg/erc20custody.sol"
Expand Down Expand Up @@ -101,7 +102,7 @@ func (r *E2ERunner) SetupEVMV2() {
require.NoError(r, err)

// deploy test dapp v2
testDAppV2Addr, txTestDAppV2, _, err := testdappv2.DeployTestDAppV2(r.EVMAuth, r.EVMClient)
testDAppV2Addr, txTestDAppV2, _, err := testdappv2.DeployTestDAppV2(r.EVMAuth, r.EVMClient, false)
require.NoError(r, err)

r.TestDAppV2EVMAddr = testDAppV2Addr
Expand All @@ -115,6 +116,11 @@ func (r *E2ERunner) SetupEVMV2() {
ensureTxReceipt(txSetCustody, "Set custody in Gateway failed")
ensureTxReceipt(txTestDAppV2, "TestDAppV2 deployment failed")

// check isZetaChain is false
isZetaChain, err := r.TestDAppV2EVM.IsZetaChain(&bind.CallOpts{})
require.NoError(r, err)
require.False(r, isZetaChain)

// whitelist the ERC20
txWhitelist, err := r.ERC20CustodyV2.Whitelist(r.EVMAuth, r.ERC20Addr)
require.NoError(r, err)
Expand Down
8 changes: 7 additions & 1 deletion e2e/runner/v2_setup_zeta.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package runner
import (
"time"

"github.com/ethereum/go-ethereum/accounts/abi/bind"
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/stretchr/testify/require"
"github.com/zeta-chain/protocol-contracts/v2/pkg/gatewayzevm.sol"
Expand Down Expand Up @@ -64,7 +65,7 @@ func (r *E2ERunner) SetZEVMContractsV2() {
require.NoError(r, err)

// deploy test dapp v2
testDAppV2Addr, txTestDAppV2, _, err := testdappv2.DeployTestDAppV2(r.ZEVMAuth, r.ZEVMClient)
testDAppV2Addr, txTestDAppV2, _, err := testdappv2.DeployTestDAppV2(r.ZEVMAuth, r.ZEVMClient, true)
require.NoError(r, err)

r.TestDAppV2ZEVMAddr = testDAppV2Addr
Expand All @@ -73,6 +74,11 @@ func (r *E2ERunner) SetZEVMContractsV2() {

ensureTxReceipt(txProxy, "Gateway proxy deployment failed")
ensureTxReceipt(txTestDAppV2, "TestDAppV2 deployment failed")

// check isZetaChain is true
isZetaChain, err := r.TestDAppV2ZEVM.IsZetaChain(&bind.CallOpts{})
require.NoError(r, err)
require.True(r, isZetaChain)
}

// UpdateChainParamsV2Contracts update the erc20 custody contract and gateway address in the chain params
Expand Down
24 changes: 24 additions & 0 deletions pkg/contracts/testdappv2/TestDAppV2.abi
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
[
{
"inputs": [
{
"internalType": "bool",
"name": "isZetaChain_",
"type": "bool"
}
],
"stateMutability": "nonpayable",
"type": "constructor"
},
{
"inputs": [],
"name": "NO_MESSAGE_CALL",
Expand Down Expand Up @@ -143,6 +154,19 @@
"stateMutability": "pure",
"type": "function"
},
{
"inputs": [],
"name": "isZetaChain",
"outputs": [
{
"internalType": "bool",
"name": "",
"type": "bool"
}
],
"stateMutability": "view",
"type": "function"
},
{
"inputs": [
{
Expand Down
2 changes: 1 addition & 1 deletion pkg/contracts/testdappv2/TestDAppV2.bin

Large diffs are not rendered by default.

39 changes: 35 additions & 4 deletions pkg/contracts/testdappv2/TestDAppV2.go

Large diffs are not rendered by default.

26 changes: 25 additions & 1 deletion pkg/contracts/testdappv2/TestDAppV2.json

Large diffs are not rendered by default.

34 changes: 34 additions & 0 deletions pkg/contracts/testdappv2/TestDAppV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,14 @@ interface IERC20 {
}

contract TestDAppV2 {
// used to simulate gas consumption
uint256[] private storageArray;

string public constant NO_MESSAGE_CALL = "called with no message";

// define if the chain is ZetaChain
bool immutable public isZetaChain;

struct zContext {
bytes origin;
address sender;
Expand Down Expand Up @@ -37,6 +43,11 @@ contract TestDAppV2 {
mapping(bytes => address) public senderWithMessage;
mapping(bytes32 => uint256) public amountWithMessage;

// the constructor is used to determine if the chain is ZetaChain
constructor(bool isZetaChain_) {
isZetaChain = isZetaChain_;
}

// return the index used for the "WithMessage" mapping when the message for calls is empty
// this allows testing the message with empty message
// this function includes the sender of the message to avoid collisions when running parallel tests with different senders
Expand Down Expand Up @@ -110,6 +121,13 @@ contract TestDAppV2 {

// Revertable interface
function onRevert(RevertContext calldata revertContext) external {

// if the chain is ZetaChain, consume gas to test the gas consumption
// we do it specifically for ZetaChain to test the outbound processing workflow
if (isZetaChain) {
consumeGas();
}

setCalledWithMessage(string(revertContext.revertMessage));
setAmountWithMessage(string(revertContext.revertMessage), 0);
senderWithMessage[revertContext.revertMessage] = revertContext.sender;
Expand All @@ -126,5 +144,21 @@ contract TestDAppV2 {
return "";
}

function consumeGas() internal {
// Approximate target gas consumption
uint256 targetGas = 500000;
// Approximate gas cost for a single storage write
uint256 storageWriteGasCost = 20000;
uint256 iterations = targetGas / storageWriteGasCost;

// Perform the storage writes
for (uint256 i = 0; i < iterations; i++) {
storageArray.push(i);
}

// Reset the storage array to avoid accumulation of storage cost
delete storageArray;
}

receive() external payable {}
}
2 changes: 1 addition & 1 deletion x/fungible/keeper/v2_deposits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func getTestDAppNoMessageIndex(

// deployTestDAppV2 deploys the test dapp v2 contract and returns its address
func deployTestDAppV2(t *testing.T, ctx sdk.Context, k *fungiblekeeper.Keeper, evmk types.EVMKeeper) common.Address {
testDAppV2, err := k.DeployContract(ctx, testdappv2.TestDAppV2MetaData)
testDAppV2, err := k.DeployContract(ctx, testdappv2.TestDAppV2MetaData, true)
require.NoError(t, err)
require.NotEmpty(t, testDAppV2)
assertContractDeployment(t, evmk, ctx, testDAppV2)
Expand Down
4 changes: 2 additions & 2 deletions zetaclient/zetacore/constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ const (
PostVoteOutboundGasLimit = 500_000

// PostVoteOutboundRevertGasLimit is the gas limit for voting on observed outbound tx for revert (when outbound fails)
// The value needs to be higher because reverting implies interacting with the EVM to perform swaps for the gas token
PostVoteOutboundRevertGasLimit = 4_000_000
// The value is set to 7M because in case of onRevert call, it might consume lot of gas
PostVoteOutboundRevertGasLimit = 7_000_000
)

// constants for monitoring tx results
Expand Down

0 comments on commit 5012d28

Please sign in to comment.