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

Adding repo concentration limit #5

Merged
merged 9 commits into from
Aug 15, 2024
Merged

Conversation

0xddong
Copy link
Collaborator

@0xddong 0xddong commented Jul 25, 2024

Summary by CodeRabbit

  • New Features

    • Introduced a discount rate mechanism for evaluating repo tokens, replacing auction-based logic.
    • Added pausing functionality in the Strategy contract for enhanced security during critical operations.
    • Enhanced event emissions to track state changes such as concentration limits and contract pausing.
  • Bug Fixes

    • Improved error handling during concentration limit tests to ensure proper enforcement of management roles.
  • Chores

    • Updated initialization logic to integrate the new discount rate adapter, enhancing financial operations.

Copy link

coderabbitai bot commented Jul 25, 2024

Warning

Rate limit exceeded

@0xddong has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 43 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between c8c52cb and 801f439.

Walkthrough

The recent changes significantly enhance the functionality and security of the repo token management system by introducing a discount rate mechanism and improved control features. The integration of the ITermDiscountRateAdapter allows for dynamic retrieval of discount rates, replacing the previous auction-based logic. Additionally, the system now supports pausing and concentration limits for repo tokens, fostering a more robust and flexible framework for managing transactions.

Changes

Files Change Summary
src/.../RepoTokenList.sol, src/.../Strategy.sol, src/.../TermDiscountRateAdapter.sol Enhanced functionality by integrating a discount rate adapter, removing auction rate retrieval, and updating method signatures.
src/.../TermVaultEventEmitter.sol, src/interfaces/.../ITermVaultEvents.sol Added functions and events for emitting notifications related to state changes, improving operational communication.
src/interfaces/.../ITermDiscountRateAdapter.sol Introduced a new interface for defining the discount rate retrieval method.
src/test/.../TestUSDCSellRepoToken.t.sol, src/test/.../TestUSDCSubmitOffer.t.sol Enhanced testing capabilities for repo token management, focusing on concentration limits and the pausing mechanism.
src/test/utils/.../Setup.sol Integrated discount rate adapter into the setup process for improved strategy initialization.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Strategy
    participant TermDiscountRateAdapter
    participant RepoTokenList

    User->>Strategy: Request to validate repo token
    Strategy->>RepoTokenList: validateRepoToken(repoToken, discountRateAdapter)
    RepoTokenList->>TermDiscountRateAdapter: getDiscountRate(repoToken)
    TermDiscountRateAdapter-->>RepoTokenList: discountRate
    RepoTokenList-->>Strategy: validationResult
    Strategy-->>User: validation response
Loading

🐰 In fields so wide and bright,
We’ve made our tokens light.
With rates that shift like breeze,
And limits set with ease.
Let’s pause and hop with cheer,
For change has brought us here!
🌟


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@0xddong 0xddong marked this pull request as ready for review July 25, 2024 21:00
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between bb351bf and 9ff455c.

Files selected for processing (10)
  • src/RepoTokenList.sol (6 hunks)
  • src/Strategy.sol (15 hunks)
  • src/TermAuctionList.sol (6 hunks)
  • src/TermDiscountRateAdapter.sol (1 hunks)
  • src/TermVaultEventEmitter.sol (1 hunks)
  • src/interfaces/term/ITermDiscountRateAdapter.sol (1 hunks)
  • src/interfaces/term/ITermVaultEvents.sol (2 hunks)
  • src/test/TestUSDCOffers.t.sol (1 hunks)
  • src/test/TestUSDCSellRepoToken.t.sol (4 hunks)
  • src/test/utils/Setup.sol (4 hunks)
Additional comments not posted (41)
src/interfaces/term/ITermDiscountRateAdapter.sol (1)

1-6: LGTM!

The interface definition is correct and adheres to best practices.

src/TermDiscountRateAdapter.sol (2)

8-13: LGTM!

The constructor correctly initializes the TERM_CONTROLLER state variable.


15-22: Ensure proper error handling and input validation.

The getDiscountRate function appears correct, but ensure that TERM_CONTROLLER.getTermAuctionResults handles errors and that ITermRepoToken(repoToken).termRepoId() is validated.

src/interfaces/term/ITermVaultEvents.sol (8)

15-15: LGTM!

The event declaration for RepoTokenConcentrationLimitUpdated is correct.


17-17: LGTM!

The event declaration for Paused is correct.


19-19: LGTM!

The event declaration for Unpaused is correct.


21-24: LGTM!

The event declaration for DiscountRateAdapterUpdated is correct.


36-36: LGTM!

The function signature for emitRepoTokenConcentrationLimitUpdated is correct.


38-38: LGTM!

The function signature for emitPaused is correct.


40-40: LGTM!

The function signature for emitUnpaused is correct.


42-45: LGTM!

The function signature for emitDiscountRateAdapterUpdated is correct.

src/TermVaultEventEmitter.sol (4)

58-60: LGTM! Ensure the event is emitted correctly.

The function correctly emits the RepoTokenConcentrationLimitUpdated event and enforces role-based access control.


62-64: LGTM! Ensure the event is emitted correctly.

The function correctly emits the Paused event and enforces role-based access control.


66-68: LGTM! Ensure the event is emitted correctly.

The function correctly emits the Unpaused event and enforces role-based access control.


70-75: LGTM! Ensure the event is emitted correctly.

The function correctly emits the DiscountRateAdapterUpdated event and enforces role-based access control.

src/test/utils/Setup.sol (2)

72-72: LGTM! Ensure the variable is initialized and used properly.

The addition of the discountRateAdapter variable appears correct.


122-128: LGTM! Ensure the adapter is properly initialized and passed.

The modification to include the discountRateAdapter in the constructor call appears correct.

src/TermAuctionList.sol (2)

139-141: LGTM! Ensure the adapter is properly passed and used.

The modification to include the discountRateAdapter parameter in the validateAndInsertRepoToken function appears correct.


Line range hint 181-212:
LGTM! Ensure the adapter is properly passed and used.

The modification to include the discountRateAdapter parameter in the getPresentValue function appears correct.

src/test/TestUSDCOffers.t.sol (1)

39-39: Verify test coverage for the new functionality.

Ensure that the new functionality to set the repo token concentration limit is adequately tested in the test suite.

src/RepoTokenList.sol (3)

8-8: LGTM!

The new import statement for ITermDiscountRateAdapter is necessary for the new discount rate mechanism.


204-205: Verify the correctness and test coverage of the new logic.

Ensure that the new logic using discountRateAdapter to get the discount rate is correctly implemented and adequately tested.


275-276: Verify the correctness and test coverage of the new parameter.

Ensure that the new repoTokenToMatch parameter is correctly utilized and adequately tested.

src/test/TestUSDCSellRepoToken.t.sol (3)

165-172: LGTM!

The new private function _sell1RepoTokenNoMintExpectRevert is well-structured and aligns with the testing objectives.


523-546: LGTM!

The new test function testConcentrationLimitFailure is comprehensive and includes scenarios for both management and non-management roles.


548-571: LGTM!

The new test function testPausing ensures that only management can pause the strategy and that deposits are prevented when the strategy is paused.

src/Strategy.sol (15)

7-8: LGTM! New imports are necessary for added functionalities.

The imports of ReentrancyGuard, Pausable, and ITermDiscountRateAdapter are appropriate for the new features introduced in the contract.

Also applies to: 15-15


34-34: LGTM! Extending Pausable and ReentrancyGuard is appropriate.

The class declaration now extends Pausable and ReentrancyGuard, which is necessary for the new functionalities.


43-43: LGTM! New error definition is appropriate.

The RepoTokenConcentrationTooHigh error is necessary for enforcing the repo token concentration limit.


50-50: LGTM! New state variables are necessary.

The state variables discountRateAdapter and repoTokenConcentrationLimit are appropriate for the new functionalities.

Also applies to: 56-57


64-67: LGTM! The pause function is correctly implemented.

The pause function calls _pause and emits a Paused event, which is appropriate for pausing the contract.


69-72: LGTM! The unpause function is correctly implemented.

The unpause function calls _unpause and emits an Unpaused event, which is appropriate for unpausing the contract.


101-106: LGTM! The setRepoTokenConcentrationLimit function is correctly implemented.

The function updates the repoTokenConcentrationLimit and emits a RepoTokenConcentrationLimitUpdated event, which is appropriate for setting the repo token concentration limit.


108-111: LGTM! The setDiscountRateAdapter function is correctly implemented.

The function updates the discountRateAdapter and emits a DiscountRateAdapterUpdated event, which is appropriate for setting the discount rate adapter.


213-213: LGTM! The _sweepAssetAndRedeemRepoTokens function is correctly updated.

The function now includes the discountRateAdapter in its parameters, which is appropriate for the new functionalities.


Line range hint 236-279:
LGTM! The sellRepoToken function is correctly updated.

The function now includes whenNotPaused and nonReentrant modifiers and incorporates the discountRateAdapter, which is appropriate for the new functionalities.


287-305: LGTM! The _validateRepoTokenConcentration function is correctly implemented.

The function validates the repo token concentration against the limit, which is appropriate for enforcing the repo token concentration limit.


537-537: LGTM! The _deployFunds function is correctly updated.

The function now includes the whenNotPaused modifier, which is appropriate for the new functionalities.


562-562: LGTM! The _freeFunds function is correctly updated.

The function now includes the whenNotPaused modifier, which is appropriate for the new functionalities.


591-591: LGTM! The _harvestAndReport function is correctly updated.

The function now includes the whenNotPaused modifier, which is appropriate for the new functionalities.


510-518: LGTM! The constructor is correctly updated.

The constructor now accepts an address for the discountRateAdapter, which is appropriate for the new functionalities.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9ff455c and 2ef255c.

Files selected for processing (1)
  • src/test/utils/Setup.sol (4 hunks)
Additional comments not posted (4)
src/test/utils/Setup.sol (4)

8-8: Import statement looks good.

The import statement for TermDiscountRateAdapter is necessary and correctly added.


72-72: Internal variable addition looks good.

The internal variable discountRateAdapter is necessary and correctly added.


98-98: Modifications to setUp function look good.

The instantiation of discountRateAdapter with the address of termController is necessary and correctly implemented.


122-130: Modifications to setUpStrategy function look good.

The inclusion of discountRateAdapter as a parameter in the Strategy constructor is necessary and correctly implemented.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Outside diff range, codebase verification and nitpick comments (2)
src/interfaces/term/ITermDiscountRateAdapter.sol (1)

1-6: LGTM! Consider adding function descriptions.

The interface is well-defined. Adding descriptions for the function can improve readability and maintainability.

+/**
+ * @dev Returns the discount rate for the specified repo token.
+ * @param repoToken The address of the repo token.
+ * @return The discount rate as a uint256.
+ */
src/interfaces/term/ITermVaultEvents.sol (1)

Line range hint 15-45:
LGTM! Consider adding descriptions for the new events and functions.

The new events and functions are well-defined. Adding descriptions can improve readability and maintainability.

+/**
+ * @dev Emitted when the repo token concentration limit is updated.
+ * @param oldLimit The old concentration limit.
+ * @param newLimit The new concentration limit.
+ */
+/**
+ * @dev Emitted when the contract is paused.
+ */
+/**
+ * @dev Emitted when the contract is unpaused.
+ */
+/**
+ * @dev Emitted when the discount rate adapter is updated.
+ * @param oldAdapter The address of the old adapter.
+ * @param newAdapter The address of the new adapter.
+ */
+/**
+ * @dev Emits the RepoTokenConcentrationLimitUpdated event.
+ * @param oldLimit The old concentration limit.
+ * @param newLimit The new concentration limit.
+ */
+/**
+ * @dev Emits the Paused event.
+ */
+/**
+ * @dev Emits the Unpaused event.
+ */
+/**
+ * @dev Emits the DiscountRateAdapterUpdated event.
+ * @param oldAdapter The address of the old adapter.
+ * @param newAdapter The address of the new adapter.
+ */
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2ef255c and 01b9653.

Files selected for processing (12)
  • src/RepoTokenList.sol (6 hunks)
  • src/Strategy.sol (16 hunks)
  • src/TermAuctionList.sol (6 hunks)
  • src/TermDiscountRateAdapter.sol (1 hunks)
  • src/TermVaultEventEmitter.sol (1 hunks)
  • src/interfaces/term/ITermDiscountRateAdapter.sol (1 hunks)
  • src/interfaces/term/ITermVaultEvents.sol (2 hunks)
  • src/test/TestUSDCOffers.t.sol (1 hunks)
  • src/test/TestUSDCSellRepoToken.t.sol (4 hunks)
  • src/test/TestUSDCSubmitOffer.t.sol (1 hunks)
  • src/test/mocks/MockTermAuctionOfferLocker.sol (1 hunks)
  • src/test/utils/Setup.sol (5 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/test/utils/Setup.sol
Additional comments not posted (31)
src/TermDiscountRateAdapter.sol (1)

1-23: LGTM! Consider adding function descriptions.

The contract is well-implemented. Adding descriptions for the functions can improve readability and maintainability.

+/**
+ * @dev Constructor that sets the term controller address.
+ * @param termController_ The address of the term controller.
+ */
+/**
+ * @dev Returns the discount rate for the specified repo token.
+ * @param repoToken The address of the repo token.
+ * @return The discount rate as a uint256.
+ */

Verify the function usage in the codebase.

Ensure that all function calls to getDiscountRate match the new implementation.

src/test/TestUSDCSubmitOffer.t.sol (2)

17-26: Ensure proper initialization of mocks.

The setup code initializes several mock contracts. Verify that all necessary components are correctly initialized and consider adding more detailed comments for clarity.


28-40: Verify test logic and assertions.

The testSubmitOffer function appears to test the submission of an offer. Ensure that all relevant assertions are included to verify the expected behavior.

src/TermVaultEventEmitter.sol (4)

58-60: LGTM! Ensure proper event emission.

The emitRepoTokenConcentrationLimitUpdated function correctly emits the event. Ensure that the event is properly handled in the consuming code.


62-64: LGTM! Ensure proper event emission.

The emitPaused function correctly emits the event. Ensure that the event is properly handled in the consuming code.


66-68: LGTM! Ensure proper event emission.

The emitUnpaused function correctly emits the event. Ensure that the event is properly handled in the consuming code.


70-75: LGTM! Ensure proper event emission.

The emitDiscountRateAdapterUpdated function correctly emits the event. Ensure that the event is properly handled in the consuming code.

src/test/mocks/MockTermAuctionOfferLocker.sol (1)

58-59: LGTM! Ensure unique and secure offer IDs.

The changes correctly enhance the uniqueness and security of the offerId by using keccak256. Ensure that this approach is consistently applied throughout the codebase.

src/TermAuctionList.sol (4)

8-8: Import statement looks good.

The addition of the ITermDiscountRateAdapter import is necessary for the new functionality.


85-85: LGTM! But verify the function usage in the codebase.

The addition of the discountRateAdapter parameter is correctly implemented.

However, ensure that all function calls to removeCompleted match the new signature.

Verification successful

Verified: All function calls to removeCompleted match the new signature.

  • src/Strategy.sol (lines where removeCompleted is called)
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `removeCompleted` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type solidity -A 5 $'removeCompleted'

Length of output: 74


Script:

#!/bin/bash
# Description: Verify all function calls to `removeCompleted` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg -A 5 'removeCompleted'

Length of output: 1162


139-141: LGTM! But verify the function usage in the codebase.

The addition of the discountRateAdapter parameter is correctly implemented.

However, ensure that all function calls to validateAndInsertRepoToken match the new signature.

Verification successful

Let's modify the rg command to correctly recognize Solidity files by specifying the .sol extension.


All function calls to validateAndInsertRepoToken match the new signature.

The function validateAndInsertRepoToken is correctly called with the discountRateAdapter parameter in all instances found.

  • src/TermAuctionList.sol
  • src/Strategy.sol
  • src/RepoTokenList.sol
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `validateAndInsertRepoToken` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type solidity -A 5 $'validateAndInsertRepoToken'

Length of output: 85


Script:

#!/bin/bash
# Description: Verify all function calls to `validateAndInsertRepoToken` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --glob '*.sol' -A 5 'validateAndInsertRepoToken'

Length of output: 1147


Line range hint 181-195: LGTM! But verify the function usage in the codebase.

The addition of the discountRateAdapter and repoTokenToMatch parameters is correctly implemented.

However, ensure that all function calls to getPresentValue match the new signature.

src/test/TestUSDCOffers.t.sol (1)

39-39: LGTM! But verify the method usage in the codebase.

The addition of the setRepoTokenConcentrationLimit method call is correctly implemented.

However, ensure that the method setRepoTokenConcentrationLimit is used correctly throughout the codebase.

Verification successful

LGTM! The method setRepoTokenConcentrationLimit is used correctly throughout the codebase.

  • The method is defined in src/Strategy.sol and updates the repoTokenConcentrationLimit.
  • It is called in the test files src/test/TestUSDCOffers.t.sol and src/test/TestUSDCSellRepoToken.t.sol with appropriate values.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `setRepoTokenConcentrationLimit` match the expected usage.

# Test: Search for the method usage. Expect: Correct usage throughout the codebase.
rg --type solidity -A 5 $'setRepoTokenConcentrationLimit'

Length of output: 89


Script:

#!/bin/bash
# Description: Verify all method calls to `setRepoTokenConcentrationLimit` match the expected usage.

# Test: Search for the method usage. Expect: Correct usage throughout the codebase.
rg --glob '*.sol' -A 5 'setRepoTokenConcentrationLimit'

Length of output: 1980

src/RepoTokenList.sol (4)

8-8: Import statement looks good.

The addition of the ITermDiscountRateAdapter import is necessary for the new functionality.


204-206: LGTM! But verify the function usage in the codebase.

The addition of the discountRateAdapter parameter is correctly implemented.

However, ensure that all function calls to validateAndInsertRepoToken match the new signature.

Verification successful

Let's adjust our approach to correctly identify and search within Solidity files.


All function calls to validateAndInsertRepoToken match the new signature.

The addition of the discountRateAdapter parameter is correctly implemented and consistently used across the codebase.

  • src/TermAuctionList.sol
  • src/Strategy.sol
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `validateAndInsertRepoToken` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type solidity -A 5 $'validateAndInsertRepoToken'

Length of output: 85


Script:

#!/bin/bash
# Description: Verify all function calls to `validateAndInsertRepoToken` match the new signature.

# Step 1: Find all Solidity files.
sol_files=$(fd --extension sol)

# Step 2: Search for the function usage within the identified Solidity files.
rg 'validateAndInsertRepoToken' $sol_files -A 5

Length of output: 3454


Line range hint 275-305: LGTM! But verify the function usage in the codebase.

The addition of the repoTokenToMatch parameter is correctly implemented.

However, ensure that all function calls to getPresentValue match the new signature.

Verification successful

Verified: All calls to getPresentValue in src/Strategy.sol match the new signature with the repoTokenToMatch parameter.

  • src/Strategy.sol: Verified correct usage of the updated getPresentValue function.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `getPresentValue` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type solidity -A 5 $'getPresentValue'

Length of output: 74


Script:

#!/bin/bash
# Description: Verify all function calls to `getPresentValue` match the new signature.

# Locate all Solidity files
fd -e sol -x rg 'getPresentValue' {} +

Length of output: 1888


Script:

#!/bin/bash
# Description: Verify that all calls to `getPresentValue` in `src/Strategy.sol` match the new signature.

# Extract more lines around the instances found in `src/Strategy.sol` to verify the function calls.
rg 'getPresentValue' src/Strategy.sol -A 10 -B 5

Length of output: 1060


204-206: LGTM! But verify the function usage in the codebase.

The addition of the discountRateAdapter parameter is correctly implemented.

However, ensure that all function calls to validateRepoToken match the new signature.

src/test/TestUSDCSellRepoToken.t.sol (3)

165-172: LGTM!

The _sell1RepoTokenNoMintExpectRevert function encapsulates the logic for testing specific scenarios involving repo tokens without minting and expecting a revert.


523-546: LGTM!

The testConcentrationLimitFailure function effectively tests the behavior when setting a repo token concentration limit, including authorization checks and expected reverts.


548-571: LGTM!

The testPausing function effectively tests the contract's behavior when paused, including authorization checks and expected reverts.

src/Strategy.sol (11)

64-67: LGTM!

The pause function allows management to pause the contract and emits an event.


69-72: LGTM!

The unpause function allows management to unpause the contract and emits an event.


101-106: LGTM!

The setRepoTokenConcentrationLimit function allows management to set the repo token concentration limit and emits an event.


108-111: LGTM!

The setDiscountRateAdapter function allows management to set the discount rate adapter and emits an event.


Line range hint 236-280:
LGTM!

The sellRepoToken function now includes checks for the contract being paused, preventing reentrant calls, and enforcing repo token concentration limits.


288-306: LGTM!

The _validateRepoTokenConcentration function validates the concentration of repo tokens and reverts if it exceeds the limit.


552-552: LGTM!

The _deployFunds function now includes a check for the contract being paused.


577-577: LGTM!

The _freeFunds function now includes a check for the contract being paused.


606-606: LGTM!

The _harvestAndReport function now includes a check for the contract being paused.


Line range hint 356-371:
LGTM!

The submitAuctionOffer function now includes checks for the contract being paused, preventing reentrant calls, and enforcing repo token concentration limits.


506-510: LGTM!

The getRepoTokenValue function now includes logic to calculate the present value of repo tokens using the discount rate adapter.

Comment on lines 43 to 45
function testEditOffer() public {

}
Copy link

Choose a reason for hiding this comment

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

Implement or remove empty test function.

The testEditOffer function is currently empty. Implement the test logic or remove the function if it is not needed.

Comment on lines 47 to 49
function testCompleteAuction() public {

}
Copy link

Choose a reason for hiding this comment

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

Implement or remove empty test function.

The testCompleteAuction function is currently empty. Implement the test logic or remove the function if it is not needed.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 01b9653 and c8c52cb.

Files selected for processing (8)
  • src/RepoTokenList.sol (7 hunks)
  • src/Strategy.sol (9 hunks)
  • src/TermDiscountRateAdapter.sol (1 hunks)
  • src/TermVaultEventEmitter.sol (1 hunks)
  • src/interfaces/term/ITermDiscountRateAdapter.sol (1 hunks)
  • src/interfaces/term/ITermVaultEvents.sol (1 hunks)
  • src/test/TestUSDCSellRepoToken.t.sol (5 hunks)
  • src/test/TestUSDCSubmitOffer.t.sol (1 hunks)
Files skipped from review due to trivial changes (1)
  • src/interfaces/term/ITermDiscountRateAdapter.sol
Files skipped from review as they are similar to previous changes (6)
  • src/RepoTokenList.sol
  • src/Strategy.sol
  • src/TermDiscountRateAdapter.sol
  • src/TermVaultEventEmitter.sol
  • src/interfaces/term/ITermVaultEvents.sol
  • src/test/TestUSDCSubmitOffer.t.sol
Additional comments not posted (3)
src/test/TestUSDCSellRepoToken.t.sol (3)

165-172: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that the _sell1RepoTokenNoMintExpectRevert function is correctly integrated and used within the codebase.


523-546: LGTM! But verify the accuracy of the revert messages.

The code changes are approved.

However, ensure that the expected revert messages are accurate and correctly implemented.

Verification successful

The expected revert messages are accurate and correctly implemented.

  • The RepoTokenConcentrationTooHigh error is correctly defined and used in the Strategy contract.
  • The test function testConcentrationLimitFailure correctly references this error.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the accuracy of the revert messages in `testConcentrationLimitFailure`.

# Test: Search for the expected revert messages. Expect: Occurrences of the revert messages being used.
rg --type solidity -A 5 $'Strategy.RepoTokenConcentrationTooHigh.selector'

Length of output: 106


Script:

#!/bin/bash
# Description: Verify the accuracy of the revert messages in `testConcentrationLimitFailure`.

# Test: Search for the expected revert messages. Expect: Occurrences of the revert messages being used.
rg 'Strategy.RepoTokenConcentrationTooHigh.selector' --glob '*.sol' -A 5

Length of output: 513


Script:

#!/bin/bash
# Description: Verify the implementation of the revert message `RepoTokenConcentrationTooHigh` in the `Strategy` contract.

# Test: Search for the implementation of `RepoTokenConcentrationTooHigh` in the `Strategy` contract.
rg 'RepoTokenConcentrationTooHigh' --glob '*.sol' -A 5

Length of output: 1140


548-571: LGTM! But verify the accuracy of the revert messages.

The code changes are approved.

However, ensure that the expected revert messages are accurate and correctly implemented.

Verification successful

The revert messages are accurately implemented.

The Pausable functionality is correctly imported from OpenZeppelin, and the revert message "Pausable: paused" is used as expected in the testPausing function.

  • Strategy contract inherits from Pausable (OpenZeppelin).
  • The _pause function from Pausable is used, which includes the revert message.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the accuracy of the revert messages in `testPausing`.

# Test: Search for the expected revert messages. Expect: Occurrences of the revert messages being used.
rg --type solidity -A 5 $'"Pausable: paused"'

Length of output: 79


Script:

#!/bin/bash
# Description: Verify the accuracy of the revert messages in `testPausing`.

# Find all Solidity files and search for the expected revert message within those files.
fd -e sol -x rg '"Pausable: paused"' {}

Length of output: 120


Script:

#!/bin/bash
# Description: Verify the accuracy of the revert messages in `testPausing`.

# Find all Solidity files and search for the expected revert message within those files, including additional lines of context.
fd -e sol -x rg -A 10 '"Pausable: paused"' {}

Length of output: 342


Script:

#!/bin/bash
# Description: Verify the implementation of the `pause` function and related revert statements.

# Find all Solidity files and search for the `pause` function and related revert statements.
fd -e sol -x rg -A 10 'function pause' {}
fd -e sol -x rg -A 10 'revert' {}

Length of output: 7628


Script:

#!/bin/bash
# Description: Verify the implementation of the `_pause` function and any references to `Pausable`.

# Find all Solidity files and search for the `_pause` function and any references to `Pausable`.
fd -e sol -x rg -A 10 '_pause' {}
fd -e sol -x rg -A 10 'Pausable' {}

Length of output: 1941

@0xddong 0xddong merged commit f7e6a82 into master Aug 15, 2024
0 of 4 checks passed
@0xddong 0xddong deleted the repo-concentration-limit branch August 15, 2024 00:05
This was referenced Oct 2, 2024
@coderabbitai coderabbitai bot mentioned this pull request Oct 10, 2024
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.

2 participants