Problem: no negative test for state check (#1254)

* Problem: no negative test for state check

* only allow status update once

Author: mmsqe

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;

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),
             )

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(

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"

integration_tests/test_ica_precompile.py

@@ -1,19 +1,20 @@
 import base64
 import json
-from enum import IntEnum
 
 import pytest
 from eth_utils import keccak
 from pystarport import cluster
 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)