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

Go 1.17, Make all Tendermint and Cosmos Versions match #590

Merged
merged 8 commits into from
Dec 6, 2021
Merged

Go 1.17, Make all Tendermint and Cosmos Versions match #590

merged 8 commits into from
Dec 6, 2021

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Oct 23, 2021

Summary of changes

Cosmos SDK 0.44.3
Tendermint 0.34.14
IBC-Go 1.2.2

This seems to improve relayer performance using both Hermes and Go relayer.

Replaces #589 and does the same thing more cleanly.

Report of required housekeeping

  • Github issue OR spec proposal link
  • Wrote tests (N/A. but let me know if any are needed)
  • Updated API documentation (client/lcd/swagger-ui/swagger.yaml) <- does not change the api
  • Added a relevant changelog entry: clog add [section] [stanza] [message]

(FOR ADMIN) Before merging

  • Added appropriate labels to PR
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)
  • Confirm added tests are consistent with the intended behavior of changes
  • Ensure all tests pass

cmd/terrad/root.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Oct 25, 2021

Codecov Report

Merging #590 (07f1019) into main (746a15f) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 07f1019 differs from pull request most recent head 9027258. Consider uploading reports for the commit 9027258 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #590      +/-   ##
==========================================
- Coverage   44.74%   44.72%   -0.02%     
==========================================
  Files         120      120              
  Lines        6969     6967       -2     
==========================================
- Hits         3118     3116       -2     
  Misses       3613     3613              
  Partials      238      238              
Impacted Files Coverage Δ
x/wasm/genesis.go 66.66% <0.00%> (-1.34%) ⬇️

@yun-yeo
Copy link
Contributor

yun-yeo commented Oct 26, 2021

Does it safe to update ibc-go from 1.1.x to 1.2.x??
No compatibility issue there?

if not I'm happy to merge this.

Dockerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@yun-yeo yun-yeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some comments.

btw I just want to check ibc-go compatibility between v1.1 and v1.2

Copy link
Contributor Author

@faddat faddat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checked changed file and have modified Dockerfile.

@YunSuk-Yeo regarding ibc-go 1.2.2.... well, I agree. I've included it here because "it works" but do not know "official guidance"

@faddat faddat changed the title Version Bumps, done more cleanly. Version Bumps Oct 29, 2021
Copy link
Contributor Author

@faddat faddat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

viewed changed files

@faddat
Copy link
Contributor Author

faddat commented Nov 15, 2021

Yeah, could go with ibc-go. We've got a wasmvm lib, at notional and I'd love your commentary on it. It is being used for juno. Https://github.com/notional-labs/wasmvm.

However, what I have done here is get this to a mergeable state. I messed something up:

IBC-Go 1.2.* has breaking changes and I should have just updated to the 1.1.* branch.

Copy link
Contributor Author

@faddat faddat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this works :)...

And you were surely right to check it.

Copy link
Contributor Author

@faddat faddat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, I realized that there were breaking changes between 1.1.* and 1.2.* but we really should use 1.1.3 otherwise we'll import multiple cosmos sdk versions and could impact stability. There's also an older build of tendermint making its way in, and I replaced that.

@faddat faddat changed the title Version Bumps Go 1.17, Make all Tendermint and Cosmos Versions match Nov 16, 2021
replace (
github.com/99designs/keyring => github.com/cosmos/keyring v1.1.7-0.20210622111912-ef00f8ac3d76
github.com/cosmos/ledger-cosmos-go => github.com/terra-money/ledger-terra-go v0.11.2
github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1
github.com/tendermint/tendermint => github.com/tendermint/tendermint v0.34.14
Copy link
Contributor

@yun-yeo yun-yeo Nov 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one is unnecessary? others seem good.

ibc-go@v1.1.3 also have tendermint@v0.34.14
cosmos/ibc-go@8193efb

@yun-yeo yun-yeo self-requested a review November 16, 2021 17:59
@yun-yeo yun-yeo merged commit c080a55 into terra-money:main Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants