Skip to content

feat(v4): UF grant work β€” async clients, ReservesLens, CI/test fixes (clean branch) - #491

Merged
ErikBjare merged 3 commits into
uniswap-python:masterfrom
TimeToBuildBob:feat/v4-uf-grant-clean
Oct 14, 2026
Merged

ErikBjare merged 3 commits into
uniswap-python:masterfrom
TimeToBuildBob:feat/v4-uf-grant-clean

Conversation

@TimeToBuildBob

Copy link
Copy Markdown
Contributor

What this is

This is a clean branch demonstrating the approach @ErikBjare described in #485:
liquid-8's changes from dev/v4-fin reapplied as a single commit on top of current master,
bypassing the 244-commit ancestry confusion caused by the original v4-fin squash merge (4c42f1f).

Why

dev/v4-fin was branched from the old v4-fin branch 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/master and upstream/dev/v4-fin HEAD.

Changes included

  • Async Uniswap v2/v3/v4 clients (asyncuniswap.py, asyncuniswap4.py + tests)
  • ReservesLens support and getPoolTVLBatch improvements
  • Universal Router and single/multi-hop swap fixes
  • Test suite fixes (RPC switch, Anvil startup poll, fixture fixes)
  • Adoption metrics snapshot script + daily workflow (UF grant D3/D4)
  • README updates
  • Minor fix: duplicate Generator import in tests/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 master rather than dev/v4-fin.

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
@TimeToBuildBob TimeToBuildBob mentioned this pull request Oct 14, 2026
@codecov

codecov Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

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.

Files with missing lines Patch % Lines
uniswap/asyncuniswap.py 71.55% 231 Missing ⚠️
uniswap/util.py 67.42% 43 Missing ⚠️
uniswap/uniswap.py 69.69% 40 Missing ⚠️
uniswap/uniswap4.py 73.46% 13 Missing ⚠️
uniswap/decorators.py 66.66% 1 Missing ⚠️
uniswap/tokens.py 80.00% 1 Missing ⚠️
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.
πŸ“’ Have feedback on the report? Share it here.

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

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.

@greptile-apps

greptile-apps Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence 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

Comment thread uniswap/asyncuniswap.py Outdated
Comment thread uniswap/asyncuniswap4.py Outdated
Comment thread uniswap/uniswap.py
Comment thread uniswap/uniswap.py Outdated
Comment thread uniswap/uniswap.py
Comment thread uniswap/uniswap.py
Comment thread uniswap/asyncuniswap4.py Outdated
Comment thread tests/test_uniswap4.py Outdated
Comment thread uniswap/uniswap.py
Comment thread poetry.lock
- 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
@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

@greptileai review

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

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.

Comment thread uniswap/asyncuniswap4.py
Comment thread uniswap/uniswap.py
@ErikBjare

Copy link
Copy Markdown
Member

@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

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

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
@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

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.

@ErikBjare

Copy link
Copy Markdown
Member

@greptileai review

@liquid-8

Copy link
Copy Markdown
Member

@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)

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

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.

@ErikBjare
ErikBjare merged commit 02c0368 into uniswap-python:master Oct 14, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants