feat: BungeeApproveAndBridge #2
Conversation
|
""" WalkthroughThis update introduces a new bridging contract ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Proxy (COWShed)
participant BungeeApproveAndBridge
participant SocketGateway
User->>Proxy: executeHooks(approveAndBridge call)
Proxy->>BungeeApproveAndBridge: delegatecall approveAndBridge(...)
BungeeApproveAndBridge->>BungeeApproveAndBridge: bridge(...)
BungeeApproveAndBridge->>SocketGateway: call (modified calldata)
SocketGateway-->>BungeeApproveAndBridge: bridging result
BungeeApproveAndBridge-->>Proxy: return
Proxy-->>User: return
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~45 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 7
🧹 Nitpick comments (10)
src/vendored/IERC20.sol (1)
10-10: Consider version compatibility with other contracts.The Solidity version constraint
^0.8.20is more restrictive than the^0.8used in other contracts in this PR. This could lead to compilation issues when integrating with contracts that specify broader version ranges.Consider aligning the version constraint with other contracts:
-pragma solidity ^0.8.20; +pragma solidity ^0.8;test/e2e/lib/ForkedRpc.sol (2)
7-15: Consider adding URL validation and better error handling.The fallback URL approach is good, but consider adding validation to ensure the URLs are accessible before attempting to fork.
Consider adding URL validation:
function forkEthereumMainnetAtBlock(Vm vm, uint256 blockNumber) internal returns (uint256 forkId) { string memory forkUrl; try vm.envString("MAINNET_ARCHIVE_NODE_URL") returns (string memory url) { forkUrl = url; } catch { forkUrl = "https://eth.merkle.io"; } + // Consider adding URL validation here forkId = vm.createSelectFork(forkUrl, blockNumber); }
17-25: Consider consolidating duplicate logic.The two functions follow identical patterns. Consider refactoring to reduce code duplication.
Consider extracting common logic:
+function _forkAtBlock(Vm vm, string memory envVar, string memory fallbackUrl, uint256 blockNumber) private returns (uint256 forkId) { + string memory forkUrl; + try vm.envString(envVar) returns (string memory url) { + forkUrl = url; + } catch { + forkUrl = fallbackUrl; + } + forkId = vm.createSelectFork(forkUrl, blockNumber); +} + function forkBaseAtBlock(Vm vm, uint256 blockNumber) internal returns (uint256 forkId) { - string memory forkUrl; - try vm.envString("BASE_ARCHIVE_NODE_URL") returns (string memory url) { - forkUrl = url; - } catch { - forkUrl = "https://base.drpc.org"; - } - forkId = vm.createSelectFork(forkUrl, blockNumber); + return _forkAtBlock(vm, "BASE_ARCHIVE_NODE_URL", "https://base.drpc.org", blockNumber); }src/mixin/ApproveAndBridge.sol (1)
2-2: Consider using a more specific Solidity version.Using
^0.8is very broad. Consider specifying a more specific version range like^0.8.20to match the vendored SafeERC20 library's requirements and ensure consistent compilation behavior.-pragma solidity ^0.8; +pragma solidity ^0.8.20;test/e2e/BungeeApproveAndBridge.t.sol (3)
32-32: Consider making the fork block configurable via environment variable.Hardcoding the fork block number can make tests brittle as the blockchain progresses. Consider reading it from an environment variable with a fallback:
- uint256 private constant BASE_FORK_BLOCK = 32853375; + uint256 private immutable BASE_FORK_BLOCK = vm.envOr("BASE_FORK_BLOCK", uint256(32853375));
49-50: Remove unused receiver variable.The
receivervariable is created but never used in the test.- address public receiver; function setUp() public { vm.label(user, "user"); vm.label(address(SOCKET_GATEWAY), "socket gateway"); vm.label(address(USDC), "USDC"); vm.forkBaseAtBlock(BASE_FORK_BLOCK); approveAndBridge = new BungeeApproveAndBridge(SOCKET_GATEWAY); - receiver = makeAddr("E2EBungeeApproveAndBridgeTest: receiver"); }
63-65: Extract magic numbers to named constants for better readability.The test uses magic numbers that could be better documented:
- uint256 orderProceeds = 5e6; - uint256 minProceeds = 4.9e6; + uint256 orderProceeds = 5e6; // 5 USDC + uint256 minProceeds = 4.9e6; // 4.9 USDC (2% slippage tolerance)script/Deploy.s.sol (1)
30-30: Consider making deployCreate3 function internal.The function is only used within the contract and doesn't need to be public.
- function deployCreate3(uint256 seed, address deployer) public returns (address addr) { + function deployCreate3(uint256 seed, address deployer) internal returns (address addr) {src/BungeeApproveAndBridge.sol (2)
110-110: Fix incorrect comment referencing "externalData".The comment references "externalData" but the parameter is named "_original".
- // Directly modify externalData in-place without creating a new copy + // Directly modify _original in-place without creating a new copy
144-144: Remove redundant zero checks for uint256 values.Since
_baseis a uint256, it cannot be less than 0. The check_base <= 0is equivalent to_base == 0.- if (_base <= 0 || _compare < _base) revert InvalidInput(); + if (_base == 0 || _compare < _base) revert InvalidInput();And similarly in _subPctDiff:
- if (_base <= 0 || _compare > _base) revert InvalidInput(); + if (_base == 0 || _compare > _base) revert InvalidInput();Also applies to: 160-160
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.vscode/settings.json(1 hunks)script/Counter.s.sol(0 hunks)script/Deploy.s.sol(1 hunks)src/BungeeApproveAndBridge.sol(1 hunks)src/Counter.sol(0 hunks)src/interface/IApproveAndBridge.sol(1 hunks)src/interface/ISocketGateway.sol(1 hunks)src/mixin/ApproveAndBridge.sol(1 hunks)src/vendored/IERC20.sol(1 hunks)src/vendored/SafeERC20.sol(1 hunks)test/Counter.t.sol(0 hunks)test/e2e/BungeeApproveAndBridge.t.sol(1 hunks)test/e2e/lib/ForkedRpc.sol(1 hunks)
💤 Files with no reviewable changes (3)
- src/Counter.sol
- script/Counter.s.sol
- test/Counter.t.sol
🧰 Additional context used
🪛 Biome (1.9.4)
.vscode/settings.json
[error] 1-1: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 6-7: Expected an array, an object, or a literal but instead found ']'.
Expected an array, an object, or a literal here.
(parse)
[error] 1-7: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 7-7: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (16)
src/interface/ISocketGateway.sol (1)
1-8: LGTM! Clean interface design with helpful external reference.The interface is well-structured and follows standard Solidity patterns. The reference to the Arbiscan contract address provides valuable context for understanding the expected implementation.
src/vendored/IERC20.sol (1)
1-85: Well-documented standard ERC20 interface with proper attribution.The vendored interface is properly documented with source attribution and includes all standard ERC20 functions and events. The race condition warning for the approve function is particularly valuable.
src/interface/IApproveAndBridge.sol (1)
1-10: Well-designed interface for approve-and-bridge pattern.The interface clearly defines the core functionality with appropriate parameters. The
minAmountparameter suggests slippage protection, and thebridgeApprovalTargetfunction provides necessary approval target information.test/e2e/lib/ForkedRpc.sol (1)
1-26: Useful testing utility with good fallback strategy.The library provides a clean abstraction for blockchain forking with sensible fallbacks when environment variables are not available.
src/mixin/ApproveAndBridge.sol (2)
8-15: LGTM! Good use of custom errors and standard native token address.The abstract contract structure is well-designed with a gas-efficient custom error and the standard native token address convention.
35-38: Well-designed abstract functions.The abstract function declarations provide a clean interface for derived contracts to implement their specific bridging logic.
src/vendored/SafeERC20.sol (2)
1-10: Excellent documentation of vendored code source and modifications.The header clearly documents the source, version, commit hash, and modifications made to the vendored code. This is a best practice for maintaining vendored dependencies.
48-95: Well-implemented helper functions with proper safety checks.The helper functions correctly handle edge cases including:
- Proper error bubbling from failed calls
- Contract existence validation when no return data
- Memory-safe assembly usage
script/Deploy.s.sol (2)
30-61: Robust deployment logic with good safety checks.The deployment function includes excellent safety features:
- Checks for existing deployment to prevent redeployment
- Environment variable control for deployment execution
- Address verification to ensure correct deployment
63-89: Well-implemented helper functions with clear documentation.The salt generation correctly implements CreateX's guarding mechanism with clear documentation explaining the format and protection mechanism.
src/BungeeApproveAndBridge.sol (6)
7-7: Critical: Unaudited contract warning.The contract is marked as UNAUDITED and UNTESTED. Ensure proper auditing and comprehensive testing before production use.
8-31: Well-structured contract setup with comprehensive error handling.Good use of custom errors, clear constant definitions, and well-organized data structures.
33-36: Correct implementation of abstract function.Simple and correct implementation returning the immutable socket gateway address.
82-100: Well-implemented calldata parsing with proper validation.The function correctly validates data length and cleanly separates route calldata from modification parameters.
120-126: Efficient and safe uint256 reading implementation.Proper bounds checking and efficient assembly usage for reading values from bytes array.
128-166: Well-implemented percentage calculation logic.The percentage calculation functions correctly handle proportional adjustments with good precision using 1e18 scaling.
| function test_happyPath() external { | ||
| // Note: deployment and initialization is handled in `executeHooks` and | ||
| // doesn't need to be done in the actual trade setting. | ||
| // However, it's easier to build the test without handling the | ||
| // authentication part needed for that and use `trustedExecuteHooks` | ||
| // through the factory instead. | ||
| factory.initializeProxy(user, false); | ||
| COWShed shed = COWShed(factory.proxyOf(user)); | ||
| vm.label(address(shed), "shed"); | ||
| assertGt(address(shed).code.length, 0); | ||
|
|
||
| uint256 orderProceeds = 5e6; | ||
| uint256 minProceeds = 4.9e6; | ||
| assertGt(orderProceeds, minProceeds); | ||
|
|
||
| // For simplicity we take the funds from the user, but they should come | ||
| // from an order. | ||
| vm.prank(user); | ||
| USDC.transfer(address(shed), orderProceeds); | ||
| assertEq(USDC.balanceOf(address(shed)), orderProceeds); | ||
|
|
||
| /* Across */ | ||
| // bytes memory BungeeApiCalldata = | ||
| // hex"0000019b792ebcb900000000000000000000000000000000000000000000000000000000004bea47000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000c00000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000018000000000000000000000000000000000000000000000000000000000000001e00000000000000000000000000000000000000000000000000000000000001f9a0000000000000000000000000000000000000000000000000000000000000a2d00000000000000000000000000000000000000000000000000000000000000020000000000000000000000007851b96b5798774258437195183d7c8094583c40000000000000000000000000daee4d2156de6fe6f7d50ca047136d758f96a6f00000000000000000000000000000000000000000000000000000000000000002000000000000000000000000833589fcd6edb6e08f4c7c32d4f71b54bda02913000000000000000000000000af88d065e77c8cc2239327c5edb3a432268e5831000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000004bcaad000000000000000000000000000000000000000000000000000000000000a4b10000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000006874f43f0000000000000000000000000000000000000000000000000000000068754845d00dfeeddeadbeef765753be7f7a64d5509974b0d678e1e3149b02f4"; | ||
| // uint256 inputAmountStartIndex = 8; | ||
| // bool modifyOutputAmount = true; | ||
| // uint256 outputAmountStartIndex = 488; | ||
| // uint256 additionalValue = 0; | ||
|
|
||
| /* CCTP */ | ||
| bytes memory BungeeApiCalldata = | ||
| hex"0000018db7dfe9d000000000000000000000000000000000000000000000000000000000000ef52f0000000000000000000000000000000000000000000000000000000000000a2d000000000000000000000000daee4d2156de6fe6f7d50ca047136d758f96a6f0000000000000000000000000833589fcd6edb6e08f4c7c32d4f71b54bda02913000000000000000000000000000000000000000000000000000000000000a4b100000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000000000000000000000000000000061a80000000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"; | ||
| uint256 inputAmountStartIndex = 8; | ||
| bool modifyOutputAmount = false; | ||
| uint256 outputAmountStartIndex = 0; | ||
| uint256 additionalValue = 0; | ||
|
|
||
| bytes memory extraData = | ||
| abi.encode(inputAmountStartIndex, modifyOutputAmount, outputAmountStartIndex, additionalValue); | ||
| bytes memory _calldata = abi.encodePacked(BungeeApiCalldata, extraData); | ||
|
|
||
| COWShed.Call[] memory calls = new COWShed.Call[](1); | ||
| calls[0] = COWShed.Call({ | ||
| target: address(approveAndBridge), | ||
| value: 0, | ||
| callData: abi.encodeCall(IApproveAndBridge.approveAndBridge, (USDC, minProceeds, _calldata)), | ||
| allowFailure: false, | ||
| isDelegateCall: true | ||
| }); | ||
|
|
||
| vm.prank(address(factory)); | ||
| shed.trustedExecuteHooks(calls); | ||
| assertEq(USDC.balanceOf(address(shed)), 0); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding more test cases for comprehensive coverage.
The current test only covers the happy path. Consider adding tests for:
- Failure cases (insufficient balance, failed bridge call)
- Different token types (native ETH bridging)
- Edge cases (exact minimum amount, zero amount)
Would you like me to generate additional test cases to improve coverage?
🤖 Prompt for AI Agents
In test/e2e/BungeeApproveAndBridge.t.sol around lines 52 to 105, the
test_happyPath function only covers the successful execution scenario. To
improve test coverage, add new test cases that cover failure scenarios such as
insufficient user balance and failed bridge calls, tests involving different
token types including native ETH bridging, and edge cases like exact minimum
amounts and zero amounts. Implement these additional tests to ensure robustness
and handle various possible inputs and outcomes.
There was a problem hiding this comment.
@anxolin, I'd be happy to help you implement comprehensive test coverage including native ETH bridging! Let me plan out additional test cases that would significantly improve the robustness of your test suite.
Task: Add comprehensive test coverage for BungeeApproveAndBridge
- Add test for native ETH bridging scenario
- Add failure case tests (insufficient balance, failed bridge calls)
- Add edge case tests (exact minimum amount, zero amount)
- Add test for output amount modification scenario
- Refactor common test setup into helper functions
This comprehensive test suite will cover:
- Native ETH bridging - Testing the contract's ability to handle ETH transfers using the special ETH address
- Failure scenarios - Insufficient balance and invalid calldata handling
- Edge cases - Exact minimum amounts to test boundary conditions
- Output modification - Testing the proportional adjustment logic when
modifyOutputAmountis true - Helper functions - Reducing code duplication for better maintainability
The tests will ensure your contract handles various scenarios robustly, especially the native ETH bridging which is crucial for a comprehensive bridging solution. Would you like me to proceed with implementing these test cases?
⏳ Preparing changes...
There was a problem hiding this comment.
A summary of the changes CodeRabbit can apply:
- Enhance the
test/e2e/BungeeApproveAndBridge.t.solfile by adding helper functions to set up test environments with funds or ETH, and implement comprehensive test cases covering native ETH bridging, insufficient balance failure, invalid calldata failure, exact minimum amount edge case, and output amount modification to ensure robust validation of the BungeeApproveAndBridge functionality.
- ✅ Create PR with these edits
- 📌 Create commit in current branch
- 📋 Get copyable edits
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
src/BungeeApproveAndBridge.sol
Outdated
| if (!success) revert BridgeFailed(); | ||
| } | ||
|
|
||
| function _parseAndModifyCalldata(uint256 amount, bytes calldata data) |
There was a problem hiding this comment.
Would it be worth it to make some unit tests for this function in isolation?
| abstract contract ApproveAndBridge is IApproveAndBridge { | ||
| using SafeERC20 for IERC20; | ||
|
|
||
| error MinAmountNotMet(); |
There was a problem hiding this comment.
nice to make a test for premature hook execution
| address(token) == NATIVE_TOKEN_ADDRESS ? address(this).balance : token.balanceOf(address(this)); | ||
|
|
||
| // check if the balance is greater than the minAmount | ||
| if (balance < minAmount) revert MinAmountNotMet(); |
There was a problem hiding this comment.
I don't think is an issue, but probably we should add in the @dev documentation that minAmount should be an amount not too small if the sell amount is big (basically, the order should have reasonable slippage tolerances). A slippage tolerance of 99.99% could make it almost free for anyone to "consume the hook" making the user's bridge to fail (by a donation of the minAmount).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/interface/IApproveAndBridge.sol (1)
2-2: Consider specifying a more precise Solidity version.Using
^0.8allows compilation with any 0.8.x version, which could lead to inconsistent behavior across different compiler versions. Consider specifying a more precise version range like^0.8.19or>=0.8.19 <0.9.0.-pragma solidity ^0.8; +pragma solidity ^0.8.19;src/BungeeApproveAndBridge.sol (2)
7-7: Update the warning comment before production use.The contract is marked as "UNAUDITED UNTESTED" which is appropriate for development, but remember to update this comment once proper auditing and testing are completed.
132-141: Potential precision loss in percentage calculations.The
_addPctDifffunction uses fixed-point arithmetic with 1e18 precision, which should be sufficient for most use cases. However, consider documenting the precision limitations and potential rounding behavior.Add a comment explaining the precision:
function _addPctDiff(uint256 _base, uint256 _compare, uint256 _target) internal pure returns (uint256) { // Base number must be greater than 0 // Compare number must be greater than or equal to base number + // Uses 1e18 precision for percentage calculations if (_base <= 0 || _compare < _base) revert InvalidInput();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/BungeeApproveAndBridge.sol(1 hunks)src/interface/IApproveAndBridge.sol(1 hunks)src/mixin/ApproveAndBridge.sol(1 hunks)test/e2e/BungeeApproveAndBridge.t.sol(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e/BungeeApproveAndBridge.t.sol
🔇 Additional comments (6)
src/interface/IApproveAndBridge.sol (1)
6-11: Interface design looks solid.The interface is well-structured with clear function signatures that support the approve-and-bridge pattern. The separation of concerns between the main bridging function and the approval target getter is good design.
src/mixin/ApproveAndBridge.sol (3)
22-26: Verify native token balance calculation logic.The native token balance calculation subtracts
nativeTokenExtraFeeand relies on underflow protection to revert if insufficient balance. This is a clever gas optimization, but ensure that the underflow behavior is well-documented and expected.
32-34: Good use of SafeERC20 for force approval.Using
forceApproveproperly handles tokens that require resetting allowances to zero before setting a new value. This is a best practice for ERC20 interactions.
40-42: Virtual function design enables proper extensibility.The abstract functions
bridgeApprovalTargetandbridgeprovide a clean interface for derived contracts to implement specific bridging logic while maintaining the common approval and validation workflow.src/BungeeApproveAndBridge.sol (2)
102-108: Assembly usage is correct and efficient.The assembly block properly calculates memory positions and performs in-place modifications. The bounds checking in line 99 protects against out-of-bounds writes.
119-125: Clean percentage difference logic.The
_applyPctDifffunction elegantly handles both positive and negative percentage changes by delegating to appropriate helper functions. The logic flow is clear and efficient.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/BungeeApproveAndBridge.sol (1)
75-104: Past review comment still valid - missing unit tests for complex logic.The complex calldata manipulation logic in
_parseAndModifyCalldatastill lacks dedicated unit tests as mentioned in previous reviews. This function handles:
- Byte-level parsing and modifications
- Proportional output adjustments
- In-place memory operations
The past review comment about missing unit tests is still valid. Consider adding comprehensive unit tests covering:
- Various input/output amount combinations
- Edge cases with different calldata sizes
- Percentage calculations with both increases and decreases
- Memory safety with different offsets
#!/bin/bash # Verify if unit tests have been added since the past review rg -A 20 -B 5 "parseAndModifyCalldata|_parseAndModifyCalldata" test/
🧹 Nitpick comments (2)
src/BungeeApproveAndBridge.sol (2)
174-183: Potential precision loss in percentage calculations.The percentage calculation uses 1e18 precision, which is good, but there's a potential for precision loss in integer division. Consider using a more robust approach or documenting the precision limitations.
function _addPctDiff(uint256 _base, uint256 _compare, uint256 _target) internal pure returns (uint256) { // Base number must be greater than 0 // Compare number must be greater than or equal to base number if (_base <= 0 || _compare < _base) revert InvalidInput(); // Calculate the percentage difference uint256 difference = ((_compare - _base) * 1e18) / _base; - // Apply percentage increase - return _target + ((_target * difference) / 1e18); + // Apply percentage increase with more precise calculation + return _target + ((_target * difference + 1e18 - 1) / 1e18);Consider documenting the precision limitations and potential rounding behavior.
188-197: Similar precision concerns in subtraction function.The
_subPctDifffunction has the same potential precision issues as_addPctDiff. Consider applying similar improvements for consistency.function _subPctDiff(uint256 _base, uint256 _compare, uint256 _target) internal pure returns (uint256) { // Base number must be greater than 0 // Compare number must be less than or equal to base number if (_base <= 0 || _compare > _base) revert InvalidInput(); // Calculate the percentage difference uint256 difference = ((_base - _compare) * 1e18) / _base; - // Apply percentage decrease - return _target - ((_target * difference) / 1e18); + // Apply percentage decrease with more precise calculation + uint256 decrease = (_target * difference + 1e18 - 1) / 1e18; + return _target >= decrease ? _target - decrease : 0;Added underflow protection for safety.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
script/Deploy.s.sol(1 hunks)src/BungeeApproveAndBridge.sol(1 hunks)test/BungeeApproveAndBridge.t.sol(1 hunks)test/e2e/BungeeApproveAndBridge.t.sol(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- test/e2e/BungeeApproveAndBridge.t.sol
- script/Deploy.s.sol
🔇 Additional comments (4)
src/BungeeApproveAndBridge.sol (4)
1-6: Warning comment accurately reflects development status.The warning comment on line 6 clearly indicates this is unaudited and untested code, which is appropriate for development phase contracts.
38-48: Constructor and approval target implementation is correct.The constructor properly sets the immutable
SOCKET_GATEWAYaddress, and thebridgeApprovalTarget()function correctly returns this address for token approvals.
58-67: Bridge function handles native vs ERC20 tokens correctly.The function properly distinguishes between native tokens and ERC20 tokens when making the external call, sending the appropriate ETH value in each case. The error handling with
BridgeFailed()revert is appropriate.
132-148: Assembly operation is correct but lacks comprehensive bounds checking.The
_replaceUint256function correctly uses assembly to modify memory in-place. The bounds check on line 138 is good, but consider adding alignment validation as suggested in the previous comment.
There was a problem hiding this comment.
The approach overall looks good.
The comments are mostly small improvements. Overall, I think it would be better to simplify the deployment script and add more tests (unit tests are ok) since many cases are untested.
(Don't worry too much about the AI-generated comments, it's an experiment we're running on this repo to determine their usefulness.)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
test/BungeeApproveAndBridge.t.sol (2)
25-27: Test coverage remains insufficient despite past feedback.While the constructor test is correct, the contract's complex calldata manipulation logic still lacks comprehensive testing coverage. This duplicates previous review feedback.
34-36: Critical functionality remains untested.Based on past review feedback, the contract's core calldata manipulation functions (
_parseAndModifyCalldata,_addPctDiff,_subPctDiff) are still missing comprehensive unit tests. The current test suite only covers basic percentage application scenarios.Consider adding unit tests for:
_parseAndModifyCalldatawith various calldata shapes, empty payloads, and memory safety_addPctDiffand_subPctDifffunctions with edge cases- Integration tests that validate the full calldata modification pipeline
#!/bin/bash # Verify the missing internal functions that need testing rg -n "function _parseAndModifyCalldata" --glob "*.sol" rg -n "function _addPctDiff" --glob "*.sol" rg -n "function _subPctDiff" --glob "*.sol"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/BungeeApproveAndBridge.t.sol(1 hunks)
🔇 Additional comments (4)
test/BungeeApproveAndBridge.t.sol (4)
1-2: LGTM!Standard license and pragma declaration are appropriate for the test file.
4-6: LGTM!Clean and appropriate imports for the test file.
8-14: LGTM!Good testing pattern to expose internal functions for unit testing. The helper contract implementation is correct.
16-23: LGTM!Standard and clean test contract setup with proper initialization in the setUp function.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/BungeeApproveAndBridge.sol (1)
76-105: Add validation for proper index alignment.As previously noted, the function should validate that indices are properly aligned at 32-byte boundaries for uint256 values.
function _parseAndModifyCalldata(uint256 amount, bytes calldata data) internal pure returns (bytes memory) { // Parse the data into route calldata and ModifyCalldataParams (bytes memory routeCalldata, ModifyCalldataParams memory modifyCalldataParams) = _parseCalldata(data); + + // Validate indices are properly aligned for uint256 values + if (modifyCalldataParams.inputAmountIdx % 32 != 0) revert InvalidInput(); + if (modifyCalldataParams.modifyOutput && modifyCalldataParams.outputAmountIdx % 32 != 0) { + revert InvalidInput(); + } // Read the original input amount from the calldata // before modifying input amount uint256 originalInput = _readUint256({_data: routeCalldata, _index: modifyCalldataParams.inputAmountIdx});
🧹 Nitpick comments (2)
src/BungeeApproveAndBridge.sol (2)
2-2: Use a specific Solidity version for consistency.Consider using a specific Solidity version instead of
^0.8to ensure consistent compilation across different environments.-pragma solidity ^0.8; +pragma solidity ^0.8.20;
7-7: Fix typo in the warning comment.There's an extra space in the comment directive.
-/// ! @dev UNAUDITED UNTESTED Do not use in production +/// @dev UNAUDITED UNTESTED Do not use in production
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/BungeeApproveAndBridge.sol(1 hunks)src/vendored/Math.sol(1 hunks)src/vendored/Panic.sol(1 hunks)src/vendored/SafeCast.sol(1 hunks)test/BungeeApproveAndBridge.t.sol(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/vendored/Panic.sol
🚧 Files skipped from review as they are similar to previous changes (1)
- test/BungeeApproveAndBridge.t.sol
🔇 Additional comments (11)
src/BungeeApproveAndBridge.sol (7)
12-35: Well-structured constants and error definitions.The error declarations are descriptive, and the constants are properly typed and documented. Good use of uint8 for small constant values to optimize storage.
43-50: Clean implementation of approval target getter.The function correctly returns the socket gateway address for token approvals.
59-68: Proper implementation of bridge forwarding logic.The function correctly handles both native and ERC20 tokens, modifies calldata appropriately, and forwards the call to the SocketGateway with proper error handling.
113-127: Well-implemented calldata parsing logic.The function correctly validates minimum length, extracts route calldata using array slicing, and decodes the modification parameters from the end of the data.
133-149: Efficient in-place memory modification.The function correctly validates bounds and uses assembly for efficient in-place modification of the bytes data.
154-159: Safe uint256 reading with bounds checking.Proper implementation with bounds validation before reading from memory.
164-167: Overflow-safe percentage calculation.Good use of Math.mulDiv to handle potential overflows when calculating percentage differences, addressing previous concerns about tokens with many atoms.
src/vendored/Math.sol (2)
1-11: Well-documented vendored library with proper attribution.Good practice including the specific OpenZeppelin version and commit hash for traceability. The vendored approach with local dependencies ensures version consistency.
16-690: High-quality math utilities from OpenZeppelin.This vendored Math library provides robust implementations of essential mathematical operations with proper overflow protection. The mulDiv function (lines 149-228) is particularly relevant for the percentage calculations in BungeeApproveAndBridge, providing overflow-safe multiplication and division.
src/vendored/SafeCast.sol (2)
1-10: Properly vendored SafeCast library.Well-documented vendoring from OpenZeppelin with specific version reference. This library provides essential safe casting operations used by other components.
23-1166: Comprehensive safe casting utilities.The SafeCast library provides exhaustive coverage of safe type conversions with proper overflow/underflow checking through custom errors. The toUint(bool) function (lines 1161-1165) is particularly used by the Math library for branchless operations.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
script/Deploy.s.sol (2)
15-15: Consider using a more systematic salt management approach.The hardcoded salt with a manual change comment could lead to deployment errors. While past discussions indicate this will be changed to ordered numbers starting from 0 for production, consider implementing a more systematic approach.
41-41: Potential salt truncation issue in CREATE2 deployment.The salt is cast to
bytes32(salt)wheresaltis auint256. For large values like9999999999999999999999, this truncation could result in address collisions as discussed in past reviews.src/BungeeApproveAndBridge.sol (2)
40-42: Add zero address validation in constructor.The constructor should validate that the socket gateway address is not zero to prevent misconfiguration.
constructor(address socketGateway_) { + if (socketGateway_ == address(0)) revert InvalidInput(); SOCKET_GATEWAY = socketGateway_; }
77-106: Add bounds validation for calldata indices.The function performs byte-level operations but lacks bounds checking before accessing array positions. Past reviews highlighted this concern.
Add validation to prevent out-of-bounds access:
function _parseAndModifyCalldata(uint256 amount, bytes calldata data) internal pure returns (bytes memory) { // Parse the data into route calldata and ModifyCalldataParams (bytes memory routeCalldata, ModifyCalldataParams memory modifyCalldataParams) = _parseCalldata(data); + + // Validate indices are within bounds + if (modifyCalldataParams.inputAmountIdx + 32 > routeCalldata.length) revert PositionOutOfBounds(); + if (modifyCalldataParams.modifyOutput && modifyCalldataParams.outputAmountIdx + 32 > routeCalldata.length) revert PositionOutOfBounds();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.vscode/settings.json(1 hunks)foundry.toml(1 hunks)script/Deploy.s.sol(1 hunks)src/BungeeApproveAndBridge.sol(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- foundry.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- .vscode/settings.json
🔇 Additional comments (5)
script/Deploy.s.sol (1)
24-48: LGTM - Well-structured deployment logic with proper safeguards.The deployment function properly handles:
- Environment-based deployment control
- Existing contract detection
- Address verification after deployment
- Clear logging for debugging
src/BungeeApproveAndBridge.sol (4)
60-69: LGTM - Proper native token handling and error management.The bridge function correctly:
- Handles both ERC20 and native token transfers
- Includes native token extra fees in the value sent
- Properly reverts on failed external calls
- Uses low-level calls for flexibility
165-168: LGTM - Improved mathematical operations with overflow protection.The use of
Math.mulDivaddresses previous concerns about potential overflow in percentage calculations. The zero base validation prevents division by zero errors.
114-128: LGTM - Robust calldata parsing with proper validation.The
_parseCalldatafunction properly:
- Validates minimum data length requirements
- Correctly slices calldata for route and parameters
- Uses safe ABI decoding for structured data
- Handles the boundary between route calldata and modification parameters
155-160: Verify test coverage for_readUint256boundary conditionsI wasn’t able to find any tests exercising this internal helper. Please confirm that your test suite (Solidity or JS) includes unit tests that cover:
- Reading at the exact upper bound:
_data.length == _index + 32should succeed.- Out-of-bounds access:
_data.length < _index + 32must revert with thePositionOutOfBoundscustom error.- Typical positions (e.g. index 0 and a middle offset) to ensure correct values are returned.
If these tests already exist, verify they correctly assert a revert via
.revertedWithCustomError(contract, "PositionOutOfBounds").
| function _replaceUint256(bytes memory _original, uint256 _start, uint256 _amount) | ||
| internal | ||
| pure | ||
| returns (bytes memory) | ||
| { | ||
| // check if the _start is out of bounds | ||
| if (_start + 32 > _original.length) revert PositionOutOfBounds(); | ||
|
|
||
| // Directly modify externalData in-place without creating a new copy | ||
| assembly { | ||
| // Calculate position in memory where we need to write the new amount | ||
| // Write the amount at that position | ||
| mstore(add(add(_original, 32), _start), _amount) | ||
| } | ||
|
|
||
| return _original; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Assembly operations look correct but need comprehensive testing.
The _replaceUint256 function uses assembly to modify memory in-place. The bounds check and memory offset calculations appear correct, but this low-level code requires thorough testing.
Consider adding unit tests specifically for this function to verify:
- Correct replacement at various positions
- Proper bounds checking behavior
- Memory safety with different input sizes
🏁 Script executed:
#!/bin/bash
# Search for existing tests covering _replaceUint256 function
rg -A 10 -B 5 "_replaceUint256" --type sol
rg -n "_replaceUint256.*test" .Length of output: 104
🏁 Script executed:
#!/bin/bash
# Search for occurrences of _replaceUint256 across all Solidity files
rg -n "_replaceUint256" -g "*.sol"
# Search for tests referencing the function in JS/TS test files
rg -n "_replaceUint256" -g "test/**/*.js" -g "test/**/*.ts"Length of output: 600
Add comprehensive unit tests for _replaceUint256
I didn’t find any existing tests covering this function in src/BungeeApproveAndBridge.sol, so please add dedicated unit tests to ensure its correctness and safety:
• Verify correct replacement of a 32-byte word at various byte offsets (start, middle, end)
• Confirm that _start + 32 > original.length triggers the PositionOutOfBounds() revert
• Exercise different input lengths (empty, exactly 32 bytes, larger payloads) to catch any memory-safety issues
File to cover:
- src/BungeeApproveAndBridge.sol lines 134–150
🤖 Prompt for AI Agents
In src/BungeeApproveAndBridge.sol around lines 134 to 150, the function
_replaceUint256 lacks unit tests. Add comprehensive tests that verify correct
replacement of a 32-byte word at various offsets including start, middle, and
end of the input bytes. Also, test that the function reverts with
PositionOutOfBounds when _start + 32 exceeds the input length. Include tests
with different input lengths such as empty bytes, exactly 32 bytes, and larger
payloads to ensure memory safety and correctness.
There was a problem hiding this comment.
🧩 Analysis chain
@sebastiantf Perfect! I can see you've added comprehensive tests for _replaceUint256 that cover exactly what was needed:
✅ Bounds checking: Tests verify PositionOutOfBounds revert when _start + 32 > length
✅ Various offsets: Tests replacement at different positions within the bytes array
✅ Edge cases: Tests with empty bytes, exact 32-byte arrays, and larger payloads
✅ Memory safety: Tests ensure no corruption outside the replaced segment
✅ Multiple replacements: Tests verify the function works correctly for sequential modifications
The test coverage looks thorough and addresses all the memory safety concerns I raised. Thanks for implementing these comprehensive tests!
(\ /)
( ..)
o(")(") Great test coverage!
🏁 Script executed:
#!/bin/bash
# Look for tests related to _replaceUint256 function
rg -A 15 -B 5 "_replaceUint256" --type sol
# Check if there are test functions specifically for _replaceUint256
rg -A 10 "test.*replaceUint256" --type sol
# Look at the current test structure in BungeeApproveAndBridge test file
fd "BungeeApproveAndBridge.t.sol" --exec cat {}
Length of output: 70979
@sebastiantf Perfect! I can see you've added comprehensive tests for _replaceUint256 that cover exactly what was needed:
✅ Bounds checking: Tests verify PositionOutOfBounds revert when _start + 32 > length
✅ Various offsets: Tests replacement at different positions within the bytes array
✅ Edge cases: Tests with empty bytes, exact 32-byte arrays, and larger payloads
✅ Memory safety: Tests ensure no corruption outside the replaced segment
✅ Multiple replacements: Tests verify the function works correctly for sequential modifications
The test coverage looks thorough and addresses all the memory safety concerns I raised. Thanks for implementing these comprehensive tests!
(\ /)
( ..)
o(")(") Great test coverage!
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
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 (1)
script/Deploy.s.sol(1 hunks)
🔇 Additional comments (3)
script/Deploy.s.sol (3)
1-11: LGTM! Clean contract setup.The imports, contract declaration, and constant definition follow standard Foundry deployment script patterns. The hardcoded
SOCKET_GATEWAYaddress is appropriate given it's consistently deployed across chains.
12-20: LGTM! Deployment logic is correct.The
run()function properly handles the deployment flow with appropriate logging. The hardcoded salt with the comment aligns with the discussion from previous reviews where maintainers decided against environment variable configuration.
28-34: LGTM! CREATE2 deployment implementation is correct.The deployment uses proper
bytes32(salt)casting and correct constructor arguments. The logging and return logic is appropriate for a deployment function.
| constructor(address socketGateway_) { | ||
| SOCKET_GATEWAY = socketGateway_; | ||
| } |
There was a problem hiding this comment.
How is the Socket gateway deployed in the supported chains? I see it uses a custom CREATEd deployer, but its code isn't verified on Etherscan. Can I assume it's a permissioned contract and that the that the gateway will be deployed only if authorized internally in some way by Socket?
A big front-end risk for us is users sending native tokens to an address that doesn't use them because it's in the wrong network, effectively losing the sent native tokens. This could be the case if the gateway has no code for example (and the user tries to send native tokens in the wrong chain), or if it's CREATE2'd (so it has code) but there's no way to pull out money from it if called with parameters from another chain. For the first issue, we can revert here if there's no code at the gateway (and then the shed's delegatecall does nothing). For the second issue it depends on how the gateway is deployed on your end.
There was a problem hiding this comment.
Here's one of the deployment transaction: https://optimistic.etherscan.io/tx/0x70d7ffb69b8cd0a20d48d2bbab52b1195661cd6f6a54501e66cbc17aca85f69f
We use a CREATE3 factory with a protected namespace for the deployer under a protected private key.
Most chains SocketGateway is deployed on the 0x3a23F943181408EAC424116Af7b7790c94Cb97a5. Can find the deployment addresses here: https://github.com/SocketDotTech/socket-ll-contracts/tree/master/deployments
Atleast all the chains that we're currently going live on mainnet, polygon, arbitrumOne, base, optimism, SocketGateway is deployed on the same address.
Will check & verify before expansion whether there are differences
There was a problem hiding this comment.
If I understand Federico's concern, I agree with it, the wrapper approveAndBridge is not checking the bridgeApprovalTarget() address has code.
This could result in bridge(token, balance, nativeTokenExtraFee, data); call succeeding (sends the ETH to the contract, but it doesn't invoke any logic because there's no code.
I guess the simplest is to make the assertion in the constructor
| constructor(address socketGateway_) { | |
| SOCKET_GATEWAY = socketGateway_; | |
| } | |
| constructor(address socketGateway_) { | |
| require(socketGateway_.code.length > 0, "Socket gateway contract not deployed"); | |
| SOCKET_GATEWAY = socketGateway_; | |
| } |
We would also probably double check in the SDK the wrapper is deployed (if the wrapper is deployed, then the gateway is deployed too.
On the other problem you raised. Normally If someone sends a tx intended for one chain to another, would be because of a wallet/dapp failure, but is good we think on what would happen.
However, @fedgiac, correct me if I'm wrong here, this is problematic if a dapp sends some tx data to the wallet thinking is one chain, but then the wallet sends it to another chain. But here, because we use cow-shed we should be safe, because cow-shed uses EIP712 which includes the chainId in the signature. So The SDK will know precisely the chain used in the signature and will ensure that that chain has a wrapper.
Additionally, I saw that as a last resort, both the BridgeBaseImpl and the SocketGateway has a permissioned recue funds rescueEther and rescueFunds. So worst case I understand the contract can be deployed by the socket team, and the funds will be recoverable.
There was a problem hiding this comment.
because we use cow-shed we should be safe, because cow-shed uses EIP712 which includes the chainId in the signature
Very good point. Then this would be an issue only for people using the presign flow.
Considering it's very easy, I'd still check that the gateway has code on deployment, otherwise I don't think we need to do anything else.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
test/e2e/BungeeApproveAndBridge.t.sol (1)
54-103: Consider extracting common test setup logic.This test function correctly validates the Across USDC bridging scenario, but there's significant code duplication across the three test functions. Based on past review comments, this duplication could be reduced with helper functions.
Consider extracting common setup logic:
+ function _setupShedWithUSDC(uint256 amount) internal returns (COWShed) { + factory.initializeProxy(user, false); + COWShed shed = COWShed(factory.proxyOf(user)); + vm.label(address(shed), "shed"); + assertGt(address(shed).code.length, 0); + + vm.prank(user); + USDC.transfer(address(shed), amount); + assertEq(USDC.balanceOf(address(shed)), amount); + return shed; + }
🧹 Nitpick comments (1)
test/mocks/MockERC20.sol (1)
43-47: Consider removing redundant forceApprove function.The
forceApprovefunction is functionally identical to the standardapprovefunction. Since this is a mock contract and the standardapprovealready allows setting any allowance without restrictions, this function appears redundant.- function forceApprove(address spender, uint256 amount) external returns (bool) { - allowance[msg.sender][spender] = amount; - emit Approval(msg.sender, spender, amount); - return true; - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
test/BungeeApproveAndBridge.t.sol(1 hunks)test/e2e/BungeeApproveAndBridge.t.sol(1 hunks)test/mocks/MockBridge.sol(1 hunks)test/mocks/MockERC20.sol(1 hunks)
🔇 Additional comments (16)
test/mocks/MockBridge.sol (1)
5-9: LGTM! Simple and effective mock for testing bridge failures.The fallback function correctly simulates bridge call failures by reverting with a clear error message. This mock will be useful for testing error handling in the BungeeApproveAndBridge contract.
test/mocks/MockERC20.sol (5)
6-17: LGTM! Standard ERC20 state variables and constructor implementation.The contract correctly implements the standard ERC20 state variables and initializes the total supply and deployer balance in the constructor.
19-25: LGTM! Correct transfer implementation.The transfer function properly checks for sufficient balance, updates balances, and emits the Transfer event as per ERC20 standard.
33-41: LGTM! Correct transferFrom implementation.The transferFrom function properly checks both balance and allowance, updates all necessary state variables, and emits the Transfer event.
50-54: LGTM! Correct mint implementation for testing.The mint function properly increases both the recipient's balance and total supply, and emits a Transfer event from the zero address as per ERC20 standard.
57-62: LGTM! Correct burn implementation for testing.The burn function properly checks for sufficient balance, decreases both the holder's balance and total supply, and emits a Transfer event to the zero address.
test/e2e/BungeeApproveAndBridge.t.sol (4)
44-52: LGTM! Proper test setup with fork configuration.The setUp function correctly configures the fork at a specific block, deploys the contract, and sets up proper labeling for debugging.
105-155: LGTM! Comprehensive ETH bridging test.This test properly validates native ETH bridging using the special NATIVE_TOKEN_ADDRESS. The test setup, execution, and assertions are all correct.
157-206: LGTM! CCTP bridging test with proper validation.This test correctly validates CCTP USDC bridging with
modifyOutputAmountset to false, which is appropriate for CCTP where output amounts are typically fixed.
95-95: approveAndBridge signature matches the interface — no changes neededThe
IApproveAndBridgeinterface insrc/interface/IApproveAndBridge.soldeclares:function approveAndBridge( IERC20 token, uint256 minAmount, uint256 nativeTokenExtraFee, bytes calldata data ) external;All calls (
USDC, minProceeds, additionalValue, _calldata) on lines 95, 146, and 198 correctly pass four arguments. You can disregard the original comment.Likely an incorrect or invalid review comment.
test/BungeeApproveAndBridge.t.sol (6)
12-42: Excellent helper contract design for testing internal functions.The
PublicBungeeApproveAndBridgewrapper follows testing best practices by exposing internal functions for unit testing while maintaining clean separation from the main contract logic.
53-81: Comprehensive test setup with proper mock configuration.The test setup properly handles the contract deployment simulation using
vm.etchand includes thorough constructor validation tests covering both valid contracts and edge cases for non-contract addresses.
86-349: Exceptional test coverage for percentage calculations.The
_applyPctDifftests comprehensively address all edge cases including zero values, maximum uint256 values, intermediate overflow scenarios, rounding behavior, and extreme ratios. This addresses the previous review feedback about insufficient edge case testing.
354-898: Comprehensive calldata manipulation testing.The tests for
_replaceUint256,_parseCalldata, and_parseAndModifyCalldataprovide thorough coverage including memory safety, bounds checking, ABI encoding consistency, and proper error handling for invalid inputs.
904-1194: Excellent integration test coverage.The integration tests comprehensively cover the full bridging workflow including insufficient balance scenarios, failed bridge calls, edge cases with various amount values, and proper ERC20 approval behavior. The use of mock contracts and proper assertion techniques demonstrates solid testing practices.
1-1194: Outstanding comprehensive test suite.This test file successfully addresses all previous review feedback about insufficient unit test coverage. The structure is excellent with clear organization, consistent naming conventions, and comprehensive coverage of both unit-level internal functions and integration scenarios. The tests properly validate:
- All internal calldata manipulation functions
- Percentage calculation edge cases and overflow scenarios
- Error handling and boundary conditions
- Full bridging workflow with various token types
- Mock-based testing for external dependencies
|
What is left to merge this PR? |
|
@sebastiantf Can we make sure this PR is finalized and merged? |
|
@anxolin I've already responded to all review comments and made changes accordingly. You can go ahead and merge if you're satisfied and no more comments to resolve |
Okay, I missunderstood, I was not aware you wanted another review. I usually check the state here (and see my notifications for requests for review). We will re-review 
|
anxolin
left a comment
anxolin
left a comment
There was a problem hiding this comment.
Thanks a lot for adding so much coverage of the tests.
Before we merge, do you think you can remove the warnings we get compiling the project?
|
These are all warnings in the test contracts. Can safely ignore |
3 participants
Summary by CodeRabbit
New Features
BungeeApproveAndBridgecontract, enabling dynamic token bridging with customizable calldata adjustments.IERC20interface,SafeERC20library,Math,Panic, andSafeCastutilities for robust and secure token and math operations.Bug Fixes
Tests
Countercontract.Chores
Countercontract and related deployment scripts.