Problem: require gas for ica packet callback is not adjusted with ctx…

… params (#1251)

* Problem: require gas for ica packet callback is not adjusted with ctx params

* use sender adr

* test ica ctrl with memo

* Update x/cronos/keeper/keeper.go

Signed-off-by: mmsqe <mavis@crypto.com>

---------

Signed-off-by: mmsqe <mavis@crypto.com>

CHANGELOG.md

@@ -16,6 +16,7 @@
 - [#1209](https://github.com/crypto-org-chain/cronos/pull/1209) Support accurate estimate gas in evm tx from relayer.
 - [#1247](https://github.com/crypto-org-chain/cronos/pull/1247) Update ethermint to develop, go-ethereum to `v1.11.2`.
 - [#1235](https://github.com/crypto-org-chain/cronos/pull/1235) Add channel detail in ica packet callback.
+- [#1251](https://github.com/crypto-org-chain/cronos/pull/1251) Adjust require gas for submitMsgs in ica precompile.
 
 ### Improvements
 

app/app.go

@@ -537,11 +537,11 @@ func New(
 		tracer,
 		evmS,
 		[]evmkeeper.CustomContractFn{
-			func(rules ethparams.Rules) vm.PrecompiledContract {
+			func(_ sdk.Context, rules ethparams.Rules) vm.PrecompiledContract {
 				return cronosprecompiles.NewRelayerContract(app.IBCKeeper, appCodec, rules, app.Logger())
 			},
-			func(rules ethparams.Rules) vm.PrecompiledContract {
-				return cronosprecompiles.NewIcaContract(&app.ICAAuthKeeper, &app.CronosKeeper, appCodec, gasConfig)
+			func(ctx sdk.Context, rules ethparams.Rules) vm.PrecompiledContract {
+				return cronosprecompiles.NewIcaContract(ctx, &app.ICAAuthKeeper, &app.CronosKeeper, appCodec, gasConfig)
 			},
 		},
 		allKeys,

go.mod

@@ -234,7 +234,7 @@ replace (
 	// TODO: remove it: https://github.com/cosmos/cosmos-sdk/issues/13134
 	github.com/dgrijalva/jwt-go => github.com/golang-jwt/jwt/v4 v4.4.2
 	github.com/ethereum/go-ethereum => github.com/crypto-org-chain/go-ethereum v1.10.20-0.20231122021350-f905c2ec3570
-	github.com/evmos/ethermint => github.com/crypto-org-chain/ethermint v0.6.1-0.20231204041029-ccb83e2e0688
+	github.com/evmos/ethermint => github.com/crypto-org-chain/ethermint v0.6.1-0.20231205070424-9f62598bff76
 	// Fix upstream GHSA-h395-qcrw-5vmq and GHSA-3vp4-m3rf-835h vulnerabilities.
 	// TODO Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409
 	github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.9.0

go.sum

@@ -484,8 +484,8 @@ github.com/crypto-org-chain/cometbft-db v0.0.0-20231011055109-57922ac52a63 h1:R1
 github.com/crypto-org-chain/cometbft-db v0.0.0-20231011055109-57922ac52a63/go.mod h1:rocwIfnS+kA060x64gkSIRvWB9StSppIkJuo5MWzL24=
 github.com/crypto-org-chain/cosmos-sdk v0.46.0-beta2.0.20231013072415-eec017435467 h1:m0/aHPIJAzi2MSP3TXzzbLTFf+koRFQiaYmerRZUtK4=
 github.com/crypto-org-chain/cosmos-sdk v0.46.0-beta2.0.20231013072415-eec017435467/go.mod h1:G384omH7cXgm90xXR7xpHvsKG7vdBaDuz4To6GpTHUU=
-github.com/crypto-org-chain/ethermint v0.6.1-0.20231204041029-ccb83e2e0688 h1:6bjEkpymCMWfhUEhTuVj8g46sGuYS1D5I6XPuSuUWm4=
-github.com/crypto-org-chain/ethermint v0.6.1-0.20231204041029-ccb83e2e0688/go.mod h1:y8DDupItqxfGglMpak/ow8E7qvqXM2Vb7t6kdb6s96Q=
+github.com/crypto-org-chain/ethermint v0.6.1-0.20231205070424-9f62598bff76 h1:mee01qkS0ZTfzOzVdwzKR60Xhixk40FXdjVV1NRems4=
+github.com/crypto-org-chain/ethermint v0.6.1-0.20231205070424-9f62598bff76/go.mod h1:y8DDupItqxfGglMpak/ow8E7qvqXM2Vb7t6kdb6s96Q=
 github.com/crypto-org-chain/go-ethereum v1.10.20-0.20231122021350-f905c2ec3570 h1:GukXF6eVCJZbK5pgTF+SHJoY9Wr0mPh95/LIOUQNMMM=
 github.com/crypto-org-chain/go-ethereum v1.10.20-0.20231122021350-f905c2ec3570/go.mod h1:DuefStAgaxoaYGLR0FueVcVbehmn5n9QUcVrMCuOvuc=
 github.com/crypto-org-chain/gravity-bridge/module/v2 v2.0.1-0.20230825054824-75403cd90c6e h1:rSTc35OBjjCBx47rHPWBCIHNGPbMnEj8f7fNcK2TjVI=

gomod2nix.toml

@@ -216,8 +216,8 @@ schema = 3
     hash = "sha256-1LRIM5dPmwjBxd57c9Y+2mZuBrbGWfMvEIlqYpdVBPA="
     replaced = "github.com/crypto-org-chain/go-ethereum"
   [mod."github.com/evmos/ethermint"]
-    version = "v0.6.1-0.20231204041029-ccb83e2e0688"
-    hash = "sha256-JluJj+FrL5gA3twvxt/IwiMxHcTKEs0ciz+MF+bHZFk="
+    version = "v0.6.1-0.20231205070424-9f62598bff76"
+    hash = "sha256-xRa8V6b9nVVtTq0L0e/Klz/2VPyNSaB6WvSUbopJlsE="
     replaced = "github.com/crypto-org-chain/ethermint"
   [mod."github.com/felixge/httpsnoop"]
     version = "v1.0.2"

integration_tests/configs/ibc.jsonnet

@@ -13,6 +13,11 @@ config {
     },
     genesis+: {
       app_state+: {
+        cronos+: {
+          params+: {
+            max_callback_gas: 50000,
+          },
+        },
         feemarket+: {
           params+: {
             no_base_fee: true,

integration_tests/cosmoscli.py

@@ -1314,6 +1314,29 @@ def icaauth_submit_tx(self, connid, tx, timeout_duration="1h", **kwargs):
             rsp = self.event_query_tx_for(rsp["txhash"])
         return rsp
 
+    def ica_ctrl_send_tx(self, connid, tx, **kwargs):
+        default_kwargs = {
+            "home": self.data_dir,
+            "node": self.node_rpc,
+            "chain_id": self.chain_id,
+            "keyring_backend": "test",
+        }
+        rsp = json.loads(
+            self.raw(
+                "tx",
+                "ica",
+                "controller",
+                "send-tx",
+                connid,
+                tx,
+                "-y",
+                **(default_kwargs | kwargs),
+            )
+        )
+        if rsp["code"] == 0:
+            rsp = self.event_query_tx_for(rsp["txhash"])
+        return rsp
+
     def ica_query_account(self, connid, owner, **kwargs):
         default_kwargs = {
             "node": self.node_rpc,

integration_tests/ibc_utils.py

@@ -622,6 +622,23 @@ def check_tx():
         assert raised
 
 
+def register_acc(cli, connid):
+    print("register ica account")
+    rsp = cli.icaauth_register_account(
+        connid, from_="signer2", gas="400000", fees="100000000basetcro"
+    )
+    _, channel_id = assert_channel_open_init(rsp)
+    wait_for_check_channel_ready(cli, connid, channel_id)
+
+    print("query ica account")
+    ica_address = cli.ica_query_account(
+        connid,
+        cli.address("signer2"),
+    )["interchain_account_address"]
+    print("ica address", ica_address, "channel_id", channel_id)
+    return ica_address, channel_id
+
+
 def funds_ica(cli, adr):
     # initial balance of interchain account should be zero
     assert cli.balance(adr) == 0

integration_tests/test_ica.py

@@ -4,10 +4,10 @@
 from pystarport import cluster
 
 from .ibc_utils import (
-    assert_channel_open_init,
     funds_ica,
     gen_send_msg,
     prepare_network,
+    register_acc,
     wait_for_check_channel_ready,
     wait_for_check_tx,
 )
@@ -31,23 +31,7 @@ def test_ica(ibc, tmp_path):
     connid = "connection-0"
     cli_host = ibc.chainmain.cosmos_cli()
     cli_controller = ibc.cronos.cosmos_cli()
-
-    def register_acc():
-        print("register ica account")
-        rsp = cli_controller.icaauth_register_account(
-            connid, from_="signer2", gas="400000", fees="100000000basetcro"
-        )
-        _, channel_id = assert_channel_open_init(rsp)
-        wait_for_check_channel_ready(cli_controller, connid, channel_id)
-
-        print("query ica account")
-        ica_address = cli_controller.ica_query_account(
-            connid, cli_controller.address("signer2")
-        )["interchain_account_address"]
-        print("ica address", ica_address, "channel_id", channel_id)
-        return ica_address, channel_id
-
-    ica_address, channel_id = register_acc()
+    ica_address, channel_id = register_acc(cli_controller, connid)
     balance = funds_ica(cli_host, ica_address)
     num_txs = len(cli_host.query_all_txs(ica_address)["txs"])
     to = cli_host.address("signer2")
@@ -90,7 +74,7 @@ def submit_msgs(msg_num, timeout_in_s=no_timeout, gas="200000"):
     assert cli_host.balance(ica_address, denom=denom) == balance
     wait_for_check_channel_ready(cli_controller, connid, channel_id, "STATE_CLOSED")
     # reopen ica account after channel get closed
-    ica_address2, channel_id2 = register_acc()
+    ica_address2, channel_id2 = register_acc(cli_controller, connid)
     assert ica_address2 == ica_address, ica_address2
     assert channel_id2 != channel_id, channel_id2
     # submit normal txs should work

integration_tests/test_ica_ctrl.py

@@ -0,0 +1,78 @@
+import json
+
+import pytest
+from pystarport import cluster
+
+from .ibc_utils import (
+    deploy_contract,
+    funds_ica,
+    gen_send_msg,
+    parse_events_rpc,
+    prepare_network,
+    register_acc,
+    wait_for_check_tx,
+)
+from .utils import CONTRACTS, wait_for_fn
+
+
+@pytest.fixture(scope="module")
+def ibc(request, tmp_path_factory):
+    "prepare-network"
+    name = "ibc_rly"
+    path = tmp_path_factory.mktemp(name)
+    yield from prepare_network(
+        path,
+        name,
+        incentivized=False,
+        connection_only=True,
+        relayer=cluster.Relayer.RLY.value,
+    )
+
+
+def test_cb(ibc):
+    connid = "connection-0"
+    cli_host = ibc.chainmain.cosmos_cli()
+    cli_controller = ibc.cronos.cosmos_cli()
+    ica_address, _ = register_acc(cli_controller, connid)
+    funds_ica(cli_host, ica_address)
+    num_txs = len(cli_host.query_all_txs(ica_address)["txs"])
+    to = cli_host.address("signer2")
+    amount = 1000
+    denom = "basecro"
+    jsonfile = CONTRACTS["TestICA"]
+    tcontract = deploy_contract(ibc.cronos.w3, jsonfile)
+    memo = {"src_callback": {"address": tcontract.address}}
+
+    def generated_tx_packet(msg_num):
+        # generate a transaction to send to host chain
+        m = gen_send_msg(ica_address, to, denom, amount)
+        msgs = []
+        for i in range(msg_num):
+            msgs.append(m)
+        data = json.dumps(msgs)
+        packet = cli_controller.ica_generate_packet_data(data, json.dumps(memo))
+        return packet
+
+    def send_tx(msg_num, gas="200000"):
+        generated_tx = json.dumps(generated_tx_packet(msg_num))
+        # submit transaction on host chain on behalf of interchain account
+        rsp = cli_controller.ica_ctrl_send_tx(
+            connid,
+            generated_tx,
+            gas=gas,
+            from_="signer2",
+        )
+        assert rsp["code"] == 0, rsp["raw_log"]
+        wait_for_check_tx(cli_host, ica_address, num_txs)
+
+    msg_num = 10
+    send_tx(msg_num)
+
+    def check_for_ack():
+        criteria = "message.action=/ibc.core.channel.v1.MsgAcknowledgement"
+        return cli_controller.tx_search(criteria)["txs"]
+
+    txs = wait_for_fn("ack change", check_for_ack)
+    events = parse_events_rpc(txs[0]["events"])
+    err = events.get("ibc_src_callback")["callback_error"]
+    assert "sender is not authenticated" in err, err

integration_tests/utils.py

@@ -92,9 +92,8 @@ def contract_path(name, filename):
 def wait_for_fn(name, fn, *, timeout=240, interval=1):
     for i in range(int(timeout / interval)):
         result = fn()
-        print("check", name, result)
         if result:
-            break
+            return result
         time.sleep(interval)
     else:
         raise TimeoutError(f"wait for {name} timeout")

x/cronos/keeper/keeper.go

@@ -291,13 +291,21 @@ func (k Keeper) onPacketResult(
 	contractAddress,
 	packetSenderAddress string,
 ) error {
+	sender, err := sdk.AccAddressFromBech32(packetSenderAddress)
+	if err != nil {
+		return fmt.Errorf("invalid bech32 address: %s, err: %w", packetSenderAddress, err)
+	}
+	senderAddr := common.BytesToAddress(sender)
 	contractAddr := common.HexToAddress(contractAddress)
+	if senderAddr != contractAddr {
+		return fmt.Errorf("sender is not authenticated: expected %s, got %s", senderAddr, contractAddr)
+	}
 	data, err := cronosprecompiles.OnPacketResultCallback(packet.SourceChannel, packet.Sequence, acknowledgement)
 	if err != nil {
 		return err
 	}
 	gasLimit := k.GetParams(ctx).MaxCallbackGas
-	_, _, err = k.CallEVM(ctx, &contractAddr, data, big.NewInt(0), gasLimit)
+	_, _, err = k.CallEVM(ctx, &senderAddr, data, big.NewInt(0), gasLimit)
 	return err
 }
 

x/cronos/keeper/precompiles/ica.go

@@ -34,6 +34,7 @@ var (
 	icaABI                 abi.ABI
 	icaCallbackABI         abi.ABI
 	icaContractAddress     = common.BytesToAddress([]byte{102})
+	icaMethodNamesByID     = map[[4]byte]string{}
 	icaGasRequiredByMethod = map[[4]byte]uint64{}
 )
 
@@ -58,6 +59,7 @@ func init() {
 		default:
 			icaGasRequiredByMethod[methodID] = 0
 		}
+		icaMethodNamesByID[methodID] = methodName
 	}
 }
 
@@ -68,15 +70,17 @@ func OnPacketResultCallback(args ...interface{}) ([]byte, error) {
 type IcaContract struct {
 	BaseContract
 
+	ctx           sdk.Context
 	cdc           codec.Codec
 	icaauthKeeper types.Icaauthkeeper
 	cronosKeeper  types.CronosKeeper
 	kvGasConfig   storetypes.GasConfig
 }
 
-func NewIcaContract(icaauthKeeper types.Icaauthkeeper, cronosKeeper types.CronosKeeper, cdc codec.Codec, kvGasConfig storetypes.GasConfig) vm.PrecompiledContract {
+func NewIcaContract(ctx sdk.Context, icaauthKeeper types.Icaauthkeeper, cronosKeeper types.CronosKeeper, cdc codec.Codec, kvGasConfig storetypes.GasConfig) vm.PrecompiledContract {
 	return &IcaContract{
 		BaseContract:  NewBaseContract(icaContractAddress),
+		ctx:           ctx,
 		cdc:           cdc,
 		icaauthKeeper: icaauthKeeper,
 		cronosKeeper:  cronosKeeper,
@@ -95,6 +99,9 @@ func (ic *IcaContract) RequiredGas(input []byte) uint64 {
 	var methodID [4]byte
 	copy(methodID[:], input[:4])
 	requiredGas, ok := icaGasRequiredByMethod[methodID]
+	if icaMethodNamesByID[methodID] == SubmitMsgsMethodName {
+		requiredGas += ic.cronosKeeper.GetParams(ic.ctx).MaxCallbackGas
+	}
 	if ok {
 		return requiredGas + baseCost
 	}

x/cronos/types/interfaces.go

@@ -15,7 +15,6 @@ import (
 	icaauthtypes "github.com/crypto-org-chain/cronos/v2/x/icaauth/types"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core"
-	ethtypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/core/vm"
 	"github.com/ethereum/go-ethereum/params"
 	evmtypes "github.com/evmos/ethermint/x/evm/types"
@@ -92,7 +91,7 @@ type Icaauthkeeper interface {
 
 // CronosKeeper defines the interface for cronos keeper
 type CronosKeeper interface {
-	CallEVM(ctx sdk.Context, to *common.Address, data []byte, value *big.Int, gasLimit uint64) (*ethtypes.Message, *evmtypes.MsgEthereumTxResponse, error)
+	GetParams(ctx sdk.Context) (params Params)
 }
 
 // IbcKeeper defines the interface for ibc keeper