ics20-tao-v2-support #1157
ics20-tao-v2-support #1157
Conversation
sangier
requested review from
AdityaSripal,
colin-axner and
damiannolan
as code owners
| ): bool { | ||
|
|
||
| // the decode function must check the payload.encoding is among those supported | ||
| success,appData=decode(payload.encoding,payload.appData) |
There was a problem hiding this comment.
this should also take in the version so that we decode into v1 or v2 appData
| var tokens []Token | ||
| var sender string | ||
| var receiver string // address to send tokens to on this chain | ||
| var finalReceiver string // final intended address in forwarding case | ||
|
|
||
| if transferVersion == "ics20-1" { | ||
| FungibleTokenPacketData data = json.unmarshal(packet.data) | ||
| FungibleTokenPacketData data = payload.encoding.unmarshal(payload.appData) |
There was a problem hiding this comment.
| FungibleTokenPacketData data = payload.encoding.unmarshal(payload.appData) | |
| FungibleTokenPacketData data = unmarshal(payload.encoding, payload.version, payload.appData) |
There was a problem hiding this comment.
payload.encoding is just a string
| currentTime() + DefaultHopTimeoutPeriod, | ||
| forwardingPayload | ||
| ) | ||
| // store packet for future sending ack |
| packet: Packet, | ||
| acknowledgement: bytes) { | ||
| sourceChannelId: bytes, | ||
| destChannelId: bytes, // Can be nullable |
There was a problem hiding this comment.
Why would this be nullable?
There was a problem hiding this comment.
Yeah, here what I wanted to express is that we call the onAckPacket with the parameters specified in the ics04, but, actually, we don't use all of them. Indeed, while all parameters are necessary for ics20v2 onAckPacket logic, destChannelId and sequence are not used in the ics20v1 onAckPacket.
So the question is how do we want to handle this difference in the interfaces?
- Do we want to have a standard way to call applications on the core side?
- Do we let the core understand how to call the specific app callback retrieving from the router the callback signature?
Option 2 should provide more flexibility and seems ok. What do you think?
| if acknowledgement.success { | ||
| FungibleTokenPacketAcknowledgement ack = FungibleTokenPacketAcknowledgement{true, "forwarded packet succeeded"} | ||
| handler.writeAcknowledgement( | ||
| prevPacket, | ||
| destChannelId, |
There was a problem hiding this comment.
We aren't using the destChannelId here. We need the dest channel of the previous packet
If its not available here then we need to store it along with the previous sequence.
| // write error acknowledgement | ||
| FungibleTokenPacketAcknowledgement ack = FungibleTokenPacketAcknowledgement{false, "forwarded packet failed"} | ||
| handler.writeAcknowledgement( | ||
| prevPacket, | ||
| destChannelId, |
| // the forwarded packet has failed, thus the funds have been refunded to the forwarding address. | ||
| // we must revert the changes that came from successfully receiving the tokens on our chain | ||
| // before propogating the error acknowledgement back to original sender chain | ||
| revertInFlightChanges(packet, prevPacket) | ||
| revertInFlightChanges(destChannelId, payload) | ||
| // write error acknowledgement | ||
| FungibleTokenPacketAcknowledgement ack = FungibleTokenPacketAcknowledgement{false, "forwarded packet timed out"} | ||
| handler.writeAcknowledgement( |
There was a problem hiding this comment.
This needs to change to not have prevPacket in argument
| function revertInFlightChanges(sentPacket: Packet, receivedPacket: Packet) { | ||
| forwardingAddress = channelForwardingAddress[receivedPacket.destChannel] | ||
| reverseEscrow = channelEscrowAddresses[receivedPacket.destChannel] | ||
| function revertInFlightChanges( |
There was a problem hiding this comment.
This function was incorrectly converted. Note that it originally took in two different packets and now you only have a single destChannelId and payload. You need to still take in the forwarded packet info and the original received packet info
| var tokens []Token | ||
| var sender string | ||
| var receiver string // address to send tokens to on this chain | ||
| var finalReceiver string // final intended address in forwarding case | ||
|
|
||
| if transferVersion == "ics20-1" { | ||
| FungibleTokenPacketData data = json.unmarshal(packet.data) | ||
| FungibleTokenPacketData data = payload.encoding.unmarshal(payload.appData) |
There was a problem hiding this comment.
payload.encoding is just a string
This PR introduces IBC v2 TAO support for the ics20 v1 and v2 trying to capture all the internal discussions.
Closes #1141