TAO V2 support in 07 client #1137

sangier · 2024-08-19T14:24:11Z

sangier · 2024-08-19T14:33:05Z

Wondering If we should add an upper bound check on the initialise function for delayTimePeriod and delayBlockPeriod. Non negativeness is guaranteed for being uint type.

AdityaSripal

Looks good! Just have a question on how we should communicate both versions here

AdityaSripal · 2024-08-19T15:58:59Z

spec/client/ics-007-tendermint-client/README.md

@@ -388,9 +390,9 @@ function verifyMembership(
  // check that the client is unfrozen or frozen at a higher height
  assert(clientState.frozenHeight === null || clientState.frozenHeight > height)
  // assert that enough time has elapsed
-  assert(currentTimestamp() >= processedTime + delayPeriodTime)
+  assert(currentTimestamp() >= processedTime + clientState.delayTimePeriod)


Hmmm I'm wondering how we should handle this since they will use DEPRECATED field for v1 implementations

Mmm, what If we do something like:
// assert(currentTimestamp() >= processedTime + delayPeriodTime) // DEPRECATED - USED in TAO V1 assert(currentTimestamp() >= processedTime + clientState.delayTimePeriod) // TAO V2 SUPPORT`

I think you guys discussed in one of the issues, but maybe it's easier to just have two exact copies of the specs as they are today, one of them we will put into the v1 folder and the other copy in v2 and we modify that one with all the eureka changes without having to deal with this problem of how we communicate in the same spec what's for v1 and what's for v2.

We could even maybe not even make a copy, but just a release branch: for example, we branch off from main and create release/v1 and that branch will keep the classic specs as they are today. And in main we just change them according to eureka. In the README of the repo we give a link to the release branch, so that anyone can take a look at the specs in their "classic" state.

What do you guys think?

I agree that to deal with this problem of how we communicate in the same spec what's for v1 and what's for v2 it's not the best thing.

I like the idea to have two separate release branch for V1 and keep on main the V2 specs, however I guess that the alternative approach to have the v1 folder, as we have done for ICS20, can be more straightforward for readers to navigate and checkout the changes we will be doing.
At the moment, I would be even fine to have a v2 folder under all ICS that needs changes, where we place all the new eureka related specs. Then once we release eureka on ibc-go we could change the folder structure as we have done in ICS20, so creating the v1 folder with classic spec and have the main readme of the folder describing the v2 spec.

Any of the approaches works fine for me, let's just decide what we wanna do!

I would be even fine to have a v2 folder instead where we place all the new eureka related specs

This also sounds good! So we can switch when the v2 specs are mature enough.

sangier · 2024-08-20T07:27:51Z

Note that the approach I took here has been:

git mv spec/client/ics-007-tendermint-client/README.md spec/client/ics-007-tendermint-client/v2/

git checkout HEAD -- spec/client/ics-007-tendermint-client/README.md

git add .

git commit -m "Copied README.md to v2/ while preserving history"

However, to then see the full commit history for the v2/README.md we would need to run
git log --follow and the v2 files appears in the PR as a new files, so it is not easy to track the change to review. Mmm.

How do you see it?

sangier · 2024-08-21T12:36:59Z

@AdityaSripal Note that the failing CI check are relative to new v2 core folder, e.g. ../ics-023-vector-commitments should now be ../../ics-023-vector-commitments. I can fix this in another PR if we want to.

sangier · 2024-08-21T13:03:04Z

spec/client/ics-007-tendermint-client/README.md


 ```typescript
 interface ClientState {
  chainID: string
  trustLevel: Rational
  trustingPeriod: uint64
  unbondingPeriod: uint64
+  delayTimePeriod: uint64 


Shall we put an upper bound check for delayPeriods in the client initialisation to avoid misconfigurations?

AdityaSripal

Nice work!

AdityaSripal

Nice work!

* 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 requested review from AdityaSripal, angbrav, colin-axner and damiannolan as code owners August 19, 2024 14:24

AdityaSripal reviewed Aug 19, 2024

View reviewed changes

sangier mentioned this pull request Aug 20, 2024

Create v2 folder with placeholder document #1142

Closed

sangier changed the base branch from main to feat/v2-spec August 21, 2024 12:11

sangier requested a review from AdityaSripal August 21, 2024 12:34

sangier commented Aug 21, 2024

View reviewed changes

AdityaSripal approved these changes Aug 21, 2024

View reviewed changes

sangier added 15 commits August 21, 2024 16:06

deprecate delay period params

c2ca116

add params into clientState

dd898bc

fix comment

9ef0ab9

include PR link in history

86c6123

fixes

e038fb1

fix comment

a55be22

fix modified field

a601770

Copied README.md to v2/ while preserving history

16f0f63

Reverted README.md to commit 825d47d

643548d

fix deadlinks

6742a50

rm v1 deprecation references

7a689b3

del v2 folder and readme

6c8d492

cp v1 spec into TAO_V1_README

f3d06a9

apply v2 changes

5da76b8

fix dead links

a7f80c1

sangier force-pushed the stefano/tao-v2-support-in-07-client branch from fb0dd55 to a7f80c1 Compare August 21, 2024 14:15

fix deadlinks 2

445db7e

sangier added 4 commits August 21, 2024 16:56

fix image links

b298a65

fixes

9e3ebfe

fixes

35414c3

fix link to v2 version

6b60c74

AdityaSripal approved these changes Aug 21, 2024

View reviewed changes

AdityaSripal merged commit f8e7134 into feat/v2-spec Aug 21, 2024
2 checks passed

AdityaSripal deleted the stefano/tao-v2-support-in-07-client branch August 21, 2024 16:34

sangier mentioned this pull request Aug 22, 2024

Modify 07-tendermint client specifications in order to also support TAO v2 spec. #1135

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

TAO V2 support in 07 client #1137

TAO V2 support in 07 client #1137

Uh oh!

sangier commented Aug 19, 2024

Uh oh!

sangier commented Aug 19, 2024

Uh oh!

AdityaSripal left a comment

Uh oh!

AdityaSripal Aug 19, 2024

Uh oh!

sangier Aug 19, 2024

Uh oh!

crodriguezvega Aug 19, 2024

Uh oh!

sangier Aug 20, 2024 •

edited

Loading

Uh oh!

crodriguezvega Aug 20, 2024

Uh oh!

sangier commented Aug 20, 2024 •

edited

Loading

Uh oh!

sangier commented Aug 21, 2024

Uh oh!

sangier Aug 21, 2024

Uh oh!

AdityaSripal left a comment

Uh oh!

AdityaSripal left a comment

Uh oh!

Uh oh!

Uh oh!

TAO V2 support in 07 client #1137

TAO V2 support in 07 client #1137

Uh oh!

Conversation

sangier commented Aug 19, 2024

Uh oh!

sangier commented Aug 19, 2024

Uh oh!

AdityaSripal left a comment

Choose a reason for hiding this comment

Uh oh!

AdityaSripal Aug 19, 2024

Choose a reason for hiding this comment

Uh oh!

sangier Aug 19, 2024

Choose a reason for hiding this comment

Uh oh!

crodriguezvega Aug 19, 2024

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crodriguezvega Aug 20, 2024

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

sangier commented Aug 21, 2024

Uh oh!

sangier Aug 21, 2024

Choose a reason for hiding this comment

Uh oh!

AdityaSripal left a comment

Choose a reason for hiding this comment

Uh oh!

AdityaSripal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sangier Aug 20, 2024 •

edited

Loading

sangier commented Aug 20, 2024 •

edited

Loading