-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat(base_layer): start migrating ethers->alloy #848
Conversation
04a9b92
to
0021b18
Compare
Benchmark movements: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #848 +/- ##
==========================================
+ Coverage 74.44% 74.50% +0.05%
==========================================
Files 364 368 +4
Lines 38395 38648 +253
Branches 38395 38648 +253
==========================================
+ Hits 28583 28794 +211
- Misses 7493 7524 +31
- Partials 2319 2330 +11 ☔ View full report in Codecov by Sentry. |
0021b18
to
35a0d49
Compare
Benchmark movements: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-starkware and @giladchase)
crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs
line 24 at r1 (raw file):
#[derive(thiserror::Error, Debug)] pub enum EthereumBaseLayerError {
Alphabetize?
Code quote:
EthereumBaseLayerError
crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs
line 84 at r1 (raw file):
// The solidity contract was pre-compiled, and only the relevant functions were kept. let abi: JsonAbi =
Is annotation still needed, given turbofish?
Code quote:
: JsonAbi
35a0d49
to
c063d27
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 3 of 5 files reviewed, 2 unresolved discussions (waiting on @dan-starkware and @elintul)
crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs
line 24 at r1 (raw file):
Previously, elintul (Elin) wrote…
Alphabetize?
Added to the refactor PR
crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs
line 84 at r1 (raw file):
Previously, elintul (Elin) wrote…
Is annotation still needed, given turbofish?
Nope, removing it in the second PR altogether.
Kept it here to keep the diff migration-only, without mixing in refactoring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dan-starkware)
c063d27
to
7be89e7
Compare
Benchmark movements: |
Benchmark movements: |
Minimal logic changes, strictly migration. Refactors of existing code are added on a separate commit on top of this one, to separate the migration from the refactor. *Notable changes* - Errors: * `ProviderError` on `ethers` was only the node_url parse error, which isn't needed for `on_http` since it already expects a `Url` argument. * `AbiError` and `BadContract` type errors are both sub-variants of the `Contract` variant, that is, both errors types are variants of `alloy_contract::Error`. - API: * Return type of the call API now only implements `IntoFuture`, rather than `Future` like on `ethers`, hence manually calling `into_future()` is required before passing it to `tokio`. * `call()` in alloy returns a dynamic type, which is alloy [discourages](https://docs.rs/alloy-dyn-abi/latest/alloy_dyn_abi/) when types are known at compilation-time. Therefore using `call_raw`, which returns `Bytes`, and casting statically. commit-id:b8b28c1a
7be89e7
to
e8546e9
Compare
Benchmark movements: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dan-starkware)
Minimal logic changes, strictly migration.
Refactors of existing code are added on a separate commit on top
of this one, to separate the migration from the refactor.
Notable changes
ProviderError
onethers
was only the node_url parse error, whichisn't needed for
on_http
since it already expects aUrl
argument.AbiError
andBadContract
type errors are both sub-variants ofthe
Contract
variant, that is, both errors types are variantsof
alloy_contract::Error
.IntoFuture
,rather than
Future
like onethers
, hence manually callinginto_future()
is required before passing it totokio
.call()
in alloy returns a dynamic type, which is alloydiscourages
when types are known at compilation-time. Therefore using
call_raw
,which returns
Bytes
, and casting statically.commit-id:b8b28c1a
Stack:
This change is