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

[LILA-6854] Address Team Omega comments, update emails and versions #56

Merged
merged 5 commits into from
Jun 10, 2024

Conversation

0xmichalis
Copy link
Member

@0xmichalis 0xmichalis commented Jun 10, 2024

@lirona
Copy link
Member

lirona commented Jun 10, 2024

there's no reason to check requestedAmount > 0 since it's a uint256, more efficient to check requestedAmount != 0
in _calculateFee
https://github.com/ToucanProtocol/dynamic-fee-pools/blob/9732dc6eb510876157c6a7c62511005417ffebe5/src/FeeCalculator.sol#L404C17-L404C36

Copy link
Member

@lirona lirona left a comment

Choose a reason for hiding this comment

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

i have 1 comment that wasn't in the audit, other than that lgtm

@0xmichalis
Copy link
Member Author

there's no reason to check requestedAmount > 0 since it's a uint256, more efficient to check requestedAmount != 0
in _calculateFee

This is true for all uint256 > 0 checks in the contracts. Updated

@0xmichalis 0xmichalis merged commit 028495a into ToucanProtocol:main Jun 10, 2024
1 check passed
@@ -2,7 +2,7 @@
//
// SPDX-License-Identifier: UNLICENSED

// If you encounter a vulnerability or an issue, please contact <info@neutralx.com>
// If you encounter a vulnerability or an issue, please contact <security@toucan.earth> or visit security.toucan.earth
Copy link
Member

Choose a reason for hiding this comment

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

https://security.toucan.earth/ would be better here.

Copy link
Member

@aspiers aspiers left a comment

Choose a reason for hiding this comment

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

Retro +1

@0xmichalis 0xmichalis deleted the audit-review branch June 10, 2024 10:25
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.

3 participants