-
Notifications
You must be signed in to change notification settings - Fork 53
chore: switch to context sender/senderEVM #257
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
Conversation
📝 WalkthroughWalkthroughThis update transitions localnet management scripts across multiple example projects from Hardhat-based commands to the Zetachain CLI, introduces readiness checks for localnet startup, and updates dependencies to use the latest Zetachain packages. The Changes
Sequence Diagram(s)Swap Contract: Token Swap and Revert FlowsequenceDiagram
participant User
participant SwapContract
participant Gateway
User->>SwapContract: swap(params)
SwapContract->>Gateway: emit TokenSwap(bytes sender, ...)
Note right of SwapContract: sender is now bytes
alt Revert Occurs
Gateway->>SwapContract: onRevert(context)
SwapContract->>Gateway: withdraw(..., bytes sender, ...)
Note right of SwapContract: sender decoded as bytes and cast as needed
end
Localnet Startup and Readiness Check (Script Flow)sequenceDiagram
participant Script
participant ZetachainCLI
Script->>ZetachainCLI: localnet start --skip sui,ton,solana --exit-on-error
loop until localnet.json exists
Script->>Script: sleep 1
end
Script->>ZetachainCLI: localnet check
Script->>ZetachainCLI: localnet stop
Issue Highlight: Recommendation: ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 5
🔭 Outside diff range comments (3)
examples/swap/scripts/localnet.sh (1)
244-244
: 🛠️ Refactor suggestionWait for CLI shutdown.
After stopping the localnet, explicitly wait on the background PID:
yarn zetachain localnet stop wait $LOCALNET_PIDThis ensures the process fully exits before the script terminates.
examples/swap/contracts/Swap.sol (2)
261-265
:⚠️ Potential issueCompile error –
IWETH9
is neither imported nor required here
IWETH9
is referenced, but the contract only importsIZRC20
/IERC20
.
Unless you deliberately depend on a wrapped-Zeta interface, this line will not compile.
UsingIZRC20
(already imported) is sufficient because it exposestransfer
.- bool success = IWETH9(params.target).transfer( + bool success = IZRC20(params.target).transfer( address(uint160(bytes20(params.to))), out );
317-318
:⚠️ Potential issueUndefined interface – replace
IUniswapV2Router01
withIUniswapV2Router02
IUniswapV2Router01
is not imported, while Router 02 is. Casting to the imported interface resolves the compile error and keeps the behaviour identical.- address zeta = IUniswapV2Router01(uniswapRouter).WETH(); + address zeta = IUniswapV2Router02(uniswapRouter).WETH();
🧹 Nitpick comments (4)
examples/call/scripts/localnet.sh (1)
39-242
: DRY out repeated readiness checks.The script invokes
yarn zetachain localnet check
over a dozen times. Extract this into a helper function for clarity and maintainability:check_localnet() { yarn zetachain localnet check } # Then replace each invocation with: check_localnetexamples/swap/scripts/localnet.sh (2)
38-136
: Factor out the repeatedlocalnet check
calls.You invoke
yarn zetachain localnet check
many times; extract it into a function:run_check() { yarn zetachain localnet check } # Then replace each occurrence with: run_checkThis reduces duplication and improves readability.
86-96
: Clean up obsolete commented code.There’s a block of commented Hardhat and localnet commands for SUI/Solana flows. If they’re not needed, remove or archive them to keep the script concise.
examples/swap/contracts/Swap.sol (1)
118-126
: Consider indexing thesender
field or emitting both forms
Changingsender
fromaddress indexed
tobytes
makes filtering for a specific sender in logs harder (bytes
is not index-able).
If off-chain consumers still need efficient queries, emit an auxiliary event or keep the fieldindexed
when the length is exactly 20 bytes.No diff provided – design choice.
Also applies to: 156-164
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
examples/call/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
examples/hello/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
examples/swap/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (7)
examples/call/package.json
(1 hunks)examples/call/scripts/localnet.sh
(20 hunks)examples/hello/package.json
(1 hunks)examples/hello/scripts/localnet.sh
(2 hunks)examples/swap/contracts/Swap.sol
(6 hunks)examples/swap/package.json
(1 hunks)examples/swap/scripts/localnet.sh
(10 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: slither (examples/swap, swap.sarif)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
🔇 Additional comments (4)
examples/call/package.json (1)
61-64
: Dependency version upgrades are consistent.The bump of
@zetachain/protocol-contracts
to13.0.0
and addition ofzetachain@^3.0.0
align with the other examples. Ensure that the@zetachain/networks
package remaining at13.0.0-rc1
is intentional and compatible with these stable versions.examples/hello/package.json (1)
60-62
: Dependencies match the overall upgrade.The update to
@zetachain/[email protected]
and addition ofzetachain@^3.0.0
mirror the other example projects. No further changes needed here.examples/swap/package.json (1)
60-63
: Confirm runtime versus dev dependency for@zetachain/toolkit
.Unlike the other examples,
swap
places@zetachain/[email protected]
independencies
. Verify if it’s required at runtime; otherwise, consider moving it todevDependencies
to reduce bundle size.examples/hello/scripts/localnet.sh (1)
3-5
: Ensure clean shutdown with a trap
The localnet is started in the background (&
) but the script does not guarantee that it is stopped if the script exits early (e.g., on error or CTRL-C). This can leave orphaned processes listening on the same ports and flake subsequent runs.set -e set -x set -o pipefail + +cleanup() { + # Stop localnet gracefully; ignore errors if it is already stopped + yarn zetachain localnet stop || true +} +trap cleanup EXIT[ suggest_essential_refactor ]
Also applies to: 25-27
yarn zetachain localnet stop |
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.
🛠️ Refactor suggestion
Ensure graceful shutdown.
After yarn zetachain localnet stop
, wait on the background process to avoid race conditions:
yarn zetachain localnet stop
wait $LOCALNET_PID
yarn zetachain localnet start --skip sui ton solana --exit-on-error & | ||
|
||
while [ ! -f "localnet.json" ]; do sleep 1; done |
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.
🛠️ Refactor suggestion
Capture and trap the background process.
Running the CLI in the background without tracking its PID can lead to orphaned processes. Consider:
yarn zetachain localnet start --skip sui ton solana --exit-on-error &
LOCALNET_PID=$!
trap "kill $LOCALNET_PID" EXIT
This ensures proper cleanup on script exit.
yarn zetachain localnet start --skip sui ton solana --exit-on-error & | ||
|
||
while [ ! -f "localnet.json" ]; do sleep 1; done |
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.
🛠️ Refactor suggestion
Manage the background CLI process.
Similar to the call
example, capture the PID and trap cleanup:
yarn zetachain localnet start --skip sui ton solana --exit-on-error &
LOCALNET_PID=$!
trap "kill $LOCALNET_PID" EXIT
This prevents orphaned processes if the script is aborted.
yarn zetachain localnet start --skip sui ton solana --exit-on-error & | ||
|
||
while [ ! -f "localnet.json" ]; do sleep 1; done |
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.
Add a timeout / readiness guard to avoid hanging forever
The loop waits indefinitely for localnet.json
. If yarn zetachain localnet start
fails or the file is written elsewhere, the script will block forever and the CI job (or developer terminal) will never complete.
-while [ ! -f "localnet.json" ]; do sleep 1; done
+# Wait up to 90 s for the localnet to become ready
+MAX_WAIT=90
+elapsed=0
+until [ -f "localnet.json" ] || [ "$elapsed" -ge "$MAX_WAIT" ]; do
+ sleep 1
+ elapsed=$((elapsed+1))
+done
+
+if [ ! -f "localnet.json" ]; then
+ echo "❌ localnet did not start in time" >&2
+ exit 1
+fi
📝 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.
yarn zetachain localnet start --skip sui ton solana --exit-on-error & | |
while [ ! -f "localnet.json" ]; do sleep 1; done | |
yarn zetachain localnet start --skip sui ton solana --exit-on-error & | |
# Wait up to 90 s for the localnet to become ready | |
MAX_WAIT=90 | |
elapsed=0 | |
until [ -f "localnet.json" ] || [ "$elapsed" -ge "$MAX_WAIT" ]; do | |
sleep 1 | |
elapsed=$((elapsed+1)) | |
done | |
if [ ! -f "localnet.json" ]; then | |
echo "❌ localnet did not start in time" >&2 | |
exit 1 | |
fi |
revertAddress: address(bytes20(sender)), | ||
callOnRevert: false, | ||
abortAddress: address(0), |
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.
🛠️ Refactor suggestion
Validate sender
length before casting to address
address(bytes20(sender))
silently truncates/zero-pads if sender.length ≠ 20
, which could mis-route funds on revert.
- revertAddress: address(bytes20(sender)),
+ revertAddress: sender.length == 20
+ ? address(bytes20(sender))
+ : address(0), // fallback / sentinel
Add a require/revert if an unexpected length is encountered to fail fast.
📝 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.
revertAddress: address(bytes20(sender)), | |
callOnRevert: false, | |
abortAddress: address(0), | |
revertAddress: sender.length == 20 | |
? address(bytes20(sender)) | |
: address(0), // fallback / sentinel | |
callOnRevert: false, | |
abortAddress: address(0), |
"@zetachain/protocol-contracts": "12.0.0-rc1", | ||
"@zetachain/protocol-contracts-solana": "2.0.0-rc1" | ||
"@zetachain/protocol-contracts": "13.0.0", | ||
"@zetachain/protocol-contracts-solana": "2.0.0-rc1", |
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.
we are already on 3.0.0 here so might be worth checking to upgrade this as well in separate PR
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.
Will do!
npx hardhat compile --force --quiet | ||
|
||
ZRC20_ETHEREUM=$(jq -r '.addresses[] | select(.type=="ZRC-20 ETH on 5") | .address' localnet.json) | ||
USDC_ETHEREUM=$(jq -r '.addresses[] | select(.type=="ERC-20 USDC" and .chain=="ethereum") | .address' localnet.json) | ||
ZRC20_USDC=$(jq -r '.addresses[] | select(.type=="ZRC-20 USDC on 97") | .address' localnet.json) | ||
ZRC20_BNB=$(jq -r '.addresses[] | select(.type=="ZRC-20 BNB on 97") | .address' localnet.json) | ||
ZRC20_SOL=$(jq -r '.addresses[] | select(.type=="ZRC-20 SOL on 901") | .address' localnet.json) | ||
# ZRC20_SOL=$(jq -r '.addresses[] | select(.type=="ZRC-20 SOL on 901") | .address' localnet.json) |
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.
just to double check if this should be commented out, same for other sol related commented code
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.
It should yes. I'll enable Solana tests after I figure out why it's failing when running in the CLI.
Depends on ZetaChain CLI with localnet that supports running in background and sender/senderEVM changes and on a stable version of protocol contracts.
sleep 15
inside tests with a loop that checks whenlocalnet.json
becomes availableSummary by CodeRabbit
New Features
Refactor