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

Natspec, Ontology, Interface cleanup #149

Merged
merged 15 commits into from
Dec 11, 2022

Conversation

0xAlcibiades
Copy link
Member

@0xAlcibiades 0xAlcibiades commented Dec 10, 2022

This PR cleans up the natspec, removes duplicate custom errors, hides some internal implemenation details around token ids, reduces the surface of the interface. It also bumps the compiler to 0.8.16 as per slither recommendation.

@0xAlcibiades 0xAlcibiades marked this pull request as draft December 10, 2022 23:15
@codecov-commenter
Copy link

codecov-commenter commented Dec 10, 2022

Codecov Report

Merging #149 (53d0c71) into audit-fixes (1b9ed25) will increase coverage by 0.24%.
The diff coverage is 92.85%.

@@               Coverage Diff               @@
##           audit-fixes     #149      +/-   ##
===============================================
+ Coverage        88.70%   88.94%   +0.24%     
===============================================
  Files                2        2              
  Lines              363      371       +8     
  Branches            53       56       +3     
===============================================
+ Hits               322      330       +8     
  Misses              31       31              
  Partials            10       10              
Impacted Files Coverage Δ
src/TokenURIGenerator.sol 71.09% <ø> (ø)
src/OptionSettlementEngine.sol 98.35% <92.85%> (+0.05%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@0xAlcibiades 0xAlcibiades changed the title WIP: Natspec, Ontology, Interface cleanup Natspec, Ontology, Interface cleanup Dec 11, 2022
@0xAlcibiades 0xAlcibiades marked this pull request as ready for review December 11, 2022 00:33
Copy link

@nickadamson nickadamson left a comment

Choose a reason for hiding this comment

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

🙏 thanks

Copy link
Contributor

@neodaoist neodaoist left a comment

Choose a reason for hiding this comment

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

Great stuff. I have some changes requested, all nits ofc but that's the point ;) Approving so I don't block

src/OptionSettlementEngine.sol Show resolved Hide resolved
src/OptionSettlementEngine.sol Show resolved Hide resolved
src/OptionSettlementEngine.sol Show resolved Hide resolved
src/OptionSettlementEngine.sol Show resolved Hide resolved
src/OptionSettlementEngine.sol Outdated Show resolved Hide resolved
test/OptionSettlementEngine.t.sol Outdated Show resolved Hide resolved
test/OptionSettlementEngine.t.sol Outdated Show resolved Hide resolved
test/OptionSettlementEngine.t.sol Show resolved Hide resolved
test/OptionSettlementEngine.t.sol Show resolved Hide resolved
test/OptionSettlementEngine.t.sol Show resolved Hide resolved
@0xAlcibiades 0xAlcibiades merged commit a78ca48 into audit-fixes Dec 11, 2022
@0xAlcibiades 0xAlcibiades deleted the chore/errors_natspec_terms branch December 11, 2022 01:31
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.

4 participants