Repository navigation
D1 UF grant - #485
D1 UF grant #485liquid-8 wants to merge 244 commits into
Conversation
This reverts commit fa75632.
This reverts commit de63ce4.
Removed unused imports and cleaned up code formatting.
Pool manager contract ABI
Deleted due to naming fix
Deleted due to naming fix
Deleted due to naming fix
Deleted due to naming fix
Deleted due to naming fix
Naming fix
Liquidity pools list management class added, UniswapV4 improvements.
refactor
refactor
refactor
moved to util
moved to types
moved to constants
Add router v2.1.1 support; add X Layer, Tempo, Robinhood networks support; remove Blast and testnets support.
Also minor improvements and docstring clarification.
This PR upgrades codebase to the latest Web3 version, adds async clients and v3 multi-hop trades, and adds async tests.
Python callers can use async clients for Uniswap requests.
v3 quotes and trades can follow multi-pool routes.
The library and its tooling now use Python 3.12 and Web3.py 8.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #485 +/- ##
==========================================
- Coverage 73.26% 70.21% -3.05%
==========================================
Files 12 14 +2
Lines 2319 4284 +1965
==========================================
+ Hits 1699 3008 +1309
- Misses 620 1276 +656 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
| tick_lower: int | ||
| tick_upper: int | ||
| liquidity_delta: int | ||
| salt: int |
There was a problem hiding this comment.
Integer salts cannot be encoded
ModifyLiquidityParams.salt is declared as an int, but modifyLiquidity requires bytes32. Both wrappers pass the value through unchanged, so callers following the new type definition cannot encode the call. Use a 32-byte value or explicitly convert the integer before passing it.
| salt: int | |
| salt: bytes |
| pools = V4pools(w3) | ||
|
|
||
| # Scan the chain (slow on first run — save to disk for reuse) | ||
| pools.fetch_poolkey_data(chunk_size=2000) |
There was a problem hiding this comment.
The example omits the required first_block argument. Copying it raises TypeError before scanning any pools. Supply a starting block, such as the mainnet deployment block used by the tests.
| pools.fetch_poolkey_data(chunk_size=2000) | |
| pools.fetch_poolkey_data(first_block=21688329, chunk_size=2000) |
| ONE_ETH = 10**18 | ||
|
|
||
| # Get the pool contract instance | ||
| pool = uni.get_pool_instance(ETH, USDC, fee=500) |
There was a problem hiding this comment.
get_pool_instance passes the zero ETH address directly to getPool, but v3 pools use WETH. The later mint_liquidity call also uses amount0 and amount1 instead of the accepted amount_0 and amount_1. Use WETH, assign amounts in the pool's token order, and correct the keyword names so users can run this example.
| """ | ||
| :param token_exact: The token for which the qty parameter is specified, either the input or output token depending on the quote type. | ||
| :param qty: The amount of the token_exact to be swapped. For an exact output quote, this is the amount of output token desired. | ||
| :param route: The list of PoolKeys representing the path of the swap in reverse order, starting with the pool containing the output token and ending with the pool containing the input token. |
There was a problem hiding this comment.
Exact-output route instructions are reversed
The docstrings tell callers to supply pools in reverse order, but encode_path_keys_output expects input-to-output order and reverses the list itself. The tests use input-to-output order. Correct the exact-output docstrings in both v4 clients so users do not build routes through the wrong pools and get failed quotes or reverted swaps.
| event_logs = await self.factory_contract.events.PoolCreated().process_receipt( | ||
| receipt | ||
| ) |
There was a problem hiding this comment.
AsyncUniswap.create_pool_instance awaits process_receipt, which reads an already-fetched receipt and returns the decoded events directly. After a pool is successfully created, this raises TypeError instead of returning its contract. Remove this await; keep the waits for sending the transaction and receiving its receipt.
| event_logs = await self.factory_contract.events.PoolCreated().process_receipt( | |
| receipt | |
| ) | |
| event_logs = self.factory_contract.events.PoolCreated().process_receipt( | |
| receipt | |
| ) |
| liquidity=10**18, # amount of liquidity to provide | ||
| amount0=ONE_ETH // 10, # max ETH to spend | ||
| amount1=340 * 10**6, # max USDC to spend |
There was a problem hiding this comment.
Mint example exceeds its budget
The example requests liquidity=10**18 while allowing only 0.1 ETH and 340 USDC. At the preceding example's price of 3400 USDC per ETH, this full-range position needs roughly 17,150 ETH and 58 million USDC, so the mint reverts. Calculate liquidity from the amounts, current price, and tick bounds using get_liquidity_for_amounts_by_ticks.
|
|
||
| ONE_ETH = 10**18 | ||
|
|
||
| tx = uni.mint_position( |
There was a problem hiding this comment.
Mint example lacks token approval
The example never approves USDC for the PositionManager. mint_position does not add that approval, so a fresh wallet cannot pay the USDC needed for this position. Show approve(USDC, approve_position_manager=True) and wait for its receipt before minting.
| tx = uni.mint_position( | |
| approval_tx = uni.approve(USDC, approve_position_manager=True) | |
| approval_receipt = uni.w3.eth.wait_for_transaction_receipt(approval_tx) | |
| assert approval_receipt["status"] | |
| tx = uni.mint_position( |
| # Retrieve the ERC-721 token ID(s) minted by this transaction | ||
| token_ids = uni.get_minted_token_id(tx.hex()) | ||
| token_id = token_ids[0] |
There was a problem hiding this comment.
The example calls get_minted_token_id immediately after broadcasting the mint. That helper uses get_transaction_receipt, not a receipt wait, so a normal pending transaction makes the lookup fail. Wait for the mint receipt and check its status before reading the token IDs.
| # Retrieve the ERC-721 token ID(s) minted by this transaction | |
| token_ids = uni.get_minted_token_id(tx.hex()) | |
| token_id = token_ids[0] | |
| # Retrieve the ERC-721 token ID(s) minted by this transaction | |
| receipt = uni.w3.eth.wait_for_transaction_receipt(tx) | |
| assert receipt["status"] | |
| token_ids = uni.get_minted_token_id(tx.hex()) | |
| token_id = token_ids[0] |
* test: switch mainnet RPC in CI tests * README update
|
@greptile-apps please review again considering convos in 4a45cf9 and ab467cd |
|
@liquid-8 Weird that you are targeting the stale/old v4-fin branch, if you target/work against master (or a branch more recently off master) you won't have all these risky conflicts. |
Unfortunately not What is really weird is why a review bot finds issues already resolved and ignores direct mentions to related convos. The current report has nothing in common with the state of affairs. I mean, if that were true, how could all tests be green? |
* test: switch mainnet RPC in CI tests * README update * async_uniswap fixture fix
|
@liquid-8 Most likely because you are working with the stale v4-fin branch which was squash merged as 4c42f1f If you created a clean branch and PR, it wouldn't get confused about the 244 commits here of which the ancestors to your changes are not on master (squashed). That's kinda what I'm saying. @TimeToBuildBob Demonstrate what I mean by making a clean PR with these changes targeting master. |
|
Done — opened #491 as the clean branch Erik described. It takes the full diff between @liquid-8 for future PRs: branch off |
|
Changes merged as part of #491 |

No description provided.