perf: shared HTTP client and latency optimizations#14
perf: shared HTTP client and latency optimizations#14gregnazario wants to merge 7 commits intomainfrom
Conversation
Eliminate per-request httpx client creation across the SDK, reducing TCP/TLS handshake overhead from 3-5 connections per transaction flow to zero (via connection pooling and keep-alive). - Add shared httpx.AsyncClient/Client to BaseSDK, BaseSDKSync, and DecibelReadDex with configurable connection pool limits - Wire shared client through fee payer submission, gas price manager, order status client, and all reader endpoints - Add close()/context manager support for proper client lifecycle - Use escalating poll intervals (200ms→500ms→1s) for faster tx confirmation on quick-confirming transactions - Add 5s timeouts to gas price estimation, simulation, order status, and confirmation polling to prevent indefinite hangs
There was a problem hiding this comment.
Pull request overview
This PR improves SDK latency and reliability by reusing pooled httpx clients across request flows, adding lifecycle management (close/context managers), tightening request timeouts, and optimizing transaction confirmation polling.
Changes:
- Introduces shared
httpxclient configuration (HTTP_LIMITS,HTTP_TIMEOUT) and wires shared clients through read/write SDK paths. - Adds
close()+ context manager support for async/sync SDK entrypoints and updates request codepaths to reuse the shared client. - Optimizes confirmation polling by using escalating delays and adds explicit 5s timeouts to selected network calls.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/decibel/_constants.py |
Adds shared httpx limits/timeout constants for pooled clients. |
src/decibel/_base.py |
Uses shared client for core write flow + adds close/context manager + faster polling delays. |
src/decibel/_gas_price_manager.py |
Adds optional injected clients and explicit 5s timeouts for gas estimation. |
src/decibel/_order_status.py |
Adds optional injected clients and explicit 5s timeouts for order status queries. |
src/decibel/read/_base.py |
Threads optional shared clients through reader request helpers. |
src/decibel/read/__init__.py |
Creates shared async client for readers and adds close/context manager. |
src/decibel/write/__init__.py |
Passes shared base client into OrderStatusClient (async/sync). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add 749 new tests (797 total) covering all SDK modules with mocked HTTP clients and blockchain interactions. Test files added: - test_utils_extended.py — FetchError, request functions, address derivation - test_base.py — BaseSDK/BaseSDKSync lifecycle, tx flow, polling - test_fee_pay.py — fee payer submission routing and serialization - test_transaction_builder.py — BCS encoding, type parsing, tx building - test_gas_price_manager.py — async/sync gas price caching - test_order_status.py — order status queries and parsing - test_exceptions.py — custom exception classes - test_pagination.py — query param construction - read/test_base_reader.py — BaseReader HTTP delegation - read/test_readers.py — all 22 reader modules - read/test_ws.py — WebSocket subscription management - write/test_write_dex.py — all write operations (async + sync) - test_admin.py — all admin operations (async + sync) Coverage: 25% → 95% (3411/3592 statements covered)
- Add explicit timeout=5.0 to gas price estimation in both async and sync BaseSDK (was relying on shared client default of 10s) - Close WebSocket subscription in DecibelReadDex.close() to prevent leaking background receive tasks and open sockets - Apply ruff formatting fixes
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add try/except for ConnectTimeout/ReadTimeout/ConnectError in sync _wait_for_transaction to match async behavior and prevent raw httpx exceptions from aborting confirmation polling - Simplify admin.usdc_balance to use shared client directly (no longer nullable after shared client changes) - Remove unused httpx import from admin.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WGB5445
left a comment
There was a problem hiding this comment.
DecibelReadDex.close() cleanup is not robust enough.
In read/init.py, await self.ws.close() is called before await self._http_client.aclose(). If the former raises an exception, the latter may not be executed.
Post a coverage summary as a PR comment using the coverage.xml already generated by pytest. Runs only on Python 3.11 matrix entry for PRs.
…nt-and-latency-optimizations
- Remove netna chain_id 208 test (netna config removed upstream) - Update builder fee tests to expect bps_to_chain_units conversion - Update twap order test to use integer bps value - Remove obsolete http_client-is-None test in admin
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 26 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_typical_run_mostly_returns_int(self) -> None: | ||
| """Statistically, with 32-bit values, zero is extremely rare.""" | ||
| results = [generate_random_replay_protection_nonce() for _ in range(50)] | ||
| non_none = [r for r in results if r is not None] | ||
| # At least 40 out of 50 should be non-None (probability of zero ~1 in 4B) | ||
| assert len(non_none) >= 40 |
There was a problem hiding this comment.
This test is probabilistic (depends on secrets.randbits returning non-zero values). Even though failure is extremely unlikely, it can still introduce flaky CI runs. Prefer making this deterministic by mocking secrets.randbits (or directly asserting behavior with explicit zero/non-zero cases) rather than relying on statistical expectations.
| def test_typical_run_mostly_returns_int(self) -> None: | |
| """Statistically, with 32-bit values, zero is extremely rare.""" | |
| results = [generate_random_replay_protection_nonce() for _ in range(50)] | |
| non_none = [r for r in results if r is not None] | |
| # At least 40 out of 50 should be non-None (probability of zero ~1 in 4B) | |
| assert len(non_none) >= 40 | |
| def test_repeated_calls_return_int_when_randbits_is_nonzero(self) -> None: | |
| side_effect = list(range(1, 101)) | |
| with patch("secrets.randbits", side_effect=side_effect): | |
| results = [generate_random_replay_protection_nonce() for _ in range(50)] | |
| assert all(isinstance(result, int) for result in results) | |
| assert all(result is not None for result in results) |
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
There was a problem hiding this comment.
The workflow grants pull-requests: write at the workflow level, which also applies to push runs. If the only write action is the PR coverage comment step, consider scoping elevated permissions to that job (or to pull_request events only) to follow least-privilege for the GITHUB_TOKEN.
Summary
httpxclient creation across the entire SDK. Previously, each API call (gas estimation, simulation, submission, confirmation polling) created and destroyed its own TCP+TLS connection — up to 5 per transaction. Now a single pooled client with keep-alive handles all requests, saving ~200-1500ms per transaction flow.BaseSDK,BaseSDKSync, andDecibelReadDexnow supportclose()and context managers (async with/with) for proper connection pool cleanup.Files changed
_constants.pyHTTP_LIMITSandHTTP_TIMEOUTshared config_base.py_gas_price_manager.py_order_status.pyread/_base.pyhttp_clientfields onReaderDeps, wired to all reader methodsread/__init__.pyDecibelReadDexcreates shared client, close/context managerwrite/__init__.pyOrderStatusClientTest plan
pytest tests/)ruff check src/)