Skip to content

old - packet forward middleware#432

Closed
faddat wants to merge 31 commits intomainfrom
faddat/pfm
Closed

old - packet forward middleware#432
faddat wants to merge 31 commits intomainfrom
faddat/pfm

Conversation

@faddat
Copy link
Copy Markdown
Contributor

@faddat faddat commented Dec 18, 2022

This PR implements the packet forward middleware.

It's using the branch referenced in this pr:

Since the implementation with ibc v4 is a bit different from v3, reviewers should have a look at:

https://github.com/cosmos/gaia/blob/6880cd7dc22ada30b9dd5737d45f1f90d38a5158/app/keepers/keepers.go#L323

We should also write some e2e tests for this integration, to ensure that it works correctly.

@faddat faddat marked this pull request as ready for review December 21, 2022 12:37
@faddat faddat enabled auto-merge December 21, 2022 12:37
@faddat
Copy link
Copy Markdown
Contributor Author

faddat commented Dec 24, 2022

This merges the faddat/make-proto-only branch in here, to ensure that it was the needed fix.

@faddat
Copy link
Copy Markdown
Contributor Author

faddat commented Dec 24, 2022

@vuong177 @reecepbcups @GNaD13 - guys, getting some pretty serious proto cross-talk here.

Also over here:

CosmosContracts/token-factory#1

@faddat
Copy link
Copy Markdown
Contributor Author

faddat commented Dec 25, 2022

@reecepbcups fyi I think that the proto changes you made to feeshare resolved the issues here.

Update: seems not.

Can you take a look at the CI logs?

We should run through what could be causing this.

idle theory:

perhaps the packet forward middleware's protos need to be built with Juno's proto's like mint, meaning that we should likely include it in this repo.

@faddat
Copy link
Copy Markdown
Contributor Author

faddat commented Dec 26, 2022

@faddat faddat added the review me Review me for merge label Dec 26, 2022
@faddat faddat requested a review from vuong177 December 28, 2022 18:22
Copy link
Copy Markdown
Contributor

@reecepbcups reecepbcups left a comment

Choose a reason for hiding this comment

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

Also should add an e2e test for this to ensure it works correctly

Comment thread app/keepers/keepers.go
@faddat
Copy link
Copy Markdown
Contributor Author

faddat commented Jan 1, 2023

@reecepbcups I agree about e2e

I also think that this is a reasonable place to segue into possibly using IBC test, maybe after we do the UNI deployment.

But I still do need to figure out how to use it to simulate the test against the live state.

@reecepbcups reecepbcups removed the review me Review me for merge label Jan 3, 2023
@reecepbcups reecepbcups changed the title packet forward middleware v13 - packet forward middleware Jan 3, 2023
@reecepbcups
Copy link
Copy Markdown
Contributor

Push the v13 if we can't figure out where test are, which just leaves oracle left for review

@faddat
Copy link
Copy Markdown
Contributor Author

faddat commented Jan 5, 2023

Yeah I think that makes perfect sense

@reecepbcups
Copy link
Copy Markdown
Contributor

Monday Meeting Pushed to v13 officially

@reecepbcups reecepbcups added the blocked We can't do this until something else is done label Jan 11, 2023
@faddat
Copy link
Copy Markdown
Contributor Author

faddat commented Jan 12, 2023

Did a merge here for some testing for:

strangelove-ventures/packet-forward-middleware#45

No change to the v13 plan though.

@reecepbcups reecepbcups linked an issue Jan 15, 2023 that may be closed by this pull request
@reecepbcups reecepbcups changed the title v13 - packet forward middleware old - packet forward middleware Jan 16, 2023
@reecepbcups
Copy link
Copy Markdown
Contributor

Closing in favor of Andrews more simple PR without go.mod replace (since its merged in now to mainline)
Will add E2E test after that

auto-merge was automatically disabled January 16, 2023 01:41

Pull request was closed

@reecepbcups reecepbcups removed the blocked We can't do this until something else is done label Jan 16, 2023
@reecepbcups reecepbcups deleted the faddat/pfm branch March 28, 2023 04:09
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.

[v12] Packet Forward Middleware

3 participants