From 36d3268f0528a26d8faaf6d4e8ad9569bee747e5 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 8 Dec 2023 16:02:04 +0800 Subject: [PATCH] Problem: no negative test for state check (#1254) * Problem: no negative test for state check * only allow status update once --- .../contracts/contracts/TestICA.sol | 10 ++++- integration_tests/cosmoscli.py | 4 +- integration_tests/ibc_utils.py | 23 ++++++++++ integration_tests/test_ica.py | 10 +++++ integration_tests/test_ica_precompile.py | 43 ++++++++----------- 5 files changed, 62 insertions(+), 28 deletions(-) diff --git a/integration_tests/contracts/contracts/TestICA.sol b/integration_tests/contracts/contracts/TestICA.sol index 120e266879..8e3b401d2d 100644 --- a/integration_tests/contracts/contracts/TestICA.sol +++ b/integration_tests/contracts/contracts/TestICA.sol @@ -11,7 +11,7 @@ contract TestICA { address constant module_address = 0x89A7EF2F08B1c018D5Cc88836249b84Dd5392905; uint64 lastSeq; enum Status { - NONE, + PENDING, SUCCESS, FAIL } @@ -78,9 +78,10 @@ contract TestICA { ); } - function callSubmitMsgs(string memory connectionID, bytes memory data, uint256 timeout) public returns (uint64) { + function callSubmitMsgs(string memory connectionID, string calldata packetSrcChannel, bytes memory data, uint256 timeout) public returns (uint64) { require(account == msg.sender, "not authorized"); lastSeq = ica.submitMsgs(connectionID, data, timeout); + statusMap[packetSrcChannel][lastSeq] = Status.PENDING; return lastSeq; } @@ -109,6 +110,11 @@ contract TestICA { function onPacketResultCallback(string calldata packetSrcChannel, uint64 seq, bool ack) external payable returns (bool) { // To prevent called by arbitrary user require(msg.sender == module_address); + Status currentStatus = statusMap[packetSrcChannel][seq]; + if (currentStatus != Status.PENDING) { + return true; + } + delete statusMap[packetSrcChannel][seq]; Status status = Status.FAIL; if (ack) { status = Status.SUCCESS; diff --git a/integration_tests/cosmoscli.py b/integration_tests/cosmoscli.py index bd228e302a..c85b1456e7 100644 --- a/integration_tests/cosmoscli.py +++ b/integration_tests/cosmoscli.py @@ -1272,7 +1272,7 @@ 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): + def ica_ctrl_send_tx(self, connid, tx, timeout_in_ns=None, **kwargs): default_kwargs = { "home": self.data_dir, "node": self.node_rpc, @@ -1287,6 +1287,8 @@ def ica_ctrl_send_tx(self, connid, tx, **kwargs): "send-tx", connid, tx, + "--relative-packet-timeout" if timeout_in_ns else None, + timeout_in_ns if timeout_in_ns else None, "-y", **(default_kwargs | kwargs), ) diff --git a/integration_tests/ibc_utils.py b/integration_tests/ibc_utils.py index 53f6b47225..6b8d776939 100644 --- a/integration_tests/ibc_utils.py +++ b/integration_tests/ibc_utils.py @@ -2,6 +2,7 @@ import json import subprocess from contextlib import contextmanager +from enum import IntEnum from pathlib import Path from typing import NamedTuple @@ -25,6 +26,10 @@ RATIO = 10**10 +class Status(IntEnum): + PENDING, SUCCESS, FAIL = range(3) + + class IBCNetwork(NamedTuple): cronos: Cronos chainmain: Chainmain @@ -622,6 +627,24 @@ def check_tx(): assert raised +def wait_for_status_change(tcontract, channel_id, seq, timeout=None): + print(f"wait for status change for {seq}") + + def check_status(): + status = tcontract.caller.getStatus(channel_id, seq) + return status + + if timeout is None: + wait_for_fn("current status", check_status) + else: + try: + print(f"should assert timeout err when pass {timeout}s") + wait_for_fn("current status", check_status, timeout=timeout) + except TimeoutError: + raised = True + assert raised + + def register_acc(cli, connid): print("register ica account") rsp = cli.icaauth_register_account( diff --git a/integration_tests/test_ica.py b/integration_tests/test_ica.py index cd2f9e8fd9..7c10d04a32 100644 --- a/integration_tests/test_ica.py +++ b/integration_tests/test_ica.py @@ -4,6 +4,7 @@ from pystarport import cluster from .ibc_utils import ( + Status, deploy_contract, funds_ica, gen_send_msg, @@ -12,6 +13,7 @@ register_acc, wait_for_check_channel_ready, wait_for_check_tx, + wait_for_status_change, ) from .utils import CONTRACTS, wait_for_fn @@ -42,6 +44,8 @@ def test_ica(ibc, tmp_path): jsonfile = CONTRACTS["TestICA"] tcontract = deploy_contract(ibc.cronos.w3, jsonfile) memo = {"src_callback": {"address": tcontract.address}} + timeout_in_ns = 6000000000 + seq = 1 def generated_tx_packet(msg_num): # generate a transaction to send to host chain @@ -60,16 +64,22 @@ def send_tx(msg_num, gas="200000"): rsp = cli_controller.ica_ctrl_send_tx( connid, generated_tx, + timeout_in_ns, gas=gas, from_="signer2", ) assert rsp["code"] == 0, rsp["raw_log"] + events = parse_events_rpc(rsp["events"]) + assert int(events.get("send_packet")["packet_sequence"]) == seq wait_for_check_tx(cli_host, ica_address, num_txs) msg_num = 10 + assert tcontract.caller.getStatus(channel_id, seq) == Status.PENDING send_tx(msg_num) balance -= amount * msg_num assert cli_host.balance(ica_address, denom=denom) == balance + wait_for_status_change(tcontract, channel_id, seq, timeout_in_ns / 1e9) + assert tcontract.caller.getStatus(channel_id, seq) == Status.PENDING def check_for_ack(): criteria = "message.action=/ibc.core.channel.v1.MsgAcknowledgement" diff --git a/integration_tests/test_ica_precompile.py b/integration_tests/test_ica_precompile.py index 4bdb127e69..3474cbd3fa 100644 --- a/integration_tests/test_ica_precompile.py +++ b/integration_tests/test_ica_precompile.py @@ -1,6 +1,5 @@ import base64 import json -from enum import IntEnum import pytest from eth_utils import keccak @@ -8,12 +7,14 @@ from web3.datastructures import AttributeDict from .ibc_utils import ( + Status, funds_ica, gen_send_msg, get_next_channel, prepare_network, wait_for_check_channel_ready, wait_for_check_tx, + wait_for_status_change, ) from .utils import ( ADDRS, @@ -35,10 +36,6 @@ amt = 1000 -class Status(IntEnum): - NONE, SUCCESS, FAIL = range(3) - - @pytest.fixture(scope="module") def ibc(request, tmp_path_factory): "prepare-network" @@ -76,11 +73,12 @@ def submit_msgs( add_delegate, expected_seq, event, + channel_id, timeout=no_timeout, amount=amt, need_wait=True, msg_num=2, - channel_id="", + with_channel_id=True, ): cli_host = ibc.chainmain.cosmos_cli() cli_controller = ibc.cronos.cosmos_cli() @@ -110,7 +108,10 @@ def submit_msgs( num_txs = len(cli_host.query_all_txs(ica_address)["txs"]) str = base64.b64decode(generated_packet["data"]) # submit transaction on host chain on behalf of interchain account - tx = func(connid, str, timeout).build_transaction(data) + if with_channel_id: + tx = func(connid, channel_id, str, timeout).build_transaction(data) + else: + tx = func(connid, str, timeout).build_transaction(data) receipt = send_transaction(w3, tx, keys) assert receipt.status == 1 if timeout < no_timeout: @@ -160,7 +161,8 @@ def test_call(ibc): False, expected_seq, contract.events.SubmitMsgsResult, - channel_id=channel_id, + channel_id, + with_channel_id=False, ) balance -= diff assert cli_host.balance(ica_address, denom=denom) == balance @@ -173,22 +175,13 @@ def test_call(ibc): True, expected_seq, contract.events.SubmitMsgsResult, - channel_id=channel_id, + channel_id, + with_channel_id=False, ) balance -= diff assert cli_host.balance(ica_address, denom=denom) == balance -def wait_for_status_change(tcontract, channel_id, seq): - print(f"wait for status change for {seq}") - - def check_status(): - status = tcontract.caller.getStatus(channel_id, seq) - return status - - wait_for_fn("current status", check_status) - - def wait_for_packet_log(start, event, channel_id, seq, status): print("wait for log arrive", seq, status) expected = AttributeDict( @@ -218,7 +211,7 @@ def test_sc_call(ibc): name = "signer2" signer = ADDRS[name] keys = KEYS[name] - default_gas = 400000 + default_gas = 500000 data = {"from": signer, "gas": default_gas} channel_id = get_next_channel(cli_controller, connid) ica_address = register_acc( @@ -270,7 +263,7 @@ def submit_msgs_ro(func, str): False, expected_seq, contract.events.SubmitMsgsResult, - channel_id=channel_id, + channel_id, ) submit_msgs_ro(tcontract.functions.delegateSubmitMsgs, str) submit_msgs_ro(tcontract.functions.staticSubmitMsgs, str) @@ -293,7 +286,7 @@ def submit_msgs_ro(func, str): True, expected_seq, contract.events.SubmitMsgsResult, - channel_id=channel_id, + channel_id, ) submit_msgs_ro(tcontract.functions.delegateSubmitMsgs, str) submit_msgs_ro(tcontract.functions.staticSubmitMsgs, str) @@ -317,9 +310,9 @@ def submit_msgs_ro(func, str): False, expected_seq, contract.events.SubmitMsgsResult, + channel_id, amount=100000001, need_wait=False, - channel_id=channel_id, ) last_seq = tcontract.caller.getLastSeq() wait_for_status_change(tcontract, channel_id, last_seq) @@ -342,9 +335,9 @@ def submit_msgs_ro(func, str): False, expected_seq, contract.events.SubmitMsgsResult, + channel_id, timeout, msg_num=100, - channel_id=channel_id, ) last_seq = tcontract.caller.getLastSeq() wait_for_status_change(tcontract, channel_id, last_seq) @@ -377,7 +370,7 @@ def submit_msgs_ro(func, str): False, expected_seq, contract.events.SubmitMsgsResult, - channel_id=channel_id2, + channel_id2, ) last_seq = tcontract.caller.getLastSeq() wait_for_status_change(tcontract, channel_id2, last_seq)