Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
minimal TLA+ spec of the xycloans monitor #149
minimal TLA+ spec of the xycloans monitor #149
Changes from 4 commits
e48b5f4
6301bee
944b58f
1198c40
7488335
ac9ca20
e0dfae5
5b2e9e1
f963df8
9d6eb5c
a7495af
7b3d551
4dfd771
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
So are we tracking storage twice?
Once here and once inside
last_tx
?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.
Yeah, my idea was that
last_tx
contains only partial storage updates like in Soroban, andstorage
contains the full storage. However, this did not work well so far.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.
Aaah, makes sense. Let's see where this goes
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.
I don't like that I have to substitute
a
in my brain. We should find a better name.Also, repeating it is confusing, as it refers to different addresses.
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.
Looking at it a second time, it feels a bit weird to have such a deeply-nested record in TLA+ only to emulate the Soroban API.
In the end, you're only using
call
andstatus
in the state machine, which could be lifted into TLA+ variables?env.storage
is already tracked in a state variable, which leaves onlyenv.current_contract_address
.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.
In general,
env
may contain other things such as the timestamp. So I would prefer to haveenv
to be clearly separated from the other things. Do you like to introduceenv
,call
, andstatus
to be three parameters instead oftx
? They are all essential to a monitor.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.
Let's keep it as is and iterate from there