Problem: require gas in relayer precompile is higher than consumed (#…

…1232) * Problem: require gas in relayer precompile is higher than consumed Update CHANGELOG.md Signed-off-by: mmsqe <[email protected]> * fix balance change in timeout test * fix test * simplify event test * fix recvPacket * add missing channel close * fix build * Update x/cronos/keeper/precompiles/relayer.go Co-authored-by: yihuang <[email protected]> Signed-off-by: mmsqe <[email protected]> * Update x/cronos/keeper/precompiles/relayer.go Co-authored-by: yihuang <[email protected]> Signed-off-by: mmsqe <[email protected]> * Update x/cronos/keeper/precompiles/relayer.go Co-authored-by: yihuang <[email protected]> Signed-off-by: mmsqe <[email protected]> --------- Signed-off-by: mmsqe <[email protected]> Co-authored-by: yihuang <[email protected]>
crypto-org-chain · Nov 6, 2023 · 376da21 · 376da21
1 parent 394bce5
commit 376da21
Show file tree

Hide file tree

Showing 12 changed files with 181 additions and 62 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,11 @@
 - [#1230](https:/crypto-org-chain/cronos/pull/1230) Fix mem store in versiondb multistore.
 - [#1233](https:/crypto-org-chain/cronos/pull/1233) Re-emit logs in callback contract.
 
+### State Machine Breaking
+
+- [#1232](https:/crypto-org-chain/cronos/pull/1232) Adjust require gas in relayer precompile to be closed with actual consumed.
+
+
 *October 17, 2023*
 
 ## v1.1.0-rc1

diff --git a/app/app.go b/app/app.go
@@ -536,7 +536,7 @@ func New(
  tracer,
  evmS,
  []vm.PrecompiledContract{
- cronosprecompiles.NewRelayerContract(app.IBCKeeper, appCodec, gasConfig),
+ cronosprecompiles.NewRelayerContract(app.IBCKeeper, appCodec, app.Logger()),
  cronosprecompiles.NewIcaContract(&app.ICAAuthKeeper, &app.CronosKeeper, appCodec, gasConfig),
  },
  allKeys,

diff --git a/integration_tests/configs/ibc_rly.jsonnet b/integration_tests/configs/ibc_rly.jsonnet
@@ -1,9 +1,24 @@
 local ibc = import 'ibc.jsonnet';
 
 ibc {
+ 'chainmain-1'+: {
+ validators: [
+ {
+ coins: '987870000000000000cro',
+ staked: '20000000000000cro',
+ mnemonic: '${VALIDATOR' + i + '_MNEMONIC}',
+ client_config: {
+ 'broadcast-mode': 'block',
+ },
+ base_port: 26800 + i * 10,
+ }
+ for i in std.range(1, 2)
+ ],
+ },
  relayer+: {
  chains: [super.chains[0] {
- precompiled_contract_address: '0x0000000000000000000000000000000000000065',
+ max_gas: 1000000,
+ gas_multiplier: 1.2,
  }] + super.chains[1:],
  },
 }
diff --git a/integration_tests/configs/ibc_rly_evm.jsonnet b/integration_tests/configs/ibc_rly_evm.jsonnet
@@ -0,0 +1,9 @@
+local ibc = import 'ibc_rly.jsonnet';
+
+ibc {
+ relayer+: {
+ chains: [super.chains[0] {
+ precompiled_contract_address: '0x0000000000000000000000000000000000000065',
+ }] + super.chains[1:],
+ },
+}
diff --git a/integration_tests/cosmoscli.py b/integration_tests/cosmoscli.py
@@ -9,6 +9,7 @@
 from collections import namedtuple
 
 import bech32
+import requests
 from dateutil.parser import isoparse
 from pystarport.utils import build_cli_args_safe, format_doc_string, interact
 
@@ -221,6 +222,17 @@ def tx_search(self, events: str):
  self.raw("query", "txs", events=events, output="json", node=self.node_rpc)
  )
 
+ def tx_search_rpc(self, criteria: str):
+ node_rpc_http = "http" + self.node_rpc.removeprefix("tcp")
+ rsp = requests.get(
+ f"{node_rpc_http}/tx_search",
+ params={
+ "query": f'"{criteria}"',
+ },
+ ).json()
+ assert "error" not in rsp, rsp["error"]
+ return rsp["result"]["txs"]
+
  def distribution_commission(self, addr):
  coin = json.loads(
  self.raw(
@@ -1238,7 +1250,7 @@ def transfer_tokens(self, from_, to, amount, **kwargs):
  "gas": "auto",
  "gas_adjustment": "1.5",
  }
- return json.loads(
+ rsp = json.loads(
  self.raw(
  "tx",
  "cronos",
@@ -1252,6 +1264,9 @@ def transfer_tokens(self, from_, to, amount, **kwargs):
  **(default_kwargs | kwargs),
  )
  )
+ if rsp["code"] == 0:
+ rsp = self.event_query_tx_for(rsp["txhash"])
+ return rsp
 
  def icaauth_register_account(self, connid, **kwargs):
  "execute on host chain to attach an account to the connection"

diff --git a/integration_tests/ibc_utils.py b/integration_tests/ibc_utils.py
@@ -217,6 +217,25 @@ def hermes_transfer(ibc):
  return src_amount
 
 
+def rly_transfer(ibc):
+ # chainmain-1 -> cronos_777-1
+ my_ibc0 = "chainmain-1"
+ my_ibc1 = "cronos_777-1"
+ channel = "channel-0"
+ dst_addr = eth_to_bech32(ADDRS["signer2"])
+ src_amount = 10
+ src_denom = "basecro"
+ path = ibc.cronos.base_dir.parent / "relayer"
+ # srcchainid dstchainid amount dst_addr srchannelid
+ cmd = (
+ f"rly tx transfer {my_ibc0} {my_ibc1} {src_amount}{src_denom} "
+ f"{dst_addr} {channel} "
+ f"--path chainmain-cronos "
+ f"--home {str(path)}"
+ )
+ subprocess.run(cmd, check=True, shell=True)
+
+
 def find_duplicate(attributes):
  res = set()
  key = attributes[0]["key"]
@@ -639,3 +658,17 @@ def gen_send_msg(sender, receiver, denom, amount):
  "to_address": receiver,
  "amount": [{"denom": denom, "amount": f"{amount}"}],
  }
+
+
+def log_gas_records(cli):
+ criteria = "tx.height >= 0"
+ txs = cli.tx_search_rpc(criteria)
+ records = []
+ for tx in txs:
+ res = tx["tx_result"]
+ actions = []
+ for event in res["events"]:
+ for attribute in event["attributes"]:
+ if attribute["key"] == "action":
+ actions.append(attribute["value"])
+ records.append(res["gas_used"])
diff --git a/integration_tests/test_ibc_rly.py b/integration_tests/test_ibc_rly.py
@@ -1,5 +1,4 @@
 import json
-import subprocess
 
 import pytest
 from eth_utils import keccak, to_checksum_address
@@ -14,6 +13,7 @@
  ibc_denom,
  ibc_incentivized_transfer,
  prepare_network,
+ rly_transfer,
 )
 from .utils import (
  ADDRS,
@@ -42,7 +42,7 @@
 @pytest.fixture(scope="module")
 def ibc(request, tmp_path_factory):
  "prepare-network"
- name = "ibc_rly"
+ name = "ibc_rly_evm"
  path = tmp_path_factory.mktemp(name)
  yield from prepare_network(
  path,
@@ -51,21 +51,6 @@ def ibc(request, tmp_path_factory):
  )
 
 
-def rly_transfer(ibc):
- # chainmain-1 -> cronos_777-1
- my_ibc0 = "chainmain-1"
- my_ibc1 = "cronos_777-1"
- path = ibc.cronos.base_dir.parent / "relayer"
- # srcchainid dstchainid amount dst_addr srchannelid
- cmd = (
- f"rly tx transfer {my_ibc0} {my_ibc1} {src_amount}{src_denom} "
- f"{eth_to_bech32(cronos_signer2)} {channel} "
- f"--path chainmain-cronos "
- f"--home {str(path)}"
- )
- subprocess.run(cmd, check=True, shell=True)
-
-
 def coin_received(receiver, amt, denom):
  return {
  "receiver": receiver,

diff --git a/integration_tests/test_ibc_rly_gas.py b/integration_tests/test_ibc_rly_gas.py
@@ -0,0 +1,31 @@
+import pytest
+from pystarport import cluster
+
+from .ibc_utils import log_gas_records, prepare_network, rly_transfer
+from .utils import wait_for_new_blocks
+
+
+@pytest.fixture(scope="module", params=["ibc_rly", "ibc_rly_evm"])
+def ibc(request, tmp_path_factory):
+ "prepare-network"
+ name = request.param
+ path = tmp_path_factory.mktemp(name)
+ yield from prepare_network(path, name, relayer=cluster.Relayer.RLY.value)
+
+
+records = []
+
+
+def test_ibc(ibc):
+ # chainmain-1 relayer -> cronos_777-1 signer2
+ cli = ibc.cronos.cosmos_cli()
+ wait_for_new_blocks(cli, 1)
+ rly_transfer(ibc)
+ diff = 0.5
+ record = log_gas_records(cli)
+ if record:
+ records.append(record)
+ if len(records) == 2:
+ for e1, e2 in zip(*records):
+ res = e2 / e1
+ assert 1 - diff <= res <= 1 + diff, res
diff --git a/integration_tests/test_ibc_timeout.py b/integration_tests/test_ibc_timeout.py
@@ -60,14 +60,18 @@ def test_cronos_transfer_timeout(ibc):
  )
  assert rsp["code"] == 0, rsp["raw_log"]
 
- new_src_balance = 0
-
  def check_balance_change():
- nonlocal new_src_balance
+ new_src_balance = get_balance(ibc.cronos, src_addr, src_denom)
+ get_balance(ibc.chainmain, dst_addr, dst_denom)
+ return old_src_balance != new_src_balance
+
+ wait_for_fn("balance has change", check_balance_change)
+
+ def check_no_change():
  new_src_balance = get_balance(ibc.cronos, src_addr, src_denom)
  get_balance(ibc.chainmain, dst_addr, dst_denom)
  return old_src_balance == new_src_balance
 
- wait_for_fn("balance no change", check_balance_change)
+ wait_for_fn("balance no change", check_no_change)
  new_dst_balance = get_balance(ibc.chainmain, dst_addr, dst_denom)
  assert old_dst_balance == new_dst_balance
diff --git a/integration_tests/test_ica_precompile.py b/integration_tests/test_ica_precompile.py
@@ -21,16 +21,11 @@
  KEYS,
  deploy_contract,
  eth_to_bech32,
- get_logs_since,
- get_method_map,
- get_topic_data,
  send_transaction,
  wait_for_fn,
 )
 
 CONTRACT = "0x0000000000000000000000000000000000000066"
-contract_info = json.loads(CONTRACT_ABIS["IICAModule"].read_text())
-method_map = get_method_map(contract_info)
 connid = "connection-0"
 no_timeout = 300000000000
 denom = "basecro"
@@ -46,7 +41,7 @@ class Status(IntEnum):
 @pytest.fixture(scope="module")
 def ibc(request, tmp_path_factory):
  "prepare-network"
- name = "ibc_rly"
+ name = "ibc_rly_evm"
  path = tmp_path_factory.mktemp(name)
  yield from prepare_network(
  path,
@@ -79,6 +74,7 @@ def submit_msgs(
  ica_address,
  add_delegate,
  expected_seq,
+ event,
  timeout=no_timeout,
  amount=amt,
  need_wait=True,
@@ -109,7 +105,6 @@ def submit_msgs(
  diff_amt += amt1
  generated_packet = cli_controller.ica_generate_packet_data(json.dumps(msgs))
  num_txs = len(cli_host.query_all_txs(ica_address)["txs"])
- start = w3.eth.get_block_number()
  str = base64.b64decode(generated_packet["data"])
  # submit transaction on host chain on behalf of interchain account
  tx = func(connid, str, timeout).build_transaction(data)
@@ -120,12 +115,9 @@ def submit_msgs(
  print(f"wait for {timeout_in_s}s")
  wait_for_check_tx(cli_host, ica_address, num_txs, timeout_in_s)
  else:
- logs = get_logs_since(w3, CONTRACT, start)
- expected = [{"seq": expected_seq}]
- assert len(logs) == len(expected)
- for i, log in enumerate(logs):
- method_name, args = get_topic_data(w3, method_map, contract_info, log)
- assert args == AttributeDict(expected[i]), [i, method_name]
+ (logs) = event.getLogs()
+ assert len(logs) > 0
+ assert logs[0].args == AttributeDict({"seq": expected_seq})
  if need_wait:
  wait_for_check_tx(cli_host, ica_address, num_txs)
  return str, diff_amt
@@ -137,6 +129,7 @@ def test_call(ibc):
  w3 = ibc.cronos.w3
  name = "signer2"
  addr = ADDRS[name]
+ contract_info = json.loads(CONTRACT_ABIS["IICAModule"].read_text())
  contract = w3.eth.contract(address=CONTRACT, abi=contract_info)
  data = {"from": ADDRS[name]}
  ica_address = register_acc(
@@ -157,6 +150,7 @@ def test_call(ibc):
  ica_address,
  False,
  expected_seq,
+ contract.events.SubmitMsgsResult,
  )
  balance -= diff
  assert cli_host.balance(ica_address, denom=denom) == balance
@@ -168,6 +162,7 @@ def test_call(ibc):
  ica_address,
  True,
  expected_seq,
+ contract.events.SubmitMsgsResult,
  )
  balance -= diff
  assert cli_host.balance(ica_address, denom=denom) == balance
@@ -194,6 +189,7 @@ def test_sc_call(ibc):
  cli_host = ibc.chainmain.cosmos_cli()
  cli_controller = ibc.cronos.cosmos_cli()
  w3 = ibc.cronos.w3
+ contract_info = json.loads(CONTRACT_ABIS["IICAModule"].read_text())
  contract = w3.eth.contract(address=CONTRACT, abi=contract_info)
  jsonfile = CONTRACTS["TestICA"]
  tcontract = deploy_contract(w3, jsonfile)
@@ -248,6 +244,7 @@ def submit_msgs_ro(func, str):
  ica_address,
  False,
  expected_seq,
+ contract.events.SubmitMsgsResult,
  )
  submit_msgs_ro(tcontract.functions.delegateSubmitMsgs, str)
  submit_msgs_ro(tcontract.functions.staticSubmitMsgs, str)
@@ -268,6 +265,7 @@ def submit_msgs_ro(func, str):
  ica_address,
  True,
  expected_seq,
+ contract.events.SubmitMsgsResult,
  )
  submit_msgs_ro(tcontract.functions.delegateSubmitMsgs, str)
  submit_msgs_ro(tcontract.functions.staticSubmitMsgs, str)
@@ -289,6 +287,7 @@ def submit_msgs_ro(func, str):
  ica_address,
  False,
  expected_seq,
+ contract.events.SubmitMsgsResult,
  amount=100000001,
  need_wait=False,
  )
@@ -310,6 +309,7 @@ def submit_msgs_ro(func, str):
  ica_address,
  False,
  expected_seq,
+ contract.events.SubmitMsgsResult,
  timeout,
  )
  last_seq = tcontract.caller.getLastSeq()