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.

Sign up for GitHub

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

Jump to bottom

chore: update to 0.47 #337

Closed
wants to merge 6 commits into from
Closed

chore: update to 0.47 #337

wants to merge 6 commits into from

Conversation

julienrbrt
Copy link
Contributor

Description

update to sdk 0.47
branch to be used to update e2e tests ibc-go: cosmos/ibc-go#2672

Checklist

  • I have made sure the upstream branch for this PR is correct
  • I have made sure this PR is ready to merge
  • I have made sure that I have assigned reviewers related to this project

Changes

  • Added ...
  • Changed ...
  • Removed ...

Screenshots

...

Testing

  • Open page ...
  • Click ...
  • Make sure that ...

Links/References

  • ...
  • ...
  • ...

Notes

  • ...
  • ...
  • ...

@julienrbrt julienrbrt marked this pull request as ready for review November 4, 2022 06:25
@julienrbrt julienrbrt requested a review from a team as a code owner November 4, 2022 06:25
@charleenfei
Copy link
Contributor

hello all, can someone review this so we can pull it into our ibc-go update to 0.47? that would be much appreciated!!

currently we are working off of @julienrbrt 's fork

@colin-axner colin-axner mentioned this pull request Dec 12, 2022
Merged
9 tasks
@agouin
Copy link
Member

agouin commented Dec 12, 2022

hello all, can someone review this so we can pull it into our ibc-go update to 0.47? that would be much appreciated!!

Sure! The changes look good, but there are some breaking changes also. Happy to get this merged once CI is passing.

/home/strangelove/actions-runner/_work/ibctest/ibctest/interchain_test.go:293
        	Error:      	no registered implementations of type types.AccountI

/home/strangelove/actions-runner/_work/ibctest/ibctest/examples/ibc/learn_ibc_test.go:104
        	Error:      	Received unexpected error:
        	            	invalid packet sequence from events : strconv.Atoi: parsing "": invalid syntax

@julienrbrt
Copy link
Contributor Author

@agouin could you create a v0.47 branch in this repo? This way this PR can be merged there, and we can collaboratively push fixes.

@colin-axner
Copy link
Contributor

Do we think the tests require an update to the cosmos/relayer?

boojamya
boojamya reviewed Dec 13, 2022
View reviewed changes
chain/internal/tendermint/events.go
@@ -13,8 +13,8 @@ func AttributeValue(events []abcitypes.Event, eventType, attrKey string) (string
continue
}
for _, attr := range event.Attributes {
if string(attr.Key) == attrKey {
return string(attr.Value), true
if attr.Key == attrKey {
Copy link
Contributor

Choose a reason for hiding this comment

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

Did some troubleshooting here. Printed out both sides of this "if" and it looked like this:
attr.Key = cGFja2V0X2RhdGE=
attrKey = packet_sequence

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice work! What chain binaries did you use to test this? It seems tendermint changed the event attribute key to be string instead of []byte. I'm wondering if the binary used is using a tendermint v0.37-rc2?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @colin-axner.
I was just using the TestLearn in examples/ibc/learn_ibc_test.go. It's using some older versions:
gaia: v7.0.0
osmosis: v11.0.0

@agouin
Copy link
Member

agouin commented Dec 13, 2022

could you create a v0.47 branch in this repo? This way this PR can be merged there, and we can collaboratively push fixes.

Sure! I pushed this v0.47 branch to our repo

@julienrbrt
Copy link
Contributor Author

julienrbrt commented Dec 13, 2022 •
edited
Loading

Sure! I pushed this v0.47 branch to our repo

Thank you! Could you open a PR from that v0.47 branch to main? I will close this in favor of that.

@julienrbrt julienrbrt closed this Dec 13, 2022
@agouin agouin mentioned this pull request Dec 13, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colin-axner colin-axner colin-axner left review comments

@boojamya boojamya boojamya left review comments

@misko9 misko9 Awaiting requested review from misko9 misko9 is a code owner automatically assigned from strangelove-ventures/interchaintest-maintainers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@julienrbrt @charleenfei @agouin @colin-axner @boojamya