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

Lemmas review #71

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Lemmas review #71

wants to merge 6 commits into from

Conversation

asymmetric
Copy link
Contributor

@asymmetric asymmetric commented Apr 30, 2020

I've been very trigger happy here, sometimes removing things I'm not 100% sure about, so we can have a discussion around the diff.

@asymmetric asymmetric requested a review from d-xo April 30, 2020 17:13

rule 75506153327051474587906755573858019282972751592871715030499431892688993766217 ==K keccakIntList(A B) => false
requires A =/=Int 0

rule 78338746147236970124700731725183845421594913511827187288591969170390706184117 ==K keccakIntList(A B) => false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is this number?

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 the slot for balanceOf[0]. both the two lines you deleted above are required afaik. They flip the order in the requires. I really had to add all four rules here or I would sometimes get stupid non deterministic failures.

@@ -75,20 +75,6 @@ library to the logical `#rangeUInt` conditions expressed within the specs.
rule A -Word B <=Int A => #rangeUInt(256, A -Int B)
requires #rangeUInt(256, A)
andBool #rangeUInt(256, B)

// add
rule (chop(X +Int Y) >=Int X) => #rangeUInt(256, X +Int Y)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we have iff in range uint256 \ X + Y, there would be no chop by the time we get to the add.

Is this here for the fail spec?

Copy link
Contributor

Choose a reason for hiding this comment

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

can't remember where these came from, they all turned up in the debugger at some point iirc.

@asymmetric
Copy link
Contributor Author

See also #63.

The base case is sufficient.
By removing extra boolean check.
The terms that end up in add/sub/mul are constrained in iff blocks, so
there's no chop.
Copy link
Contributor

@d-xo d-xo left a comment

Choose a reason for hiding this comment

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

doc changes are all 👍 ✨

Apart from that I don't think we should touch any existing lemma at this point unless we believe it to be actually wrong.

@@ -75,20 +75,6 @@ library to the logical `#rangeUInt` conditions expressed within the specs.
rule A -Word B <=Int A => #rangeUInt(256, A -Int B)
requires #rangeUInt(256, A)
andBool #rangeUInt(256, B)

// add
rule (chop(X +Int Y) >=Int X) => #rangeUInt(256, X +Int Y)
Copy link
Contributor

Choose a reason for hiding this comment

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

can't remember where these came from, they all turned up in the debugger at some point iirc.


rule 75506153327051474587906755573858019282972751592871715030499431892688993766217 ==K keccakIntList(A B) => false
requires A =/=Int 0

rule 78338746147236970124700731725183845421594913511827187288591969170390706184117 ==K keccakIntList(A B) => false
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 the slot for balanceOf[0]. both the two lines you deleted above are required afaik. They flip the order in the requires. I really had to add all four rules here or I would sometimes get stupid non deterministic failures.

@asymmetric
Copy link
Contributor Author

Non-deterministic failures suck, yes. Otherwise I'd be fine with cosmetic cleanups, as long as everything is green.

@asymmetric
Copy link
Contributor Author

They are obviously low-prio.

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.

2 participants