Skip to content
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

chore: use alloy zksync #608

Merged
merged 24 commits into from
Nov 21, 2024
Merged

chore: use alloy zksync #608

merged 24 commits into from
Nov 21, 2024

Conversation

nbaztec
Copy link
Collaborator

@nbaztec nbaztec commented Oct 11, 2024

Motivation

Migrate away from the unmaintained zksync-web3-rs.

Solution

Use alloy-zksync crate.

@nbaztec nbaztec marked this pull request as draft October 11, 2024 22:18
@tomimor tomimor added challenging 🏴‍☠️ Indicates a difficult item p1 🟠 Indicates high priority item upstream-parity 🟰 Needed for upstream feature parity labels Oct 29, 2024
@nbaztec nbaztec marked this pull request as ready for review November 19, 2024 12:27
@nbaztec nbaztec requested a review from a team as a code owner November 19, 2024 12:27
crates/cast/bin/cmd/send.rs Outdated Show resolved Hide resolved
@nbaztec nbaztec requested a review from Karrq November 20, 2024 09:08
Copy link
Contributor

@Karrq Karrq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I thought with alloy-zksync we could get rid of a lot of overrides, perhaps we are still lacking the necessary abstractions?
For example, we shouldn't need to have ZkCast, or so I thought - unless it's something that we can achieve, but in a separate PR

crates/forge/bin/cmd/create.rs Outdated Show resolved Hide resolved
crates/forge/bin/cmd/create.rs Show resolved Hide resolved
crates/forge/bin/cmd/create.rs Show resolved Hide resolved
crates/forge/bin/cmd/create.rs Show resolved Hide resolved
crates/forge/bin/cmd/create.rs Show resolved Hide resolved
crates/forge/bin/cmd/create.rs Show resolved Hide resolved
crates/zksync/core/src/lib.rs Show resolved Hide resolved
@nbaztec
Copy link
Collaborator Author

nbaztec commented Nov 20, 2024

Cast uses Provider<AnyNetwork> whereas we need to use Provider<Zksync>, moreover the defined methods rely on concrete tx types defined on AnyNetwork::TransactionRequest an not on Zkysnc::TransactionRequest. So generalizing them doesn't give us much.

Karrq
Karrq previously approved these changes Nov 20, 2024
Karrq
Karrq previously approved these changes Nov 20, 2024
crates/forge/bin/cmd/create.rs Outdated Show resolved Hide resolved
Karrq
Karrq previously approved these changes Nov 21, 2024
Jrigada
Jrigada previously approved these changes Nov 21, 2024
@nbaztec nbaztec dismissed stale reviews from Jrigada and Karrq via 7796e5d November 21, 2024 13:03
Copy link
Contributor

@Karrq Karrq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Send it 🚀

Copy link
Contributor

@Jrigada Jrigada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nbaztec nbaztec merged commit f75d7bf into main Nov 21, 2024
13 checks passed
@nbaztec nbaztec deleted the nish-alloy-zksync branch November 21, 2024 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
challenging 🏴‍☠️ Indicates a difficult item p1 🟠 Indicates high priority item upstream-parity 🟰 Needed for upstream feature parity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants