-
Notifications
You must be signed in to change notification settings - Fork 61
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
Update tutorials for Gateway on testnet #499
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Warning Rate limit exceeded@fadeev has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 54 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces significant updates to several tutorial files and their metadata, primarily focusing on enhancing the clarity and functionality of the tutorials related to message passing, token swapping, and NFT minting on ZetaChain. Key changes include renaming and rebranding existing tutorials, introducing new tutorials, and modifying tutorial content to reflect the latest contract functionalities and deployment instructions. The updates aim to provide developers with accurate and streamlined guidance for building cross-chain applications. 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 (
|
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: 8
🧹 Outside diff range and nitpick comments (7)
src/pages/developers/tutorials/localnet.mdx (1)
148-152
: Consider adding a note block for better visibility.
This is important information about address persistence and verification. Consider wrapping it in a note or warning block to make it more prominent.
-These addresses correspond to the contracts deployed on your local blockchain.
-You can interact with them using your preferred development tools. Note that
-while these addresses remain persistent between restarts, they may vary on
-different instances of localnet. For the latest list of addresses, please refer
-to the table displayed in the terminal window where localnet is running.
+:::note Important
+These addresses correspond to the contracts deployed on your local blockchain.
+You can interact with them using your preferred development tools. Note that
+while these addresses remain persistent between restarts, they may vary on
+different instances of localnet. For the latest list of addresses, please refer
+to the table displayed in the terminal window where localnet is running.
+:::
src/pages/developers/tutorials/swap-any.mdx (1)
Line range hint 79-108
: Add bounds checking for Bitcoin message parsing.
The Bitcoin message parsing lacks proper length validation before accessing bytes at specific offsets, which could lead to out-of-bounds access.
Consider adding explicit length checks:
if (context.chainID == BITCOIN) {
+ require(message.length >= 20, "Invalid message length for target");
params.target = BytesHelperLib.bytesToAddress(message, 0);
+ require(message.length >= 40, "Invalid message length for recipient");
params.to = abi.encodePacked(
BytesHelperLib.bytesToAddress(message, 20)
);
if (message.length >= 41) {
params.withdraw = BytesHelperLib.bytesToBool(message, 40);
}
}
src/pages/developers/tutorials/hello.mdx (4)
20-20
: Enhance Gateway availability notice.
Consider providing more specific information about the Gateway's status and any limitations or considerations for both localnet and testnet environments.
-<Alert>This tutorial relies on the Gateway, which is currently available only on localnet and testnet.</Alert>
+<Alert>
+ This tutorial relies on the Gateway contract, which is:
+ - Available on localnet for development and testing
+ - Available on testnet for pre-production validation
+ - Not yet available on mainnet
+</Alert>
Line range hint 134-136
: Update function name in documentation.
The documentation refers to onCrossChainCall
but the contract implements onCall
. Update the documentation to reflect the current implementation.
-interface, which requires the implementation of `onCrossChainCall` and
-`onRevert` functions for handling cross-chain interactions.
+interface, which requires the implementation of `onCall` and `onRevert`
+functions for handling cross-chain interactions.
Line range hint 141-149
: Update parameter type in documentation.
The documentation describes zContext
but the contract uses MessageContext
. Update the documentation to match the implementation.
-The `onCrossChainCall` function is a special handler that gets triggered when
+The `onCall` function is a special handler that gets triggered when
the contract receives a call from a connected chain through the gateway. This
function processes the incoming data, which includes:
-- `context`: A `zContext` struct containing:
+- `context`: A `MessageContext` struct containing:
- `origin`: The address (EOA or contract) that initiated the gateway call on
the connected chain.
- `chainID`: The integer ID of the connected chain from which the cross-chain
284-289
: Use placeholder addresses in examples.
Replace specific contract addresses with clearly marked placeholders to prevent confusion when users have different deployment addresses.
-📜 Contract address: 0x84eA74d481Ee0A5332c457a4d796187F6Ba67fEB
+📜 Contract address: <HELLO_CONTRACT_ADDRESS>
-📜 Contract address: 0x9E545E3C0baAB3E08CdfD552C960A1050f373042
+📜 Contract address: <ECHO_CONTRACT_ADDRESS>
- --contract 0x9E545E3C0baAB3E08CdfD552C960A1050f373042 \
- --receiver 0x84eA74d481Ee0A5332c457a4d796187F6Ba67fEB \
+ --contract <ECHO_CONTRACT_ADDRESS> \
+ --receiver <HELLO_CONTRACT_ADDRESS> \
- --contract 0x84eA74d481Ee0A5332c457a4d796187F6Ba67fEB \
- --receiver 0x9E545E3C0baAB3E08CdfD552C960A1050f373042 \
+ --contract <HELLO_CONTRACT_ADDRESS> \
+ --receiver <ECHO_CONTRACT_ADDRESS> \
- --contract 0x84eA74d481Ee0A5332c457a4d796187F6Ba67fEB \
- --receiver 0x9E545E3C0baAB3E08CdfD552C960A1050f373042 \
+ --contract <HELLO_CONTRACT_ADDRESS> \
+ --receiver <ECHO_CONTRACT_ADDRESS> \
Also applies to: 302-303, 335-336, 362-363
src/pages/developers/tutorials/nft.mdx (1)
3-4
: Add security considerations section to the tutorial.
The tutorial should include a section about security considerations and best practices, such as:
- Importance of proper access control
- Handling of cross-chain transaction failures
- Gas considerations for cross-chain operations
- Potential risks of token ID collisions
🧰 Tools
🪛 LanguageTool
[typographical] ~3-3: It appears that a comma is missing./components/shared"; In this tutorial you will learn how to create a universa...
Context: ... } from "
(DURING_THAT_TIME_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
- src/pages/developers/tutorials/_meta.json (1 hunks)
- src/pages/developers/tutorials/hello.mdx (9 hunks)
- src/pages/developers/tutorials/localnet.mdx (2 hunks)
- src/pages/developers/tutorials/nft.mdx (1 hunks)
- src/pages/developers/tutorials/swap-any.mdx (7 hunks)
- src/pages/developers/tutorials/swap-tss.mdx (0 hunks)
- src/pages/developers/tutorials/swap.mdx (5 hunks)
💤 Files with no reviewable changes (1)
- src/pages/developers/tutorials/swap-tss.mdx
🧰 Additional context used
🪛 LanguageTool
src/pages/developers/tutorials/nft.mdx
[typographical] ~3-3: It appears that a comma is missing./components/shared"; In this tutorial you will learn how to create a universa...
Context: ... } from "
(DURING_THAT_TIME_COMMA)
[uncategorized] ~268-~268: Possible missing comma found.
Context: ...d with the gas limit on the destination chain and isArbitraryCall
(the second param...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~271-~271: Possible missing article found.
Context: ...estination chain, but without providing address of the universal contract making the ca...
(AI_HYDRA_LEO_MISSING_THE)
[formatting] ~274-~274: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ... isArbitraryCall
to false is important, because the contract on a connected chain must ...
(COMMA_BEFORE_BECAUSE)
[grammar] ~306-~306: The word ‘withdraw’ is not a noun. Did you mean “withdrawal”?
Context: ... connected chain. First, it queries the withdraw fee on the destination chain. Then, it ...
(PREPOSITION_VERB)
[uncategorized] ~467-~467: Possible missing comma found.
Context: ...connected chain. To transfer an NFT to ZetaChain the destination address must be specifi...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~470-~470: Possible missing comma found.
Context: ...To transfer an NFT to another connected chain the destination address must be the ZRC...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~474-~474: Possible missing comma found.
Context: ...C-20 ETH address. When transferring to ZetaChain a no asset gateway.call
is executed, ...
(AI_HYDRA_LEO_MISSING_COMMA)
[formatting] ~474-~474: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...in a no asset gateway.call
is executed, because cross-chain calls to ZetaChain do not r...
(COMMA_BEFORE_BECAUSE)
[uncategorized] ~511-~511: Possible missing preposition found.
Context: ... addresses, mint an NFT and transfer it ZetaChain → Ethereum → BNB → ZetaChain: ``` ./sc...
(AI_HYDRA_LEO_MISSING_TO)
🔇 Additional comments (9)
src/pages/developers/tutorials/_meta.json (3)
8-10
: LGTM: Improved tutorial title and description.
The rename from "First Universal App on Localnet" to "Message Passing" better reflects the core concept being taught, while the updated description provides clearer learning objectives focused on cross-chain functionality.
13-15
: LGTM: Simplified tutorial title with updated chain examples.
The removal of environment specificity from the title and the updated chain examples (Ethereum, Solana, Bitcoin) provide a more accurate representation of the tutorial's scope.
18-21
: LGTM: Well-structured new tutorials with clear progression.
The new tutorials "Swap Any Token" and "NFT" are logical additions:
- "Swap Any Token" builds upon the basic swap tutorial
- "NFT" introduces cross-chain NFT functionality
- Both have appropriate time estimates and clear descriptions
Let's verify the tutorial ordering makes sense:
Also applies to: 22-25
✅ Verification successful
Let me verify the tutorial content structure more thoroughly before making a final determination.
Tutorial sequence and structure verification complete
The tutorial ordering in _meta.json follows a logical progression:
- Getting Started (intro) → basic setup
- Message Passing (hello) → fundamentals
- Swap → basic swap functionality
- Swap Any Token → advanced swap features
- NFT → different use case
- Localnet → development environment
All tutorial files exist in the correct location and follow the natural learning curve from basic concepts to more advanced implementations. The time estimates are appropriate, with more complex tutorials (swap-any, nft) allocated 60 minutes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the referenced MDX files exist and check their content structure
# Test: Check if the new tutorial files exist
echo "Verifying new tutorial files..."
for file in swap-any.mdx nft.mdx; do
if ! fd -t f "^${file}$" src/pages/developers/tutorials/; then
echo "Warning: Missing tutorial file: $file"
fi
done
# Test: Check content structure in new tutorials
echo "Checking tutorial structure..."
rg -l "^#\s+Prerequisites|^#\s+Introduction" src/pages/developers/tutorials/
Length of output: 532
Script:
#!/bin/bash
# Check the content and ordering of tutorials in _meta.json
echo "Checking tutorial ordering in _meta.json..."
cat src/pages/developers/tutorials/_meta.json
# Check content of both new tutorials
echo -e "\nChecking swap-any.mdx content structure..."
rg -A 5 "^#\s+" src/pages/developers/tutorials/swap-any.mdx
echo -e "\nChecking nft.mdx content structure..."
rg -A 5 "^#\s+" src/pages/developers/tutorials/nft.mdx
# List all tutorial files to verify complete sequence
echo -e "\nListing all tutorial files..."
fd . src/pages/developers/tutorials/ -e mdx
Length of output: 1834
src/pages/developers/tutorials/localnet.mdx (1)
123-145
: LGTM! Well-structured contract address tables.
The tables are well-organized with:
- Clear separation between EVM and ZetaChain contracts
- Consistent formatting
- Valid Ethereum addresses
- Comprehensive coverage of essential contracts including the new Uniswap Router entry
src/pages/developers/tutorials/swap-any.mdx (1)
25-25
: LGTM: Alert placement and messaging.
The alert effectively communicates Gateway availability limitations at the appropriate point in the tutorial flow.
src/pages/developers/tutorials/swap.mdx (4)
43-43
: LGTM: Clear and accurate Gateway availability message.
259-267
: LGTM: Clear localnet setup instructions.
Line range hint 273-282
: LGTM: Improved deployment workflow using yarn scripts.
290-290
: LGTM: Clear and well-documented swap command examples.
The commands are properly formatted with consistent contract addresses and clear parameter explanations.
Also applies to: 320-320
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: 2
🧹 Outside diff range and nitpick comments (5)
src/components/Docs/components/ContractAddresses.tsx (2)
Line range hint
41-60
: Add error handling for failed fetch requests.The current implementation doesn't handle network failures or invalid responses explicitly. This could lead to a poor user experience if the GitHub URLs are inaccessible.
Consider implementing error handling and a retry mechanism:
const fetchAndGroupAddresses = async () => { setIsLoading(true); + try { + const fetchWithRetry = async (url: string, retries = 3) => { + for (let i = 0; i < retries; i++) { + try { + const response = await fetch(url); + if (!response.ok) throw new Error(`HTTP error! status: ${response.status}`); + return response; + } catch (e) { + if (i === retries - 1) throw e; + await new Promise(resolve => setTimeout(resolve, 1000 * Math.pow(2, i))); + } + } + }; - const responses = await Promise.all([fetch(addressesUrl.testnet), fetch(addressesUrl.mainnet)]); + const responses = await Promise.all([ + fetchWithRetry(addressesUrl.testnet), + fetchWithRetry(addressesUrl.mainnet) + ]); const [testnetData, mainnetData] = await Promise.all(responses.map((res) => res.json())); setGroupedData({ testnet: sortGroupedData(groupDataByChain(testnetData)), mainnet: sortGroupedData(groupDataByChain(mainnetData)), }); + } catch (error) { + console.error('Failed to fetch addresses:', error); + // Handle error state appropriately + } finally { setIsLoading(false); + } };
Line range hint
36-40
: Implement caching mechanism for better performance.The contract addresses are fetched on every component mount, which could be optimized by implementing a caching strategy.
Consider implementing a caching mechanism:
const CACHE_DURATION = 5 * 60 * 1000; // 5 minutes interface CacheEntry { data: ContractAddressesByChain; timestamp: number; } const addressCache: Record<NetworkType, CacheEntry | null> = { testnet: null, mainnet: null, }; const isCacheValid = (cache: CacheEntry | null) => { return cache && Date.now() - cache.timestamp < CACHE_DURATION; }; // Update the useEffect to use cache useEffect(() => { const fetchAndGroupAddresses = async () => { setIsLoading(true); try { const results: Record<NetworkType, ContractAddressesByChain> = { testnet: {}, mainnet: {}, }; for (const network of ['testnet', 'mainnet'] as NetworkType[]) { if (isCacheValid(addressCache[network])) { results[network] = addressCache[network]!.data; continue; } const response = await fetch(addressesUrl[network]); const data: ContractAddressData[] = await response.json(); const grouped = sortGroupedData(groupDataByChain(data)); addressCache[network] = { data: grouped, timestamp: Date.now(), }; results[network] = grouped; } setGroupedData(results); } catch (error) { console.error('Failed to fetch addresses:', error); } finally { setIsLoading(false); } }; fetchAndGroupAddresses(); }, []);src/pages/developers/tutorials/hello.mdx (3)
Line range hint
2-20
: Update function name references in the introductionThe introduction refers to
onCrossChainCall
, but the actual implementation usesonCall
. This inconsistency might confuse readers.Update the introduction to match the implementation:
-Within `onCrossChainCall`, the contract decodes the `name` from the `message` +Within `onCall`, the contract decodes the `name` from the `message`
Line range hint
68-94
: Add access control and input validation to critical functionsThe
onCall
function lacks proper access control and input validation:
- Only the gateway should be able to call this function
- Input parameters should be validated
Add the following security measures:
+ error InvalidCaller(); + error InvalidInput(); function onCall( MessageContext calldata context, address zrc20, uint256 amount, bytes calldata message ) external override { + if (msg.sender != address(gateway)) revert InvalidCaller(); + if (message.length == 0) revert InvalidInput(); string memory name = abi.decode(message, (string)); emit HelloEvent("Hello on ZetaChain", name); }
255-273
: Enhance deployment instructions with prerequisitesThe deployment section should include:
- Required environment variables
- Network configuration
- Account permissions and funding requirements
Add a prerequisites section before deployment:
### Prerequisites for Deployment 1. Set up environment variables: ```bash export PRIVATE_KEY=your_private_key export ETHERSCAN_API_KEY=your_api_key # For contract verification
- Ensure your account:
- Has sufficient funds for deployment
- Has the necessary permissions
- Is properly configured in hardhat.config.js
</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 0c0bbf57fa6d8e0b14f272c263b85c1d511ea702 and 2bf4d7b6decdbdf73843f5f525b7effb7c99d8f3. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `src/components/Docs/components/ContractAddresses.tsx` (1 hunks) * `src/pages/developers/tutorials/hello.mdx` (10 hunks) * `src/pages/developers/tutorials/swap.mdx` (5 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * src/pages/developers/tutorials/swap.mdx </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
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: 1
🧹 Outside diff range and nitpick comments (2)
src/pages/developers/tutorials/nft.mdx (2)
164-205
: Consider implementing a circuit breaker pattern for cross-chain transfers.The
onCall
function handles critical cross-chain transfers but lacks an emergency stop mechanism. This could be problematic if issues are detected in the cross-chain transfer logic.Consider adding a circuit breaker:
+ bool public paused; + + modifier whenNotPaused() { + require(!paused, "Contract is paused"); + _; + } + + function setPaused(bool _paused) external onlyOwner { + paused = _paused; + emit PausedStateChanged(_paused); + } + + event PausedStateChanged(bool isPaused); - function onCall( + function onCall( MessageContext calldata context, address zrc20, uint256 amount, bytes calldata message - ) external override { + ) external override whenNotPaused {
3-4
: Minor grammatical improvements needed.Consider adding a comma after "In this tutorial" for better readability.
Apply this change:
-In this tutorial you will learn how to create a universal ERC-721 NFT that can +In this tutorial, you will learn how to create a universal ERC-721 NFT that can🧰 Tools
🪛 LanguageTool
[typographical] ~3-
3: It appears that a comma is missing./components/shared"; In this tutorial you will learn how to create a universa...
Context: ... } from "(DURING_THAT_TIME_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/pages/developers/tutorials/nft.mdx
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
src/pages/developers/tutorials/nft.mdx
[typographical] ~3-3: It appears that a comma is missing./components/shared"; In this tutorial you will learn how to create a universa...
Context: ... } from "
(DURING_THAT_TIME_COMMA)
[formatting] ~283-~283: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ... isArbitraryCall
to false is important, because the contract on a connected chain must ...
(COMMA_BEFORE_BECAUSE)
[grammar] ~315-~315: The word ‘withdraw’ is not a noun. Did you mean “withdrawal”?
Context: ... connected chain. First, it queries the withdraw fee on the destination chain. Then, it ...
(PREPOSITION_VERB)
[formatting] ~483-~483: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...in a no asset gateway.call
is executed, because cross-chain calls to ZetaChain do not r...
(COMMA_BEFORE_BECAUSE)
@brewmaster012 @bbbeeeee @andresaiello @zeta-chain/fullstack please, review. |
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.
utACK
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.
LGTM
Depends on zeta-chain/example-contracts#207
Summary by CodeRabbit