Closed skosito closed 4 months ago
The changes involve refactoring Sender
to SenderZEVM
in the ZEVM contract to create consistency with the ZEVM environment, adding new tasks in localnet.ts
for interacting with EVM and ZEVM contracts, and introducing a worker script (worker.ts
) to deploy and interact across environments. A new scripts/readme.md
file documents the worker script functionalities.
File(s) | Change Summary |
---|---|
pkg/contracts/.../senderzevm.go |
Refactor: Renamed entities related to Sender contract to SenderZEVM for consistent naming. |
tasks/localnet.ts |
New Tasks: Added tasks for calling contracts and managing assets between ZEVM and EVM environments. |
scripts/worker.ts |
New Script: Introduced worker script for deployment and interaction across EVM and ZEVM contracts. |
scripts/readme.md |
Documentation: Added documentation for the worker script, outlining its purpose and usage. |
sequenceDiagram
participant Worker as worker.ts
participant EVM as EVM
participant ZEVM as ZEVM
participant GatewayEVM as GatewayEVM
participant GatewayZEVM as GatewayZEVM
Worker->>EVM: Deploy ReceiverEVM Contract
Worker->>ZEVM: Deploy SenderZEVM Contract
loop Management
note over Worker: Mint and Transfer Tokens
Worker->>EVM: Mint ERC-20
Worker->>ZEVM: Mint ZRC-20
end
loop Interactions
note over Worker: Calls, Withdrawals, Deposits through Gateways
Worker->>GatewayZEVM: Call ReceiverEVM
GatewayZEVM->>EVM: Execute Call
Worker->>GatewayEVM: Call SenderZEVM
GatewayEVM->>ZEVM: Execute Call
end
In a world of EVM and ZEVM blend,
We renamed contracts to better portend.
Tasks now call, withdraw, and send,
Local nets and workers, they extend.
With code aligned from start to end,
Deployments easier, changes mend.
🐇✨
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?
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 59.25%. Comparing base (
6bf41ee
) to head (be1f176
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
start localnet (these 2 commands in 2 separate terminals so you can follow logs) yarn localnode yarn localnet
What is exactly localnet
if localnode
represent the Hardhat node? Couldn't we think about merging both?
start localnet (these 2 commands in 2 separate terminals so you can follow logs) yarn localnode yarn localnet
What is exactly
localnet
iflocalnode
represent the Hardhat node? Couldn't we think about merging both?
good point, localnet is not good name, it is deploying contracts on localnode and listening to events and calling contracts, so maybe it should be called worker instead of localnet
i dont think we should merge these 2, its better to run separately and see logs of both
Do I understand correctly that this essentially deploys protocol contracts to a chain running locally managed by Hardhat. Then it monitors existing connected blockchains like Sepolia for incoming transactions (acting sort of like zetaclient, but without voting and consensus) and processes these calls by calling universal apps on this chain that's running in Hardhat.
Do I understand correctly that this essentially deploys protocol contracts to a chain running locally managed by Hardhat. Then it monitors existing connected blockchains like Sepolia for incoming transactions (acting sort of like zetaclient, but without voting and consensus) and processes these calls by calling universal apps on this chain that's running in Hardhat.
yes, but currently there is no external blockchain to be monitored, but hardhat local node is playing the role of both evm and zevm, and worker is basically simplified zetaclient
@coderabbitai review
@coderabbitai review
@coderabbitai how are these files ignored:
scripts/worker.ts is excluded by none and included by none
tasks/localnet.ts is excluded by none and included by none
they are included in config:
reviews:
path_filters:
- "pkg/**"
- "typechain-types/**"
- "docs/**"
- "data/**"
path_instructions:
- path: 'contracts/**'
instructions: >-
Review the Solidity contracts for security vulnerabilities and best practices.
- path: 'test/**'
instructions: >-
Review the test files for proper coverage, edge cases, and best practices.
- path: 'scripts/**'
instructions: >-
Review the scripts for best practices.
- path: 'tasks/**'
instructions: >-
Review the tasks for best practices.
auto_review:
base_branches:
- main
[!TIP] For best results, initiate chat on the files or code changes.
The list of workflows did not include "CodeRabbit Review". The correct workflow name needs to be identified. Here are the workflows available:
Since there is no workflow named "CodeRabbit Review", it might be possible that the workflow related to the review process is named differently or not present.
@skosito, it appears that there is no workflow named "CodeRabbit Review" in the repository. Could you please confirm the exact name of the workflow that handles the review process? This will help in accurately retrieving the configuration settings and resolving the issue with ignored files.
@coderabbitai full review
@coderabbitai full review
@coderabbitai full review
@coderabbitai full review
I get this error when starting localnet:
[NODE] <--- JS stacktrace --->
[NODE]
[NODE] FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
[NODE] 1: 0x1026f2984 node::Abort() [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node]
[NODE] 2: 0x1026f2b74 node::ModifyCodeGenerationFromStrings(v8::Local<v8::Context>, v8::Local<v8::Value>, bool) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node]
[NODE] 3: 0x1028356e8 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node]
[NODE] 4: 0x1028356ac v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node]
[NODE] 5: 0x1029b74d4 v8::internal::Heap::GarbageCollectionReasonToString(v8::internal::GarbageCollectionReason) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node]
[NODE] 6: 0x1029b5f70 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node]
[NODE] 7: 0x1029c173c v8::internal::Heap::AllocateRawWithLightRetrySlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node]
[NODE] 8: 0x1029c17d0 v8::internal::Heap::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node]
[NODE] 9: 0x102993e10 v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationType, v8::internal::AllocationOrigin) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node]
[NODE] 10: 0x102cd96c8 v8::internal::Runtime_AllocateInYoungGeneration(int, unsigned long*, v8::internal::Isolate*) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node]
[NODE] 11: 0x102fef8ec Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node]
[NODE] 12: 0x107eb3b70
As suggested on Internet, you can increase the memory for JS, just wondering why this is necessary? I'm gonna invetstigate further
export NODE_OPTIONS="--max-old-space-size=8192"
@lumtis I was getting the same error, then switched to nodejs v20 it's gone.
I get this error when starting localnet:
[NODE] <--- JS stacktrace ---> [NODE] [NODE] FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory [NODE] 1: 0x1026f2984 node::Abort() [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node] [NODE] 2: 0x1026f2b74 node::ModifyCodeGenerationFromStrings(v8::Local<v8::Context>, v8::Local<v8::Value>, bool) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node] [NODE] 3: 0x1028356e8 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node] [NODE] 4: 0x1028356ac v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node] [NODE] 5: 0x1029b74d4 v8::internal::Heap::GarbageCollectionReasonToString(v8::internal::GarbageCollectionReason) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node] [NODE] 6: 0x1029b5f70 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node] [NODE] 7: 0x1029c173c v8::internal::Heap::AllocateRawWithLightRetrySlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node] [NODE] 8: 0x1029c17d0 v8::internal::Heap::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node] [NODE] 9: 0x102993e10 v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationType, v8::internal::AllocationOrigin) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node] [NODE] 10: 0x102cd96c8 v8::internal::Runtime_AllocateInYoungGeneration(int, unsigned long*, v8::internal::Isolate*) [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node] [NODE] 11: 0x102fef8ec Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit [/Users/lucas/.nvm/versions/node/v16.20.2/bin/node] [NODE] 12: 0x107eb3b70
As suggested on Internet, you can increase the memory for JS, just wondering why this is necessary? I'm gonna invetstigate further
export NODE_OPTIONS="--max-old-space-size=8192"
thanks for checking, i didnt have this error locally
it could be that hardhat is spending a lot of resources, did you also try this https://hardhat.org/hardhat-runner/docs/troubleshooting/common-problems#out-of-memory-errors-when-compiling-large-projects
using different node version might also help as @fadeev suggested
@fadeev I think this makes sense if you can review this one
This looks good.
Not blocking for this PR, but I would see two improvement for the developer.
- Packaging the command to reduce the logs to what is essential to developer: showing account the two gateway addresses:
Local ZetaChain contracts environment deployed Account 1: ... ... Gateway EVM: 0x... Gateway ZEVM: 0x...
Maybe could be done with some flags.
- Even further to simplify dev development, shipping a Hardhat plugin for devs to use to integrate the gateway contracts and worker in their own localnet.
import "zeta-contracts-localnet"; function deployScript{ // ... // Deploy ZetaChain contracts environment, return gateway addresses and start the work process in the background var gatewayZEVM, gatewayEVM = setupLocalZetaChain(...) // dev do their own deployment here with their contracts using gateways // ... }
Not knowledgeable enough on hardhat to know if this is possible
These are not blocking for this PR, this is further work for devEx purposes
thanks for review, I fixed the comments, except hardhat plugin part, maybe we can discuss that and research options
thanks for review, I fixed the comments, except hardhat plugin part, maybe we can discuss that and research options
For the cleaned up log, is there maybe a solution, like a flag, to just show the WORKER logs?
This is really cool! A few thoughts.
Should this eventually move into a separate repo? Pros: we don't mix tools and protocol contracts, we don't have to explicitly exclude this code from the audit. Cons: slower iterations as protocol contracts live in one repo, localnet lives in a different repo.
Package localnet
as a Hardhat task that can be imported either by the toolkit or directly by the template, so that it can be distributed a bit more easily.
Should we find a different name for this? This feels more like a simulation rather than a local network.
Would it make sense to start testing this with the examples we have in the example-contracts
repo?
thanks for review, I fixed the comments, except hardhat plugin part, maybe we can discuss that and research options
For the cleaned up log, is there maybe a solution, like a flag, to just show the WORKER logs?
yes there is a way, i added to scripts/readme.md
This is really cool! A few thoughts.
Should this eventually move into a separate repo? Pros: we don't mix tools and protocol contracts, we don't have to explicitly exclude this code from the audit. Cons: slower iterations as protocol contracts live in one repo, localnet lives in a different repo.
Package
localnet
as a Hardhat task that can be imported either by the toolkit or directly by the template, so that it can be distributed a bit more easily.Should we find a different name for this? This feels more like a simulation rather than a local network.
thanks for review - i would say currently this can help us speed up development as we work on v2 contracts so until its in development phase i would say it is beneficial to stay in same repo as it would evolve with contracts
later we can discuss moving it and packaging it differently
@lumtis what do you think?
This is really cool! A few thoughts. Should this eventually move into a separate repo? Pros: we don't mix tools and protocol contracts, we don't have to explicitly exclude this code from the audit. Cons: slower iterations as protocol contracts live in one repo, localnet lives in a different repo. Package
localnet
as a Hardhat task that can be imported either by the toolkit or directly by the template, so that it can be distributed a bit more easily. Should we find a different name for this? This feels more like a simulation rather than a local network.thanks for review - i would say currently this can help us speed up development as we work on v2 contracts so until its in development phase i would say it is beneficial to stay in same repo as it would evolve with contracts
later we can discuss moving it and packaging it differently
@lumtis what do you think?
Yeah, I would tend toward keep the main command in this repo as being part of the development workflow and provide a packaged plugin or so reusable in a utility
start local hardhat node and worker which deploys evm/zevm contracts and listen to Call/WithdrawAndCall/Deposit events
also, adding 4 hardhat tasks to interact with localnet (we can add more in follow up PRs if needed):
Summary by CodeRabbit
New Features
Documentation