TAO V2 support in 07 client #1137
Conversation
sangier
requested review from
AdityaSripal,
angbrav,
colin-axner and
damiannolan
as code owners
|
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. |
| @@ -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) | |||
There was a problem hiding this comment.
Hmmm I'm wondering how we should handle this since they will use DEPRECATED field for v1 implementations
There was a problem hiding this comment.
Mmm, what If we do something like:
// assert(currentTimestamp() >= processedTime + delayPeriodTime) // DEPRECATED - USED in TAO V1 assert(currentTimestamp() >= processedTime + clientState.delayTimePeriod) // TAO V2 SUPPORT`
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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.
|
Note that the approach I took here has been:
However, to then see the full commit history for the v2/README.md we would need to run How do you see it? |
|
@AdityaSripal Note that the failing CI check are relative to new v2 core folder, e.g. |
|
|
||
| ```typescript | ||
| interface ClientState { | ||
| chainID: string | ||
| trustLevel: Rational | ||
| trustingPeriod: uint64 | ||
| unbondingPeriod: uint64 | ||
| delayTimePeriod: uint64 |
There was a problem hiding this comment.
Shall we put an upper bound check for delayPeriods in the client initialisation to avoid misconfigurations?
sangier
force-pushed
the
stefano/tao-v2-support-in-07-client
branch
from
fb0dd55
to
a7f80c1
Compare
Closes #1135