-
Notifications
You must be signed in to change notification settings - Fork 64
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
chore: more permissive solidity pragma #442
chore: more permissive solidity pragma #442
Conversation
📝 WalkthroughWalkthroughThis pull request involves a comprehensive update to the Solidity version pragma across multiple contract files in the project. The changes consistently modify the version specification from a fixed version Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (19)
✅ Files skipped from review due to trivial changes (19)
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
|
There was a problem hiding this 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
🧹 Nitpick comments (5)
v1/contracts/zevm/interfaces/zContract.sol (1)
Line range hint
10-12
: Consider reusing or unifyingzContext
andrevertContext
StructrevertContext
closely resembleszContext
. If the fields and usage are mostly similar, consider abstracting common elements to reduce duplication and enforce consistency.v2/test/utils/SystemContractMock.sol (1)
Line range hint
39-41
: Minor event emission detail
Currently, theSetWZeta
event emits the updated storage variablewZetaContractAddress
. For clarity, consider directly emitting the function parameteraddr
if that more accurately reflects the new value being set.function setWZETAContractAddress(address addr) external { wZetaContractAddress = addr; - emit SetWZeta(wZetaContractAddress); + emit SetWZeta(addr); }v2/scripts/deploy/deterministic/UpgradeGatewayEVM.s.sol (1)
Line range hint
21-22
: Consider adding more comprehensive upgrade validation.While the basic state checks for
tssAddress
andzetaToken
are good, consider adding more comprehensive validation:
- Verify all storage slots that should remain unchanged
- Add assertions for any new state variables
- Validate any modified function behaviors
v2/scripts/upgrade/SimulateERC20CustodyUpgrade.s.sol (1)
2-2
: Consider pinning the version for deployment scriptsWhile making the pragma more permissive is the goal, deployment and upgrade scripts often benefit from using fixed versions to ensure reproducible deployments. Consider keeping this script pinned to
0.8.26
to maintain deployment consistency.v1/contracts/evm/ERC20Custody.sol (1)
3-3
: Consider adding an upper bound to the pragma versionWhile the change to
^0.8.7
is safe as the 0.8.x series maintains backwards compatibility, consider adding an upper bound (e.g.,pragma solidity ^0.8.7 <0.9.0;
) to prevent potential compatibility issues with future 0.9.x releases, especially given this is a critical custody contract handling user funds.-pragma solidity ^0.8.7; +pragma solidity ^0.8.7 <0.9.0;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (80)
v1/contracts/evm/ERC20Custody.sol
(1 hunks)v1/contracts/evm/Zeta.eth.sol
(1 hunks)v1/contracts/evm/ZetaConnector.base.sol
(1 hunks)v1/contracts/evm/ZetaConnector.eth.sol
(1 hunks)v1/contracts/evm/ZetaConnector.non-eth.sol
(1 hunks)v1/contracts/evm/interfaces/ConnectorErrors.sol
(1 hunks)v1/contracts/evm/interfaces/ZetaInteractorErrors.sol
(1 hunks)v1/contracts/evm/testing/ERC20Mock.sol
(1 hunks)v1/contracts/evm/testing/ZetaInteractorMock.sol
(1 hunks)v1/contracts/evm/testing/ZetaReceiverMock.sol
(1 hunks)v1/contracts/evm/tools/ZetaInteractor.sol
(1 hunks)v1/contracts/evm/tools/ZetaTokenConsumerPancakeV3.strategy.sol
(1 hunks)v1/contracts/evm/tools/ZetaTokenConsumerTrident.strategy.sol
(1 hunks)v1/contracts/evm/tools/ZetaTokenConsumerUniV2.strategy.sol
(1 hunks)v1/contracts/evm/tools/ZetaTokenConsumerUniV3.strategy.sol
(1 hunks)v1/contracts/evm/tools/ZetaTokenConsumerZEVM.strategy.sol
(1 hunks)v1/contracts/zevm/SystemContract.sol
(1 hunks)v1/contracts/zevm/ZRC20.sol
(1 hunks)v1/contracts/zevm/ZetaConnectorZEVM.sol
(1 hunks)v1/contracts/zevm/interfaces/ISystem.sol
(1 hunks)v1/contracts/zevm/interfaces/IUniswapV2Router01.sol
(1 hunks)v1/contracts/zevm/interfaces/IUniswapV2Router02.sol
(1 hunks)v1/contracts/zevm/interfaces/IWZETA.sol
(1 hunks)v1/contracts/zevm/interfaces/IZRC20.sol
(1 hunks)v1/contracts/zevm/interfaces/zContract.sol
(1 hunks)v1/contracts/zevm/testing/SystemContractMock.sol
(1 hunks)v2/contracts/Errors.sol
(1 hunks)v2/contracts/Revert.sol
(1 hunks)v2/contracts/evm/ERC20Custody.sol
(1 hunks)v2/contracts/evm/GatewayEVM.sol
(1 hunks)v2/contracts/evm/ZetaConnectorBase.sol
(1 hunks)v2/contracts/evm/ZetaConnectorNative.sol
(1 hunks)v2/contracts/evm/ZetaConnectorNonNative.sol
(1 hunks)v2/contracts/evm/interfaces/IERC20Custody.sol
(1 hunks)v2/contracts/evm/interfaces/IGatewayEVM.sol
(1 hunks)v2/contracts/evm/interfaces/IZetaConnector.sol
(1 hunks)v2/contracts/evm/interfaces/IZetaNonEthNew.sol
(1 hunks)v2/contracts/zevm/GatewayZEVM.sol
(1 hunks)v2/contracts/zevm/SystemContract.sol
(1 hunks)v2/contracts/zevm/ZRC20.sol
(1 hunks)v2/contracts/zevm/interfaces/IGatewayZEVM.sol
(1 hunks)v2/contracts/zevm/interfaces/ISystem.sol
(1 hunks)v2/contracts/zevm/interfaces/IWZETA.sol
(1 hunks)v2/contracts/zevm/interfaces/IZRC20.sol
(1 hunks)v2/contracts/zevm/interfaces/UniversalContract.sol
(1 hunks)v2/scripts/deploy/DeployZRC20.s.sol
(1 hunks)v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol
(1 hunks)v2/scripts/deploy/deterministic/DeployERC20CustodyImplementation.s.sol
(1 hunks)v2/scripts/deploy/deterministic/DeployGatewayEVM.s.sol
(1 hunks)v2/scripts/deploy/deterministic/DeployGatewayEVMImplementation.s.sol
(1 hunks)v2/scripts/deploy/deterministic/DeployGatewayZEVM.s.sol
(1 hunks)v2/scripts/deploy/deterministic/DeployGatewayZEVMImplementation.s.sol
(1 hunks)v2/scripts/deploy/deterministic/DeployTestERC20.s.sol
(1 hunks)v2/scripts/deploy/deterministic/DeployZetaConnectorNonNative.s.sol
(1 hunks)v2/scripts/deploy/deterministic/UpgradeGatewayEVM.s.sol
(1 hunks)v2/scripts/upgrade/SimulateERC20CustodyUpgrade.s.sol
(1 hunks)v2/scripts/upgrade/SimulateGatewayEVMUpgrade.s.sol
(1 hunks)v2/scripts/upgrade/SimulateGatewayZEVMUpgrade.s.sol
(1 hunks)v2/test/ERC20Custody.t.sol
(1 hunks)v2/test/GatewayEVM.t.sol
(1 hunks)v2/test/GatewayEVMZEVM.t.sol
(1 hunks)v2/test/GatewayZEVM.t.sol
(1 hunks)v2/test/ZRC20.t.sol
(1 hunks)v2/test/ZetaConnectorNative.t.sol
(1 hunks)v2/test/ZetaConnectorNonNative.t.sol
(1 hunks)v2/test/fuzz/ERC20CustodyEchidnaTest.sol
(1 hunks)v2/test/fuzz/GatewayEVMEchidnaTest.sol
(1 hunks)v2/test/utils/IReceiverEVM.sol
(1 hunks)v2/test/utils/ReceiverEVM.sol
(1 hunks)v2/test/utils/SenderZEVM.sol
(1 hunks)v2/test/utils/SystemContractMock.sol
(1 hunks)v2/test/utils/TestERC20.sol
(1 hunks)v2/test/utils/TestUniversalContract.sol
(1 hunks)v2/test/utils/WZETA.sol
(1 hunks)v2/test/utils/Zeta.non-eth.sol
(1 hunks)v2/test/utils/upgrades/ERC20CustodyUpgradeTest.sol
(1 hunks)v2/test/utils/upgrades/GatewayEVMUpgradeTest.sol
(1 hunks)v2/test/utils/upgrades/GatewayZEVMUpgradeTest.sol
(1 hunks)v2/test/utils/upgrades/ZetaConnectorNativeUpgradeTest.sol
(1 hunks)v2/test/utils/upgrades/ZetaConnectorNonNativeUpgradeTest.sol
(1 hunks)
✅ Files skipped from review due to trivial changes (59)
- v1/contracts/evm/ZetaConnector.non-eth.sol
- v2/test/utils/IReceiverEVM.sol
- v2/contracts/evm/interfaces/IZetaNonEthNew.sol
- v2/contracts/evm/ZetaConnectorBase.sol
- v2/contracts/zevm/interfaces/IZRC20.sol
- v1/contracts/zevm/interfaces/IUniswapV2Router01.sol
- v2/test/utils/WZETA.sol
- v1/contracts/zevm/SystemContract.sol
- v2/scripts/upgrade/SimulateGatewayEVMUpgrade.s.sol
- v2/test/GatewayEVMZEVM.t.sol
- v2/contracts/evm/interfaces/IGatewayEVM.sol
- v1/contracts/zevm/testing/SystemContractMock.sol
- v1/contracts/evm/testing/ERC20Mock.sol
- v2/contracts/zevm/interfaces/IWZETA.sol
- v1/contracts/evm/tools/ZetaTokenConsumerPancakeV3.strategy.sol
- v2/contracts/Errors.sol
- v2/scripts/deploy/deterministic/DeployZetaConnectorNonNative.s.sol
- v2/scripts/deploy/deterministic/DeployGatewayZEVMImplementation.s.sol
- v2/scripts/deploy/deterministic/DeployGatewayZEVM.s.sol
- v1/contracts/evm/ZetaConnector.eth.sol
- v1/contracts/evm/tools/ZetaTokenConsumerTrident.strategy.sol
- v1/contracts/zevm/interfaces/IZRC20.sol
- v1/contracts/zevm/interfaces/ISystem.sol
- v2/test/ZetaConnectorNative.t.sol
- v1/contracts/evm/tools/ZetaInteractor.sol
- v2/test/utils/SenderZEVM.sol
- v1/contracts/evm/interfaces/ConnectorErrors.sol
- v2/contracts/evm/ERC20Custody.sol
- v1/contracts/zevm/interfaces/IWZETA.sol
- v1/contracts/evm/testing/ZetaReceiverMock.sol
- v2/contracts/evm/interfaces/IERC20Custody.sol
- v2/contracts/zevm/interfaces/IGatewayZEVM.sol
- v2/test/ZRC20.t.sol
- v2/test/utils/ReceiverEVM.sol
- v1/contracts/evm/tools/ZetaTokenConsumerUniV2.strategy.sol
- v2/test/utils/Zeta.non-eth.sol
- v1/contracts/zevm/ZetaConnectorZEVM.sol
- v1/contracts/evm/tools/ZetaTokenConsumerZEVM.strategy.sol
- v2/contracts/Revert.sol
- v2/contracts/zevm/interfaces/ISystem.sol
- v2/contracts/zevm/SystemContract.sol
- v1/contracts/evm/interfaces/ZetaInteractorErrors.sol
- v2/scripts/upgrade/SimulateGatewayZEVMUpgrade.s.sol
- v2/test/fuzz/GatewayEVMEchidnaTest.sol
- v2/contracts/zevm/GatewayZEVM.sol
- v2/contracts/evm/ZetaConnectorNonNative.sol
- v2/test/GatewayEVM.t.sol
- v2/test/ZetaConnectorNonNative.t.sol
- v1/contracts/evm/ZetaConnector.base.sol
- v2/scripts/deploy/DeployZRC20.s.sol
- v2/contracts/evm/GatewayEVM.sol
- v2/contracts/zevm/ZRC20.sol
- v2/test/GatewayZEVM.t.sol
- v2/test/utils/upgrades/ZetaConnectorNonNativeUpgradeTest.sol
- v2/contracts/evm/ZetaConnectorNative.sol
- v2/scripts/deploy/deterministic/DeployGatewayEVM.s.sol
- v2/test/utils/TestUniversalContract.sol
- v2/test/fuzz/ERC20CustodyEchidnaTest.sol
- v1/contracts/zevm/ZRC20.sol
🧰 Additional context used
📓 Learnings (1)
v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol (1)
Learnt from: lumtis
PR: zeta-chain/protocol-contracts#419
File: v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol:68-86
Timestamp: 2024-11-12T13:20:12.658Z
Learning: In the `ERC20Custody` contract, the `WITHDRAWER_ROLE` is intentionally not granted to the admin during the admin transition in the `DeployERC20Custody` deployment script.
🔇 Additional comments (23)
v2/test/utils/upgrades/GatewayZEVMUpgradeTest.sol (1)
2-2
: Switched from pinned compiler version to caret range.
This update to pragma solidity ^0.8.26;
allows for more permissive patch and minor version usage within 0.8.x. Ensure that this broader range is tested with newer compiler releases to avoid unexpected incompatibilities.
v1/contracts/zevm/interfaces/zContract.sol (2)
2-2
: Use of caret version for Solidity
Great job updating the pragma from a fixed to a caret version. This provides more flexibility with newer minor releases while retaining the same major version, expanding compatibility.
Line range hint 28-34
: New onRevert
function
The new onRevert
function definition complements the existing cross-chain interface. Ensure that you have corresponding tests to verify edge cases and call correctness.
Would you like assistance creating tests that trigger and validate this new onRevert
method?
v2/contracts/zevm/interfaces/UniversalContract.sol (1)
2-2
: More permissive Solidity pragma
Switching to ^0.8.26
aligns with the upgrades throughout the codebase. Keep an eye on any compiler-related warnings or potential breaking changes if a future minor release of Solidity alters language features.
v2/scripts/deploy/deterministic/DeployERC20CustodyImplementation.s.sol (1)
2-2
: Consistent caret usage in deployment script
Maintaining the same caret version as the rest of the codebase helps keep deployments in sync with the contract versions deployed. This looks good.
v2/test/utils/SystemContractMock.sol (1)
2-2
: Perpetuating caret version in tests
Using ^0.8.26
ensures the test utilities stay aligned with the main contracts’ compiler versions.
v2/test/utils/upgrades/ZetaConnectorNativeUpgradeTest.sol (1)
2-2
: Adopt caret syntax for Solidity compiler version.
Switching from a pinned version (0.8.26
) to a caret version (^0.8.26
) provides flexibility when compiling with higher patch and minor releases within the 0.8 series. This approach is typically safe for production, but keep an eye on compiler release notes to ensure that newly introduced changes don’t break existing functionality.
v2/scripts/deploy/deterministic/DeployERC20Custody.s.sol (1)
2-2
: Use of caret version for broader Solidity compatibility.
Changing pragma solidity 0.8.26;
to pragma solidity ^0.8.26;
allows leveraging beneficial compiler optimizations in newer 0.8.x releases. Confirm that your contracts and tests remain valid across these minor and patch updates to avoid unforeseen breakages.
v2/test/utils/upgrades/ERC20CustodyUpgradeTest.sol (1)
2-2
: Enabling permissive Solidity version.
By adopting ^0.8.26
, you ensure forward compatibility with upcoming patch versions. Be sure to re-run tests whenever the compiler version changes to detect any unexpected behavior caused by future compiler updates.
v2/test/utils/upgrades/GatewayEVMUpgradeTest.sol (1)
2-2
: Transition to caret version pragma.
This update expands permissible compiler versions and helps future-proof the contract. Verify that library dependencies and any external tooling are also aligned with the broader range of compiler versions to avoid compatibility pitfalls.
v2/test/ERC20Custody.t.sol (1)
2-2
: Allowing future compiler updates is aligned with your PR objectives.
Switching to ^0.8.26
expands compatibility with minor and patch updates. Be sure to run tests periodically with newer compiler versions to catch potential breaking changes early.
v1/contracts/evm/Zeta.eth.sol (1)
2-2
: Caret pragma enables more permissive compiler usage.
Using ^0.8.7
is consistent with your goal of allowing new minor and patch versions. However, confirm CI coverage against a range of compiler versions to ensure forward compatibility.
v1/contracts/evm/testing/ZetaInteractorMock.sol (1)
2-2
: Shifting to caret version broadens compatibility.
Maintaining test suites across multiple Solidity 0.8.x releases is advised to prevent unexpected issues from minor version changes.
v2/test/utils/TestERC20.sol (1)
2-2
: Compatibility improvement via caret version.
This aligns with the aim of making the code more permissive. Keep robust tests to guard against potential quirks introduced by future compiler minor releases.
v2/scripts/deploy/deterministic/DeployTestERC20.s.sol (1)
2-2
: LGTM! Pragma change is safe for this deployment script.
The change to ^0.8.26
is appropriate as:
- The script uses standard Solidity features available since 0.8.0
- No version-specific dependencies are present
- The caret ensures compatibility with future 0.8.x versions while preventing breaking changes from 0.9.0
v2/scripts/deploy/deterministic/DeployGatewayEVMImplementation.s.sol (1)
2-2
: Verify version compatibility with imported contracts.
While the pragma change to ^0.8.26
is appropriate for the script itself, we should ensure version compatibility with imported contracts, particularly GatewayEVM.sol
.
Run this script to verify pragma versions across related contracts:
✅ Verification successful
Version compatibility verified across all v2 contracts
Based on the search results, all contracts in the v2 directory, including the main GatewayEVM.sol
contract and its dependencies, consistently use the same pragma version ^0.8.26
. This includes:
- The deployment script
DeployGatewayEVMImplementation.s.sol
- The main contract
v2/contracts/evm/GatewayEVM.sol
- All related interfaces and dependencies in v2
The version compatibility is maintained across all relevant contracts, ensuring consistent behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check pragma versions in imported contracts
# Expected: All imported contracts should use compatible versions (^0.8.x)
echo "Checking pragma versions in imported contracts:"
rg -A 1 "pragma solidity" "contracts/evm/GatewayEVM.sol"
Length of output: 230
Script:
#!/bin/bash
# Let's first find the correct path to GatewayEVM contract
fd GatewayEVM.sol
# Then check pragma versions in all related files
echo "Checking pragma versions in all related files:"
rg -A 1 "pragma solidity" $(fd -t f -e sol)
Length of output: 15299
v2/scripts/deploy/deterministic/UpgradeGatewayEVM.s.sol (1)
2-2
: Verify version compatibility with OpenZeppelin dependencies.
The pragma change to ^0.8.26
requires careful consideration due to:
- Dependencies on OpenZeppelin contracts
- Use of proxy upgrade patterns
Run these checks to verify compatibility:
✅ Verification successful
Version compatibility verified with OpenZeppelin dependencies
The pragma change to ^0.8.26
is compatible with the project dependencies:
- OpenZeppelin contracts and contracts-upgradeable are at version ^5.0.2, which supports Solidity ^0.8.20
- The GatewayEVM contract and its upgrade test both use ^0.8.26
- All OpenZeppelin imports are from the upgradeable contracts collection, which is appropriate for proxy-based upgrades
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify version compatibility across the upgrade path
# Expected: All contracts should use compatible versions
echo "Checking OpenZeppelin contract versions:"
rg -A 1 "@openzeppelin" "package.json"
echo "Checking pragma versions in test utils:"
rg -A 1 "pragma solidity" "test/utils/GatewayEVMUpgradeTest.sol"
Length of output: 411
Script:
#!/bin/bash
# Let's try to find the relevant files and dependencies first
echo "Finding OpenZeppelin related imports:"
rg -l "@openzeppelin"
echo -e "\nFinding all Solidity files and their pragma versions:"
fd -e sol -x head -n 1 {}
echo -e "\nChecking for package management files:"
fd -e json
Length of output: 41694
Script:
#!/bin/bash
# Let's check the OpenZeppelin dependencies and pragma versions in v2
echo "Checking OpenZeppelin dependencies in v2/package.json:"
cat v2/package.json | grep -A 5 "dependencies"
echo -e "\nChecking pragma versions in relevant contract files:"
echo "1. GatewayEVM contract:"
head -n 1 v2/contracts/evm/GatewayEVM.sol
echo -e "\n2. OpenZeppelin imports in GatewayEVM:"
rg -A 1 "@openzeppelin" v2/contracts/evm/GatewayEVM.sol
echo -e "\n3. Related upgrade test files:"
rg -A 1 "pragma solidity" v2/test/utils/upgrades/GatewayEVMUpgradeTest.sol
Length of output: 1446
v1/contracts/zevm/interfaces/IUniswapV2Router02.sol (1)
2-2
: LGTM with version compatibility note
The pragma change to ^0.8.7
is acceptable for this interface. Since this is a UniswapV2 interface, it's stable and unlikely to be affected by minor version changes. However, ensure that the implementing contracts are tested with the same compiler version used in production to avoid any potential interface compatibility issues.
Let's verify the compiler versions used across the codebase:
✅ Verification successful
Pragma version change is compatible with the codebase
Looking at the pragma versions across the codebase:
- In v1 (where the file under review is located), most contracts use
^0.8.7
or^0.8.0
- The only exceptions are UniswapV2-related contracts that use older versions (0.5.16, 0.6.6) which is expected as they are from the original Uniswap codebase
- The change from
0.8.7
to^0.8.7
inIUniswapV2Router02.sol
aligns with the versioning pattern used in other v1 contracts - The interface is stable and the caret version is safe as it only allows patch updates within 0.8.x
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all Solidity files and their pragma versions
echo "Checking Solidity pragma versions across the codebase..."
rg -n "pragma solidity" --type sol
Length of output: 187
Script:
#!/bin/bash
# Find all Solidity files and their pragma versions using ripgrep without file type filter
echo "Checking Solidity pragma versions across the codebase..."
rg -n "pragma solidity" --type-add 'sol:*.sol' --type sol
# Also try to find all .sol files to ensure we're not missing any
echo -e "\nListing all .sol files in the repository:"
fd -e sol
Length of output: 26637
v2/scripts/upgrade/SimulateERC20CustodyUpgrade.s.sol (1)
Line range hint 1-24
: Verify upgrade simulation coverage
The upgrade simulation script contains important state verification but could benefit from additional checks:
- Verification of implementation contract initialization state
- Events emission verification
- Access control verification
Let's check for any existing upgrade tests:
✅ Verification successful
Let's check for specific upgrade test implementations to verify the coverage of state verification, events, and access control.
Let's examine the actual test implementations to verify the coverage of state verification, events, and access control during upgrades.
Let's examine the actual test files to verify the upgrade test coverage.
Upgrade simulation coverage is comprehensive
The codebase has thorough upgrade test coverage that verifies all the aspects mentioned in the review comment:
-
Implementation contract initialization state is verified through:
testUpgradeAndWithdraw()
which tests state preservation after upgradetestTSSUpgrade()
which verifies role assignments and state transitions
-
Events emission verification is covered by:
- Event assertions in
testUpgradeAndWithdraw()
verifying the newWithdrawnV2
event - Event checks in
testTSSUpgrade()
forUpdatedCustodyTSSAddress
- Event assertions in
-
Access control verification is handled through:
- Role-based access control tests like
testTSSUpgradeFailsIfSenderIsNotTSSUpdater()
- Permission checks in
testUpgradeAndWithdraw()
maintaining proper authorization
- Role-based access control tests like
The upgrade simulation script complements these tests by providing a dry-run environment for the upgrade process.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for upgrade-related test files
echo "Checking for upgrade tests..."
fd -e sol -e js -e ts . | rg -i "upgrade|test"
# Look for upgrade-related test patterns
echo "Checking upgrade test coverage..."
rg -A 5 "function test.*[Uu]pgrade" --type sol
Length of output: 11783
Script:
#!/bin/bash
# Look for ERC20Custody upgrade tests
echo "Checking ERC20Custody upgrade tests..."
rg -A 10 "contract.*ERC20CustodyUpgradeTest"
# Look for upgrade test implementations
echo "Checking upgrade test implementations..."
rg -A 10 "function test.*upgrade" v2/test/utils/upgrades/ERC20CustodyUpgradeTest.sol
Length of output: 29701
Script:
#!/bin/bash
# Look for upgrade test implementation content
echo "Checking ERC20CustodyUpgradeTest content..."
cat v2/test/utils/upgrades/ERC20CustodyUpgradeTest.sol
# Look for test files that verify upgrade state
echo "Checking upgrade state verification tests..."
rg -A 10 "test.*state.*after.*upgrade" v2/test/ERC20Custody.t.sol
Length of output: 8980
Script:
#!/bin/bash
# Check ERC20Custody test file for upgrade tests
echo "Checking ERC20Custody tests..."
cat v2/test/ERC20Custody.t.sol
# Look for upgrade-related test patterns in all test files
echo "Checking all upgrade test patterns..."
rg -A 10 "function test.*upgrade|function test.*implementation|function test.*state" v2/test/
Length of output: 28735
v2/contracts/evm/interfaces/IZetaConnector.sol (2)
Line range hint 1-42
: Documentation meets NatSpec standards
The interface is well-documented with NatSpec comments, which is excellent for maintaining clarity across version changes.
2-2
: LGTM with version consistency note
The pragma change to ^0.8.26
is acceptable for this interface. However, ensure that all v2 contracts maintain consistent versioning to avoid potential compatibility issues.
Let's verify version consistency in v2:
✅ Verification successful
Version consistency verified across v2 contracts
The verification shows that all Solidity files in the v2 directory consistently use pragma solidity ^0.8.26
, including:
- All contracts in
v2/contracts/evm/
andv2/contracts/zevm/
- All interfaces in
v2/contracts/evm/interfaces/
andv2/contracts/zevm/interfaces/
- All test files and deployment scripts
The pragma change to ^0.8.26
in IZetaConnector.sol
aligns with the versioning used throughout the v2 codebase, ensuring compatibility across all components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check version consistency in v2 directory
echo "Checking version consistency in v2..."
rg -n "pragma solidity" "v2/"
# Look for any remaining fixed versions
echo "Checking for any remaining fixed versions..."
rg -n "pragma solidity [0-9]" --type sol
Length of output: 4101
v1/contracts/evm/tools/ZetaTokenConsumerUniV3.strategy.sol (2)
2-3
: Verify pragma consistency across the entire codebase
While the changes to use ^0.8.7
in these files are safe, it's important to ensure all contracts in the codebase use consistent pragma versions to prevent any potential compatibility issues during compilation and deployment.
Let's verify pragma consistency across the codebase:
2-2
: Consider aligning pragma version with Uniswap V3 dependencies
While the change to ^0.8.7
is safe, consider checking and aligning the pragma version with the Uniswap V3 contracts you're integrating with to ensure optimal compatibility. Also, adding an upper bound would prevent potential issues with future 0.9.x releases.
Let's verify the Solidity versions used by the dependencies:
✅ Verification successful
The pragma version ^0.8.7 is compatible with Uniswap V3 dependencies
Based on the verification results:
-
The file imports Uniswap V3 dependencies:
@uniswap/v3-periphery/contracts/interfaces/ISwapRouter.sol
@uniswap/v3-core/contracts/interfaces/IUniswapV3Factory.sol
@uniswap/v3-core/contracts/interfaces/IUniswapV3Pool.sol
-
Uniswap V3 contracts are designed to be compatible with Solidity 0.8.x, and the pragma version
^0.8.7
is within this range.
The current pragma version is appropriate and doesn't require any changes. The suggestion to align versions was unnecessary as ^0.8.7
is already compatible with Uniswap V3's requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Solidity versions used in dependencies
# Check OpenZeppelin contracts
echo "Checking OpenZeppelin contracts:"
rg -A 1 "pragma solidity" node_modules/@openzeppelin/contracts/
# Check Uniswap V3 contracts
echo "Checking Uniswap V3 contracts:"
rg -A 1 "pragma solidity" node_modules/@uniswap/v3-core/contracts/
rg -A 1 "pragma solidity" node_modules/@uniswap/v3-periphery/contracts/
Length of output: 599
Script:
#!/bin/bash
# Let's try to find the actual dependencies and their locations
echo "Checking package.json for dependencies:"
fd -t f "package.json" --exec cat {}
echo -e "\nChecking for Solidity files in the repository:"
fd -e sol --exec head -n 1 {}
Length of output: 38800
Script:
#!/bin/bash
# Let's try to find the actual Solidity files in the repository with their pragma versions
echo "Checking Solidity files in v1/contracts/evm/tools directory:"
find v1/contracts/evm/tools -type f -name "*.sol" -exec head -n 3 {} \;
echo -e "\nChecking for Uniswap V3 related imports:"
rg -l "import.*@uniswap/v3" v1/contracts/evm/tools/ZetaTokenConsumerUniV3.strategy.sol
Length of output: 972
Script:
#!/bin/bash
# Let's check the actual imports and dependencies in the file
echo "Checking imports in ZetaTokenConsumerUniV3.strategy.sol:"
rg "import.*@uniswap/v3" v1/contracts/evm/tools/ZetaTokenConsumerUniV3.strategy.sol
echo -e "\nChecking package.json for @uniswap/v3 version:"
rg "@uniswap/v3.*\":" -A 1 package.json
Length of output: 617
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution.
I think it would be ok to make this change
Need to run
yarn generate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jhuntbach-bc generate is still failing, you might need to upgrade Foundry with foundryup to generate same bytecode as in the CI
When I run
And then a bunch of contracts fail to compile and files in |
What is your forge and node version? |
e834549
to
5254d13
Compare
I've rebased this onto the main branch, and still getting the same issues, even with node The PR is jut a find/replace for |
we can do this, yes |
Great thanks. I see you've started work on #451, so I'll close this PR |
Yeah, have actually some similar issues, I think the CI use a different version than locally when using this pragma, we will check further this week |
Addresses #441
Summary by CodeRabbit
Chores
0.8.26
to^0.8.26
across multiple contract filesDocumentation
UniversalContract
New Features
MessageContext
struct andUniversalContract
interface inUniversalContract.sol