-
Notifications
You must be signed in to change notification settings - Fork 48
Re routing example #180
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
Re routing example #180
Conversation
| /** | ||
| * @notice Rule to verify data integrity during storage pointer operations | ||
| */ | ||
| rule testDataIntegrity(env e) { |
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.
what's interesting in this rule?
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 am not sure what our policy for the Certora Examples here is, but this rule seems to be a sanity check that re-routing summaries doesn't break the data, while it's good to have, my opinion is that this should rather be a test in EVMVerifier than living here in examples, as it is just unnecessary noise for the example. @yoav-el-certora what do you think?
If you agree, I'll remove the test.
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.
Not sure what was the motivation for writing this example.
If you both agree, we can probably remove it.
| "Current data length should match new data length"; | ||
|
|
||
| // Verify operation counter was incremented | ||
| assert operationCounter() == initialCounter + 1, |
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 this will fail, right?
| * @notice Invariant to ensure operation counter only increases | ||
| */ | ||
| invariant operationCounterMonotonic() | ||
| operationCounter() >= 0 |
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 with the summary it will not increase, right?
|
|
||
| ## Problem Statement | ||
|
|
||
| In traditional CVL specifications, internal functions that accept storage pointers (especially mappings) are difficult to summarize because: |
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 traditional CVL specifications, internal functions that accept storage pointers (especially mappings) are difficult to summarize because: | |
| Internal functions that accept storage pointers (especially mappings) can not be summarized to a CVL function that uses the arguments |
|
Assigning @johspaeth (or someone from his team) to complete and merge this Example. |
johspaeth
left a comment
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.
From my side, this is good to go modulo deletion of the noise that I will take care of when Yoav agrees.
@nd-certora can you re-review?
| /** | ||
| * @notice Rule to verify data integrity during storage pointer operations | ||
| */ | ||
| rule testDataIntegrity(env e) { |
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 am not sure what our policy for the Certora Examples here is, but this rule seems to be a sanity check that re-routing summaries doesn't break the data, while it's good to have, my opinion is that this should rather be a test in EVMVerifier than living here in examples, as it is just unnecessary noise for the example. @yoav-el-certora what do you think?
If you agree, I'll remove the test.
| /** | ||
| * @notice Invariant to ensure operation counter only increases | ||
| */ | ||
| invariant operationCounterMonotonic() | ||
| operationCounter() >= 0 | ||
| { | ||
| preserved { | ||
| require operationCounter() <= max_uint256 - 1; | ||
| } | ||
| } |
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 invariant is excluded from verification in .conf and I don't see it's value. I removed it to reduce the noise here.
# Conflicts: # CVLByExample/README.md
* Add an example on how to reason about events in CVL (#174) * 7.26.0 Release (#163) * Niv/cert 8248 revert example (#155) * CERT 8248 Add Revert Example * Update README * Address Christiane review * Update example based on Nurit Review * Clean * Update config.yml (#160) Co-authored-by: yoav-el-certora <122207807+yoav-el-certora@users.noreply.github.com> * Example ready * christiane cr * Code reviews --------- Co-authored-by: Niv vaknin <122722245+nivcertora@users.noreply.github.com> Co-authored-by: liav-certora <114004726+liav-certora@users.noreply.github.com> Co-authored-by: liav-certora <liav@certora.com> * Bug fixed * Add an example on how to reason about events in CVL * Addressing code reviews * Addressing Christiane's CR * Issue with merge resolution * Revert "Bug fixed" This reverts commit 33b87b9. * Reverse incorrect change after merge --------- Co-authored-by: yoav-el-certora <122207807+yoav-el-certora@users.noreply.github.com> Co-authored-by: Niv vaknin <122722245+nivcertora@users.noreply.github.com> Co-authored-by: liav-certora <114004726+liav-certora@users.noreply.github.com> Co-authored-by: liav-certora <liav@certora.com> Co-authored-by: Otakar <Otakar@certora.com> * Removed Process from conf file * Removed send_only from conf file * add a simple example with a transient field and hooks and direct storage accesses on it * remove rule sanity Co-authored-by: Johannes Späth <johspaeth@users.noreply.github.com> * remove ambiguity * CERT 8687 Realistic example (#167) * CERT 8687 Realistic example * fix spec, sanity issue * fix * require invariant example * Update README * Update CVLByExample/Invariant/RequireInvariantArray/README.md * Addressing Nurit's CR --------- Co-authored-by: Nurit Dor <57101353+nd-certora@users.noreply.github.com> Co-authored-by: Johannes Späth <johspaeth@users.noreply.github.com> Co-authored-by: Johannes Spaeth <johannes@certora.com> * Re routing example (#180) * Re routing example * Updating example --------- Co-authored-by: Johannes Spaeth <johannes@certora.com> * Niv/fix sanity failure (#183) * Fix some of the sanity failure * Fixing specification to not have sanity failures runFullPool.conf -> https://vaas-stg.certora.com/output/53900/a7ab7f221da84eb4accea1c95e936803?anonymousKey=e063b8e60bae944b4a9d0d0431999b5bd4b578ea runBroken.con -> https://vaas-stg.certora.com/output/53900/c85b8c49fec24d32ac0de00043d7ddd6?anonymousKey=a6c585f91d010a7c13f1c6d8935143b894a0c05d * Updates to spec * Self-Review --------- Co-authored-by: Johannes Spaeth <johannes@certora.com> Co-authored-by: yoav-el-certora <122207807+yoav-el-certora@users.noreply.github.com> * Added fixes to breaking changes (#190) * foundry toml * comment * branch * branch * branch * Add an example for internal function calls (#191) * add an example for internal function calls * Update CVLByExample/InternalFunctionsFromCVL/README.md Co-authored-by: Johannes Späth <johspaeth@users.noreply.github.com> --------- Co-authored-by: Johannes Späth <johspaeth@users.noreply.github.com> * Added foundry installation to CI (#194) --------- Co-authored-by: Johannes Späth <johspaeth@users.noreply.github.com> Co-authored-by: Niv vaknin <122722245+nivcertora@users.noreply.github.com> Co-authored-by: liav-certora <114004726+liav-certora@users.noreply.github.com> Co-authored-by: liav-certora <liav@certora.com> Co-authored-by: Otakar <Otakar@certora.com> Co-authored-by: Christiane Goltz <christiane@certora.com> Co-authored-by: Nurit Dor <57101353+nd-certora@users.noreply.github.com> Co-authored-by: Johannes Spaeth <johannes@certora.com> Co-authored-by: rahav <rahav@certora.com> Co-authored-by: Rahav <103361134+rahav-certora@users.noreply.github.com> Co-authored-by: Naftali Goldstein <44599898+naftali-g@users.noreply.github.com>
No description provided.