stricter fee check rounding

[mcclurejt]

Resolved Issues

• Fixes https://github.com/spearbit-audits/delv-review/issues/30

Description

The rounding for fees was previously done to increase the number of values for which the condition was false, however since a true condition results in revert, the rounding had the counterintuitive effect of actually making the check less strict.

By rounding in the opposite direction, we make the fee checks more strict.

Review Checklists

Please check each item before approving the pull request. While going through the checklist, it is recommended to leave comments on items that are referenced in the checklist to make sure that they are reviewed. If there are multiple reviewers, copy the checklists into sections titled ## [Reviewer Name]. If the PR doesn't touch Solidity and/or Rust, the corresponding checklist can be removed.

@jalextowle

Solidity

• [x] Tokens
  • [ ] Do all approve calls use forceApprove?
  • [ ] Do all transfer calls use safeTransfer?
  • [ ] Do all transferFrom calls use msg.sender as the from address?
    • [ ] If not, is the function access restricted to prevent unauthorized token spend?
• [x] Low-level calls (call, delegatecall, staticcall, transfer, send)
  • [ ] Is the returned success boolean checked to handle failed calls?
  • [ ] If using delegatecall, are there strict access controls on the addresses that can be called? It shouldn't be possible to delegatecall arbitrary addresses, so the list of possible targets should either be immutable or tightly controlled by an admin.
• [x] Reentrancy
  • [ ] Are functions that make external calls or transfer ether marked as nonReentrant?
    • [ ] If not, is there documentation that explains why reentrancy is not a concern or how it's mitigated?
• [x] Gas Optimizations
  • [ ] Is the logic as simple as possible?
  • [ ] Are the storage values that are used repeatedly cached in stack or memory variables?
  • [ ] If loops are used, are there guards in place to avoid out-of-gas issues?
• [x] Visibility
  • [ ] Are all payable functions restricted to avoid stuck ether?
• [x] Math
  • [ ] Is all of the arithmetic checked or guarded by if-statements that will catch underflows?
  • [ ] If Safe functions are altered, are potential underflows and overflows caught so that a failure flag can be thrown?
  • [ ] Are all of the rounding directions clearly documented?
• [x] Testing
  • [ ] Are there new or updated unit or integration tests?
  • [ ] Do the tests cover the happy paths?
  • [ ] Do the tests cover the unhappy paths?
  • [ ] Are there an adequate number of fuzz tests to ensure that we are covering the full input space?

@jrhea

Solidity

• [x] Tokens
  • [ ] Do all approve calls use forceApprove?
  • [ ] Do all transfer calls use safeTransfer?
  • [ ] Do all transferFrom calls use msg.sender as the from address?
    • [ ] If not, is the function access restricted to prevent unauthorized token spend?
• [x] Low-level calls (call, delegatecall, staticcall, transfer, send)
  • [ ] Is the returned success boolean checked to handle failed calls?
  • [ ] If using delegatecall, are there strict access controls on the addresses that can be called? It shouldn't be possible to delegatecall arbitrary addresses, so the list of possible targets should either be immutable or tightly controlled by an admin.
• [x] Reentrancy
  • [ ] Are functions that make external calls or transfer ether marked as nonReentrant?
    • [ ] If not, is there documentation that explains why reentrancy is not a concern or how it's mitigated?
• [x] Gas Optimizations
  • [ ] Is the logic as simple as possible?
  • [ ] Are the storage values that are used repeatedly cached in stack or memory variables?
  • [ ] If loops are used, are there guards in place to avoid out-of-gas issues?
• [x] Visibility
  • [ ] Are all payable functions restricted to avoid stuck ether?
• [x] Math
  • [ ] Is all of the arithmetic checked or guarded by if-statements that will catch underflows?
  • [ ] If Safe functions are altered, are potential underflows and overflows caught so that a failure flag can be thrown?
  • [ ] Are all of the rounding directions clearly documented?
• [x] Testing
  • [ ] Are there new or updated unit or integration tests?
  • [ ] Do the tests cover the happy paths?
  • [ ] Do the tests cover the unhappy paths?
  • [ ] Are there an adequate number of fuzz tests to ensure that we are covering the full input space?

[github-actions[bot]]

Hyperdrive Gas Benchmark

Benchmark suite	Current: a048083c462cd39f5dfbd0e352c90ee653b15771	Previous: 46c834d403754cb2c6c572563bccd65634c54aff	Deviation	Status
addLiquidity: min	33915 gas	1546 gas	2093.7257%	🚨
addLiquidity: avg	145061 gas	67936 gas	113.5260%	🚨
addLiquidity: max	405253 gas	293159 gas	38.2366%	🚨
checkpoint: min	29232 gas	1182 gas	2373.0964%	🚨
checkpoint: avg	104086 gas	48477 gas	114.7121%	🚨
checkpoint: max	211255 gas	191559 gas	10.2819%	🚨
closeLong: min	31428 gas	1492 gas	2006.4343%	🚨
closeLong: avg	132779 gas	29374 gas	352.0290%	🚨
closeLong: max	341737 gas	152303 gas	124.3797%	🚨
closeShort: min	31349 gas	1494 gas	1998.3266%	🚨
closeShort: avg	127813 gas	33444 gas	282.1702%	🚨
closeShort: max	201614 gas	149156 gas	35.1699%	🚨
initialize: min	31283 gas	1451 gas	2055.9614%	🚨
initialize: avg	252987 gas	213962 gas	18.2392%	🚨
initialize: max	322173 gas	253953 gas	26.8632%	🚨
openLong: min	33503 gas	1499 gas	2135.0233%	🚨
openLong: avg	166897 gas	51658 gas	223.0806%	🚨
openLong: max	252525 gas	185562 gas	36.0866%	🚨
openShort: min	33848 gas	1520 gas	2126.8421%	🚨
openShort: avg	169830 gas	51482 gas	229.8823%	🚨
openShort: max	363609 gas	181325 gas	100.5289%	🚨
redeemWithdrawalShares: min	31227 gas	1488 gas	1998.5887%	🚨
redeemWithdrawalShares: avg	63490 gas	22375 gas	183.7542%	🚨
redeemWithdrawalShares: max	160647 gas	109301 gas	46.9767%	🚨
removeLiquidity: min	31323 gas	1530 gas	1947.2549%	🚨
removeLiquidity: avg	219414 gas	151225 gas	45.0911%	🚨
removeLiquidity: max	376714 gas	325781 gas	15.6341%	🚨

This comment was automatically generated by workflow using github-action-benchmark.