Merge swap tutorials in one #517
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request involves the deletion of the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/pages/developers/tutorials/swap-any.mdx(0 hunks)src/pages/developers/tutorials/swap.mdx(9 hunks)
💤 Files with no reviewable changes (1)
- src/pages/developers/tutorials/swap-any.mdx
🧰 Additional context used
🪛 LanguageTool
src/pages/developers/tutorials/swap.mdx
[uncategorized] ~349-~349: Loose punctuation mark.
Context: ... of information: - address target: The ZRC-20 address of the target token ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~350-~350: Loose punctuation mark.
Context: ...get token on ZetaChain. - bytes to: The recipient's address on the destinat...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~353-~353: Loose punctuation mark.
Context: ...ns like Bitcoin. - bool withdraw: Indicates whether to withdraw the swapp...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (3)
src/pages/developers/tutorials/swap.mdx (3)
83-88: LGTM: Well-implemented upgradeable pattern
The contract correctly implements the OpenZeppelin upgradeable pattern with proper initialization and ownership controls.
Also applies to: 114-124
Line range hint 264-336: LGTM: Robust withdrawal and revert handling
The implementation provides comprehensive error handling and recovery mechanisms for failed withdrawals.
346-420: LGTM: High-quality documentation
The documentation is comprehensive, well-structured, and provides clear examples for both testnet and localnet deployments.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~349-~349: Loose punctuation mark.
Context: ... of information: - address target: The ZRC-20 address of the target token ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~350-~350: Loose punctuation mark.
Context: ...get token on ZetaChain. - bytes to: The recipient's address on the destinat...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~353-~353: Loose punctuation mark.
Context: ...ns like Bitcoin. - bool withdraw: Indicates whether to withdraw the swapp...
(UNLIKELY_OPENING_PUNCTUATION)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/pages/developers/frontend/universalkit.mdx (2)
Line range hint
1-14: Enhance the warning message for production readinessThe current alert about testnet usage is good, but could be more comprehensive for production-grade documentation.
Consider expanding the alert message to include:
-<Alert> The component library is in under active development and currently is recommended for testnet use only.</Alert> +<Alert type="warning"> + ⚠️ Production Notice: + This component library is under active development and is currently recommended for testnet use only. + Breaking changes may occur without notice. + For production deployments, please await stable release announcements. +</Alert>🧰 Tools
🪛 LanguageTool
[uncategorized] ~142-~142: Possible missing comma found.
Context: ...ns on connected chains For cross-chain swaps the component depends on a universal sw...(AI_HYDRA_LEO_MISSING_COMMA)
Line range hint
156-182: Enhance code examples for production readinessThe current example uses a hardcoded contract address without proper context or error handling.
Consider updating the example to be more production-ready:
const contract = "0xb459F14260D1dc6484CE56EB0826be317171e91F"; // universal swap contract + +// In production, consider using environment variables +const UNIVERSAL_SWAP_CONTRACT = process.env.NEXT_PUBLIC_UNIVERSAL_SWAP_CONTRACT; +if (!UNIVERSAL_SWAP_CONTRACT) { + throw new Error("Universal swap contract address not configured"); +} const Page = () => { return ( <div className="m-4"> <div className="flex justify-end gap-2 mb-10"> <ConnectBitcoin /> <ConnectButton label="Connect EVM" showBalance={false} /> </div> <div className="flex justify-center"> <div className="w-[400px]"> - <Swap contract={contract} /> + <Swap contract={UNIVERSAL_SWAP_CONTRACT} /> </div> </div> </div> ); };🧰 Tools
🪛 LanguageTool
[uncategorized] ~142-~142: Possible missing comma found.
Context: ...ns on connected chains For cross-chain swaps the component depends on a universal sw...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/pages/developers/frontend/universalkit.mdx(1 hunks)src/pages/developers/tutorials/_meta.json(0 hunks)
💤 Files with no reviewable changes (1)
- src/pages/developers/tutorials/_meta.json
Co-authored-by: Hernan Clich <[email protected]>
Co-authored-by: Hernan Clich <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/pages/developers/tutorials/swap.mdx (2)
94-98: Enhance error messages for better debuggingThe current error messages are too generic, making it difficult to identify the specific cause of failures.
Consider making the error messages more descriptive:
- error InvalidAddress(); - error Unauthorized(); - error ApprovalFailed(); - error TransferFailed(); + error InvalidAddress(address addr); + error Unauthorized(address sender, address expected); + error ApprovalFailed(address token, address spender, uint256 amount); + error TransferFailed(address token, address from, address to, uint256 amount);
349-354: Fix markdown formatting inconsistenciesThere are some formatting inconsistencies in the documentation.
Apply these changes:
- Remove extra spaces before bullet points
- Add consistent spacing after bullet points
- Ensure uniform indentation in the list items
🧰 Tools
🪛 LanguageTool
[uncategorized] ~349-~349: Loose punctuation mark.
Context: ... of information: -address target: The ZRC-20 address of the target token ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~350-~350: Loose punctuation mark.
Context: ...get token on ZetaChain. -bytes to: The recipient's address on the destinat...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~353-~353: Loose punctuation mark.
Context: ...ns like Bitcoin. -bool withdraw: Indicates whether to withdraw the swapp...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/pages/developers/tutorials/swap.mdx(9 hunks)
🧰 Additional context used
🪛 LanguageTool
src/pages/developers/tutorials/swap.mdx
[uncategorized] ~349-~349: Loose punctuation mark.
Context: ... of information: - address target: The ZRC-20 address of the target token ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~350-~350: Loose punctuation mark.
Context: ...get token on ZetaChain. - bytes to: The recipient's address on the destinat...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~353-~353: Loose punctuation mark.
Context: ...ns like Bitcoin. - bool withdraw: Indicates whether to withdraw the swapp...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~400-~400: Possible missing comma found.
Context: ...kens are approved, either combined or separately depending on whether they are the same....
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (1)
src/pages/developers/tutorials/swap.mdx (1)
255-262:
Add slippage protection to prevent sandwich attacks
The swap is performed without specifying a minimum output amount, which could lead to significant slippage or sandwich attacks.
This issue was previously identified. The fix involves adding a minOutputAmount parameter to protect users from unfavorable trades.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/pages/developers/tutorials/swap.mdx(9 hunks)
🧰 Additional context used
🪛 LanguageTool
src/pages/developers/tutorials/swap.mdx
[uncategorized] ~345-~345: Loose punctuation mark.
Context: ... of information: - address target: The ZRC-20 address of the target token ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~346-~346: Loose punctuation mark.
Context: ...get token on ZetaChain. - bytes to: The recipient's address on the destinat...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~349-~349: Loose punctuation mark.
Context: ...ns like Bitcoin. - bool withdraw: Indicates whether to withdraw the swapp...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~396-~396: Possible missing comma found.
Context: ...kens are approved, either combined or separately depending on whether they are the same....
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (3)
src/pages/developers/tutorials/swap.mdx (3)
255-262: Remove hardcoded minimum output amount
Using 0 as the minimum output amount is unsafe and could lead to significant slippage or sandwich attacks.
334-336: Enhance upgrade implementation with additional safeguards
The current upgrade implementation lacks important safety measures and transparency features.
187-228:
Add slippage and deadline protection
The swap function should include slippage protection and a deadline parameter to prevent sandwich attacks and stale transactions.
function swap(
address inputToken,
uint256 amount,
address targetToken,
bytes memory recipient,
- bool withdrawFlag
+ bool withdrawFlag,
+ uint256 minOutputAmount,
+ uint256 deadline
) public {
+ if (block.timestamp > deadline) revert DeadlineExpired();
bool success = IZRC20(inputToken).transferFrom(
msg.sender,
address(this),
amount
);
if (!success) {
revert TransferFailed();
}
(uint256 out, address gasZRC20, uint256 gasFee) = handleGasAndSwap(
inputToken,
amount,
- targetToken
+ targetToken,
+ minOutputAmount
);Likely invalid or redundant comment.
We have two Swap tutorials. One is meant to be slightly more compact, the other one more feature-rich (for example, being able to swap to ZETA on ZetaChain and swap directly from ZetaChain). The problem is, both contracts have grown fairly large and the LOC difference between them is no longer significant. I've decided to merge these contracts into one so that developers don't have to choose which tutorial to follow.
I've also modified the code to be upgradeable and added a companion contract.
Related: zeta-chain/example-contracts#223
Summary by CodeRabbit
New Features
Swapcontract tutorial with a clearer step-by-step process for creating cross-chain swaps.Alertcomponent in the UniversalKit documentation to indicate active development status.Bug Fixes
Documentation
Chores
SwapToAnyTokenand its associated metadata entry.