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

FV for Safe v1.5 Audit #3

Merged
merged 27 commits into from
Jan 15, 2025
Merged

FV for Safe v1.5 Audit #3

merged 27 commits into from
Jan 15, 2025

Conversation

derek-certora
Copy link

Summary

This is a draft PR to make it easy to get code review on the new rules being written.

Files

The new spec and conf files are located in respective v1.5 files, in order to keep the new spec separate (for now) from the old spec.

Note

This is a rebased version of a the previous working branch, derek/audit, onto a more recent commit given by the customer.

Copy link

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


Derek Sorensen seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

Derek Sorensen added 2 commits December 16, 2024 13:51
…le guards; this commit ports a rule from Safe.spec so that rules on guards are in the same spec, adds some functionality to the Safe harness, and makes two mocks for the module and transaction guards.
@coveralls
Copy link

coveralls commented Dec 16, 2024

Pull Request Test Coverage Report for Build 12796391062

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.814%

Totals Coverage Status
Change from base Build 12350895163: 0.0%
Covered Lines: 495
Relevant Lines: 507

💛 - Coveralls

Derek Sorensen added 2 commits December 16, 2024 14:02
… an enabled module before making the successful call, and adds documentation that refers to an added function to the Safe Harness
@derek-certora
Copy link
Author

All the rules are passing now, this codebase also incorporates changes suggested by @nd-certora on the previous draft PR.

Derek Sorensen added 2 commits December 16, 2024 14:23
Copy link
Author

@derek-certora derek-certora Dec 16, 2024

Choose a reason for hiding this comment

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

The rule guardAddressChange was moved from Safe.spec to Guards.spec so that all the rules about guards are in one place.

Copy link

@nd-certora nd-certora left a comment

Choose a reason for hiding this comment

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

looks better

Derek Sorensen added 8 commits December 17, 2024 16:44
…anity on the invariant fallbackHanlderNeverSelf, adding the static Dummy to the spec, and strengthening from satisfy to assert
…n and txnGuardCalled are vioalted otherwise
…olve some CVL issues, filter the invariant fallbackHanlderNeverSelf for sanity, etc. overall improvement of rules for fallback
Copy link

@jhoenicke jhoenicke left a comment

Choose a reason for hiding this comment

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

Left some comments, but looks good overall.

@derek-certora derek-certora marked this pull request as ready for review January 15, 2025 20:23
@derek-certora derek-certora changed the title Audit Draft PR for Code Review FV for Safe v1.5 Audit Jan 15, 2025
@derek-certora
Copy link
Author

merging in preparation for PR to client

@derek-certora derek-certora merged commit a38212b into main Jan 15, 2025
16 of 25 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants