Skip to content
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

CW + SDK v46 + ibcv3 #764

Closed
wants to merge 98 commits into from
Closed

CW + SDK v46 + ibcv3 #764

wants to merge 98 commits into from

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Feb 23, 2022

This draft PR is a part of the initiative described here:

https://twitter.com/gadikian/status/1496100856235950086?s=20&t=yIA9N83HtFoWVNCXKbagbw

and here:

cosmos/cosmos-sdk#11096

Notes and questions:

  • V46 refactors gov. We are unsure if we should use new or legacy proposal types. Currently we are using legacy.
  • We're using the arm support branch of wasmvm, just to ease the development workflow. This could be removed if need be.
  • ICA: I am not sure if there is integration work needed, where and how

@faddat faddat changed the title CW + SDK v46 CW + SDK v46 + ibcv3 Feb 23, 2022
@faddat
Copy link
Contributor Author

faddat commented Mar 7, 2022

Status Update

In order to make this work properly, Khanh has made a PR to ibc-go which will end up in the ibc spec:

cosmos/ibc-go#1076

cosmos/ibc-go#1075

cosmos/ibc#674

From here, we are going to begin actively testing the work found here in craft economy

The goal is basically the same as always-- to provide a featureful platform for blockchain development that tracks:

  • cosmos-sdk
  • wasmd
  • ibc-go

And to create a templating tool that lives in the SDK, which ships with all features turned on so that it will be useful in a CI system, which will also land in the SDK repo.

Probably it won't make sense to merge this into the master branch, but it may make a lot of sense to change the base branch and bring it here, maybe just for the purpose of visibility.

@faddat faddat marked this pull request as ready for review March 8, 2022 11:58
@faddat faddat requested a review from alpe as a code owner March 8, 2022 11:58
@faddat
Copy link
Contributor Author

faddat commented Mar 8, 2022

Only problem remaining is the benchmarks and I suspect that's trivial.

@codecov
Copy link

codecov bot commented Mar 8, 2022

Codecov Report

Merging #764 (d86ce08) into master (e7a3ac3) will decrease coverage by 2.55%.
The diff coverage is 63.47%.

❗ Current head d86ce08 differs from pull request most recent head 348f50a. Consider uploading reports for the commit 348f50a to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #764      +/-   ##
==========================================
- Coverage   58.64%   56.09%   -2.56%     
==========================================
  Files          50       51       +1     
  Lines        5845     6008     +163     
==========================================
- Hits         3428     3370      -58     
- Misses       2166     2379     +213     
- Partials      251      259       +8     
Impacted Files Coverage Δ
app/test_access.go 0.00% <0.00%> (ø)
x/wasm/client/cli/gov_tx.go 0.00% <0.00%> (ø)
x/wasm/keeper/handler_plugin.go 85.14% <ø> (ø)
x/wasm/keeper/ibc.go 77.77% <ø> (ø)
x/wasm/keeper/query_plugins.go 80.06% <ø> (ø)
x/wasm/module.go 48.86% <ø> (+0.54%) ⬆️
x/wasm/types/keys.go 51.11% <ø> (ø)
x/wasm/types/privval.go 0.00% <0.00%> (ø)
app/export.go 11.53% <3.33%> (-0.59%) ⬇️
app/test_helpers.go 2.53% <7.89%> (+1.77%) ⬆️
... and 19 more

@ethanfrey
Copy link
Member

Hi @faddat I am just back from vacation and seeing this.

We do not plan to add v0.46 support to wasmd:main anytime soon, but do plan to add ibcv3 support soon.
If you want to pull out just the ibcv3 part of this into another PR, I can review it later this week. It would keep the v0.46 fork closer to the main branch.

@ethanfrey
Copy link
Member

We won't work on v0.46 for some time. Not looking into this until then.

But I will review #793 shortly, which is compatible with main

@ethanfrey
Copy link
Member

Closing this for now, we have taken in all parts except for 0.46 upgrade.

I would be happy later for a new only 0.46 upgrade when we have a stable 1.0-beta.
Or if there is anything else you want to back port (like ibcv3 - that was super helpful), feel free to open a new PR just on that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants