Fix constructor network handling#465
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev/v4-fin #465 +/- ##
==============================================
- Coverage 76.87% 76.83% -0.05%
==============================================
Files 12 12
Lines 2249 2253 +4
==============================================
+ Hits 1729 1731 +2
- Misses 520 522 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Greptile SummaryThis PR adds an explicit check in the
Confidence Score: 3/5The constructor's network validation is only partially fixed — chains known to the library but unsupported by v4 contracts still crash with a raw KeyError. The change improves the error path for truly unrecognized chain IDs, but leaves a real gap: networks such as Sepolia, Fantom, and several legacy testnets are in _netid_to_name and will pass the new guard, then immediately raise an unhandled KeyError in the v4 contract address lookups. Callers cannot rely on getting a descriptive error when connecting to a known-but-v4-unsupported network. uniswap/uniswap4.py — the constructor network check and the downstream contract address lookups need to be reviewed together. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Uniswap4.__init__ called] --> B[Fetch chain_id via w3.net.version]
B --> C{chain_id in _netid_to_name?}
C -- No --> D[raise Exception: Unknown netid\n✅ New explicit error]
C -- Yes --> E[self.net_name = _netid_to_name chain_id]
E --> F{net_name in v4 contract maps?\ne.g. sepolia, fantom, xdai}
F -- No --> G[KeyError on address lookup\n⚠️ Still unhandled]
F -- Yes --> H[Load quoter, router, stateview,\npermit2, pool_manager, etc.]
H --> I[Constructor complete]
Reviews (1): Last reviewed commit: "Fix constructor network handling" | Re-trigger Greptile |
| if self.net_id in _netid_to_name: | ||
| self.net_name = _netid_to_name[self.net_id] | ||
| else: | ||
| raise Exception(f"Unknown netid: {self.net_id}") | ||
|
|
||
| logger.info(f"Using {self.w3} ('{self.net_name}', netid: {self.net_id})") | ||
| quoter_address = _quoter_contract_addresses_v4[self.net_name] | ||
| router_address = _router_contract_addresses_v4[self.net_name] |
There was a problem hiding this comment.
KeyError
_netid_to_name contains many networks that have no entries in the v4 contract address maps (_quoter_contract_addresses_v4, _router_contract_addresses_v4, etc.): sepolia, fantom, xdai, ropsten, görli, kovan, binance_testnet, arbitrum_testnet, harmony_mainnet, and harmony_testnet. A caller connecting to any of those chains will pass the new if self.net_id in _netid_to_name guard but then immediately hit an uncaught KeyError: 'sepolia' on the contract address lookup (line 135 onward), producing a cryptic error that is harder to diagnose than the message raised on line 132. Since the purpose of this change is explicit network error handling, the guard should also cover the case where the network name has no corresponding v4 contract addresses.
| if self.net_id in _netid_to_name: | ||
| self.net_name = _netid_to_name[self.net_id] | ||
| else: | ||
| raise Exception(f"Unknown netid: {self.net_id}") |
There was a problem hiding this comment.
Exception is less Pythonic and harder for callers to catch specifically. ValueError is the idiomatic built-in for receiving an invalid value, and it is consistent with how the rest of the library signals bad user input.
| if self.net_id in _netid_to_name: | |
| self.net_name = _netid_to_name[self.net_id] | |
| else: | |
| raise Exception(f"Unknown netid: {self.net_id}") | |
| if self.net_id in _netid_to_name: | |
| self.net_name = _netid_to_name[self.net_id] | |
| else: | |
| raise ValueError(f"Unknown netid: {self.net_id}") |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
No description provided.