-
Notifications
You must be signed in to change notification settings - Fork 445
feat(wip): r/aib/ibc
#4655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(wip): r/aib/ibc
#4655
Conversation
🛠 PR Checks Summary🔴 Maintainers must be able to edit this pull request (more info) Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🔴 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
30cc5e4 to
01c52d9
Compare
7ad0dd2 to
9a80efd
Compare
66e6889 to
6e65ab7
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
c7c216b to
6a1d70a
Compare
5b90837 to
6e1f8c8
Compare
The IBC protocol is based on merkle proofs, and over the years a standard of merkle proof representation has emerged: [ICS23](https://github.com/cosmos/ics23). The simplest way for Gno to be compatible with this protocol is to migrate the iavl implementation to [`cosmos/iavl`](https://github.com/cosmos/iavl), which already supports ICS23. ```diff - Therefore, this PR deprecates `tm2/pkg/iavl` in favour of imported `cosmos/iavl`. + Therefore, this PR replaces the code of `tm2/pkg/iavl` with the code of `cosmos/iavl@v1.3.5`. ``` Note that `cosmos/iavl` is just a maintened version of `tm2/pkg/iavl`, which has received numerous improvements and bug fixes over the years. In addition to the benefit of making Gno IBC-ready, this change also eliminates some drawbacks of `tm2/pkg/iavl`: - `tm2/pkg/iavl` uses the legacy `range-proof` under the hood which is considered unsafe. It has an exploited vulnerability (see DragonFruit [[1](https://github.com/cosmos/iavl/issues/584)][[2](https://forum.cosmos.network/t/cosmos-sdk-security-advisory-dragonfruit/7614)]) - `tm2/pkg/iavl` proof generation is buggy: see #4369 This change only concerns the proof system. The Gno light client realm which is the other important piece of the IBCv2 connection between Gnoland and AtomOne is still under progress [here](#4655). > [!IMPORTANT] > This [change](https://github.com/gnolang/gno/pull/4656/files#diff-4f1d80300014eb607e44892f6ebdcc31de2c0815669acd54f805b83501311b7fL419) to the commit info hashing is causing a app hash mismatch when restarting a node with a binary built from this branch. > ``` > panic: state.AppHash does not match AppHash after replay. Got > 76BB396891B9F4415AC49FB72CF81933F407AE6A2C04551814719C9A908B2E8E, expected 8EDAB60FFD0B2C6BB5AC398417CF0AFBFE6E7D491C830F831F9395DB7FAC522A. > ``` > Unfortunately without it some tests are failing because ics23 does not expect to have hashed value. As a result, upgrading a mainnet launched using `tm2/pkg/iavl` to a new version with `cosmos/iavl` might be challenging. It's probably a good idea to merge this change for the initial mainnet. ## For reviewers - Most of the diff is related to the `DB` interface, because most of declared methods return an error in `comos/iavl` - When possible I added a comment for change related to the difference between `tm2/pkg/iavl` and `cosmos/iavl` implementation. - Lazy loading has been removed in `cosmos/iavl` so I had no other choice than removing the option from the Gno store. Anyway this feature was not used. --------- Co-authored-by: n0izn0iz <n0izn0iz@users.noreply.github.com> Co-authored-by: Miloš Živković <milos.zivkovic@tendermint.com>
e4e1a8a to
ecf952e
Compare
…ng#4656) The IBC protocol is based on merkle proofs, and over the years a standard of merkle proof representation has emerged: [ICS23](https://github.com/cosmos/ics23). The simplest way for Gno to be compatible with this protocol is to migrate the iavl implementation to [`cosmos/iavl`](https://github.com/cosmos/iavl), which already supports ICS23. ```diff - Therefore, this PR deprecates `tm2/pkg/iavl` in favour of imported `cosmos/iavl`. + Therefore, this PR replaces the code of `tm2/pkg/iavl` with the code of `cosmos/iavl@v1.3.5`. ``` Note that `cosmos/iavl` is just a maintened version of `tm2/pkg/iavl`, which has received numerous improvements and bug fixes over the years. In addition to the benefit of making Gno IBC-ready, this change also eliminates some drawbacks of `tm2/pkg/iavl`: - `tm2/pkg/iavl` uses the legacy `range-proof` under the hood which is considered unsafe. It has an exploited vulnerability (see DragonFruit [[1](https://github.com/cosmos/iavl/issues/584)][[2](https://forum.cosmos.network/t/cosmos-sdk-security-advisory-dragonfruit/7614)]) - `tm2/pkg/iavl` proof generation is buggy: see gnolang#4369 This change only concerns the proof system. The Gno light client realm which is the other important piece of the IBCv2 connection between Gnoland and AtomOne is still under progress [here](gnolang#4655). > [!IMPORTANT] > This [change](https://github.com/gnolang/gno/pull/4656/files#diff-4f1d80300014eb607e44892f6ebdcc31de2c0815669acd54f805b83501311b7fL419) to the commit info hashing is causing a app hash mismatch when restarting a node with a binary built from this branch. > ``` > panic: state.AppHash does not match AppHash after replay. Got > 76BB396891B9F4415AC49FB72CF81933F407AE6A2C04551814719C9A908B2E8E, expected 8EDAB60FFD0B2C6BB5AC398417CF0AFBFE6E7D491C830F831F9395DB7FAC522A. > ``` > Unfortunately without it some tests are failing because ics23 does not expect to have hashed value. As a result, upgrading a mainnet launched using `tm2/pkg/iavl` to a new version with `cosmos/iavl` might be challenging. It's probably a good idea to merge this change for the initial mainnet. ## For reviewers - Most of the diff is related to the `DB` interface, because most of declared methods return an error in `comos/iavl` - When possible I added a comment for change related to the difference between `tm2/pkg/iavl` and `cosmos/iavl` implementation. - Lazy loading has been removed in `cosmos/iavl` so I had no other choice than removing the option from the Gno store. Anyway this feature was not used. --------- Co-authored-by: n0izn0iz <n0izn0iz@users.noreply.github.com> Co-authored-by: Miloš Živković <milos.zivkovic@tendermint.com>
1bce2ec to
349cdae
Compare
e676815 to
9c5eb09
Compare
fail for now bc not possible to mint the ibc denom
|
Moved everything in a separate repo since it became too big to be reviewed, and also because I can point to a fork of gno with the required changes: |
The
r/aib/ibc/corerealm provides the expected functions,CreateClient,RegisterCounterParty,UpdateClient,UpgradeClient,SendPacket,RecvPacket,AcknowlegdmentandTimeout. Most of them are TODOs for now, but they are callable.The
r/aib/ibc/appsrealm provides applications that implements thep/aib/ibc/app.IBCAppinterface. Such apps must be registered into thecoremodule using thecore.RegisterApp()function.To run a gno node, clone this repo, checkout the
aib/feat/ibcbranch, runmake installand finally rungnodevinside the folder. This will pop a node and you should be able to see the functions at http://localhost:8888/r/aib/ibc/core$help. This help page also gives instructions to call the functions, but only withMsgCall. This kind of call won't work with most of the IBC functions, because they use complex args. See the README for detailed call instructions.Once created, clients are visible here http://localhost:8888/r/aib/ibc/core:clients