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

Return result and acknowledgement from RelayPacket #2956

Closed
3 tasks
migueldingli1997 opened this issue Dec 19, 2022 · 2 comments · Fixed by #3986
Closed
3 tasks

Return result and acknowledgement from RelayPacket #2956

migueldingli1997 opened this issue Dec 19, 2022 · 2 comments · Fixed by #3986
Assignees
Labels
testing Testing package and unit/integration tests type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Milestone

Comments

@migueldingli1997
Copy link

migueldingli1997 commented Dec 19, 2022

Summary

Return result and acknowledgement from RelayPacket function for possibility of applying checks or further operations based on these values.

Problem Definition

I have found the RelayPacket function to be very useful for tests where the packet is parsed from events and then relayed. An example from this repository that served as inspiration is the following:

res, err := suite.chainB.SendMsgs(transferMsg)
suite.Require().NoError(err) // message committed
packet, err := ibctesting.ParsePacketFromEvents(res.GetEvents())
suite.Require().NoError(err)
err = path.RelayPacket(packet)
suite.Require().NoError(err) // relay committed

As my code has gotten more complex I have found myself wanting to relay packets that were committed on the counterparty chain as a result of a packet relayed on the original chain. For example, an interchain accounts transaction with a MsgTransfer will create a packet on the counterparty chain during the ICA host's OnRecvPacket.

The way I see it, the easiest way to then relay this second packet would be to get a hold of the OnRecvPacket's events and look for the packet there. However the RelayPacket function currently discards the result containing the events.

So what I'm hoping for here is:

  • RelayPacket starts returning the result (and might as well return the acknowledgement as well).
  • Find an easy way to do this without requiring changes to RelayPacket. Perhaps the events can be obtained in another way?

Proposal

From:

func (path *Path) RelayPacket(packet channeltypes.Packet) error

To:

func (path *Path) RelayPacket(packet channeltypes.Packet) (res *sdk.Result, ack []byte, err error)

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@crodriguezvega crodriguezvega added testing Testing package and unit/integration tests type: refactor Architecture, code or CI improvements that may or may not tackle technical debt. labels Dec 20, 2022
@crodriguezvega
Copy link
Contributor

crodriguezvega commented Mar 9, 2023

Hi @migueldingli1997. Sorry for the late reply...

We have discussed this issue and we have a proposal: we would like to keep RelayPacket as is, but instead add a new function (tentatively called RelayPacketWithResults), with the same functionality as RelayPacket, but that will return what you need (i.e (res *sdk.Result, ack []byte, err error)). That way the existing RelayPacket function doesn't need to change and usages of it can remain the same.

How does this sound? Would it work for you?

@migueldingli1997
Copy link
Author

That would work @crodriguezvega :)

@crodriguezvega crodriguezvega added this to the v8.0.0 milestone May 19, 2023
@crodriguezvega crodriguezvega moved this to Todo in ibc-go May 19, 2023
@crodriguezvega crodriguezvega self-assigned this Jun 26, 2023
@crodriguezvega crodriguezvega linked a pull request Jun 30, 2023 that will close this issue
9 tasks
@github-project-automation github-project-automation bot moved this from Todo to Done in ibc-go Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Testing package and unit/integration tests type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants