Alloy UniswapV3Pool bindings - #3662
Conversation
…sePoolFactory # Conflicts: # crates/shared/src/sources/balancer_v2/pool_fetching/registry.rs
# Conflicts: # crates/shared/src/sources/balancer_v2/pool_fetching/pool_storage.rs # crates/shared/src/sources/balancer_v2/pool_fetching/registry.rs
# Conflicts: # crates/shared/src/event_handling.rs # crates/shared/src/sources/balancer_v2/pool_fetching/pool_storage.rs # crates/shared/src/sources/balancer_v2/pool_fetching/registry.rs
| // AlloyUniswapV3PoolEvents doesn't derive Clone, so we need this wrapper | ||
| #[derive(Clone)] |
There was a problem hiding this comment.
Given that they are open for PRs and that it's an easy fix we should probably just upstream the needed changes to alloy.
There was a problem hiding this comment.
I can look into that later. Can we merge this PR as is to start properly testing the liquidity with alloy? Possibly in prod.
There was a problem hiding this comment.
We actually need not only Clone, but also PartialEq and Debug.
There was a problem hiding this comment.
Sure, but assuming those can be derived on the alloy events that would be a 2 line PR.
There was a problem hiding this comment.
extra_derives(<paths...>): adds extra #[derive(...)] attributes to all generated types.
There was a problem hiding this comment.
Sure, but assuming those can be derived on the alloy events that would be a 2 line PR.
I might have missed something, but it is not.
Please read the PR description carefully 🙂
There was a problem hiding this comment.
Actually, I think I found the solution. Will open a PR shortly.
There was a problem hiding this comment.
There was a problem hiding this comment.
I updated the PR to migrate to alloy bindings, but I currently use a dependency to my opened PR alloy-rs/core#1017. I can't reproduce the issue in the alloy repo tests for some reason, but it is easy to reproduce it here for some reason 🤷
# Conflicts: # crates/contracts/src/alloy.rs # crates/contracts/src/lib.rs # crates/shared/src/event_handling.rs
MartinquaXD
left a comment
MartinquaXD
left a comment
There was a problem hiding this comment.
It's a bit unfortunate that we need the WithAddress wrapper because alloy events don't contain the address or log index on their own.
If this is useful for other indexing logic we should consider moving this wrapper type into ethrpc/alloy to make it reusable.
Overall just nits.
| set.insert(PoolCreated::SIGNATURE_HASH); | ||
| Filter::new() | ||
| .event_signature(hashset![PoolCreated::SIGNATURE_HASH]) | ||
| .event_signature(set) |
There was a problem hiding this comment.
nit: Does this work? I think event_signatures takes anything that can be converted into a FilterSet. Same comment applies to the other instances of event_signature().
| .event_signature(set) | |
| .event_signature([PoolCreated::SIGNATURE_HASH].into_iter()) |
There was a problem hiding this comment.
For some reason it stopped working when i updated the alloy version.
| { | ||
| match event.inner() { | ||
| UniswapV3PoolEvents::Burn(burn) => { | ||
| let tick_lower = BigInt::from(burn.tickLower.as_i32()); |
There was a problem hiding this comment.
Bit surprising that the contract returns int24 but we currently parse the subgraph response via BigInt. Could you check if we can turn the tick_lower field in the subgraph response into an i32 as well?
There was a problem hiding this comment.
Probably yes, but that requires changing types in many places. Worth a separate PR.
|
That is weird. I can't reproduce locally any of the CI issues. Running on the same rust version... |
Description
Migrates
UniswapV3PoolSC bindings to alloy.Tests heavily utilize SC events generated by alloy, but they don't implement
Eq,PartialEqandDebugrequired for proper tests, so I had to implement some wrappers. The issue is opened alloy-rs/core#1009How to test
Existing test + mainnet shadow.