deps!: bump ibc-go to v4.3.0 and remove dragonberry replace #2185
Conversation
faddat
requested review from
alexanderbez,
zmanian,
yaruwangway,
crodriguezvega,
jackzampolin,
mmulji-ic,
glnro,
jtremback,
mpoke,
sainoe,
MSalopek and
shaspitz
as code owners
mpoke
added
the
state-machine-breaking
|
Hi @faddat. Thanks for the PR. Could you please resolve the conflicts? |
|
Also, could you please bump packet-forward-middleware to v4.0.5? Thanks. |
|
yes absolutely |
|
to ensure smoothness, this version of gaia, should depend on a version of ics with this PR merged: |
go.mod
Outdated
|
|
||
| // TODO: remove it: https://github.com/cosmos/cosmos-sdk/issues/13134 | ||
| github.com/dgrijalva/jwt-go => github.com/golang-jwt/jwt/v4 v4.4.2 | ||
| // TODO Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409 | ||
| github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.8.1 | ||
|
|
||
| // use cosmos style protobufs | ||
| github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1 | ||
| github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.2-alpha.regen.4 |
There was a problem hiding this comment.
Why is this change necessary?
There was a problem hiding this comment.
Ditto, seems an accidental regression.
There was a problem hiding this comment.
Hey there guys that's not an accidental regression it fixes compatibility issues like big time
There was a problem hiding this comment.
Unless I'm just wrong in which case I'm just wrong let's proceed but that extra three there causes stuff to get wonky
There was a problem hiding this comment.
Could you please share some context on why v1.3.3-alpha.regen.1 is problematic and v1.3.2-alpha.regen.4 is preferred? SDK v0.45.x (the version of SDK for v10) uses the former, see https://github.com/cosmos/cosmos-sdk/blob/b05b6fe651514c11af3d4160f7c75fbaad92d5db/go.mod#L154
There was a problem hiding this comment.
@faddat please revert this change.
mpoke
changed the title
It's not necessary as the version in the Gaia go.mod will take priority. It is preferable though as ICS 1.1.0 is not tested with SDK v0.45.15-ics. As this change is state breaking, we can fix this in the next major Gaia release, i.e., v10. |
|
Where's the branch to Target for version 10 or should we make this that branch? |
|
Sure thing.
…On Wed, Apr 19, 2023, 1:28 AM Marius Poke ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In go.mod
<#2185 (comment)>:
>
// TODO: remove it: cosmos/cosmos-sdk#13134
github.com/dgrijalva/jwt-go => github.com/golang-jwt/jwt/v4 v4.4.2
// TODO Remove it: cosmos/cosmos-sdk#10409
github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.8.1
// use cosmos style protobufs
- github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1
+ github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.2-alpha.regen.4
@faddat <https://github.com/faddat> please revert this change.
—
Reply to this email directly, view it on GitHub
<#2185 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABWPVCO72KF5YZF3ZNPVVBTXB3FNJANCNFSM6AAAAAAU3RCIOY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
hey sorry about slow! I couldn't find this pr |
|
@mpoke nice catch, thank you. I'd meant to adjust the version not in the replaces but in the main section of go.mod, |
go.mod
Outdated
| github.com/cosmos/ibc-go/v4 v4.2.0 | ||
| github.com/cosmos/interchain-security v1.1.0 | ||
| github.com/cosmos/ibc-go/v4 v4.3.0 | ||
| github.com/cosmos/interchain-security v1.2.0 |
There was a problem hiding this comment.
Please keep the PR contained to one issue. There is no need to bump the version of ICS in the same PR where we bump ibc-go.
There was a problem hiding this comment.
there is. At least, while I do understand that gaia's version will dominate, I've seen a lot of kaboomies.
To prevent them I like to ensure that dependencies use the exact same versions of ecosystem libraries
There was a problem hiding this comment.
I'll revert, but wish to note that I think it is safer to group this stuff
There was a problem hiding this comment.
@faddat Thank you for sharing your concern. If you think that bumping ibc-go to v4.3.0 requires a bump of ICS as well, please open an issue describing the problem in details. We can discuss your concerns on that issue and decide on how to best address them.
From the description and title of this PR, the goal is to
Removes the now-unnecessary dragonberry replace and updates to ibc-go v4.3.0
Adding multiple changes to the same PR without properly documenting it, will lead to difficulties in maintaining Gaia in the long run. For example, github.com/cosmos/interchain-security v1.2.0 contains multiple changes that need to be properly tested before adding them to Gaia. Moreover, v1.2.0 still uses ibc-go v4.2.0.
Removes the now-unnecessary dragonberry replace and updates to ibc-go v4.3.0
Closes #2280