-
Notifications
You must be signed in to change notification settings - Fork 396
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
ICS04: adds event emission in recvPacket
to align with implementation
#844
ICS04: adds event emission in recvPacket
to align with implementation
#844
Conversation
@@ -670,7 +681,19 @@ function recvPacket( | |||
// for unordered channels we must set the receipt so it can be verified on the other side | |||
// this receipt does not contain any data, since the packet has not yet been processed | |||
// it's the sentinel success receipt: []byte{0x01} | |||
abortTransactionUnless(provableStore.get(packetReceiptPath(packet.destPort, packet.destChannel, packet.sequence) === null)) | |||
packetRecepit = provableStore.get(packetReceiptPath(packet.destPort, packet.destChannel, packet.sequence)) | |||
if (packetRecepit != null) { |
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.
I noticed that we are also emitting an event in this case in ibc-go, so I added it here as well. I hope that's ok.
@@ -679,8 +702,14 @@ function recvPacket( | |||
} | |||
|
|||
// log that a packet has been received | |||
emitLogEntry("recvPacket", {sequence: packet.sequence, timeoutHeight: packet.timeoutHeight, port: packet.destPort, channel: packet.destChannel, | |||
timeoutTimestamp: packet.timeoutTimestamp, data: packet.data}) | |||
emitLogEntry("recvPacket", { |
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.
I thought it would help readability to break the long line...
…ent-for-already-received-packets-on-ordered-channels
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.
LGTM. Just some nits. See comments below.
…ent-for-already-received-packets-on-ordered-channels
Closes: #755