GeezerVison

Codecov Report

❌ Patch coverage is 71.29145% with 329 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.61%. Comparing base (aa2c09a) to head (e3493c3).
⚠️ Report is 1 commits behind head on master.

@@            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.
📢 Have feedback on the report? Share it here.

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.

Confidence Score: 5/5

[High impact] The latest change appears safe to merge and does not change runtime behavior.

Summary

The PR adds async clients, custom routes, async V4 pool discovery, and dependency and test updates.

• V3 quotes and swaps can follow custom token routes.
• Callers can await Uniswap V2 and V3 client operations.
• V4 pool reserve queries can return batched and paged results.
• Callers can await Uniswap V4 client and pool-cache operations.
• The project updates its Web3 setup and runs CI on versions 2–4.

_{Reviews (3) · Last reviewed commit: "fix(typecheck): remove unused type: igno..." · Reviewed by Greptile}

@greptileai review

Addressed the Greptile findings in commit d464d8f:

P1 fixes:

• Restored @check_approval on make_trade, make_trade_output, add_liquidity, remove_liquidity (removed in the clean-branch squash — fixes first-trade approval failure)
• Removed the duplicate path reassignment in the V2 exact-output branch that overwrote custom routes with the default WETH path
• Added ValueError when V3 route is supplied without fees (incomplete route now fails fast instead of silently falling back to single-pool quote)
• Fixed async client __init__ to derive the sync provider URL from the supplied AsyncWeb3 instance, so PROVIDER env var is not required when a web3 parameter is given
• Added asyncio.Lock (_nonce_lock) in AsyncUniswap4._build_and_send_tx — nonce is reserved while holding the lock before yielding to I/O, preventing concurrent trades from sharing a nonce

P2 fixes:

• Removed incorrect await on _name.decode() / _symbol.decode() in AsyncUniswap4.get_token (.decode() is sync, not an awaitable — was raising TypeError for bytes32 tokens)
• Restored the Anvil startup readiness poll in test_uniswap4.py fixture (the clean-branch squash reverted the socket.create_connection poll from test(v4): poll Anvil startup instead of a fixed sleep #486 back to sleep(3))
• Added per-instance cache (_weth_address_cache) to get_weth_address to avoid repeated RPC calls on every quote/swap path

Not fixed / deferred:

• Native ETH routes with custom V3 route+fees (Greptile finding 5): these now fail at _token_to_token_swap_input with a ValueError due to the ETH_ADDRESS guard already present there — the correct fix is to extend _eth_to_token_swap_input to accept route/fees and route custom multi-hop ETH swaps through exactInput with msg.value. Filed as a follow-up.
• Lock file duplicate entries (Greptile finding 10): regenerating poetry.lock requires poetry installed; will address in a separate commit.

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.

Fixed the typecheck failure: removed spurious # type: ignore[return-value] on get_weth_address (the cache attribute is typed as ChecksumAddress, same as the return type, so mypy no longer needs the suppression).

The v3 test failure (test_get_tvl_in_pool_on_chain[3-DAI-USDC]) is a transient RPC rate limit from the free Cloudflare plan (HTTP 408 - Request timeout on the free plan) — not a code regression. CI will retry on the new push.

@greptileai review

@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

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.