Skip to content

PR #2477 review: deferred minor cleanups (subgraph, http_server, tendermint handlers, docs) #2482

@DavidMinarsch

Description

@DavidMinarsch

Overview

Small cleanups deferred from the PR #2477 review thread (#2477 (comment)). None are regressions; all are incremental improvements.

Items

1. Defensive guard in autonomy/chain/subgraph/client.py:83. payload["data"] can KeyError if a response has neither an errors key nor a data key. Add:

if "data" not in payload:
    raise RuntimeError(f"Subgraph returned malformed response: {payload!r}")

2. Body-size cap in autonomy/deploy/_http_server.py:_serve. self.rfile.read(content_length) is unbounded. Add a module-level MAX_CONTENT_LENGTH = 10 * 1024 * 1024 and return a 413 if the declared length exceeds it. Matches Flask's MAX_CONTENT_LENGTH pattern. Localhost-only so low severity; cheap defense-in-depth.

3. 404/500 error-handler mimetype mismatch. Both autonomy/deploy/generators/localhost/tendermint/app.py:347-354 and deployments/Dockerfiles/tendermint/app.py:322, 328 return plain-text bodies ("Not Found", "Error Closing Node") with mimetype="application/json". Switch to jsonify({"error": "..."}) so the body matches the declared mimetype and other handlers.

4. Document _to_response supported shapes. autonomy/deploy/_http_server.py:_to_response supports only the 2-tuple (body, status) form; Flask also supports (body, status, headers) and (body, headers). Zero current callers use those, but the restriction should be explicit in the docstring so future handler return-value surprises don't silently break.

5. encode.py:73 docstring typo. "Create method struction defintion." → "Create the struct definition string."

6. Clarify pyproject.toml dev-vs-distribution split. The TODO(cleanup) block's comment above the shim entries should explicitly say "[tool.poetry.dependencies] models the dev environment the repo is checked out into; setup.py remains the authoritative PyPI manifest and is intentionally narrower".

7. Lean-install CI smoke test. CI's install_check job currently does pip install .[all] + autonomy --help. Add a parallel step that does pip install . (no extras) + autonomy --help to catch regressions where a module accidentally grows a top-level import web3 / import aiohttp / etc.

8. Real-EthereumApi integration test for encode.py. The current unit tests in packages/valory/contracts/gnosis_safe/tests/test_encode.py use a deterministic ledger_api.api.codec._registry.get_encoder stub. A mock-based test won't catch an upstream eth_abi refactor that changes the _registry shape. Add one test that instantiates a real EthereumApi (no network) and exercises encode_value through it.

9. Cache get_requests_connection_error(). autonomy/chain/exceptions.py:25-48 re-imports requests.exceptions.ConnectionError on every except clause evaluation. Low cost but also low value to reimport. A module-level cache (_CACHED: Optional[Type[BaseException]] = None) would tighten it without changing behaviour.

Non-goals

  • Style-level refactor of the 41 cast(EthereumApi, ledger_api) call sites to a helper (review add finalization to price_estimation_skill #12): explicitly declined. Adds a stack frame to every chain-side exception for marginal readability gain.

Context

See PR #2477 review #2477 (comment) (non-blockers #3, #4, #5, #6, #8, #9, #11 + PR description cleanup).

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions