ics20-tao-v2-support #1157

sangier · 2024-10-31T12:40:42Z

This PR introduces IBC v2 TAO support for the ics20 v1 and v2 trying to capture all the internal discussions.

AdityaSripal

Nice start! The ics20-v1 spec looks correct, but there are some mistakes on the ics20-v2 side

AdityaSripal · 2024-11-01T14:31:36Z

spec/app/ics-020-fungible-token-transfer/README.md

+  ): bool {
+
+  // the decode function must check the payload.encoding is among those supported 
+  success,appData=decode(payload.encoding,payload.appData)


this should also take in the version so that we decode into v1 or v2 appData

AdityaSripal · 2024-11-01T14:33:59Z

spec/app/ics-020-fungible-token-transfer/README.md

  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)


Suggested change

FungibleTokenPacketData data = payload.encoding.unmarshal(payload.appData)

FungibleTokenPacketData data = unmarshal(payload.encoding, payload.version, payload.appData)

payload.encoding is just a string

AdityaSripal · 2024-11-01T14:38:18Z

spec/app/ics-020-fungible-token-transfer/README.md

      currentTime() + DefaultHopTimeoutPeriod,
+      forwardingPayload
    )
    // store packet for future sending ack


Fix comment

AdityaSripal · 2024-11-01T15:17:41Z

spec/app/ics-020-fungible-token-transfer/v1/README.md

-  packet: Packet,
-  acknowledgement: bytes) {
+  sourceChannelId: bytes,
+  destChannelId: bytes, // Can be nullable


Why would this be nullable?

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?

spec/app/ics-020-fungible-token-transfer/README.md

AdityaSripal · 2024-11-01T15:29:35Z

spec/app/ics-020-fungible-token-transfer/README.md

    if acknowledgement.success {
      FungibleTokenPacketAcknowledgement ack = FungibleTokenPacketAcknowledgement{true, "forwarded packet succeeded"}
      handler.writeAcknowledgement(
-        prevPacket,
+        destChannelId,


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.

AdityaSripal · 2024-11-01T15:30:04Z

spec/app/ics-020-fungible-token-transfer/README.md

      // write error acknowledgement
      FungibleTokenPacketAcknowledgement ack = FungibleTokenPacketAcknowledgement{false, "forwarded packet failed"}
      handler.writeAcknowledgement(
-        prevPacket,
+        destChannelId,


AdityaSripal · 2024-11-01T15:33:01Z

spec/app/ics-020-fungible-token-transfer/README.md

    // 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(


This needs to change to not have prevPacket in argument

AdityaSripal · 2024-11-01T15:43:11Z

spec/app/ics-020-fungible-token-transfer/README.md

-function revertInFlightChanges(sentPacket: Packet, receivedPacket: Packet) {
-  forwardingAddress = channelForwardingAddress[receivedPacket.destChannel]
-  reverseEscrow = channelEscrowAddresses[receivedPacket.destChannel]
+function revertInFlightChanges(


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

Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com>

AdityaSripal

Excellent work! Everything looks good now, will commit the small fixes and then merge

AdityaSripal · 2024-11-06T18:15:09Z

spec/app/ics-020-fungible-token-transfer/README.md

  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)


payload.encoding is just a string

spec/app/ics-020-fungible-token-transfer/README.md

* add v2 folder * create structure and paste v1 specs for now * TAO V2 support in 07 client (#1137) * deprecate delay period params * add params into clientState * fix comment * include PR link in history * fixes * fix comment * fix modified field * Copied README.md to v2/ while preserving history * Reverted README.md to commit 825d47d * fix deadlinks * rm v1 deprecation references * del v2 folder and readme * cp v1 spec into TAO_V1_README * apply v2 changes * fix dead links * fix deadlinks 2 * fix image links * fixes * fixes * fix link to v2 version --------- Co-authored-by: Stefano Angieri <stefano@interchain.io> * 02-client-TAOv2 (#1147) * rm delayPeriods * add msg and hanlder * fixes * del things * fix order * typos * review comments --------- Co-authored-by: Stefano Angieri <stefano@interchain.io> * 24-host: Reduce provable surface area to only packet flow keys (#1144) * start 24-host by removing unnecessary requirements and moving some provable keys to private keys * fix layout rendering * fix links * address Stefano's comments * high level path space and commitment specification --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io> * 04-Packet-Flow-v2 (#1148) * start removing channel refs * add counterparty refs * introducing multi-packet data structure * add multi-data packet * introduce packet handlers * send packet handl * change packet commitment paths * fix paths * mod sendPacket * mod receive * allign with new packet structure * mod ack and paths * change counterparty def * discussions-related-fixes * add sketch packet flow * rework + ack logic * fixes + timeout logic * start addressing review * addressing review comments * fixes * client-creation-registration * ante-error-post conditions sendPacket * fixes * handlers pre-error-post conditions * fixes * add mermaid diagrams, fixes * router fixes * improvements and fixes * minor fix * improvements * adding considerations * add condition table * change condition table format * multi-payload,paths,router,acks fixes * improve createClient conditions * table fix * registerChannel table * fixes * improvements * fixes * setup fixes * diagram fixes * fix diagram * test fix * two and three step setup * send-receive conditions * router fixes * createChannl conditions * register table fixes * conditions tables * table fixes * self review * self review * add soundness and correctness * self review * self review * fixes * minor fixes * rename folder * polish * change registerChannel to registerCounterparty * Apply suggestions from code review Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * address review comments * rm counterparty channel id check in send packet * mod writeAck function * delete packet once async ack has been written * fix callbacks * fixes * fix mermaid * rm relayer from SendPack * Apply suggestions from code review --------- Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * ics20-tao-v2-support (#1157) * start v2 changes * mod onRecv callback * mod revert,refund * wip refactor * wip-packet-reasoning * callbacks interface update * fixes * v2Tao support in v1 * Apply suggestions from code review Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * fix sequence type * unrmarshal utility function * fix write ack * fix revertInFlightsChanges * fixes * rm relayer from onSend * Apply suggestions from code review --------- Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * ICS24: Restructure provable packet keys (#1155) * imp: note that commitments must be lexographically ordered to maintain soundness (#1153) * restructure ibc keys * add value instead of redundant provable store column * chore: remove myself as codeowner (#1156) * Fix variable name (#1162) * fix link and put new provable keys into 04-channel spec --------- Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: Christoph Otter <chipshort@tutanota.com> * add timeoutTimestamp in forwarding (#1163) * Packet Spec: Hash App data rather than standardizing encoding (#1152) * add packet and acknowledgement structs and commitment details * add CBOR as suggested encoding and SENTINEL_ACKNOWLEDGEMENT value * explain each field in the code * change id to channel * switch cbor encoding to hashing in packet commitment * switch cbor encoding to hashing in acknowledgement * Apply suggestions from code review Co-authored-by: sangier <45793271+sangier@users.noreply.github.com> * add recursive hashing suggestion from amulet * prepend protocol byte --------- Co-authored-by: sangier <45793271+sangier@users.noreply.github.com> * Move PACKET.md (#1166) * move PACKET.md to the right folder * linter * linter * ICS4: Create 32 byte commitment (#1168) * move protocol version inside final hash * Update spec/core/v2/ics-004-packet-semantics/PACKET.md Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com> --------- Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com> * ICS4: Event Emission Specification (NO PSEUDOCODE) (#1172) * event specification * specify events for channel creation * add client id to register counterparty events * rename emitLogEntry to emitEvents * address reviews * fix typo * remove explicit event implementation and include event writeup on what should be included * continue changes * add packet handler to v2 spec * move to v2 folder in top-level spec * simplify the spec * rename folder for more visibility * specify client handler specification * cleanup ICS-2 and start with ICS24 * ics5 port specification * halfway through ICS26 * specify packet callbacks * move folders to core * fix table * write overview doc * add link to payload docs * header corrections * address marius reviews and lint lists * revert non-core changes * revert client changes * satisfy linter * fix links * fix more linting issues * lint --------- Co-authored-by: sangier <45793271+sangier@users.noreply.github.com> Co-authored-by: Stefano Angieri <stefano@interchain.io> Co-authored-by: Carlos Rodriguez <carlos@interchain.io> Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: Christoph Otter <chipshort@tutanota.com> Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>

sangier added 8 commits October 29, 2024 16:45

start v2 changes

e2ce002

mod onRecv callback

3b44aa5

mod revert,refund

d140755

wip refactor

9bf517b

wip-packet-reasoning

be7e7ac

callbacks interface update

41e8181

fixes

7287aa1

v2Tao support in v1

99eaf71

sangier marked this pull request as ready for review October 31, 2024 17:04

sangier requested review from AdityaSripal, colin-axner and damiannolan as code owners October 31, 2024 17:04

AdityaSripal suggested changes Nov 1, 2024

View reviewed changes

sangier and others added 6 commits November 4, 2024 15:41

Apply suggestions from code review

adbd5c9

Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com>

fix sequence type

db19eaa

unrmarshal utility function

d08eb7e

fix write ack

d2ebdb3

fix revertInFlightsChanges

9e87aee

fixes

7949b90

sangier requested a review from AdityaSripal November 5, 2024 15:37

rm relayer from onSend

8c20ec0

AdityaSripal approved these changes Nov 6, 2024

View reviewed changes

Apply suggestions from code review

44e31ee

AdityaSripal merged commit af15ac1 into feat/v2-spec Nov 6, 2024
2 checks passed

AdityaSripal deleted the stefano/1141-ic20-tao-v2 branch November 6, 2024 18:27

sangier mentioned this pull request Nov 8, 2024

ICS20: Support v2 TAO in ICS20 spec #1141

Closed

	FungibleTokenPacketData data = payload.encoding.unmarshal(payload.appData)
	FungibleTokenPacketData data = unmarshal(payload.encoding, payload.version, payload.appData)

ics20-tao-v2-support #1157

ics20-tao-v2-support #1157

Uh oh!

Conversation

sangier commented Oct 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AdityaSripal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AdityaSripal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sangier commented Oct 31, 2024 •

edited

Loading