Update DepositContract.t.sol #2098
Conversation
Fixed some aesthetic errors. Signed-off-by: MaxBT <[email protected]>
WalkthroughThe changes made to the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as DepositContractTest
participant Setup as SetUp Function
participant TestFunc as Test Functions
participant Constants as Constants
Test->>Setup: Call setUp()
Setup->>Constants: Use OWNER and DEPOSITOR
Setup-->>Test: Setup complete
Test->>TestFunc: Execute test functions
TestFunc->>Constants: Access DEPOSITOR
TestFunc-->>Test: Tests executed with constants
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range comments (1)
contracts/test/staking/DepositContract.t.sol (1)
Line range hint
98-106: Consider using consistent units throughout the test.There's a potential confusion in the test_Deposit function:
- Line 98 uses
32 etherfor vm.deal- Line 102 uses
32e9for the expected eventWhile these values are equivalent (1 ether = 1e18 wei, and 32e9 gwei = 32 ether), using consistent units would make the test more readable and maintainable.
Consider using either:
- vm.deal(DEPOSITOR, 32 ether); - vm.prank(DEPOSITOR); - vm.expectEmit(true, true, true, true); - emit IDepositContract.Deposit( - VALIDATOR_PUBKEY, STAKING_CREDENTIALS, 32e9, _create96Byte(), 0 - ); + uint256 depositAmount = 32 ether; + vm.deal(DEPOSITOR, depositAmount); + vm.prank(DEPOSITOR); + vm.expectEmit(true, true, true, true); + emit IDepositContract.Deposit( + VALIDATOR_PUBKEY, STAKING_CREDENTIALS, depositAmount, _create96Byte(), 0 + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- contracts/test/staking/DepositContract.t.sol (9 hunks)
🔇 Additional comments (6)
contracts/test/staking/DepositContract.t.sol (6)
10-13: LGTM! Good use of constants for test addresses.Converting the addresses to constants improves code maintainability and makes the test setup more explicit. The initialization in setUp properly uses these constants for contract deployment and permissions setup.
Also applies to: 22-22, 30-32
35-38: LGTM! Comprehensive validation of public key length requirements.The tests properly validate both fuzzing and static scenarios for invalid public key lengths. Good use of vm.prank with the DEPOSITOR constant for permission control.
Also applies to: 49-54
58-64: LGTM! Well-structured credential validation tests.The tests effectively cover both fuzzing and static scenarios for invalid credential lengths. Proper error validation and permission handling is maintained.
Also applies to: 70-73
109-115: LGTM! Thorough validation of minimum deposit requirements.The tests effectively cover both fuzzing and static scenarios for insufficient deposits, with proper bounds checking and error validation.
Also applies to: 121-127
Line range hint
132-138: LGTM! Comprehensive validation of Gwei divisibility requirements.The tests effectively cover various scenarios for non-Gwei-divisible amounts, including edge cases both above and below the expected deposit amount.
Also applies to: 143-156
173-189: LGTM! Well-structured multiple deposit validation.The test effectively validates multiple deposits with proper event emission checks and deposit count verification. Good use of vm.startPrank for handling multiple transactions from the same address.
Fixed some aesthetic errors.
Summary by CodeRabbit
Bug Fixes
test_Depositfunction.testFuzz_DepositCount.New Features
OWNERandDEPOSITORto improve clarity in test cases.Refactor