Problem: ibc callback is not used (#1185)

* add cb larger maxCallbackGas * add interface * go mod tidy * gen binding pack method add cb * use cb events * add max_callback_gas in param * Apply suggestions from code review * use default maxCallbackGas * fix test * revert events * fix signature * notify caller * fix migrate fix test fix lint test param * add cb signature * json unmarshal and base64 decode for dapps * interface only when bind * Apply suggestions from code review * notify directly from middleware * make use of fee type * change ack to bool * fix unmarshal * timeout should be treated as failure * add change doc * fix gas limit * fix interface * fix test --------- Co-authored-by: HuangYi <[email protected]>
crypto-org-chain · Sep 27, 2023 · aee4960 · aee4960
1 parent f11271b
commit aee4960
Show file tree

Hide file tree

Showing 25 changed files with 483 additions and 69 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,6 +25,7 @@
 - [cronos#1163](https://github.com/crypto-org-chain/cronos/pull/1163) Support stateful precompiled contract for ica.
 - [cronos#837](https://github.com/crypto-org-chain/cronos/pull/837) Support stateful precompiled contract for bank.
 - [cronos#1184](https://github.com/crypto-org-chain/cronos/pull/1184) Update ibc-go to `v7.3.1`.
+- [cronos#1185](https://github.com/crypto-org-chain/cronos/pull/1185) Support ibc callback.
 
 ### Bug Fixes
 

diff --git a/app/app.go b/app/app.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"math"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -98,6 +99,7 @@ import (
 	upgradeclient "github.com/cosmos/cosmos-sdk/x/upgrade/client"
 	upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
 	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
+	ibccallbacks "github.com/cosmos/ibc-go/modules/apps/callbacks"
 	ibcfee "github.com/cosmos/ibc-go/v7/modules/apps/29-fee"
 	ibcfeekeeper "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/keeper"
 	ibcfeetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types"
@@ -551,7 +553,7 @@ func New(
 		[]vm.PrecompiledContract{
 			cronosprecompiles.NewBankContract(app.BankKeeper, appCodec),
 			cronosprecompiles.NewRelayerContract(app.IBCKeeper, appCodec),
-			cronosprecompiles.NewIcaContract(&app.ICAAuthKeeper, appCodec),
+			cronosprecompiles.NewIcaContract(&app.ICAAuthKeeper, &app.CronosKeeper, appCodec),
 		},
 		allKeys,
 	)
@@ -655,8 +657,13 @@ func New(
 	icaAuthModule := icaauth.NewAppModule(appCodec, app.ICAAuthKeeper)
 	icaAuthIBCModule := icaauth.NewIBCModule(app.ICAAuthKeeper)
 
-	icaControllerIBCModule := icacontroller.NewIBCMiddleware(icaAuthIBCModule, app.ICAControllerKeeper)
-	icaControllerStack := ibcfee.NewIBCMiddleware(icaControllerIBCModule, app.IBCFeeKeeper)
+	var icaControllerStack porttypes.IBCModule
+	icaControllerStack = icacontroller.NewIBCMiddleware(icaAuthIBCModule, app.ICAControllerKeeper)
+	icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)
+	// Since the callbacks middleware itself is an ics4wrapper, it needs to be passed to the ica controller keeper
+	app.ICAControllerKeeper.WithICS4Wrapper(icaControllerStack.(porttypes.Middleware))
+	// we don't limit gas usage here, because the cronos keeper will use network parameter to control it.
+	icaControllerStack = ibccallbacks.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper, app.CronosKeeper, math.MaxUint64)
 
 	// Create static IBC router, add transfer route, then set and seal it
 	ibcRouter := porttypes.NewRouter()

diff --git a/client/docs/swagger-ui/swagger.yaml b/client/docs/swagger-ui/swagger.yaml
@@ -452,6 +452,9 @@ paths:
                     title: the admin address who can update token mapping
                   enable_auto_deployment:
                     type: boolean
+                  max_callback_gas:
+                    type: string
+                    format: uint64
             description: >-
               QueryParamsResponse is the response type for the Query/Params RPC
               method.
@@ -43648,6 +43651,9 @@ definitions:
         title: the admin address who can update token mapping
       enable_auto_deployment:
         type: boolean
+      max_callback_gas:
+        type: string
+        format: uint64
     description: Params defines the parameters for the cronos module.
   cronos.QueryParamsResponse:
     type: object
@@ -43666,6 +43672,9 @@ definitions:
             title: the admin address who can update token mapping
           enable_auto_deployment:
             type: boolean
+          max_callback_gas:
+            type: string
+            format: uint64
     description: QueryParamsResponse is the response type for the Query/Params RPC method.
   cronos.QueryPermissionsResponse:
     type: object

diff --git a/go.mod b/go.mod
@@ -14,6 +14,7 @@ require (
 	github.com/cosmos/cosmos-proto v1.0.0-beta.2
 	github.com/cosmos/cosmos-sdk v0.47.5
 	github.com/cosmos/gogoproto v1.4.10
+	github.com/cosmos/ibc-go/modules/apps/callbacks v0.1.1-0.20230831194909-17cf1260a9cd
 	github.com/cosmos/ibc-go/v7 v7.3.1-0.20230920070810-c3261472c815
 	github.com/crypto-org-chain/cronos/store v0.0.4
 	github.com/crypto-org-chain/cronos/versiondb v0.0.0-00010101000000-000000000000

diff --git a/go.sum b/go.sum
@@ -452,6 +452,8 @@ github.com/cosmos/gogoproto v1.4.10 h1:QH/yT8X+c0F4ZDacDv3z+xE3WU1P1Z3wQoLMBRJoK
 github.com/cosmos/gogoproto v1.4.10/go.mod h1:3aAZzeRWpAwr+SS/LLkICX2/kDFyaYVzckBDzygIxek=
 github.com/cosmos/iavl v0.21.0-alpha.1.0.20230904092046-df3db2d96583 h1:3Matt7/LjZiZkIBPalYazOZcw2B05Ch14dU5TJyqJEc=
 github.com/cosmos/iavl v0.21.0-alpha.1.0.20230904092046-df3db2d96583/go.mod h1:WO7FyvaZJoH65+HFOsDir7xU9FWk2w9cHXNW1XHcl7A=
+github.com/cosmos/ibc-go/modules/apps/callbacks v0.1.1-0.20230831194909-17cf1260a9cd h1:KVnAwC1d0b+LWrVi+U1rex0e9LlyGTZ17zYhU3S4il8=
+github.com/cosmos/ibc-go/modules/apps/callbacks v0.1.1-0.20230831194909-17cf1260a9cd/go.mod h1:h+JtOsdOs/ikuntjZFXOAa8qnXUfgkTcRSHaTTcAM+M=
 github.com/cosmos/ibc-go/v7 v7.3.1-0.20230920070810-c3261472c815 h1:raSo7w7B3IXCb7DZozHWz8ajG7HLWZw9foiyCbEgInI=
 github.com/cosmos/ibc-go/v7 v7.3.1-0.20230920070810-c3261472c815/go.mod h1:wvx4pPBofe5ZdMNV3OFRxSI4auEP5Qfqf8JXLLNV04g=
 github.com/cosmos/ics23/go v0.10.0 h1:iXqLLgp2Lp+EdpIuwXTYIQU+AiHj9mOC2X9ab++bZDM=

diff --git a/gomod2nix.toml b/gomod2nix.toml
@@ -154,6 +154,9 @@ schema = 3
   [mod."github.com/cosmos/iavl"]
     version = "v0.21.0-alpha.1.0.20230904092046-df3db2d96583"
     hash = "sha256-3Va8Ljq63IXty0oHlRpqfsC6WsMut6TWZ2R+/nYtfTU="
+  [mod."github.com/cosmos/ibc-go/modules/apps/callbacks"]
+    version = "v0.1.1-0.20230831194909-17cf1260a9cd"
+    hash = "sha256-kVvGNQt3A1H6pQs3YtMvx1t9nNcL6ClKnIfsR24OTi8="
   [mod."github.com/cosmos/ibc-go/v7"]
     version = "v7.3.1-0.20230920070810-c3261472c815"
     hash = "sha256-x/D64hmU+aOc5sm8RzXMB+8y+530+CCEik/Zpj3Rf9A="

diff --git a/integration_tests/contracts/contracts/TestICA.sol b/integration_tests/contracts/contracts/TestICA.sol
@@ -7,7 +7,12 @@ contract TestICA {
     address constant icaContract = 0x0000000000000000000000000000000000000066;
     IICAModule ica = IICAModule(icaContract);
     address account;
+    // sha256('cronos-evm')[:20]
+    address constant module_address = 0x89A7EF2F08B1c018D5Cc88836249b84Dd5392905;
     uint64 lastAckSeq;
+    bool lastAck;
+    mapping (uint64 => bool) public acknowledgement;
+    event OnPacketResult(uint64 seq, bool ack);
 
     function encodeRegister(string memory connectionID, string memory version) internal view returns (bytes memory) {
         return abi.encodeWithSignature(
@@ -92,4 +97,18 @@ contract TestICA {
     function getLastAckSeq() public view returns (uint256) {
         return lastAckSeq;
     }
-}
+
+    function getLastAck() public view returns (bool) {
+        return lastAck;
+    }
+
+    function onPacketResultCallback(uint64 seq, bool ack) external payable returns (bool) {
+        // To prevent called by arbitrary user
+        require(msg.sender == module_address);
+        lastAckSeq = seq;
+        lastAck = ack;
+        acknowledgement[seq] = ack;
+        emit OnPacketResult(seq, ack);
+        return true;
+    }
+}
diff --git a/integration_tests/test_gov_update_params.py b/integration_tests/test_gov_update_params.py
@@ -9,18 +9,20 @@ def test_gov_update_params(cronos, tmp_path):
     proposal = tmp_path / "proposal.json"
     # governance module account as signer
     signer = "crc10d07y265gmmuvt4z0w9aw880jnsr700jdufnyd"
+    params = {
+        "cronos_admin": "crc12luku6uxehhak02py4rcz65zu0swh7wjsrw0pp",
+        "enable_auto_deployment": False,
+        "ibc_cro_denom": "ibc/6411AE2ADA1E73DB59DB151"
+        "A8988F9B7D5E7E233D8414DB6817F8F1A01600000",
+        "ibc_timeout": "96400000000000",
+        "max_callback_gas": "400000",
+    }
     proposal_src = {
         "messages": [
             {
                 "@type": "/cronos.MsgUpdateParams",
                 "authority": signer,
-                "params": {
-                    "cronos_admin": "crc12luku6uxehhak02py4rcz65zu0swh7wjsrw0pp",
-                    "enable_auto_deployment": False,
-                    "ibc_cro_denom": "ibc/6411AE2ADA1E73DB59DB151"
-                    "A8988F9B7D5E7E233D8414DB6817F8F1A01600000",
-                    "ibc_timeout": "96400000000000",
-                },
+                "params": params,
             }
         ],
         "deposit": "1basetcro",
@@ -35,10 +37,4 @@ def test_gov_update_params(cronos, tmp_path):
     print("check params have been updated now")
     rsp = cli.query_params()
     print("params", rsp)
-    assert rsp == {
-        "cronos_admin": "crc12luku6uxehhak02py4rcz65zu0swh7wjsrw0pp",
-        "enable_auto_deployment": False,
-        "ibc_cro_denom": "ibc/6411AE2ADA1E73DB59DB151"
-        "A8988F9B7D5E7E233D8414DB6817F8F1A01600000",
-        "ibc_timeout": "96400000000000",
-    }
+    assert rsp == params
diff --git a/integration_tests/test_ica_precompile.py b/integration_tests/test_ica_precompile.py
@@ -191,6 +191,8 @@ def submit_msgs_ro(func, str):
     submit_msgs_ro(tcontract.functions.staticSubmitMsgs, str)
     assert tcontract.caller.getLastAckSeq() == seq
     balance -= amt
+    ack = tcontract.caller.getLastAck()
+    assert ack == tcontract.caller.acknowledgement(seq), ack
     assert cli_host.balance(ica_address, denom=denom) == balance
     seq = 2
     str = submit_msgs(
@@ -206,3 +208,4 @@ def submit_msgs_ro(func, str):
     balance -= amt
     balance -= amt1
     assert cli_host.balance(ica_address, denom=denom) == balance
+    assert ack == tcontract.caller.acknowledgement(seq), ack
diff --git a/integration_tests/test_upgrade.py b/integration_tests/test_upgrade.py
@@ -186,6 +186,7 @@ def test_cosmovisor_upgrade(custom_cronos: Cronos, tmp_path_factory):
 
     rsp = cli.query_params("icaauth")
     assert rsp["params"]["min_timeout_duration"] == "3600s", rsp
+    assert cli.query_params()["max_callback_gas"] == "300000", rsp
 
     # migrate to sdk v0.47
     custom_cronos.supervisorctl("stop", "all")

diff --git a/proto/cronos/cronos.proto b/proto/cronos/cronos.proto
@@ -13,6 +13,7 @@ message Params {
   // the admin address who can update token mapping
   string cronos_admin           = 3;
   bool   enable_auto_deployment = 4;
+  uint64 max_callback_gas       = 5;
 }
 
 // TokenMappingChangeProposal defines a proposal to change one token mapping.

diff --git a/scripts/gen-bindings-contracts b/scripts/gen-bindings-contracts
@@ -4,9 +4,11 @@ solc08 --abi --bin x/cronos/events/bindings/src/CosmosTypes.sol -o build --overw
 solc08 --abi --bin x/cronos/events/bindings/src/Relayer.sol -o build --overwrite
 solc08 --abi --bin x/cronos/events/bindings/src/Bank.sol -o build --overwrite
 solc08 --abi --bin x/cronos/events/bindings/src/ICA.sol -o build --overwrite
+solc08 --abi --bin x/cronos/events/bindings/src/ICACallback.sol -o build --overwrite
 
 
 abigen --pkg lib --abi build/CosmosTypes.abi --bin build/CosmosTypes.bin --out x/cronos/events/bindings/cosmos/lib/cosmos_types.abigen.go --type CosmosTypes
 abigen --pkg relayer --abi build/IRelayerModule.abi --bin build/IRelayerModule.bin --out x/cronos/events/bindings/cosmos/precompile/relayer/i_relayer_module.abigen.go --type RelayerModule
 abigen --pkg bank --abi build/IBankModule.abi --bin build/IBankModule.bin --out x/cronos/events/bindings/cosmos/precompile/bank/i_bank_module.abigen.go --type BankModule
 abigen --pkg ica --abi build/IICAModule.abi --bin build/IICAModule.bin --out x/cronos/events/bindings/cosmos/precompile/ica/i_ica_module.abigen.go --type ICAModule
+abigen --pkg icacallback --abi build/IICACallback.abi --bin build/IICACallback.bin --out x/cronos/events/bindings/cosmos/precompile/icacallback/i_ica_callback.abigen.go --type ICACallback