fix(contracts-rfq): TokenZapV1 native gas token behaviour [SLT-389]
#3418
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested reviewers
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
Documentation and Community
|
Deploying sanguine-fe with
|
| Latest commit: |
2305d93
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://75b0d3b4.sanguine-fe.pages.dev |
| Branch Preview URL: | https://fix-token-zap-native-gas-tok.sanguine-fe.pages.dev |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3418 +/- ##
===================================================
+ Coverage 3.10368% 41.84492% +38.74124%
===================================================
Files 44 87 +43
Lines 2932 2992 +60
Branches 0 82 +82
===================================================
+ Hits 91 1252 +1161
+ Misses 2838 1737 -1101
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ 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: 3
🧹 Outside diff range and nitpick comments (5)
packages/contracts-rfq/test/mocks/WETHMock.sol (3)
1-9: Add IWETH interface compatibility documentationThe contract implementation looks good, but consider documenting that this mock implements the standard WETH interface (IWETH) for clarity in testing scenarios.
Add interface documentation:
/// @notice WETH mock for testing purposes. DO NOT USE IN PRODUCTION. +/// @dev Implements standard WETH interface for testing contract WETHMock is ERC20 {
18-21: Consider adding reentrancy protectionWhile the function follows checks-effects-interactions pattern, consider adding OpenZeppelin's
nonReentrantmodifier for additional security, as it deals with ETH transfers.+import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol"; -contract WETHMock is ERC20 { +contract WETHMock is ERC20, ReentrancyGuard { // ... - function withdraw(uint256 amount) external { + function withdraw(uint256 amount) external nonReentrant {
23-25: Consider emitting a Deposit eventWhile the implementation is correct, consider emitting a
Depositevent for better testability and standard WETH interface compliance.+ event Deposit(address indexed dst, uint256 wad); + function deposit() public payable { _mint(msg.sender, msg.value); + emit Deposit(msg.sender, msg.value); }packages/contracts-rfq/test/zaps/TokenZapV1.t.sol (1)
181-183: Consider changing function visibility tointernalThe
getZapDataWithdrawfunction is only used within the contract. Changing its visibility tointernalenhances encapsulation.Apply this diff to adjust the function visibility:
-function getZapDataWithdraw(uint256 amount) public view returns (bytes memory) { +function getZapDataWithdraw(uint256 amount) internal view returns (bytes memory) {packages/contracts-rfq/contracts/zaps/TokenZapV1.sol (1)
31-33: Consider emitting an event in thereceive()function for transparencyAdding an event emission when the contract receives ETH can help track deposits and enhance transparency.
Apply the following diff to emit an event when ETH is received:
+event Received(address indexed sender, uint256 amount); receive() external payable { + emit Received(msg.sender, msg.value); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol(2 hunks)packages/contracts-rfq/test/mocks/WETHMock.sol(1 hunks)packages/contracts-rfq/test/zaps/TokenZapV1.t.sol(5 hunks)
🔇 Additional comments (11)
packages/contracts-rfq/test/mocks/WETHMock.sol (2)
11-13: LGTM! Correctly implements standard WETH receive behavior
The receive function properly forwards incoming ETH to the deposit function, matching standard WETH behavior.
15-16: LGTM! Well-documented test exclusion pattern
The empty test function is properly documented and serves its purpose for coverage exclusion.
packages/contracts-rfq/test/zaps/TokenZapV1.t.sol (8)
8-11: Imports are appropriate and necessary
The new imports for NonPayableRecipient, RecipientMock, and WETHMock are required for the additional test cases and are correctly included.
23-25: Variable declarations are correct
The declarations of weth, payableMock, and nonPayableMock are appropriate and necessary for the new tests.
34-36: Initialization in setUp() is proper
The setUp() function correctly initializes the mock contracts and sets up the testing environment.
158-167: Test handles excess msg.value correctly
The test_zap_native_msgValueHigherThanAmount function effectively tests the scenario where msg.value exceeds the specified amount. The assertions ensure the correct handling of surplus funds.
169-177: Test correctly utilizes existing native balance
The test_zap_native_msgValueLowerThanAmount_extraNative function properly tests the usage of both msg.value and the contract's existing native balance.
185-210: New test functions enhance coverage
The added test functions for withdrawing and depositing native tokens comprehensively cover various scenarios, improving test coverage.
240-265: Tests for transferring native tokens to EOAs are well-implemented
The functions test_zap_withdraw_transferNativeEOA_placeholderZero and test_zap_withdraw_transferNativeEOA_placeholderNonZero correctly test transfers to externally owned accounts.
295-320: Tests for transferring native tokens to contracts are appropriate
The functions test_zap_withdraw_transferNativeContract_placeholderZero and test_zap_withdraw_transferNativeContract_placeholderNonZero effectively test transfers to contracts, ensuring the TokenZapV1 handles these cases properly.
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol (1)
70-75: Properly handling transfers when payload is empty
The implementation correctly handles the case where payload is empty by using Address.sendValue to transfer ETH directly to the target, accommodating both EOAs and contracts.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol (3)
31-33: Consider adding an event emission for received ETHWhile the receive function is correctly implemented, adding an event emission would improve transparency and make it easier to track ETH deposits.
+ event EthReceived(address indexed sender, uint256 amount); receive() external payable { + emit EthReceived(msg.sender, msg.value); }
36-39: Enhance security warning in documentationWhile the documentation mentions that leftover funds can be claimed by anyone, consider making the security implications more prominent by adding an explicit @security tag.
/// @notice Performs a Zap action using the specified token and amount. This amount must be previously /// transferred to this contract (could also be supplied as msg.value if the token is native gas token). /// Zap action will be performed forwarding full `msg.value` for ERC20s or `amount` for native gas token. /// Note: all funds remaining after the Zap action is performed can be claimed by anyone. /// Make sure to spend the full balance during the Zaps and avoid sending extra funds if a single Zap is performed. + /// @security IMPORTANT: Any excess tokens or ETH sent to this contract can be claimed by anyone. Always ensure + /// the exact amount is sent and fully utilized in the Zap operation.
71-80: Add event emission for transfersConsider adding event emissions for both direct transfers and contract calls to improve traceability and debugging capabilities.
+ event ZapTransfer(address indexed target, uint256 value, bool isDirectTransfer); if (payload.length == 0) { Address.sendValue({recipient: payable(target), amount: msgValue}); + emit ZapTransfer(target, msgValue, true); } else { Address.functionCallWithValue({target: target, data: payload, value: msgValue}); + emit ZapTransfer(target, msgValue, false); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol(2 hunks)
🔇 Additional comments (1)
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol (1)
50-59: Verify security implications of removed msg.value check
The removal of the explicit msg.value check for native gas tokens means users could accidentally send more ETH than intended. While this aligns with the documented behavior that excess funds can be claimed by anyone, it increases the risk of user error.
✅ Verification successful
Removal of msg.value check is safe with proper documentation
The implementation is secure because:
- The code explicitly documents the behavior regarding excess funds in the comments
- Balance checks are performed downstream in the Address library functions
- The contract is designed as a non-custodial pass-through, where any excess funds can be retrieved through subsequent zap calls
- The implementation matches the documented behavior that excess funds can be claimed by anyone
The removal of the explicit msg.value check aligns with the contract's design principles and is properly documented to warn users about the implications of sending excess funds.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any existing front-running or ETH claim patterns in the codebase
rg -A 5 "sendValue|call{value|transfer"
Length of output: 168
Script:
#!/bin/bash
# Search for ETH transfer patterns with proper regex escaping
rg -A 5 "sendValue|call\{value|transfer\("
# Also search for potential claim or sweep functions
rg -i "(claim|sweep|withdraw|collect).*balance"
Length of output: 106474
Script:
#!/bin/bash
# Search for zap implementation and related functions
rg -A 5 "function zap|function sweep|function claim" packages/contracts-rfq/contracts/zaps/
# Search for any balance checks or validations
rg -A 3 "require.*balance|assert.*balance" packages/contracts-rfq/contracts/zaps/
Length of output: 901
Script:
#!/bin/bash
# Search for the entire zap function implementation and surrounding context
rg -B 5 -A 20 "function zap.*external.*payable" packages/contracts-rfq/contracts/zaps/
# Search for any downstream balance checks in Address library
rg -A 5 "function (sendValue|functionCallWithValue)" packages/contracts-rfq/contracts/
Length of output: 3591
Changes to gas cost
🧾 Summary (50% most significant diffs)
Full diff report 👇
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol (2)
31-33: Consider adding an event emission for better transparency.While the
receive()function implementation is correct, adding an event emission would improve transparency and make it easier to track incoming ETH.+ event NativeTokenReceived(address indexed sender, uint256 amount); receive() external payable { + emit NativeTokenReceived(msg.sender, msg.value); }
77-79: Consider adding revert reason for failed callsWhen the target contract call fails, it would be helpful to include a custom error message that provides more context about the failure.
- Address.functionCallWithValue({target: target, data: payload, value: msgValue}); + try Address.functionCallWithValue({target: target, data: payload, value: msgValue}) { + } catch Error(string memory reason) { + revert TokenZapV1__TargetCallFailed(reason); + } catch { + revert TokenZapV1__TargetCallFailed("unknown error"); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol(2 hunks)
🔇 Additional comments (3)
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol (3)
18-22: LGTM! Documentation improvements enhance clarity.
The updated documentation clearly explains the contract's capabilities and includes important warnings about fund handling, particularly for native gas tokens.
35-39: LGTM! Documentation clearly explains the native token behavior.
The updated documentation effectively communicates the handling of msg.value and includes important warnings about fund management.
50-53:
Reconsider removing the msg.value validation
The removal of the msg.value check, while documented, could lead to unexpected behavior where users accidentally send more ETH than intended. Consider keeping a validation to ensure exact amounts.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol (2)
18-22: LGTM! Consider adding revert behavior documentation.The enhanced documentation clearly explains the contract's purpose and token handling behavior. Consider adding a note about specific conditions that would cause the contract to revert.
/// @notice Facilitates atomic token operations known as "Zaps", allowing the execution of predefined actions /// on behalf of users, such as deposits or swaps. Supports ERC20 tokens and native gas tokens (e.g., ETH). /// @dev Tokens must be transferred to the contract before execution, native tokens could be provided as `msg.value`. /// This contract is stateless and does not hold assets between Zaps; leftover tokens can be claimed by anyone. -/// Ensure that Zaps fully utilize tokens or revert to prevent the loss of funds. +/// Ensure that Zaps fully utilize tokens or revert to prevent the loss of funds. +/// The contract will revert if: target is zero address, payload length exceeds maximum, or target contract reverts.
36-43: LGTM! Consider adding example usage.The documentation clearly explains the function's behavior and requirements. Consider adding a @notice example showing typical usage patterns for both ERC20 and native gas tokens.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol(2 hunks)packages/contracts-rfq/test/mocks/WETHMock.sol(1 hunks)packages/contracts-rfq/test/zaps/TokenZapV1.t.sol(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/contracts-rfq/test/mocks/WETHMock.sol
🔇 Additional comments (13)
packages/contracts-rfq/test/zaps/TokenZapV1.t.sol (8)
8-9: Imports are appropriate
The added imports (NonPayableRecipient, RecipientMock, WETHMock, Address) are necessary for the new test cases and are correctly included.
Also applies to: 11-11, 13-13
23-25: New variables declared correctly
The declarations of weth, payableMock, and nonPayableMock are appropriate and necessary for the added tests.
34-36: Proper initialization in setUp function
The initialization of weth, payableMock, and nonPayableMock in the setUp function ensures the tests have the correct setup.
158-167: Test handles excess msg.value correctly
The test_zap_native_msgValueHigherThanAmount function accurately tests the scenario where msg.value exceeds amount. The logic verifies that the extra funds are retained in the contract.
170-177: Test combines msg.value and existing native balance effectively
The test_zap_native_msgValueLowerThanAmount_extraNative function correctly tests the use of both msg.value and existing balance to meet the required amount.
444-444: Incorrect error selector in expected revert
The vm.expectRevert uses Address.AddressInsufficientBalance.selector, which is not defined in the Address library. Instead, match the revert reason string from the Address utility.
Apply this diff:
-vm.expectRevert(abi.encodeWithSelector(Address.AddressInsufficientBalance.selector, tokenZap));
+vm.expectRevert("Address: insufficient balance");453-453: Incorrect expected revert reason
The vm.expectRevert uses Address.FailedInnerCall.selector, which is not defined in the Address library. Use the correct revert reason from the Address utility.
Apply this diff:
-vm.expectRevert(Address.FailedInnerCall.selector);
+vm.expectRevert("Address: unable to send value, recipient may have reverted");482-482:
Incorrect error selector in expected revert
The vm.expectRevert uses Address.AddressEmptyCode.selector, which is not defined in the Address library. Use the correct revert reason from the Address utility.
Apply this diff:
-vm.expectRevert(abi.encodeWithSelector(Address.AddressEmptyCode.selector, user));
+vm.expectRevert("Address: call to non-contract");Likely invalid or redundant comment.
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol (5)
30-30: LGTM! Good security practice.
The addition of a custom error for zero address validation improves both security and gas efficiency.
52-52: LGTM! Essential security check added.
Zero address validation is a critical security measure.
74-83: Verify EOA transfer handling security.
The new empty payload handling for native token transfers to EOAs introduces potential risks:
- MEV sandwich attacks on transfers
- Unintended token leakage if target is compromised
Consider:
- Adding transfer amount limits
- Implementing a whitelist for EOA targets
- Adding transfer events for better tracking
✅ Verification successful
Let me gather more information about the security measures around native token transfers.
Let me check for additional security measures and validations in the TokenZapV1 contract.
Empty payload native token transfers are adequately protected
The implementation has sufficient security measures for native token transfers to EOAs:
- The contract is stateless and doesn't custody funds between zaps
- Balance checks are enforced by OpenZeppelin's
Address.sendValue - Target address validation prevents transfers to zero address
- The contract clearly documents that it's designed for atomic operations and leftover funds can be claimed by anyone
The identified risks are mitigated:
- MEV sandwich attacks: Not a significant concern as this is an atomic operation
- Token leakage: The contract is explicitly designed to be non-custodial and documents that any leftover funds can be claimed by anyone
Additional security measures like transfer limits or whitelisting would go against the contract's atomic, stateless design pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar transfer patterns
ast-grep --pattern 'Address.sendValue($recipient, $amount)'
# Look for security measures around EOA transfers
rg -A 3 "sendValue|transfer\("
Length of output: 162217
Script:
#!/bin/bash
# Look for security measures around native token transfers in TokenZapV1
rg -A 5 "sendValue|transfer\(" packages/contracts-rfq/contracts/zaps/TokenZapV1.sol
# Check for events and access controls
ast-grep --pattern 'event $name($$$)' packages/contracts-rfq/contracts/zaps/TokenZapV1.sol
# Look for owner/admin controls
rg -A 3 "onlyOwner|require\(" packages/contracts-rfq/contracts/zaps/TokenZapV1.sol
Length of output: 1151
Script:
#!/bin/bash
# Look for the full TokenZapV1 contract implementation
cat packages/contracts-rfq/contracts/zaps/TokenZapV1.sol
# Check for security-related imports
rg -l "ReentrancyGuard|Ownable|Pausable" packages/contracts-rfq/contracts/zaps/
# Look for validation checks in the contract
rg -B 2 -A 2 "require|assert|revert" packages/contracts-rfq/contracts/zaps/TokenZapV1.sol
Length of output: 11078
32-34: Consider adding access control or time-lock for leftover ETH.
While the documentation warns about leftover ETH being claimable by anyone, this could be exploited by MEV bots or attackers. Consider:
- Adding access control for claiming leftover ETH
- Implementing a time-lock mechanism
- Adding an explicit claim function with proper access control
Let's check if there are any similar patterns in the codebase:
53-62: Verify the implications of removing msg.value check.
The removal of msg.value validation allows using pre-transferred native tokens but could lead to unexpected behavior if users send incorrect amounts.
Consider adding events to track:
- Pre-transferred native token usage
- Excess token amounts
✅ Verification successful
Let me gather more specific information about the token handling patterns.
Removal of msg.value check is intentional and safe with proper documentation
The code explicitly documents that not checking msg.value is a design choice to support pre-transferred native tokens, with clear warnings about the implications:
- The comment clearly states the behavior: "we don't check the amount that was transferred to TokenZapV1 (or msg.value for native gas tokens)"
- Users are properly warned: "Transferring more than
amountwill lead to remaining funds in TokenZapV1" - Clear guidance is provided: "Ensure that you send the exact amount for a single Zap or spend the full balance for multiple
zap()calls" - Balance checks are still performed at critical points: "balance check is performed within
Address.sendValueorAddress.functionCallWithValue"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar native token handling patterns
rg -B 2 -A 2 "msg.value"
# Search for potential security implications
ast-grep --pattern 'if ($token == $NATIVE_TOKEN) {
$$$
}'
Length of output: 136240
Script:
#!/bin/bash
# Search for token validation patterns in TokenZapV1
rg -B 5 -A 5 "NATIVE_GAS_TOKEN" packages/contracts-rfq/contracts/zaps/TokenZapV1.sol
# Search for msg.value validation patterns
ast-grep --pattern 'require($msg.value == $amount, $_)'
# Search for balance checks
ast-grep --pattern 'require(address(this).balance >= $amount, $_)'
Length of output: 2401
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
packages/contracts-rfq/test/mocks/WETHMock.sol (3)
16-17: Consider using coverage configuration instead of empty functions.While the empty function works for coverage exclusion, consider using coverage tool configurations (like solidity-coverage's skipFiles or .solcover.js) for a cleaner approach.
19-22: Consider adding input validation.While the implementation is secure, consider adding a check for zero amount to prevent unnecessary transactions.
function withdraw(uint256 amount) external { + require(amount > 0, "WETHMock: cannot withdraw 0"); _burn(msg.sender, amount); Address.sendValue(payable(msg.sender), amount); }
24-26: LGTM! Consider adding a zero-value check.The deposit function correctly mints WETH tokens. Consider adding a zero-value check for consistency with the withdraw function.
function deposit() public payable { + require(msg.value > 0, "WETHMock: cannot deposit 0"); _mint(msg.sender, msg.value); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol(2 hunks)packages/contracts-rfq/test/mocks/WETHMock.sol(1 hunks)packages/contracts-rfq/test/zaps/TokenZapV1.t.sol(7 hunks)
🔇 Additional comments (8)
packages/contracts-rfq/test/mocks/WETHMock.sol (2)
1-10: LGTM! Good choice of dependencies and clear documentation.
The contract is well-structured with appropriate imports from OpenZeppelin. The pragma version ^0.8.20 is recent and includes important security features.
12-14: LGTM! Correct implementation of the receive function.
The receive function properly handles direct ETH transfers by calling deposit(), following the standard WETH pattern.
packages/contracts-rfq/contracts/zaps/TokenZapV1.sol (1)
52-52:
Duplicate Comment: Ensure msg.value matches amount for native gas tokens
As previously noted, when token == NATIVE_GAS_TOKEN, msgValue is set to amount, which may differ from the actual msg.value sent. To prevent discrepancies and unintended accumulation of funds in the contract, consider adding a check to ensure msg.value equals amount when handling native gas tokens.
Apply the following diff to enforce the check:
if (token == NATIVE_GAS_TOKEN) {
+ require(msg.value == amount, "TokenZapV1: msg.value must equal amount for native gas tokens");
msgValue = amount;
// No approval needed since native tokens don't use allowances.
// Note: balance check is performed within `Address.sendValue` or `Address.functionCallWithValue` below.
}packages/contracts-rfq/test/zaps/TokenZapV1.t.sol (5)
378-378: Use abi.encode instead of abi.encodePacked to prevent ambiguity
When encoding multiple parameters, abi.encodePacked can lead to ambiguous encoding, potentially causing decoding issues. Consider using abi.encode to preserve type information and prevent collisions.
Apply this diff:
-return abi.encodePacked(ZapDataV1.VERSION, amountPosition, address(0), payload);
+return abi.encode(ZapDataV1.VERSION, amountPosition, address(0), payload);422-423: Incorrect error selector in expected revert
The vm.expectRevert uses Address.AddressEmptyCode.selector, which is not defined in the Address library. Instead, use the revert reason string from OpenZeppelin's Address utility when calling a non-contract address.
Apply this diff to correct the expected revert:
-vm.expectRevert(abi.encodeWithSelector(Address.AddressEmptyCode.selector, user));
+vm.expectRevert("Address: call to non-contract");444-444: Incorrect error selector in expected revert
The vm.expectRevert is using an incorrect error selector Address.AddressInsufficientBalance.selector. The Address library does not define AddressInsufficientBalance. Instead, the error message should match the revert reason from OpenZeppelin's Address utility.
Apply this diff to correct the expected revert:
-vm.expectRevert(abi.encodeWithSelector(Address.AddressInsufficientBalance.selector, tokenZap));
+vm.expectRevert("Address: insufficient balance");453-453: Incorrect expected revert reason
The vm.expectRevert uses Address.FailedInnerCall.selector, which is not defined in the Address library. The correct revert message should reflect the failure in sending value.
Apply this diff to correct the expected revert:
-vm.expectRevert(Address.FailedInnerCall.selector);
+vm.expectRevert("Address: unable to send value, recipient may have reverted");482-482: Incorrect error selector in expected revert
The vm.expectRevert uses Address.AddressEmptyCode.selector, which is not defined in the Address library. Use the revert reason string from OpenZeppelin's Address utility when calling a non-contract address.
Apply this diff:
-vm.expectRevert(abi.encodeWithSelector(Address.AddressEmptyCode.selector, user));
+vm.expectRevert("Address: call to non-contract");
Description
Modified
TokenZapV1handling of the native gas token, making it more similar to approach it uses for ERC20s:receive()to allow the contract to receive native gas token outside ofzap()call.TokenZapV1to use previously transferred or otherwise acquired native gas token.Summary by CodeRabbit
New Features
receive()function in theTokenZapV1contract to accept ETH directly.Bug Fixes
TokenZapV1contract.Tests
TokenZapV1Testcontract, including edge cases for deposits and withdrawals.