-
Notifications
You must be signed in to change notification settings - Fork 629
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
deps: upgrade of SDK to 0.46 and tendermint to 0.35 #1205
deps: upgrade of SDK to 0.46 and tendermint to 0.35 #1205
Conversation
Will there not be a v0.45 compatible version? |
You mean SDK 0.45 with Tendermint 0.35? We could look into it... |
Codecov Report
@@ Coverage Diff @@
## carlos/upgrade-sdk-0.46-tendermint-0.35 #1205 +/- ##
===========================================================================
- Coverage 79.82% 79.47% -0.35%
===========================================================================
Files 151 151
Lines 10907 10945 +38
===========================================================================
- Hits 8706 8699 -7
- Misses 1776 1819 +43
- Partials 425 427 +2
|
go.mod
Outdated
@@ -4,119 +4,142 @@ module github.com/cosmos/ibc-go/v3 | |||
|
|||
replace github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1 | |||
|
|||
replace github.com/zondax/hid => github.com/zondax/hid v0.9.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to add this to make the branch build on my Mac running Big Sur. I will take it off before merging.
Just did a quick test of trying to upgrade to Tendermint 0.35 with SDK 0.45.1 and I get errors like this:
So not sure if it will be possible... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice start! Left several suggestions after briefly diving through a lot of the changes. I'm wondering if we should split up this update. For example, the IBC ante handler changes are technically not required to update to v0.46 (simply remove them), then we can have a followup pr which properly fixes the code. The old structure of the ante handler code doesn't make sense to just copy over given the new API
hey! |
This has been moved to Edit: also, |
right! just saw a reference to the old path here: ibc-go/testing/simapp/params/amino.go Line 8 in 34f03df
|
hey, I'm really happy to see the progress here and also to be getting help on discerning exactly which changes were needed for v46 compatiblity. For our upgrade, the primary authors were @catShaark and @nguyenvuong1122000. In the coming week, I'll be updating the testnet code for https://github.com/notional-labs/craft to the beta1 cosmos-sdk (or possibly release/v0.46.x) and we should have some more input on the work here. |
Thanks for pointing this out! I just pushed a fix. |
…SDK v0.46 (#1288) * refactor: simplify ibc redundancy check used as sdk middleware Instead of having the function checkRedundancy check if the call is on CheckTx or SimulateTx, only call the function on CheckTx or SimulateTx * add godoc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! I still need to rereview the sdk middleware code and I am still concerned about updating the ABCIResponsesResultHash
, implementing the function ourself defeats the purpose of the test added to indicate when the expected downstream behaviour changes
I would prefer updates to newer release (ie beta3) to be done in a separate pr since this one already has many changes. Ideally we merge this as soon as the ABCIResponsesResultHash
is resolved
@@ -390,7 +392,7 @@ func writeFile(name string, dir string, contents []byte) error { | |||
return err | |||
} | |||
|
|||
err = tmos.WriteFile(file, contents, 0644) | |||
err = ioutil.WriteFile(file, contents, 0644) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side note: running this branch locally deletes a local genesis file which causes other branches (without the SDK update) cmd_test.go
tests to break. I think this line is the culprit. I guess there isn't much to do other than just get this merged asap :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK pending the merge of #1349 (approval only applies to merging into the 0.46 feature branch, not main)
…age responses (#1349) * begin refactoring ack_test * fix test due to delayed block execution * refactor: switch ics27 response creation to use new SDK version, update tests * fix import
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
…ibc-go into carlos/upgrade-pr-branch
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Another PR will be opened when there's an SDK 0.46 RC pre-release. |
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ### Summary by CodeRabbit - **Documentation**: Updated the file structure for better organization. Introduced a new specification file "Block Validity" for enhanced clarity on block and header validation in our blockchain system. - **New Feature**: Added detailed descriptions and requirements for verifying blocks and headers, including basic validation and verification against the previous block. This will improve the reliability and security of our blockchain system. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Connor O'Hara <connor@switchboard.xyz> Co-authored-by: nashqueue <99758629+nashqueue@users.noreply.github.com>
Description
Upgrade of SDK to 0.46-alpha3 and Tendermint to 0.35.
This PR is based on the excellent work done by @faddat on his PR. This PR is basically a subset of all the changes included in #949 (i.e. it adds only the changes strictly necessary for the upgrade of SDK and Tendermint) to narrow the scope and make the review easier.
TODO:
closes: #909 #539
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes