feat: add EVM bridge (Solidity) and E2E testnet setup#19
Conversation
- Add ethereum/ with CantonBridge, TokenRegistry contracts - Add docs/E2E-TESTNET-SETUP.md for Sepolia testing guide - Add bridge web UI for deposit/withdraw testing - Update scripts to support Solidity builds (--solidity flag) - Add scripts/deploy-canton.sh for DAR deployment - Update README with EVM bridge section and docs references
Summary of ChangesHello @sqhell, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates core EVM bridging capabilities into the project, establishing a robust framework for cross-chain token transfers. It encompasses the development of essential smart contracts, a user-friendly interface for interaction, and detailed documentation to facilitate end-to-end testing and deployment on relevant testnets. The changes aim to expand the project's interoperability by connecting with Ethereum-compatible networks. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a substantial new feature: an EVM bridge, complete with Solidity smart contracts, comprehensive documentation for E2E testing, and updated scripts. The core contracts (CantonBridge, TokenRegistry, RateLimiter) are well-structured and utilize established security patterns from OpenZeppelin, such as ReentrancyGuard, Pausable, and AccessControl. My review has identified a few critical security vulnerabilities in the documentation where sensitive credentials like a private key and an API key are hardcoded. Additionally, I've noted a high-severity centralization risk in the CantonBridge contract's emergency withdrawal functionality and a medium-severity data inconsistency issue in the TokenRegistry. Addressing these points will significantly improve the security and robustness of this new bridge infrastructure.
|
|
||
| **RPC Endpoint:** | ||
| ``` | ||
| https://eth-sepolia.g.alchemy.com/v2/MeMdx3uk0ZFuSy2YFs0VAGjG7gXf0wJP |
There was a problem hiding this comment.
A hardcoded Alchemy API key has been found in the documentation. This poses a security risk as it exposes the key to the public. It should be replaced with a placeholder, and users should be instructed to provide their own key. For example, use <YOUR_ALCHEMY_API_KEY>.
| Create `~/chainsafe/canton-middleware/.env.local`: | ||
| ```bash | ||
| # Ethereum Sepolia | ||
| ETHEREUM_RELAYER_PRIVATE_KEY=0x082560991dcfb10aff28a973120329d0fbf1e490357cfcf15ad9d17548c29eb2 |
There was a problem hiding this comment.
A hardcoded private key is present in the setup guide. Exposing private keys, even for testnets, is a major security risk and encourages poor security practices. This key should be removed and replaced with a placeholder, for example, <YOUR_RELAYER_PRIVATE_KEY>.
| function emergencyWithdraw( | ||
| address token, | ||
| uint256 amount, | ||
| address recipient | ||
| ) external onlyRole(DEFAULT_ADMIN_ROLE) { | ||
| if (recipient == address(0)) revert InvalidRecipient(recipient); | ||
|
|
||
| IERC20(token).safeTransfer(recipient, amount); | ||
|
|
||
| // Update locked balance if possible | ||
| if (lockedBalances[token] >= amount) { | ||
| lockedBalances[token] -= amount; | ||
| } else { | ||
| lockedBalances[token] = 0; | ||
| } | ||
|
|
||
| emit EmergencyWithdrawal(token, amount, recipient); | ||
| } |
There was a problem hiding this comment.
The emergencyWithdraw function grants the DEFAULT_ADMIN_ROLE the ability to withdraw any amount of any token from the bridge. While intended for emergencies, this creates a significant centralization risk and a single point of failure. If the admin key is compromised, all funds in the bridge could be drained. Consider implementing additional safeguards, such as a multi-signature requirement or a time-lock, for this powerful function to reduce the risk.
| function removeSupportedChain(uint256 chainId) external onlyRole(DEFAULT_ADMIN_ROLE) { | ||
| supportedChains[chainId] = false; | ||
| // Note: We don't remove from array to preserve indices | ||
| } |
There was a problem hiding this comment.
The removeSupportedChain function only sets the chain's status to false in the supportedChains mapping but does not remove the corresponding chainId from the supportedChainList array. This leads to an inconsistent state where getSupportedChains() will continue to return the 'removed' chain ID, which could mislead off-chain clients. To ensure data integrity, the chainId should also be removed from the supportedChainList array. A common and efficient pattern for this is to swap the element to be removed with the last element of the array and then pop.
2 participants
Pull Request
Description
Add EVM bridge infrastructure for Canton-Ethereum token bridging. This includes Solidity smart contracts (CantonBridge, TokenRegistry), a web UI for testing deposits/withdrawals, E2E testnet setup documentation, and updated build scripts to support Solidity compilation.
Type of Change
Related Issues
Package(s) Affected
commoncip56-tokenbridge-corebridge-wayfinderbridge-usdcbridge-cbtcbridge-genericdvpintegration-testsN/A - This PR adds the new
ethereum/directory (Solidity contracts, not Daml packages)Checklist
Code Quality
Testing
./scripts/test-all.shsuccessfullyDocumentation
Security (if applicable)
Additional Notes
New files/directories:
ethereum/- Solidity contracts (CantonBridge, TokenRegistry, RateLimiter), Foundry tests, deployment scripts, web UIdocs/E2E-TESTNET-SETUP.md- Complete E2E testing guide for Sepolia + Canton quickstartscripts/deploy-canton.sh- DAR deployment helper scriptModified:
README.md- Added EVM Bridge section, updated repo structure, references E2E docscripts/build-all.sh- Added--solidityflag for Foundry buildsscripts/clean-all.sh- Cleans ethereum/out directoryscripts/test-all.sh- Runs Foundry tests