-
Notifications
You must be signed in to change notification settings - Fork 625
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
feat(core): Wire packet handler to core message server #7091
feat(core): Wire packet handler to core message server #7091
Conversation
// send packet | ||
_, err := suite.chainA.App.GetPacketServer().SendPacket(suite.chainA.GetContext(), nil, packet.SourceChannel, packet.SourcePort, packet.DestinationPort, packet.TimeoutHeight, packet.TimeoutTimestamp, packet.AppVersion, packet.Data) | ||
sequence, err := path.EndpointA.SendPacketV2(defaultTimeoutHeight, disabledTimeoutTimestamp, "", ibctesting.MockPacketData) |
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.
added for convenience, will chage rest to use where applicable
|
||
ack = mock.MockAcknowledgement | ||
err = suite.chainB.App.GetPacketServer().WriteAcknowledgement(suite.chainB.GetContext(), nil, packet, ack) | ||
err = path.EndpointB.RecvPacket(packet) |
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.
can now directly use this
|
||
case channeltypes.IBC_VERSION_2: | ||
packetHandler = k.PacketServerKeeper | ||
module = msg.Packet.DestinationPort |
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.
was not sure if we want to do fallback yet. can tweak and add tho
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 work!!
|
||
TimeoutPacket( | ||
ctx sdk.Context, | ||
chanCap *capabilitytypes.Capability, |
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.
Hmm if there wasn't originally a chanCapability in this signature why do we need to add it only to then remove it later?
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.
because I inlined timeoutExecuted
and need to have the signatures matching (TimeoutPacket/OnClose
didn't require capability previously), unfortunate but pile it on the list of clean-ups after port-router!
was looking into amending Ideally should copy over tests, trim down ordered cases, amend functions and just dump them in a new file. |
Description
still pending on making TimeoutExecuted private and tweaking as many tests as I can.
closes: #7061
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/
).godoc
comments.Files changed
in the GitHub PR explorer.SonarCloud Report
in the comment section below once CI passes.