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

Fixed and More Consistent Pragmas #458

Merged
merged 3 commits into from
Jul 5, 2024
Merged

Conversation

nlordell
Copy link
Collaborator

@nlordell nlordell commented Jul 4, 2024

Fixes hats-finance#17

This PR changes the pragmas in the passkeys contract to be:

  1. More consistent: we use ^0.8.20 everywhere for "non-top-level" contracts. The version was chosen as this it the first version that supports all of the compiler features that we make use of. Notably, we have @custom: NatSpec items which are only officially supported as of v0.8.20 (incidentally, this is the same version used by OpenZeppelin contracts for similar reason: source). The choice to use floating pragmas here is to make using these support contracts easier across other projects (namely, the WebAuthn and P256 libraries are useful outside of this project). Furthermore, we use ^ versions so that breaking changes introduced in a potential Solidity v0.9 would not affect the security and integrity of the contract code.
  2. Use fixed pragmas for "top-level" contracts that we deploy:
    • SafeWebAuthnProxyFactory
    • SafeWebAuthnSharedSigner
    • FCLP256Verifier

I am aware that Solidity v0.8.20 doesn't play nice with chains like BNB by default, however this can be worked around by explicitly by setting the EVM version target (as we do in this project) and do not believe this is an issue.

This PR changes the pragmas in the passkeys contract to be:
1. More consistent: we use `^0.8.20` everywhere for "non-top-level"
   contracts. The version was chosen as this it the first version that
   supports all of the compiler features that we make use of. Notably,
   we have `@custom:` NatSpec items which are only officially supported
   as of v0.8.20. The choice to use floating pragmas here is to make
   using these support contracts easier across other projects (namely,
   the `WebAuthn` and `P256` libraries are useful outside of this
   project).
2. Use fixed pragmas for "top-level" contracts that we deploy:
    - `SafeWebAuthnProxyFactory`
    - `SafeWebAuthnSharedSigner`
    - `FCLP256Verifier`

I am aware that Solidity v0.8.20 doesn't play nice with chains like BNB
by default, however this can be worked around by explicitly by setting
the EVM version target (as we do in this project) and do not believe
this is an issue.
@nlordell nlordell requested a review from a team as a code owner July 4, 2024 15:39
@nlordell nlordell requested review from akshay-ap, mmv08 and remedcu and removed request for a team July 4, 2024 15:39
@coveralls
Copy link

coveralls commented Jul 5, 2024

Pull Request Test Coverage Report for Build 9806169607

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 100.0%

Totals Coverage Status
Change from base Build 9806149214: 0.0%
Covered Lines: 39
Relevant Lines: 39

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jul 5, 2024

Pull Request Test Coverage Report for Build 9806169608

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 100.0%

Totals Coverage Status
Change from base Build 9806149214: 0.0%
Covered Lines: 90
Relevant Lines: 90

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jul 5, 2024

Pull Request Test Coverage Report for Build 9806232134

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 100.0%

Totals Coverage Status
Change from base Build 9806149214: 0.0%
Covered Lines: 90
Relevant Lines: 90

💛 - Coveralls

@nlordell nlordell merged commit bb5e1f7 into main Jul 5, 2024
10 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 2024
@nlordell nlordell deleted the hats/17-consitent-pragmas branch July 5, 2024 10:52
@nlordell nlordell restored the hats/17-consitent-pragmas branch July 5, 2024 10:52
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.

Usage of floating pragma
3 participants