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

Audit/sherlock audit v2 #247

Merged
merged 4 commits into from
Nov 5, 2021
Merged

Audit/sherlock audit v2 #247

merged 4 commits into from
Nov 5, 2021

Conversation

Alexangelj
Copy link
Contributor

@Alexangelj Alexangelj commented Nov 2, 2021

Changelog

  • Updates PrimitiveFactory to reference the stableDecimals if the error is thrown for the stableDecimals, rather than passing in the riskyDecimals
  • Updates natspec comments on create parameters to be inclusive of sigma boundaries, and outside of min liquidity range
  • Updates create's strike argument from a uint256 to a uint128. Removes the downcasting of it.
  • Updates the Create event arguments to match respective types
  • Gas optimization in SwapDetails struct, and variable packing the unlocked state variable with an immutable address
  • Updates natspec on MinLiquidityError, thrown when delLiquidity is less than or equal to MIN_LIQUIDITY

@SherlockProtocol
Copy link
Contributor

Hi @Alexangelj

The report stated in L-05

Besides, as the comment says, the MinLiquidityError error is thrown when the liquidity is lower than the minimum amount of liquidity. However, if delLiquidity equals MIN_LIQUIDITY, a MinLiquidityError error is thrown as well.

This finding doesn't seem to be addressed in this PR @ https://github.com/primitivefinance/primitive-v2-core/blob/audit/sherlock-audit-v2/contracts/PrimitiveEngine.sol#L167

@Alexangelj
Copy link
Contributor Author

Hi @Alexangelj

The report stated in L-05

Besides, as the comment says, the MinLiquidityError error is thrown when the liquidity is lower than the minimum amount of liquidity. However, if delLiquidity equals MIN_LIQUIDITY, a MinLiquidityError error is thrown as well.

This finding doesn't seem to be addressed in this PR @ https://github.com/primitivefinance/primitive-v2-core/blob/audit/sherlock-audit-v2/contracts/PrimitiveEngine.sol#L167

Ah, I updated the MIN_LIQUIDITY natspec to describe less than or equal to. So the delLiquidity must be greater than min liquidity. Else, it'd be possible to mint 0 zero liquidity on create calls if delLiquidity is equal to the min.

@SherlockProtocol SherlockProtocol merged commit f54c8b9 into main Nov 5, 2021
@Alexangelj
Copy link
Contributor Author

🎉 This PR is included in version 1.12.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Alexangelj Alexangelj deleted the audit/sherlock-audit-v2 branch November 30, 2021 20:41
Alexangelj pushed a commit that referenced this pull request Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants