Skip to content
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.

Sign up for GitHub

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

TAO V2 support in 07 client #1137

Merged
merged 20 commits into from
Aug 21, 2024
Merged

TAO V2 support in 07 client #1137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c2ca116
deprecate delay period params
sangier Aug 19, 2024
dd898bc
add params into clientState
sangier Aug 19, 2024
9ef0ab9
fix comment
sangier Aug 19, 2024
86c6123
include PR link in history
sangier Aug 19, 2024
e038fb1
fixes
sangier Aug 19, 2024
a55be22
fix comment
sangier Aug 19, 2024
a601770
fix modified field
sangier Aug 19, 2024
16f0f63
Copied README.md to v2/ while preserving history
sangier Aug 20, 2024
643548d
Reverted README.md to commit 825d47d79b9ebd40be26e8b637f697dc8b005ac4
sangier Aug 20, 2024
6742a50
fix deadlinks
sangier Aug 20, 2024
7a689b3
rm v1 deprecation references
sangier Aug 20, 2024
6c8d492
del v2 folder and readme
sangier Aug 21, 2024
f3d06a9
cp v1 spec into TAO_V1_README
sangier Aug 21, 2024
5da76b8
apply v2 changes
sangier Aug 21, 2024
a7f80c1
fix dead links
sangier Aug 21, 2024
445db7e
fix deadlinks 2
sangier Aug 21, 2024
b298a65
fix image links
sangier Aug 21, 2024
9e3ebfe
fixes
sangier Aug 21, 2024
35414c3
fixes
sangier Aug 21, 2024
6b60c74
fix link to v2 version
sangier Aug 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions spec/client/ics-007-tendermint-client/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ implements: 2
version compatibility: ibc-go v7.0.0
author: Christopher Goes <[email protected]>
created: 2019-12-10
modified: 2019-12-19
modified: 2024-08-19
---

## Synopsis
Expand Down Expand Up @@ -59,14 +59,16 @@ This specification depends on correct instantiation of the [Tendermint consensus

### Client state

The Tendermint client state tracks the current revision, current validator set, trusting period, unbonding period, latest height, latest timestamp (block time), and a possible frozen height.
The Tendermint client state tracks the current revision, current validator set, trusting period, unbonding period, delayTimePeriod, delayBlockPeriod, latest height, latest timestamp (block time), and a possible frozen height.

```typescript
interface ClientState {
chainID: string
trustLevel: Rational
trustingPeriod: uint64
unbondingPeriod: uint64
delayTimePeriod: uint64
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

delayBlockPeriod: uint64
latestHeight: Height
frozenHeight: Maybe<uint64>
upgradePath: []string
Expand Down Expand Up @@ -377,8 +379,6 @@ These functions utilise the `proofSpecs` with which the client was initialised.
function verifyMembership(
clientState: ClientState,
height: Height,
delayTimePeriod: uint64,
delayBlockPeriod: uint64,
proof: CommitmentProof,
path: CommitmentPath,
value: []byte
Expand All @@ -388,9 +388,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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@sangier sangier Aug 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// assert that enough blocks have elapsed
assert(currentHeight() >= processedHeight + delayPeriodBlocks)
assert(currentHeight() >= processedHeight + clientState.delayBlockPeriod)
// fetch the previously verified commitment root & verify membership
// Implementations may choose how to pass in the identifier
// ibc-go provides the identifier-prefixed store to this method
Expand All @@ -406,8 +406,6 @@ function verifyMembership(
function verifyNonMembership(
clientState: ClientState,
height: Height,
delayTimePeriod: uint64,
delayBlockPeriod: uint64,
proof: CommitmentProof,
path: CommitmentPath
): Error {
Expand All @@ -416,9 +414,9 @@ function verifyNonMembership(
// 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)
// assert that enough blocks have elapsed
assert(currentHeight() >= processedHeight + delayPeriodBlocks)
assert(currentHeight() >= processedHeight + clientState.delayBlockPeriod)
// fetch the previously verified commitment root & verify membership
// Implementations may choose how to pass in the identifier
// ibc-go provides the identifier-prefixed store to this method
Expand Down Expand Up @@ -452,8 +450,11 @@ Not applicable. Alterations to the client verification algorithm will require a
## History

December 10th, 2019 - Initial version

December 19th, 2019 - Final first draft

August 19th, 2024 - [Support for IBC/TAO V2](https://github.com/cosmos/ibc/pull/1137)

## Copyright

All content herein is licensed under [Apache 2.0](https://www.apache.org/licenses/LICENSE-2.0).
Loading