Skip to content

Add Alloy-rs and remove ethers #85

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

androolloyd
Copy link

this is reopened from : #84 but from an isolated fork branch for the work

Copy link

@0xPatryk 0xPatryk left a comment

Choose a reason for hiding this comment

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

Looks good for the start!

I've also started posting it on Alloy so I can help you implement some features!

@@ -14,7 +14,12 @@ repository = "https://github.com/hyperliquid-dex/hyperliquid-rust-sdk"
[dependencies]
chrono = "0.4.26"
env_logger = "0.10.0"
ethers = {version = "2.0.14", features = ["eip712", "abigen"]}
alloy-primitives = { version = "0.8.21", features = ["serde"] }

Choose a reason for hiding this comment

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

Could you use alloy with features instead of everything as a single dependency?

It's easier for dependency management and you don't end up with a mess of dependencies.

Not 100% sure, but this should be it. Alloy core should have remaining deps

Suggested change
alloy-primitives = { version = "0.8.21", features = ["serde"] }
alloy = { version = "0.8", features = ["serde", "signer-local"] }

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I also agree with this, much cleaner this way

@@ -1,49 +1,24 @@
use alloy_primitives::{Address, U256};
use alloy_signer_local::PrivateKeySigner;

Choose a reason for hiding this comment

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

And then use alloy::primitives:...
and so on

@llvm-x86
Copy link
Contributor

any breaking changes?

@llvm-x86
Copy link
Contributor

@namn-grg @0xPatryk what's the plan for integration?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants