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.

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

Connection Handshake (ICS3) L2 TLA+ spec #58

Merged
merged 44 commits into from
May 21, 2020

Conversation

adizere
Copy link
Member

@adizere adizere commented Apr 21, 2020

WIP for : #4

Rendered version here: ch.tla.


For contributor use:

  • Unit tests written
  • Added test to CI if applicable
  • Updated CHANGELOG_PENDING.md
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments
  • Re-reviewed Files changed in the Github PR explorer

Copy link
Collaborator

@ancazamfir ancazamfir left a comment

Choose a reason for hiding this comment

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

Added some comments. I think we should discuss at this point.

verification/spec/connection-handshake/chm.tla Outdated Show resolved Hide resolved
verification/spec/connection-handshake/chm.tla Outdated Show resolved Hide resolved
verification/spec/connection-handshake/chm.tla Outdated Show resolved Hide resolved
verification/spec/connection-handshake/chm.tla Outdated Show resolved Hide resolved
verification/spec/connection-handshake/chm.tla Outdated Show resolved Hide resolved
verification/spec/connection-handshake/chm.tla Outdated Show resolved Hide resolved
***************************************************************************)
ConnectionEnds ==
[
connectionID : ConnectionIDs \union {Null},
Copy link
Contributor

@milosevic milosevic Apr 24, 2020

Choose a reason for hiding this comment

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

Is there a need having Null here as connectionEnd is never initialised with null fields? The same for clientIDs.

(***
* Initial value for a connection end.
**)
ConnectionEndInitValue ==
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably not needed.

(***
* Initial values for a connection.
**)
ConnectionInitValue ==
Copy link
Contributor

Choose a reason for hiding this comment

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

The same here.

CHModuleInitValue ==
[chainHeight |-> 1,
connectionState |-> "UNINT",
connection |-> ConnectionInitValue ]
Copy link
Contributor

Choose a reason for hiding this comment

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

We should just define NilConnection as operator and that should be initial value of the connection field.

connection : Connections]

NextEnv ==
\/ OnConnInitMsg
Copy link
Contributor

Choose a reason for hiding this comment

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

We can probably simplify this with inMsg' \in ConnectionHandshakeMessages.

*****************************************************************************)

Init ==
turn = "env" \* Initially, the environment takes a turn.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
turn = "env" \* Initially, the environment takes a turn.
/\ turn = "env" \* Initially, the environment takes a turn.

Copy link
Contributor

@milosevic milosevic left a comment

Choose a reason for hiding this comment

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

Great work!It turns out that modelling connection handshake with underlying mechanisms turns out not to be simple task. I would suggest to try to address comments and to merge this PR. Then we could consider as a follow up work to think about integration with relayer spec which would allow us to simplify some stuff in this spec that are posing us problems. In my view, all proof related business will be moved outside protocol spec and we can at the protocol level assume that we only receive messages that have passed light client related proof checks. Protocol will also not need to care about height business as it will be managed outside the protocols (by chain abstraction). Hopefully this will make a bit easier expressing what are the conditions in which we expect protocol to terminate and also it would allow us to point out the problems in the current design and help us verify potential protocol improvements.

@adizere adizere changed the title [WIP] Connection Handshake TLA Connection Handshake (ICS3) L2 TLA+ spec May 20, 2020
@adizere adizere merged commit fee17f8 into master May 21, 2020
@adizere adizere deleted the adi/connection_handshake_tla branch May 21, 2020 09:23
adizere added a commit that referenced this pull request May 22, 2020
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
…tion_handshake_tla

Connection Handshake (ICS3) L2 TLA+ spec
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants