Txfusion - ZKSync support #1
Conversation
mshojaei-txfusion
changed the title
…txfusion/sending-messages
solidity/core-utils/evm/.gitkeep
Outdated
| # Find most recently modified JSON build artifact | ||
| if [ "$(uname)" = "Darwin" ]; then | ||
| # for local flow | ||
| jsonFiles=$(find "$artifactsDir" -type f -name "*.json" -exec stat -f "%m %N" {} \; | sort -rn | head -n 1 | cut -d' ' -f2-) | ||
| else | ||
| # for CI flow | ||
| jsonFiles=$(find "$artifactsDir" -type f -name "*.json" -exec stat -c "%Y %n" {} \; | sort -rn | head -n 1 | cut -d' ' -f2-) | ||
| fi | ||
|
|
||
| if [ ! -f "$jsonFiles" ]; then | ||
| echo 'Failed to find build artifact' | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
would be nice to deduplicate this recently modified artifact finding/checking
solidity/package.json
Outdated
| "./artifacts": "./dist/zksync/artifacts/index.js", | ||
| "./artifacts/*": "./dist/zksync/artifacts/*.js" |
There was a problem hiding this comment.
perhaps ./artifacts -> ./zksync-artifacts/, to leave the door open for other types of VM artifacts
| solidity: { | ||
| version: '0.8.19', | ||
| settings: { | ||
| optimizer: { | ||
| enabled: true, | ||
| runs: 999_999, | ||
| }, | ||
| }, | ||
| }, | ||
| gasReporter: { | ||
| currency: 'USD', | ||
| }, |
There was a problem hiding this comment.
is it possible/worth the effort to reuse solidity/gasReporter/mocha/warnings config from the main hardhat.config.cts?
There was a problem hiding this comment.
It could be possible in theory but not worth the effort.
| "@hyperlane-xyz/sdk": "workspace:^", | ||
| "@hyperlane-xyz/utils": "workspace:^", |
There was a problem hiding this comment.
probably want to revert this? it should already be using your local versions of packages
| import { Signer } from 'ethers'; | ||
| import { Wallet } from 'zksync-ethers'; |
There was a problem hiding this comment.
maybe say explicitly whether it's ethers/zksync, for easier reading in the rest of the file?
| import { Signer } from 'ethers'; | |
| import { Wallet } from 'zksync-ethers'; | |
| import { Signer as ethersSigner } from 'ethers'; | |
| import { Wallet zkSyncWallet } from 'zksync-ethers'; |
| "@hyperlane-xyz/core": "file:../../solidity", | ||
| "@hyperlane-xyz/utils": "workspace:^", |
There was a problem hiding this comment.
similar to prev comment, should revert
There was a problem hiding this comment.
the hardhat import might break in published packages, as we only specify it as a dev dependency. also this doesn't seem specific to zksync, and I can't see where this is currently being used
| @@ -77,14 +78,14 @@ async function addressToImpersonatedSigner( | |||
| * @param key a private key | |||
| * @returns a signer for the private key | |||
| */ | |||
| function privateKeyToSigner(key: string): ethers.Wallet { | |||
| function privateKeyToSigner(key: string): Wallet { | |||
There was a problem hiding this comment.
so we can use zksync ethers here and it'll still be backwards compatible with non-zksync chains?
There was a problem hiding this comment.
According to the testing that we have done so far, zksync wallet is fully compatible with ethers.Wallet. Also, in the getContext function, the function for getting signer is called before MultiProvider is instantiated, so there is not way of passing ProtocolType to the getSigner function.
For now this works, but in the future it would require heavier refactoring for supporting different types of signers per protocol.
| @@ -21,10 +21,12 @@ import type { | |||
| Transaction as VTransaction, | |||
| TransactionReceipt as VTransactionReceipt, | |||
| } from 'viem'; | |||
| import * as zk from 'zksync-ethers'; | |||
There was a problem hiding this comment.
do we need everything? would be nice to have it similar to the existing format by only importing the relevant items and adding the zk prefix
e.g
zk.Contractends up beingZKSyncContractor something to that effectzk.types.TransactionRequest->ZKSyncTransactionRequest
There was a problem hiding this comment.
I know there's some differences, but would be good to deduplicate as much as possible with the existing ContractVerifier - perhaps there's an abstract verifier with the base functionality that the default + zksync verifier inherit from
| this.addVerificationArtifacts({ chain, artifacts: [verificationInput] }); | ||
|
|
||
| // try verifying contract | ||
| try { | ||
| await this.contractVerifier?.verifyContract(chain, verificationInput); | ||
| if (isZKSyncChain) { | ||
| const verifier = new ZKSyncContractVerifier(this.multiProvider); |
There was a problem hiding this comment.
set it up once at the start when we set up the base contractVerifier? that we we don't need to create a new one for every verifyContract call
…perlane core deploy` command
Improvement/verification
Description
PR for reviewing zksync specific changes