Yearn audit variable optimizations #119
Conversation
WalkthroughThe pull request introduces modifications to three Solidity files: Changes
Possibly related issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #119 +/- ##
==========================================
- Coverage 88.96% 88.90% -0.06%
==========================================
Files 5 5
Lines 607 604 -3
Branches 90 90
==========================================
- Hits 540 537 -3
Misses 56 56
Partials 11 11 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/RepoTokenList.sol(2 hunks)src/Strategy.sol(2 hunks)src/TermAuctionList.sol(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/RepoTokenList.sol
[warning] 357-357: src/RepoTokenList.sol#L357
Added line #L357 was not covered by tests
🔇 Additional comments (4)
src/RepoTokenList.sol (2)
350-351: Function signature change improves code clarity
The removal of discountRate from the return tuple simplifies the function interface since the rate is only used internally.
357-357:
Add test coverage for early return path
The early return path for matured repoTokens is not covered by tests. This is a critical validation path that should be tested.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 357-357: src/RepoTokenList.sol#L357
Added line #L357 was not covered by tests
src/TermAuctionList.sol (1)
190-190: LGTM: Correctly adapted to new function signature
The code properly handles the updated return values from validateAndInsertRepoToken, correctly unpacking only the boolean and timestamp values.
src/Strategy.sol (1)
1087-1088: LGTM: Correctly adapted to new function signature
The code properly handles the updated return values from validateAndInsertRepoToken, correctly unpacking only the boolean and timestamp values.
| @@ -374,20 +373,20 @@ | |||
| discountRate = rate == 0 ? ZERO_AUCTION_RATE : rate; | |||
| } catch { | |||
| discountRate = INVALID_AUCTION_RATE; | |||
| return (false, discountRate, redemptionTimestamp); | |||
| return (false, redemptionTimestamp); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consistent error handling pattern needed
The function has inconsistent error handling:
- Line 357: Returns false with redemptionTimestamp
- Line 376: Returns false with redemptionTimestamp
- Line 383: Returns false with redemptionTimestamp
- Line 389: Returns true with redemptionTimestamp
Consider using a more consistent pattern, either:
- Always revert with
InvalidRepoTokenfor invalid cases (commented out in line 357) - Return false with a specific error code/reason
Also applies to: 383-383, 389-389
Summary by CodeRabbit
New Features
Bug Fixes
Documentation