feat(btc-staking): UTXOGateway and BTC staking test #132
Conversation
WalkthroughThis pull request introduces comprehensive changes across the Exocore project's codebase, focusing on library updates, file structure reorganization, and the introduction of a new UTXO Gateway contract. The primary modifications involve migrating from LayerZero V2 libraries to the LayerZero Labs namespace, restructuring script and deployment directories, and adding a new contract for handling Bitcoin-like token operations. Changes
Sequence DiagramsequenceDiagram
participant Owner
participant UTXOGateway
participant Witnesses
participant AssetContract
Owner->>UTXOGateway: Initialize Gateway
Owner->>UTXOGateway: Add Witnesses
Owner->>UTXOGateway: Activate Staking
Witnesses->>UTXOGateway: Submit Proof
UTXOGateway->>AssetContract: Verify and Process Stake
AssetContract-->>UTXOGateway: Confirm Stake
Possibly Related PRs
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
adu-web3
force-pushed
the
btc-gateway
branch
from
7e98598 to
698feb0
Compare
|
✅ The Slither Analysis workflow has completed successfully. Check the workflow run for details. (b06f029) |
|
✅ The Solhint workflow has completed successfully. Check the workflow run for details. (b06f029) |
|
❌ The Forge CI workflow has failed! Check the workflow run for details. (b06f029) |
|
⏭ The Compare Storage Layouts workflow was skipped. Check the workflow run for details. (b06f029) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🔭 Outside diff range comments (3)
script/deployBeaconOracle.s.sol (1)
Line range hint
21-24: Consider parameterizing hardcoded values.The beacon oracle address and timestamp are hardcoded:
- The beacon oracle address
0xd3D285cd1516038dAED61B8BF7Ae2daD63662492should be configurable- The timestamp
1_715_918_948(May 17, 2024) is a future date and should be dynamically setConsider:
- Moving the beacon oracle address to a configuration file
- Using a dynamic timestamp based on deployment time
- beaconOracle = EigenLayerBeaconOracle(0xd3D285cd1516038dAED61B8BF7Ae2daD63662492); - (bool success,) = - address(beaconOracle).call(abi.encodeWithSelector(beaconOracle.addTimestamp.selector, 1_715_918_948)); + beaconOracle = EigenLayerBeaconOracle(config.beaconOracleAddress); + (bool success,) = + address(beaconOracle).call(abi.encodeWithSelector(beaconOracle.addTimestamp.selector, block.timestamp));script/16_UpgradeExoCapsule.s.sol (1)
The review comment is correct - safety checks are necessary for the upgrade operation
The suggested safety checks are valid and align with the codebase patterns:
- Using
address(0)in ExoCapsule constructor bypasses validation in ExoCapsuleStorage- The ownership and implementation verifications follow security patterns seen in other contracts (e.g., OAppCoreUpgradeable)
- The suggested diff provides appropriate safety measures consistent with the codebase's practices
🔗 Analysis chain
Line range hint
26-31: Add safety checks for beacon upgrade operation.The upgrade operation lacks proper safety checks:
- No verification of the new implementation
- No error handling for the upgrade operation
- Initializing capsule with address(0) could be problematic
Consider adding these safety checks:
vm.startBroadcast(deployer.privateKey); console.log("owner", capsuleBeaconContract.owner()); - ExoCapsule capsule = new ExoCapsule(address(0)); + // Verify deployer is the owner + require(capsuleBeaconContract.owner() == deployer.addr, "Not authorized"); + + // Deploy new implementation with proper initialization + ExoCapsule capsule = new ExoCapsule(config.initialAddress); + + // Verify new implementation + require(address(capsule).code.length > 0, "Invalid implementation"); + + // Perform upgrade capsuleBeaconContract.upgradeTo(address(capsule)); + + // Verify upgrade + require(capsuleBeaconContract.implementation() == address(capsule), "Upgrade failed"); vm.stopBroadcast();🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find ExoCapsule contract definition ast-grep --pattern 'contract ExoCapsule { $$$ }' # Find other upgrade scripts fd -e sol -e s.sol upgrade # Find beacon contract implementation ast-grep --pattern 'contract $_ is BeaconProxy { $$$ }' # Search for upgrade patterns rg -A 5 "upgradeTo\("Length of output: 718
Script:
#!/bin/bash # Find ExoCapsule source file fd -t f -e sol ExoCapsule # Search for constructor implementation ast-grep --pattern 'constructor($$$) { $$$ }' # Look for beacon contract usage rg -l "BeaconProxy|UpgradeableBeacon" # Check initialization patterns rg -A 5 "constructor.*address.*\)"Length of output: 5893
script/19_DeployFaucet.s.sol (1)
Line range hint
41-44: Parameterize initialization values and add proxy deployment checks.The faucet deployment has several areas for improvement:
- Hardcoded initialization amount of 1 ether
- Empty proxy initialization data
- Lack of proxy deployment verification
Consider:
- Making the initialization amount configurable
- Adding proper initialization data
- Adding deployment verification checks
- faucet.initialize(exocoreValidatorSet.addr, exoEthFaucet ? tokenAddr : address(0), 1 ether); + // Get configurable amount from environment + uint256 initAmount = vm.envUint("FAUCET_INIT_AMOUNT"); + + // Prepare initialization data + bytes memory initData = abi.encodeCall( + CombinedFaucet.initialize, + (exocoreValidatorSet.addr, exoEthFaucet ? tokenAddr : address(0), initAmount) + ); + + // Deploy proxy with initialization data + TransparentUpgradeableProxy proxy = new TransparentUpgradeableProxy( + address(faucetLogic), + address(proxyAdmin), + initData + ); + + // Verify deployment + CombinedFaucet faucet = CombinedFaucet(payable(address(proxy))); + require(faucet.owner() == exocoreValidatorSet.addr, "Initialization failed");
🧹 Nitpick comments (22)
src/libraries/SignatureVerifier.sol (1)
7-64: Consider using OpenZeppelin's ECDSA library for signature verificationTo enhance security and maintainability, consider using OpenZeppelin's
ECDSAlibrary, which provides robust and well-tested functions for signature verification, including handling of Ethereum Signed Messages and support for EIP-2098 short signatures. Leveraging a widely used library can reduce potential bugs and improve code reliability.test/hardhat/integration/btc-stake-e2e.test.js (3)
38-129: Add error handling for network failures infundBitcoinAddressThe
fundBitcoinAddressfunction may encounter network errors when making API calls. Consider adding specific error handling for network failures to provide clearer feedback and retry mechanisms.
208-215: Calculate transaction fees dynamically increateStakingTransactionThe transaction fee is hardcoded as
1000satoshis. To ensure timely confirmations and adapt to network conditions, consider calculating the fee based on the transaction size and current fee rates.Apply this diff to calculate fees dynamically:
const fee = 1000; // 1000 satoshis fee +// Alternatively, calculate fee based on transaction size and fee rate +const txSize = psbt.virtualSize(); +const feeRate = await getFeeRate(); // Implement getFeeRate() to fetch current fee rates +const fee = txSize * feeRate;
289-293: Handle potential errors when funding the staker's Bitcoin addressWhen calling
fundBitcoinAddress, consider adding a try-catch block to handle any potential errors and provide informative feedback if the funding process fails.src/storage/UTXOGatewayStorage.sol (3)
471-485: Enhance validation inisValidOperatorAddressThe
isValidOperatorAddressfunction only checks the length and prefix of the address. For better security and validation, consider implementing full Bech32 address decoding and checksum verification to ensure the operator address is valid.
492-496: Consider overflow checks in_verifyInboundNonceWhile Solidity 0.8+ automatically reverts on overflows, explicitly handling potential overflows can improve code readability and safety. Consider adding a check or comment to clarify.
468-485: Visibility ofisValidOperatorAddressfunctionThe
isValidOperatorAddressfunction is marked aspublic. If it's only used internally, consider changing the visibility tointernalorprivateto reduce the contract's public interface.Apply this diff to change the function visibility:
-function isValidOperatorAddress(string calldata addressToValidate) public pure returns (bool) { +function isValidOperatorAddress(string calldata addressToValidate) internal pure returns (bool) {src/core/UTXOGateway.sol (4)
792-800: Handle unsuccessful delegation in_delegatefunctionIn the
_delegatefunction, the booleansuccessis returned but not immediately acted upon. Ensure that the calling function properly handles cases when delegation fails and provides appropriate feedback.
730-756: Add checks for potential overflows when incrementing noncesWhen incrementing
delegationNonce,pegOutNonce, andinboundNonce, consider adding checks to prevent potential overflows, even though Solidity 0.8+ handles them. This can improve code clarity and safety.
813-815: Consistent error messages in_registerAddressIn the
_registerAddressfunction, different require statements use different error messages. For consistency and clarity, consider using custom errors or consistent revert messages.Apply this diff to use consistent error handling:
-require(depositor.length > 0 && exocoreAddress != address(0), "Invalid address"); -require(inboundRegistry[clientChainId][depositor] == address(0), "Depositor address already registered"); -require(outboundRegistry[clientChainId][exocoreAddress].length == 0, "Exocore address already registered"); +if (depositor.length == 0 || exocoreAddress == address(0)) { + revert Errors.InvalidAddress(); +} +if (inboundRegistry[clientChainId][depositor] != address(0)) { + revert Errors.AddressAlreadyRegistered(); +} +if (outboundRegistry[clientChainId][exocoreAddress].length != 0) { + revert Errors.AddressAlreadyRegistered(); +}
240-250: Avoid code duplication inprocessStakeMessageThe
processStakeMessagefunction andsubmitProofForStakeMsgshare similar logic. Consider refactoring common functionality into internal functions to adhere to the DRY (Don't Repeat Yourself) principle.test/foundry/unit/UTXOGateway.t.sol (1)
488-585: Consider refactoring repeated mocking code into helper functions to improve maintainabilityThe setup of mock calls to external precompile addresses is repeated across multiple tests (e.g., lines 491-521 and 534-576). Extracting this setup into helper functions would reduce code duplication and improve readability.
src/interfaces/precompiles/IDelegation.sol (2)
80-86: Add documentation for the BTC gateway delegation method.The new method lacks documentation explaining:
- Purpose and use cases
- Parameter descriptions
- Security considerations
Add documentation similar to existing methods:
+ /// TRANSACTIONS + /// @dev delegate the client chain BTC assets to the operator through BTC gateway + /// @param clientChainId The layerZero chainID of the client chain + /// @param assetsAddress The client chain asset address + /// @param stakerAddress The staker address + /// @param operatorAddr The operator address to delegate to + /// @param opAmount The delegation amount
88-94: Add documentation for the BTC gateway undelegation method.The new method lacks documentation explaining:
- Purpose and use cases
- Parameter descriptions
- Security considerations
Add documentation similar to existing methods:
+ /// TRANSACTIONS + /// @dev undelegate the client chain BTC assets from the operator through BTC gateway + /// @param clientChainId The layerZero chainID of the client chain + /// @param assetsAddress The client chain asset address + /// @param stakerAddress The staker address + /// @param operatorAddr The operator address to undelegate from + /// @param opAmount The undelegation amountscript/hardhat/deploy-and-setup-utxogateway.script.js (1)
18-26: Consider using Promise.all for parallel transactions.The sequential distribution of ether to accounts could be optimized by using
Promise.allto execute transactions in parallel.- for (const account of initialAccounts) { - const tx = await faucet.sendTransaction({ - to: account.address, - value: ethers.parseEther("1"), - }); - await tx.wait(); - assert(await ethers.provider.getBalance(account.address) > 0, "no enough balance for account") - } + await Promise.all(initialAccounts.map(async (account) => { + const tx = await faucet.sendTransaction({ + to: account.address, + value: ethers.parseEther("1"), + }); + await tx.wait(); + assert(await ethers.provider.getBalance(account.address) > 0, "no enough balance for account") + }));test/hardhat/integration/btc-stake.test.js (2)
1-92: Test setup looks good but consider adding error handling for ETH transfers.The test setup is well-structured with clear initialization of accounts and contract deployment. However, the ETH transfer operations could benefit from error handling.
Consider adding try-catch blocks for ETH transfers:
- const tx = await faucet.sendTransaction({ - to: account.address, - value: ethers.parseEther("1"), - }); + try { + const tx = await faucet.sendTransaction({ + to: account.address, + value: ethers.parseEther("1"), + }); + await tx.wait(); + } catch (error) { + console.error(`Failed to transfer ETH to ${account.address}:`, error); + throw error; + }
141-247: Consider adding negative test cases for BTC staking.While the happy path is well tested, consider adding test cases for:
- Invalid message signatures
- Insufficient proofs
- Duplicate proof submissions
- Invalid staking amounts
Example negative test case:
it("should fail when submitting duplicate proofs", async () => { // ... setup code ... // Submit first proof await utxoGateway.connect(relayer).submitProofForStakeMsg( witness1.address, stakeMsg, proofs[0] ); // Try to submit the same proof again await expect( utxoGateway.connect(relayer).submitProofForStakeMsg( witness1.address, stakeMsg, proofs[0] ) ).to.be.revertedWith("Duplicate proof"); });src/libraries/Errors.sol (1)
317-398: LGTM! Comprehensive error handling for UTXO operations.The error definitions are well-structured and cover all critical aspects of UTXO operations, including witness management, transaction validation, and state checks.
Consider adding these additional error cases for enhanced robustness:
- Timeout-related errors for witness consensus
- Gas limit exceeded errors for complex operations
script/deployments/deployedContracts.json (1)
24-31: Ensure configuration consistency across deployment files.The UTXOGateway configuration matches the localnet deployment file but inherits the same witness configuration issue. Consider consolidating deployment configurations into a single source of truth to prevent inconsistencies.
package.json (1)
40-42: Move hardhat-gas-reporter to devDependencies.The
hardhat-gas-reporterpackage is a development tool and should be indevDependenciesinstead ofdependencies."dependencies": { "@openzeppelin/upgrades-core": "^1.40.0", - "ethers": "^6.9.0", - "hardhat-gas-reporter": "^1.0.9" + "ethers": "^6.9.0" }, "devDependencies": { + "hardhat-gas-reporter": "^1.0.9", // ... other dev dependencies }script/11_SetPeers.s.sol (2)
Line range hint
1-1: Update Solidity version to match the project configuration.The script uses Solidity 0.8.19, but the project configuration in hardhat.config.js (mentioned in the AI summary) uses 0.8.22.
-pragma solidity ^0.8.19; +pragma solidity ^0.8.22;
Line range hint
42-45: Optimize gas usage in setPeer transactions.The current implementation makes separate transactions for
setPeercalls. Consider batching these operations to reduce gas costs.Would you like me to propose an implementation for batching these operations?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (67)
.gitignore(1 hunks)hardhat.config.js(2 hunks)package.json(1 hunks)remappings.txt(0 hunks)script/10_DeployExocoreGatewayOnly.s.sol(3 hunks)script/11_SetPeers.s.sol(2 hunks)script/12_RedeployClientChainGateway.s.sol(3 hunks)script/13_DepositValidator.s.sol(2 hunks)script/14_CorrectBootstrapErrors.s.sol(4 hunks)script/15_DeploySafeMulstisigWallet.s.sol(1 hunks)script/16_UpgradeExoCapsule.s.sol(1 hunks)script/17_WithdrawalValidator.s.sol(4 hunks)script/18_SimulateReceive.s.sol(1 hunks)script/19_DeployFaucet.s.sol(1 hunks)script/1_Prerequisites.s.sol(2 hunks)script/2_DeployBoth.s.sol(2 hunks)script/3_Setup.s.sol(2 hunks)script/4_Deposit.s.sol(2 hunks)script/5_Withdraw.s.sol(2 hunks)script/6_CreateExoCapsule.s.sol(3 hunks)script/7_DeployBootstrap.s.sol(3 hunks)script/8_RegisterValidatorsAndDelegate.s.sol(1 hunks)script/9_ExtendBootstrapTime.s.sol(1 hunks)script/BaseScript.sol(1 hunks)script/TestPrecompileErrorFixed.s.sol(0 hunks)script/TestPrecompileErrorFixed_Deploy.s.sol(0 hunks)script/TokenTransfer.s.sol(0 hunks)script/deployBeaconOracle.s.sol(1 hunks)script/deployments/deployedContracts.json(1 hunks)script/deployments/utxo-gateway-exocore_localnet.json(1 hunks)script/hardhat/deploy-and-setup-utxogateway.script.js(1 hunks)slither.config.json(1 hunks)src/core/BaseRestakingController.sol(1 hunks)src/core/ClientChainGateway.sol(1 hunks)src/core/ExocoreGateway.sol(1 hunks)src/core/RewardVault.sol(1 hunks)src/core/UTXOGateway.sol(1 hunks)src/interfaces/IClientChainGateway.sol(1 hunks)src/interfaces/IExocoreGateway.sol(1 hunks)src/interfaces/precompiles/IAssets.sol(6 hunks)src/interfaces/precompiles/IDelegation.sol(1 hunks)src/libraries/Errors.sol(1 hunks)src/libraries/ExocoreBytes.sol(1 hunks)src/libraries/SignatureVerifier.sol(1 hunks)src/lzApp/OAppCoreUpgradeable.sol(1 hunks)src/lzApp/OAppReceiverUpgradeable.sol(1 hunks)src/lzApp/OAppSenderUpgradeable.sol(1 hunks)src/storage/UTXOGatewayStorage.sol(1 hunks)test/foundry/Delegation.t.sol(1 hunks)test/foundry/DepositThenDelegateTo.t.sol(1 hunks)test/foundry/DepositWithdrawPrinciple.t.sol(1 hunks)test/foundry/ExocoreDeployer.t.sol(1 hunks)test/foundry/Governance.t.sol(1 hunks)test/foundry/TvlLimits.t.sol(1 hunks)test/foundry/unit/ClientChainGateway.t.sol(1 hunks)test/foundry/unit/ExocoreGateway.t.sol(2 hunks)test/foundry/unit/UTXOGateway.t.sol(1 hunks)test/hardhat/client-chain/deploy_fixture.js(0 hunks)test/hardhat/client-chain/gateway.test.js(0 hunks)test/hardhat/exocore/deploy_fixture.js(0 hunks)test/hardhat/exocore/gateway.test.js(0 hunks)test/hardhat/integration/btc-stake-e2e.test.js(1 hunks)test/hardhat/integration/btc-stake.test.js(1 hunks)test/mocks/AssetsMock.sol(6 hunks)test/mocks/DelegationMock.sol(5 hunks)test/mocks/ExocoreGatewayMock.sol(1 hunks)test/mocks/NonShortCircuitLzEndpointMock.sol(0 hunks)
💤 Files with no reviewable changes (9)
- test/hardhat/exocore/gateway.test.js
- test/hardhat/client-chain/gateway.test.js
- script/TestPrecompileErrorFixed.s.sol
- script/TokenTransfer.s.sol
- remappings.txt
- script/TestPrecompileErrorFixed_Deploy.s.sol
- test/hardhat/client-chain/deploy_fixture.js
- test/mocks/NonShortCircuitLzEndpointMock.sol
- test/hardhat/exocore/deploy_fixture.js
✅ Files skipped from review due to trivial changes (19)
- test/foundry/DepositWithdrawPrinciple.t.sol
- .gitignore
- script/18_SimulateReceive.s.sol
- src/lzApp/OAppCoreUpgradeable.sol
- src/lzApp/OAppReceiverUpgradeable.sol
- src/lzApp/OAppSenderUpgradeable.sol
- test/foundry/TvlLimits.t.sol
- script/BaseScript.sol
- test/foundry/Governance.t.sol
- src/core/BaseRestakingController.sol
- test/foundry/ExocoreDeployer.t.sol
- test/foundry/Delegation.t.sol
- src/core/ClientChainGateway.sol
- src/interfaces/IClientChainGateway.sol
- test/foundry/DepositThenDelegateTo.t.sol
- test/foundry/unit/ExocoreGateway.t.sol
- src/interfaces/IExocoreGateway.sol
- src/core/ExocoreGateway.sol
- test/mocks/ExocoreGatewayMock.sol
🧰 Additional context used
📓 Learnings (1)
test/mocks/AssetsMock.sol (1)
Learnt from: adu-web3
PR: ExocoreNetwork/exocore-contracts#23
File: test/mocks/AssetsMock.sol:9-28
Timestamp: 2024-11-12T10:03:05.536Z
Learning: The `AssetsMock` contract is used for simulating a production precompiled contract, and its complexity in data structures is intentional to mimic production behavior.
🪛 Biome (1.9.4)
hardhat.config.js
[error] 21-21: This property value named chainId is later overwritten by an object member with the same name.
Overwritten with this value.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property value named chainId
(lint/suspicious/noDuplicateObjectKeys)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (47)
test/mocks/AssetsMock.sol (2)
37-39: Verify staker address validation for BTC chain IDsIn the
depositLSTfunction, the staker address length check is skipped for BTC chain IDs (clientChainLzId == clientBtcChainId). Please verify that omitting this validation for BTC is intentional and does not introduce security vulnerabilities.
81-82: Verify withdrawal logic for BTC and unregistered tokensIn the
withdrawLSTfunction, the condition allows withdrawals for BTC (even if not registered) but disallows withdrawals for ETH and unregistered tokens. Please confirm that this behavior is intentional and aligns with the expected policies for token withdrawals.src/interfaces/precompiles/IAssets.sol (3)
10-45: New structsTokenInfoandStakerBalanceare well-definedThe introduction of
TokenInfoandStakerBalancestructs provides comprehensive data structures for managing token information and staker balances. The fields are appropriately defined to encapsulate necessary details.
158-162: Ensure proper access control forupdateAuthorizedGatewaysfunctionThe
updateAuthorizedGatewaysfunction allows updating authorized gateways. Please ensure that appropriate access controls are implemented to prevent unauthorized modifications to the list of authorized gateways.
180-199: Addition of query functions enhances contract interactionThe new functions
getTokenInfoandgetStakerBalanceByTokenimprove the interface by providing essential data retrieval capabilities for tokens and staker balances, facilitating better contract interaction.test/hardhat/integration/btc-stake-e2e.test.js (1)
96-99: Adjust dust threshold check for the change outputWhen calculating the change output, you check if
change > 546. The dust threshold can vary depending on the Bitcoin network and policy settings. Ensure that the dust limit is appropriate for the regtest network or make it configurable.script/9_ExtendBootstrapTime.s.sol (1)
16-16: File path updated correctlyThe file path for reading deployed contracts has been updated to match the new directory structure, ensuring the script functions properly.
hardhat.config.js (2)
20-31: Verify environment variables for local testingThe configuration now uses multiple test account private keys from environment variables. Ensure these are properly documented and included in the local development setup instructions.
Consider creating a
.env.examplefile with placeholder values for all required environment variables to help other developers set up their local environment.🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: This property value named chainId is later overwritten by an object member with the same name.
Overwritten with this value.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property value named chainId(lint/suspicious/noDuplicateObjectKeys)
8-8: Verify compatibility with upgraded Solidity versionThe upgrade to Solidity 0.8.22 includes breaking changes and new features. Please ensure all smart contracts, especially the UTXOGateway and BTC staking contracts, are compatible with this version.
✅ Verification successful
Solidity 0.8.22 upgrade is compatible with the codebase
All core contracts, including UTXOGateway and staking-related contracts, use ^0.8.19 which is compatible with 0.8.22. The few contracts using different versions are either non-critical components or external interfaces that won't be affected by this upgrade.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential compatibility issues in Solidity files echo "Searching for Solidity files with explicit version pragma..." fd -e sol | xargs grep -l "pragma solidity"Length of output: 3815
Script:
#!/bin/bash # Check pragma versions in core contracts echo "=== Checking pragma versions in core contracts ===" rg "pragma solidity" src/core/ -B 0 -A 1 echo -e "\n=== Checking pragma versions in interfaces ===" rg "pragma solidity" src/interfaces/ -B 0 -A 1 echo -e "\n=== Looking for potential version-specific features ===" rg "pragma solidity [\^>=]0\.8\." -l | xargs rg -l "assembly|unchecked|custom error|error [A-Z]|using.*for.*global"Length of output: 3758
script/6_CreateExoCapsule.s.sol (2)
11-12: LGTM: Updated LayerZero import pathsThe import paths have been updated to use the new
@layerzerolabsnamespace, which aligns with the library's organization.
26-26: LGTM: Improved deployment artifacts organizationMoving JSON files to the
script/deployments/directory improves the organization of deployment-related artifacts.Also applies to: 56-56
script/15_DeploySafeMulstisigWallet.s.sol (1)
27-27: LGTM: Consistent deployment artifacts locationMoving the safe contracts JSON file to
script/deployments/maintains consistency with the project's deployment file organization.src/core/RewardVault.sol (1)
28-29: LGTM: Added critical address validationThe zero-address check for the gateway parameter is a crucial security measure that prevents initialization with an invalid address.
Let's verify that similar checks exist in related contracts:
✅ Verification successful
Zero-address validation is consistently implemented
The zero-address check for the gateway parameter is consistently implemented across related contracts:
RewardVault.sol:require(gateway_ != address(0), "Gateway address cannot be zero")ExoCapsule.sol:require(gateway_ != address(0), "ExoCapsule: gateway address can not be empty")Vault.sol:if (underlyingToken_ == address(0) || gateway_ == address(0))🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for zero-address checks in gateway-related contracts ast-grep --pattern 'require($_ != address(0), $_)'Length of output: 52
Script:
#!/bin/bash # Search for various forms of zero-address validation echo "=== Searching require statements ===" rg -U "require\(.*address.*!= address\(0\).*\)" --type sol echo -e "\n=== Searching revert statements ===" rg -U "revert.*address.*zero" --type sol echo -e "\n=== Searching if conditions ===" rg -U "if.*address.*== address\(0\).*revert" --type sol echo -e "\n=== Searching custom errors ===" rg "error.*ZeroAddress" --type solLength of output: 638
Script:
#!/bin/bash # Find Solidity files first echo "=== Finding Solidity files ===" fd "\.sol$" echo -e "\n=== Searching gateway initialization and zero-address checks ===" rg -U "gateway.*=.*" -A 2 -B 2 echo -e "\n=== Searching zero-address validations ===" rg -U "require\(.*!= address\(0\).*\)" echo -e "\n=== Searching constructor and initialize patterns ===" rg -U "function (constructor|initialize).*\{" -A 5Length of output: 75015
script/10_DeployExocoreGatewayOnly.s.sol (2)
3-3: LGTM: Updated LayerZero import pathThe import path has been updated to use the new
@layerzerolabsnamespace, maintaining consistency with the library organization.
19-19: LGTM: Consistent deployment artifacts organizationJSON files have been moved to the
script/deployments/directory, maintaining a consistent structure for deployment artifacts.Also applies to: 58-58
script/1_Prerequisites.s.sol (2)
5-5: LGTM! Library update aligned with LayerZero V2 migration.The import path has been updated to use the official LayerZero Labs namespace.
76-76: Verify deployment artifacts directory exists.The JSON output path has been updated to use a
deploymentssubdirectory.✅ Verification successful
Directory structure verified successfully
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the deployments directory exists if [ ! -d "script/deployments" ]; then echo "Warning: script/deployments directory does not exist" exit 1 fiLength of output: 35
Script:
#!/bin/bash # List all directories under script/ echo "=== Directories under script/ ===" fd --type d . script/ # Check for JSON files in script/ and subdirectories echo -e "\n=== JSON files in script/ ===" fd --extension json . script/ # Look for directory creation/setup in script files echo -e "\n=== Directory setup in scripts ===" rg -l "mkdir.*deployments|mkdirp.*deployments" script/Length of output: 1191
script/5_Withdraw.s.sol (1)
11-12: LGTM! Consistent library updates.Import paths have been updated to use the official LayerZero Labs namespace, maintaining consistency across the codebase.
src/interfaces/precompiles/IDelegation.sol (1)
80-94: Verify replay protection mechanism for BTC gateway operations.The new BTC gateway methods don't include the
lzNonceparameter present in the regular delegation methods. This could affect replay protection.script/4_Deposit.s.sol (2)
10-11: LGTM! Consistent library updates.Import paths have been updated to use the official LayerZero Labs namespace, maintaining consistency across the codebase.
23-23: Verify JSON file path consistency.The JSON input path has been updated to match the new deployments directory structure.
✅ Verification successful
JSON file path is correctly structured and consistent
The file exists at
script/deployments/deployedContracts.jsonand is consistently referenced across multiple deployment scripts and tests. The path aligns with the repository's organized structure where all deployment-related JSON files are stored in thescript/deployments/directory.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the deployedContracts.json exists in the new location if [ ! -f "script/deployments/deployedContracts.json" ]; then echo "Warning: deployedContracts.json not found in script/deployments/" exit 1 fiLength of output: 58
Script:
#!/bin/bash # Check directory structure echo "=== Directory Structure ===" ls -R script/ 2>/dev/null || echo "script/ directory not found" echo -e "\n=== All JSON Files ===" fd -e json echo -e "\n=== References to deployedContracts.json ===" rg "deployedContracts.json" -lLength of output: 2909
script/8_RegisterValidatorsAndDelegate.s.sol (1)
51-51: LGTM! File path update aligns with new directory structure.The change to use the new
deploymentsdirectory is consistent with the project's reorganization.script/12_RedeployClientChainGateway.s.sol (2)
22-22: LGTM! Updated LayerZero import path.The import path has been updated to use the new
@layerzerolabsnamespace, which is the correct package.
34-34: LGTM! File paths updated to use deployments directory.File paths have been consistently updated to use the new
deploymentsdirectory structure.Also applies to: 89-89
script/13_DepositValidator.s.sol (2)
10-12: LGTM! Updated LayerZero imports.The imports have been correctly updated to use the new
@layerzerolabsnamespace.
38-39: LGTM! File paths updated to use appropriate subdirectories.Files have been organized into appropriate subdirectories:
- Validator proof moved to
data/directory- Deployed contracts moved to
deployments/directorytest/mocks/DelegationMock.sol (1)
9-9: LGTM! Mapping name change improves clarity.The rename from
delegateTotodelegateToRecordsbetter describes the purpose of the mapping.Also applies to: 44-44, 66-69, 113-113
script/hardhat/deploy-and-setup-utxogateway.script.js (1)
63-67: Verify the security implications of the required proofs parameter.The hardcoded value of 3 for
requiredProofsis a critical security parameter. Ensure this value aligns with the security requirements of the system.Run this script to check if this value is consistent across the codebase:
✅ Verification successful
The requiredProofs value of 3 is secure and properly validated
The value is protected by:
- Explicit bounds (1-10) defined in contract storage
- Validation in both initialization and updates
- Owner-only modification with pause protection
- Event emission for transparency
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for requiredProofs parameter usage and its values rg -A 2 "requiredProofs"Length of output: 7697
Script:
#!/bin/bash # Search for MIN_REQUIRED_PROOFS and MAX_REQUIRED_PROOFS constants and related documentation rg -B 2 -A 2 "MIN_REQUIRED_PROOFS|MAX_REQUIRED_PROOFS" # Also check for any documentation in README or docs fd -e md | xargs rg -i "proof|witness|consensus" -A 2Length of output: 1253
script/3_Setup.s.sol (2)
12-13: LGTM! Updated import paths for LayerZero V2.The import paths have been correctly updated to use the new
@layerzerolabs/lz-evm-protocol-v2package.
24-24: LGTM! Updated file path for deployedContracts.json.The file path has been correctly updated to use the new
deploymentsdirectory structure.script/14_CorrectBootstrapErrors.s.sol (2)
18-18: LGTM! Updated import path for LayerZero V2.The import path has been correctly updated to use the new
@layerzerolabs/lz-evm-protocol-v2package.
40-40: LGTM! Updated file paths for JSON files.The file paths have been correctly updated to use the new
deploymentsdirectory structure:
- prerequisiteContracts.json
- deployedBootstrapOnly.json
- correctBootstrapErrors.json
Also applies to: 59-59, 139-139
script/7_DeployBootstrap.s.sol (2)
17-17: LGTM! Updated import path for LayerZero V2.The import path has been correctly updated to use the new
@layerzerolabs/lz-evm-protocol-v2package.
32-32: LGTM! Updated file paths for JSON files.The file paths have been correctly updated to use the new
deploymentsdirectory structure:
- prerequisiteContracts.json
- deployedBootstrapOnly.json
Also applies to: 150-150
script/17_WithdrawalValidator.s.sol (2)
12-13: LGTM! Library imports updated to use the new LayerZero package.The imports have been correctly updated to use
@layerzerolabs/lz-evm-protocol-v2instead of the older@layerzero-v2package.
45-45: LGTM! JSON file paths updated for better organization.The file paths have been correctly updated to use the new directory structure:
- Deployment data moved to
script/deployments/- Withdrawal proof data moved to
script/data/Also applies to: 98-98, 105-105, 121-121, 128-128
script/2_DeployBoth.s.sol (2)
17-17: LGTM! LayerZero endpoint import updated.The import has been correctly updated to use
@layerzerolabs/lz-evm-protocol-v2.
28-28: LGTM! Prerequisites file path updated.The file path has been correctly updated to use the new
script/deployments/directory.test/hardhat/integration/btc-stake.test.js (2)
94-114: LGTM! Comprehensive deployment verification.The test thoroughly verifies:
- Contract deployment
- Owner settings
- Witness authorization
- Threshold configuration
- Gateway authorization
116-139: LGTM! Thorough BTC staking activation test.The test properly verifies:
- Staking activation
- Client chain registration
- Token registration with correct parameters
test/foundry/unit/ClientChainGateway.t.sol (1)
5-5: LGTM! LayerZero imports updated consistently.The imports have been correctly updated to use
@layerzerolabs/lz-evm-protocol-v2package, maintaining consistency with other files.Also applies to: 7-8
slither.config.json (1)
2-2: Reconsider excluding the mapping-deletion detector.Excluding the mapping-deletion detector from Slither analysis could hide potential issues with improper storage cleanup in mappings, which is particularly important for upgradeable contracts like UTXOGateway.
package.json (1)
18-37: Review dependency version constraints.Several dependencies lack version constraints or use potentially unstable versions:
@evmos/evmosjsis pinned to a git branch (main) which could lead to unexpected breaking changeshardhatv2.22.17 is very recent and might have stability issuesPlease verify the stability of these versions by running:
script/deployBeaconOracle.s.sol (1)
5-5: LGTM! Import path updated to use LayerZero Labs namespace.The import path has been correctly updated to use the new LayerZero Labs namespace.
script/16_UpgradeExoCapsule.s.sol (1)
15-15: LGTM! JSON file path updated to new deployments directory.The path has been correctly updated to use the new standardized deployments directory structure.
script/19_DeployFaucet.s.sol (2)
19-19: LGTM! JSON file path updated to new deployments directory.The path has been correctly updated to use the new standardized deployments directory structure.
Line range hint
45-49: Consider storing deployment addresses securely.The comment indicates addresses are intentionally kept private, but console logging them might expose them in logs.
Consider:
- Using a secure logging mechanism
- Encrypting sensitive data before logging
- Implementing proper access controls for deployment logs
| (r, s) = abi.decode(signature, (bytes32, bytes32)); | ||
| v = uint8(signature[64]); | ||
|
|
There was a problem hiding this comment.
Fix incorrect decoding of 65-byte signatures to prevent runtime errors
When handling signatures of length 65 bytes, the code attempts to decode (r, s) using abi.decode(signature, (bytes32, bytes32)); on a signature that is 65 bytes long. This will cause a decoding error because abi.decode expects the bytes array length to match the total size of the decoded types (which is 64 bytes for two bytes32 values). To correctly extract r, s, and v from a 65-byte signature, you should manually read the bytes.
Apply this diff to fix the issue:
} else if (signature.length == 65) {
- (r, s) = abi.decode(signature, (bytes32, bytes32));
- v = uint8(signature[64]);
+ // Extract r, s, and v from the signature
+ assembly {
+ r := mload(add(signature, 0x20)) // first 32 bytes
+ s := mload(add(signature, 0x40)) // next 32 bytes
+ v := byte(0, mload(add(signature, 0x60))) // final byte
+ }
+ // Ensure v value is properly formatted.
+ if (v != 27 && v != 28) {
+ revert BadSignatureV(v);
+ }
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (r, s) = abi.decode(signature, (bytes32, bytes32)); | |
| v = uint8(signature[64]); | |
| } else if (signature.length == 65) { | |
| // Extract r, s, and v from the signature | |
| assembly { | |
| r := mload(add(signature, 0x20)) // first 32 bytes | |
| s := mload(add(signature, 0x40)) // next 32 bytes | |
| v := byte(0, mload(add(signature, 0x60))) // final byte | |
| } | |
| // Ensure v value is properly formatted. | |
| if (v != 27 && v != 28) { | |
| revert BadSignatureV(v); | |
| } | |
| } else { |
| it("should complete the full staking flow", async function() { | ||
| // Get initial balance | ||
| const [success, initialBalance] = await assetsPrecompile.getStakerBalanceByToken( | ||
| CLIENT_CHAIN.BTC, | ||
| ethers.getBytes(staker.address), | ||
| BTC_ID | ||
| ); | ||
|
|
||
| // Create and broadcast the Bitcoin transaction | ||
| const txid = await createStakingTransaction( | ||
| staker.privateKey, | ||
| BITCOIN_VAULT_ADDRESS, | ||
| depositAmount | ||
| ); | ||
| console.log('Staking transaction broadcasted. TXID:', txid); | ||
|
|
||
| // Wait for Bitcoin confirmation | ||
| console.log('Waiting for Bitcoin confirmation...'); | ||
| const confirmedTx = await waitForBitcoinConfirmation(txid); | ||
| console.log('Transaction confirmed with', confirmedTx.confirmations, 'confirmations'); | ||
|
|
||
| // Wait for DepositCompleted event | ||
| console.log('Waiting for DepositCompleted event...'); | ||
| return new Promise((resolve, reject) => { | ||
| const timeout = setTimeout(() => { | ||
| reject(new Error('Timeout waiting for DepositCompleted event')); | ||
| }, 60000); | ||
|
|
||
| utxoGateway.on('DepositCompleted', async ( | ||
| clientChainId, | ||
| txTag, | ||
| depositorExoAddr, | ||
| depositorClientChainAddr, | ||
| amount, | ||
| updatedBalance | ||
| ) => { | ||
| try { | ||
| if (depositorExoAddr.toLowerCase() === staker.address.toLowerCase()) { | ||
| clearTimeout(timeout); | ||
|
|
||
| // Verify final balance | ||
| const [finalSuccess, finalBalance] = await assetsPrecompile.getStakerBalanceByToken( | ||
| CLIENT_CHAIN.BTC, | ||
| ethers.getBytes(staker.address), | ||
| BTC_ID | ||
| ); | ||
|
|
||
| expect(finalSuccess).to.be.true; | ||
| const expectedIncrease = ethers.parseUnits('0.01', 8); // 0.01 BTC in satoshis | ||
|
|
||
| // Check balance components | ||
| expect(finalBalance[0]).to.equal(CLIENT_CHAIN.BTC); // clientChainID | ||
| expect(ethers.hexlify(finalBalance[1])).to.equal(ethers.hexlify(staker.address)); // stakerAddress | ||
| expect(ethers.hexlify(finalBalance[2])).to.equal(ethers.hexlify(BTC_ID)); // tokenId | ||
| expect(finalBalance[3]).to.equal(expectedIncrease); // balance | ||
| expect(finalBalance[7]).to.equal(expectedIncrease); // totalDeposited | ||
|
|
||
| console.log('Deposit completed successfully'); | ||
| console.log('Initial balance:', initialBalance ? initialBalance[3] : 0); | ||
| console.log('Final balance:', finalBalance[3].toString()); | ||
|
|
||
| resolve(); | ||
| } | ||
| } catch (error) { | ||
| clearTimeout(timeout); | ||
| reject(error); | ||
| } | ||
| }); | ||
| }); | ||
| }).timeout(120000); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure proper cleanup after the test completes
In the test case "should complete the full staking flow", ensure that event listeners are properly removed after the test to prevent side effects on subsequent tests.
Apply this diff to remove the event listener after it's no longer needed:
utxoGateway.on('DepositCompleted', async (
// ... parameters ...
) => {
// ... test logic ...
+ utxoGateway.removeAllListeners('DepositCompleted');
resolve();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("should complete the full staking flow", async function() { | |
| // Get initial balance | |
| const [success, initialBalance] = await assetsPrecompile.getStakerBalanceByToken( | |
| CLIENT_CHAIN.BTC, | |
| ethers.getBytes(staker.address), | |
| BTC_ID | |
| ); | |
| // Create and broadcast the Bitcoin transaction | |
| const txid = await createStakingTransaction( | |
| staker.privateKey, | |
| BITCOIN_VAULT_ADDRESS, | |
| depositAmount | |
| ); | |
| console.log('Staking transaction broadcasted. TXID:', txid); | |
| // Wait for Bitcoin confirmation | |
| console.log('Waiting for Bitcoin confirmation...'); | |
| const confirmedTx = await waitForBitcoinConfirmation(txid); | |
| console.log('Transaction confirmed with', confirmedTx.confirmations, 'confirmations'); | |
| // Wait for DepositCompleted event | |
| console.log('Waiting for DepositCompleted event...'); | |
| return new Promise((resolve, reject) => { | |
| const timeout = setTimeout(() => { | |
| reject(new Error('Timeout waiting for DepositCompleted event')); | |
| }, 60000); | |
| utxoGateway.on('DepositCompleted', async ( | |
| clientChainId, | |
| txTag, | |
| depositorExoAddr, | |
| depositorClientChainAddr, | |
| amount, | |
| updatedBalance | |
| ) => { | |
| try { | |
| if (depositorExoAddr.toLowerCase() === staker.address.toLowerCase()) { | |
| clearTimeout(timeout); | |
| // Verify final balance | |
| const [finalSuccess, finalBalance] = await assetsPrecompile.getStakerBalanceByToken( | |
| CLIENT_CHAIN.BTC, | |
| ethers.getBytes(staker.address), | |
| BTC_ID | |
| ); | |
| expect(finalSuccess).to.be.true; | |
| const expectedIncrease = ethers.parseUnits('0.01', 8); // 0.01 BTC in satoshis | |
| // Check balance components | |
| expect(finalBalance[0]).to.equal(CLIENT_CHAIN.BTC); // clientChainID | |
| expect(ethers.hexlify(finalBalance[1])).to.equal(ethers.hexlify(staker.address)); // stakerAddress | |
| expect(ethers.hexlify(finalBalance[2])).to.equal(ethers.hexlify(BTC_ID)); // tokenId | |
| expect(finalBalance[3]).to.equal(expectedIncrease); // balance | |
| expect(finalBalance[7]).to.equal(expectedIncrease); // totalDeposited | |
| console.log('Deposit completed successfully'); | |
| console.log('Initial balance:', initialBalance ? initialBalance[3] : 0); | |
| console.log('Final balance:', finalBalance[3].toString()); | |
| resolve(); | |
| } | |
| } catch (error) { | |
| clearTimeout(timeout); | |
| reject(error); | |
| } | |
| }); | |
| }); | |
| }).timeout(120000); | |
| it("should complete the full staking flow", async function() { | |
| // Get initial balance | |
| const [success, initialBalance] = await assetsPrecompile.getStakerBalanceByToken( | |
| CLIENT_CHAIN.BTC, | |
| ethers.getBytes(staker.address), | |
| BTC_ID | |
| ); | |
| // Create and broadcast the Bitcoin transaction | |
| const txid = await createStakingTransaction( | |
| staker.privateKey, | |
| BITCOIN_VAULT_ADDRESS, | |
| depositAmount | |
| ); | |
| console.log('Staking transaction broadcasted. TXID:', txid); | |
| // Wait for Bitcoin confirmation | |
| console.log('Waiting for Bitcoin confirmation...'); | |
| const confirmedTx = await waitForBitcoinConfirmation(txid); | |
| console.log('Transaction confirmed with', confirmedTx.confirmations, 'confirmations'); | |
| // Wait for DepositCompleted event | |
| console.log('Waiting for DepositCompleted event...'); | |
| return new Promise((resolve, reject) => { | |
| const timeout = setTimeout(() => { | |
| reject(new Error('Timeout waiting for DepositCompleted event')); | |
| }, 60000); | |
| utxoGateway.on('DepositCompleted', async ( | |
| clientChainId, | |
| txTag, | |
| depositorExoAddr, | |
| depositorClientChainAddr, | |
| amount, | |
| updatedBalance | |
| ) => { | |
| try { | |
| if (depositorExoAddr.toLowerCase() === staker.address.toLowerCase()) { | |
| clearTimeout(timeout); | |
| // Verify final balance | |
| const [finalSuccess, finalBalance] = await assetsPrecompile.getStakerBalanceByToken( | |
| CLIENT_CHAIN.BTC, | |
| ethers.getBytes(staker.address), | |
| BTC_ID | |
| ); | |
| expect(finalSuccess).to.be.true; | |
| const expectedIncrease = ethers.parseUnits('0.01', 8); // 0.01 BTC in satoshis | |
| // Check balance components | |
| expect(finalBalance[0]).to.equal(CLIENT_CHAIN.BTC); // clientChainID | |
| expect(ethers.hexlify(finalBalance[1])).to.equal(ethers.hexlify(staker.address)); // stakerAddress | |
| expect(ethers.hexlify(finalBalance[2])).to.equal(ethers.hexlify(BTC_ID)); // tokenId | |
| expect(finalBalance[3]).to.equal(expectedIncrease); // balance | |
| expect(finalBalance[7]).to.equal(expectedIncrease); // totalDeposited | |
| console.log('Deposit completed successfully'); | |
| console.log('Initial balance:', initialBalance ? initialBalance[3] : 0); | |
| console.log('Final balance:', finalBalance[3].toString()); | |
| utxoGateway.removeAllListeners('DepositCompleted'); | |
| resolve(); | |
| } | |
| } catch (error) { | |
| clearTimeout(timeout); | |
| reject(error); | |
| } | |
| }); | |
| }); | |
| }).timeout(120000); |
| const deploymentsDir = path.join(__dirname, '../deployments'); | ||
| if (!fs.existsSync(deploymentsDir)) { | ||
| fs.mkdirSync(deploymentsDir); | ||
| } | ||
| fs.writeFileSync( | ||
| path.join(deploymentsDir, `utxo-gateway-${network.name}.json`), | ||
| JSON.stringify(deploymentInfo, null, 2) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for file operations.
File system operations should include proper error handling to manage potential failures gracefully.
- if (!fs.existsSync(deploymentsDir)) {
- fs.mkdirSync(deploymentsDir);
- }
- fs.writeFileSync(
- path.join(deploymentsDir, `utxo-gateway-${network.name}.json`),
- JSON.stringify(deploymentInfo, null, 2)
- );
+ try {
+ if (!fs.existsSync(deploymentsDir)) {
+ fs.mkdirSync(deploymentsDir, { recursive: true });
+ }
+ fs.writeFileSync(
+ path.join(deploymentsDir, `utxo-gateway-${network.name}.json`),
+ JSON.stringify(deploymentInfo, null, 2)
+ );
+ console.log("Successfully saved deployment info");
+ } catch (error) {
+ console.error("Failed to save deployment info:", error);
+ throw error;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const deploymentsDir = path.join(__dirname, '../deployments'); | |
| if (!fs.existsSync(deploymentsDir)) { | |
| fs.mkdirSync(deploymentsDir); | |
| } | |
| fs.writeFileSync( | |
| path.join(deploymentsDir, `utxo-gateway-${network.name}.json`), | |
| JSON.stringify(deploymentInfo, null, 2) | |
| ); | |
| const deploymentsDir = path.join(__dirname, '../deployments'); | |
| try { | |
| if (!fs.existsSync(deploymentsDir)) { | |
| fs.mkdirSync(deploymentsDir, { recursive: true }); | |
| } | |
| fs.writeFileSync( | |
| path.join(deploymentsDir, `utxo-gateway-${network.name}.json`), | |
| JSON.stringify(deploymentInfo, null, 2) | |
| ); | |
| console.log("Successfully saved deployment info"); | |
| } catch (error) { | |
| console.error("Failed to save deployment info:", error); | |
| throw error; | |
| } |
| "scripts": { | ||
| "test": "echo \"Error: no test specified\" && exit 1", | ||
| "deploy:utxo-gateway": "hardhat run script/hardhat/deploy-and-setup-utxogateway.script.js --network exocore_localnet" | ||
| }, |
There was a problem hiding this comment.
Fix the test script implementation.
The current test script exits with an error message, which contradicts the PR's focus on adding unit tests for BTC staking. This should be updated to run the actual test suite.
- "test": "echo \"Error: no test specified\" && exit 1",
+ "test": "hardhat test",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "scripts": { | |
| "test": "echo \"Error: no test specified\" && exit 1", | |
| "deploy:utxo-gateway": "hardhat run script/hardhat/deploy-and-setup-utxogateway.script.js --network exocore_localnet" | |
| }, | |
| "scripts": { | |
| "test": "hardhat test", | |
| "deploy:utxo-gateway": "hardhat run script/hardhat/deploy-and-setup-utxogateway.script.js --network exocore_localnet" | |
| }, |
| string memory deployed = vm.readFile("script/deployments/deployedBootstrapOnly.json"); | ||
| bootstrapAddr = stdJson.readAddress(deployed, ".clientChain.bootstrap"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for JSON file reading.
The vm.readFile and stdJson.readAddress operations lack proper error handling. If the files don't exist or the JSON structure is invalid, it will fail silently.
- string memory deployed = vm.readFile("script/deployments/deployedBootstrapOnly.json");
- bootstrapAddr = stdJson.readAddress(deployed, ".clientChain.bootstrap");
+ try vm.readFile("script/deployments/deployedBootstrapOnly.json") returns (string memory deployed) {
+ try stdJson.readAddress(deployed, ".clientChain.bootstrap") returns (address addr) {
+ bootstrapAddr = addr;
+ } catch {
+ revert("Invalid JSON structure or missing bootstrap address");
+ }
+ } catch {
+ revert("Failed to read deployedBootstrapOnly.json");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| string memory deployed = vm.readFile("script/deployments/deployedBootstrapOnly.json"); | |
| bootstrapAddr = stdJson.readAddress(deployed, ".clientChain.bootstrap"); | |
| try vm.readFile("script/deployments/deployedBootstrapOnly.json") returns (string memory deployed) { | |
| try stdJson.readAddress(deployed, ".clientChain.bootstrap") returns (address addr) { | |
| bootstrapAddr = addr; | |
| } catch { | |
| revert("Invalid JSON structure or missing bootstrap address"); | |
| } | |
| } catch { | |
| revert("Failed to read deployedBootstrapOnly.json"); | |
| } |
Description
since we have refactored the original
ExocoreBTCGatewayin https://github.com/ExocoreNetwork/exocore-contracts/tree/btc-bridge and add unit tests to make btc staking work, we should also perform some e2e test on a local exocore node to make sureexocoredworks well withUTXOGatewaycontract.As we need to do integration test with exocore's assets precompiles, and even with other off-chain extensions like bridge, using hardhat test is a better way to work around some of foundry's problem related with custom stateful precompiles
Summary by CodeRabbit
Based on the comprehensive summary, here are the concise release notes:
New Features
Improvements
Bug Fixes
Chores