Repository navigation
feat(v4): UF grant work β async clients, ReservesLens, CI/test fixes (clean branch) - #491
Conversation
Clean branch off master containing liquid-8's changes from dev/v4-fin: - Async Uniswap v2/v3/v4 clients (asyncuniswap.py, asyncuniswap4.py) - ReservesLens support and getPoolTVLBatch improvements - Universal Router and single/multi-hop swap fixes - Test suite fixes (RPC switch, Anvil startup, fixture fixes) - Adoption metrics snapshot script + daily workflow (UF grant D3/D4) - README updates Clean squash of dev/v4-fin changes reapplied on top of current master, resolving the 244-commit ancestry confusion from the original v4-fin squash merge (4c42f1f). Also fixes duplicate Generator import in tests/test_uniswap.py (ruff F811). Git-Session-Id: 15b18137-18fa-53a4-92e2-759ce27705c7
Codecov Reportβ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #491 +/- ##
==========================================
- Coverage 73.26% 70.61% -2.65%
==========================================
Files 12 14 +2
Lines 2319 4302 +1983
==========================================
+ Hits 1699 3038 +1339
- Misses 620 1264 +644 β View full report in Codecov by Harness. π New features to boost your workflow:
|
|
CI first wave: build β typecheck β test-v2 β test-v4 β β test-v3 and Greptile still pending. Coverage dropped from 73% β 59% as expected (new async client code without full test harness). Will address Greptile findings once review lands. |
|
- Restore @check_approval on make_trade/make_trade_output/add_liquidity/ remove_liquidity (P1: first token trades fail without router allowance) - Remove path overwrite after cost calculation in V2 exact-output branch (P1: custom route was silently replaced with default WETH route) - Raise ValueError when V3 route is given without fees in get_price and make_trade (P1: incomplete routes silently fell back to single-pool quote) - Fix async client init to derive provider URL from supplied AsyncWeb3 so PROVIDER env var is not required when web3 is given (P1) - Add asyncio.Lock (_nonce_lock) in AsyncUniswap4 and reserve nonce inside the lock before yielding to prevent concurrent trades sharing a nonce (P1) - Remove incorrect await on bytes32 _name.decode()/_symbol.decode() in AsyncUniswap4.get_token β decode() returns str, not awaitable (P2) - Restore Anvil startup readiness poll in test_uniswap4.py fixture, replacing sleep(3) that was reverted from uniswap-python#486 (P2) - Cache get_weth_address result per client instance to avoid repeated RPC calls on every quote/swap path (P2) Co-Authored-By: Bob <bob@superuserlabs.org> Git-Session-Id: d44b7f42-192e-588b-98db-435d6ad11d1c
|
@greptileai review |
|
Addressed the Greptile findings in commit d464d8f: P1 fixes:
P2 fixes:
Not fixed / deferred:
Greptile re-review triggered. |
|
@TimeToBuildBob I have previously raised concerns, and I still have concerns about the dual implementation, where we have both an async implementation and a sync implementation to maintain. It's liable to cause drift and requires changes to be implemented in two places, etc. I raised this during planning for the grant and suggested solutions. I was against async generally because I thought it was not very necessary or sort of a waste of effort, since async in Python isn't great. Regardless, I wanted us to at least have a single implementation and use some kind of framework there where you write it async and can then compile it into an async thing (which I've heard about or read about). That was apparently not adopted or fully considered or chosen in the end, but I want to flag this here, and I'm wondering if we should do that anyway. I want you to open an issue about this |
|
Agreed: maintaining two handwritten implementations invites exactly the drift this review exposed. I will open the issue you requested to evaluate an async source of truth with generated synchronous clients (for example unasync), including provider initialization, API compatibility, and a generation/parity CI check. I will keep that architectural decision separate from the immediate correctness and CI repairs here rather than silently choosing a framework in this fix session. |
β¦address _weth_address_cache is typed as ChecksumAddress (same as return type), so the ignore comment is now redundant and mypy correctly flags it. Git-Session-Id: c95ac4a4-64bd-42b3-9f79-c55ca4dfb9cf
|
Fixed the typecheck failure: removed spurious The v3 test failure ( |
|
@greptileai review |
Worth mentioning, web3py itself uses this approach exactly. They provide separate classes for sync and async versions. Implementations are almost identical, I cant see significant problems in development and maintenance. Also, I can't agree that async in Python is not oke now (unlike multithreading, but that's literally different) |
|
Issue opened as committed: #492 β "refactor: single async source of truth with generated/wrapped sync client (eliminate dual-implementation drift)". That's the right place to resolve the architecture question, and liquid-8's web3.py precedent is good data to include there. Status for D1 (deadline 2026-10-15): CI is all green (build β typecheck β test-v2 β test-v3 β test-v4 β , Greptile 5/5). The sync/async drift concern is now tracked in #492 rather than blocking this PR. |
What this is
This is a clean branch demonstrating the approach @ErikBjare described in #485:
liquid-8's changes from
dev/v4-finreapplied as a single commit on top of currentmaster,bypassing the 244-commit ancestry confusion caused by the original v4-fin squash merge (
4c42f1f).Why
dev/v4-finwas branched from the oldv4-finbranch before it was squash-merged into master.Git therefore sees 244 commits in the PR that "aren't on master," even though most of that code
IS in master β it just lives in the squash commit. Review tools (Greptile, Codecov) get confused
and report re-finding issues already fixed.
This clean branch has only 1 commit on top of master, containing exactly the diff between
upstream/masterandupstream/dev/v4-finHEAD.Changes included
asyncuniswap.py,asyncuniswap4.py+ tests)getPoolTVLBatchimprovementsGeneratorimport intests/test_uniswap.py(ruff F811)Relationship to #485
This is a clean rebase of liquid-8's work. If they want to continue iterating,
they should create new branches off
masterrather thandev/v4-fin.