-
Notifications
You must be signed in to change notification settings - Fork 359
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
ICS3 connection versioning / model checking in TLA+ #211
Conversation
…rs into ilina/ics3_versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! A few comments on the disclosure-log.md
Will check TLA+ later today
|
||
__Context__. | ||
The original issue triggering this discussion is here: [cosmos/ics/#459](https://github.com/cosmos/ics/issues/459). | ||
Briefly, version negotiation in the ICS3 handshake can interfere in various ways, breaking either the safety or liveness of this protocol. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current liveness issues are expected as the two parties do not agree on a version. Not sure if and how these should be reported. If there would be liveness issue due to protocol bugs (e.g. the two parties do not agree even if there is one common version) the format here would be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When it comes to liveness, this can suffer in two cases:
- due to empty version intersection, case (a),
- due to mode
onTryNonDet
, case (c)
The current liveness issues are expected as the two parties do not agree on a version. Not sure if and how these should be reported.
So this concerns case (a). My initial thought was to classify this as a hidden assumption, which we disclose here. We thus provide a clear reason to make this assumption explicit in the ICS.
If there would be liveness issue due to protocol bugs (e.g. the two parties do not agree even if there is one common version) the format here would be fine.
I agree this is not a bug and rather it's a case of underspecification, but I would say the format here is good for this in fact.
Latest version of disclosure log rendered here: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
) * connection handshake with versions, non-deterministic choice of version * added different version picking modes * added comments explaining version picking * connection spec fix * bug in Environment.tla * added overwrite mode * overwrite mode fixes * Update README.md * cleanup in connection handshake spec * fix in sending Ack message * Typos; cleaner Concurrency; added Utils; * Added a WIP marker for the disclosure log * Traces; WIP for ACK cases * Disclosure log ready for review * Added summary table cf. Anca's suggestion. Co-authored-by: istoilkovska <anili100@gmail.com>
Discussion ground for our work to close informalsystems/ibc-rs#117.
Will also close informalsystems/ibc-rs#116.
Description
For contributor use:
docs/
) and code commentsFiles changed
in the Github PR explorer