feat: introduce zksync bridges utility functions #2667
Conversation
…n-txfusion-bridge-functions
|
|
@nikola-bozin-txfusion is attempting to deploy a commit to the Wevm Team on Vercel. A member of the Team first needs to authorize it. |
jxom
force-pushed
the
main
branch
3 times, most recently
from
b56e162
to
ad2831b
Compare
package.json
Outdated
| "dependencies": { | ||
| "nopt": "^7.2.1" | ||
| } |
There was a problem hiding this comment.
| "dependencies": { | |
| "nopt": "^7.2.1" | |
| } |
There was a problem hiding this comment.
Are these chains "official" ZKsync local chains? If so, maybe better named as zksyncLocalChain.... Also, what is the difference between zkSyncChainL1 and zkSyncLocalNodeL1?
src/zksync/constants/number.ts
Outdated
| export const REQUIRED_L2_GAS_PRICE_PER_PUBDATA = 800 | ||
|
|
||
| export const REQUIRED_L1_TO_L2_GAS_PER_PUBDATA_LIMIT = 800n | ||
|
|
||
| export const L1_FEE_ESTIMATION_COEF_NUMERATOR = 12 | ||
| export const L1_FEE_ESTIMATION_COEF_DENOMINATOR = 10 | ||
|
|
||
| export const ADDRESS_MODULO = 2n ** 160n | ||
|
|
||
| export const DEFAULT_POLLING_INTERVAL_MS = 500 |
There was a problem hiding this comment.
src/zksync/utils instead of src/zksync/bridge-utils to conform to codebase convention.
|
|
||
| export type ApplyL1ToL2AliasReturnType = Address | ||
|
|
||
| export function applyL1ToL2Alias( |
There was a problem hiding this comment.
Would be good to have some JSDoc with a brief explanation of what this utility does (and possibly a link to more info). I am not quite sure what this utility's purpose is by looking at it. By adding JSDoc, this means we can make this utility as part of the public API if we ever need to.
| import type { PublicZkSyncRpcSchema } from '../types/eip1193.js' | ||
|
|
||
| export type SendRawTransactionWithDetailedOutputParameters = { | ||
| signedTx: Hex |
There was a problem hiding this comment.
| signedTx: Hex | |
| serializedTransaction: Hex |
Keep this consistent with Viem's sendRawTransaction
| export type TransactionWithDetailedOutput = { | ||
| transactionHash: Hash | ||
| storageLogs: Array<{ | ||
| address: Address | ||
| key: string | ||
| writtenValue: string | ||
| }> | ||
| events: Array<{ | ||
| address: Address | ||
| topics: Hex[] | ||
| data: Hex | ||
| blockHash: Hash | null | ||
| blockNumber: bigint | null | ||
| l1BatchNumber: bigint | null | ||
| transactionHash: Hash | ||
| transactionIndex: bigint | ||
| logIndex: bigint | null | ||
| transactionLogIndex: bigint | null | ||
| logType: string | null | ||
| removed: boolean | ||
| }> | ||
| } |
There was a problem hiding this comment.
Can we put this in src/zksync/types/transaction.ts?
| export type DepositTransactionExtended = DepositTransaction & { | ||
| bridgehubContractAddress: Address | ||
| l2ChainId: bigint | ||
| eRC20DefaultBridgeData: Hex |
There was a problem hiding this comment.
why is it "extended"? might be worth adding a comment with a link to docs if this is a zksync standard thing.
| bridgeAddresses?: GetDefaultBridgeAddressesReturnType | ||
| } | ||
|
|
||
| export interface Overrides |
There was a problem hiding this comment.
Also a bit confused why we even need Overrides. Why can't we explicitly state the properties instead of putting them in an overrides property? This is what we already do for core Viem Actions.
e.g.
approveErc20L1({
// ...
maxFeePerGas: 69420n
})versus.
approveErc20L1({
// ...
overrides: { maxFeePerGas: 69420n }
})|
|
||
| export interface Overrides | ||
| extends Omit<ZksyncTransactionRequest, 'to' | 'data'> { | ||
| gasLimit: bigint |
There was a problem hiding this comment.
Looks like there is a stray column in the name too src/chains/definitions/zkSyncChainL2.ts
jxom
force-pushed
the
main
branch
5 times, most recently
from
a126da0
to
8821e73
Compare
Overview
This PR introduces helper utility functions required for Deposit & Withdraw functionality.
Summary
PR-Codex overview
This PR adds support for zkSync CLI local nodes, implements bridge functionalities, and enhances testing coverage.
Detailed summary