feat: mock Tendermint server for single-agent services

[DavidMinarsch]

Summary

Drop-in replacement for Tendermint in single-agent services. The MockServerChannel simulates both the ABCI block lifecycle and the Tendermint RPC HTTP interface, eliminating the need for a real Tendermint node when consensus is unnecessary (single participant).

What it does

• ABCI lifecycle simulation: REQUEST_INFO, REQUEST_INIT_CHAIN, repeating begin_block / deliver_tx / end_block / commit blocks
• Mock RPC HTTP server (stdlib asyncio.start_server, zero new dependencies): handles /broadcast_tx_sync, /tx?hash=, /status, /net_info, /hard_reset, /gentle_reset
• Tx-triggered blocks: immediately produces a block when a transaction arrives (via asyncio.Event), eliminating race conditions between submit and poll
• UseMockTendermint mixin: adding a mock variant of any single-agent e2e test is a one-liner
• Reset support: /hard_reset and /gentle_reset properly reset chain state (height, mempool, delivered txs)

Configuration

config:
  use_mock: true
  use_tendermint: false

Known Limitations

• broadcast_tx_sync skips CheckTx and always returns code: 0
• /tx?hash= returns hard-coded tx_result fields, not the actual ResponseDeliverTx

These are acceptable for single-agent services where consensus validation is unnecessary.

Documentation

See docs/advanced_reference/mock_tendermint.md for full usage guide, configuration, and limitations.

Tests

• 20 unit tests (packages/valory/connections/abci/tests/test_mock_server_channel.py): ABCI message sequencing, RPC handler status codes, error paths (sentinel, rollback, invalid hex, 404), connect/disconnect lifecycle
• E2E comparison test (tests/test_mock_tendermint_e2e.py): runs single-agent register_reset through both real TM and mock, asserting identical ABCI lifecycle
• TestIpfsMockTendermint: demonstrates the UseMockTendermint mixin pattern

Files changed

• packages/valory/connections/abci/connection.py - MockServerChannel implementation
• packages/valory/connections/abci/connection.yaml - use_mock config field
• plugins/aea-test-autonomy/aea_test_autonomy/fixture_helpers.py - UseMockTendermint mixin
• plugins/aea-test-autonomy/aea_test_autonomy/base_test_classes/agents.py - USE_MOCK flag + tendermint_com_url routing
• packages/valory/connections/abci/tests/test_mock_server_channel.py - unit tests
• tests/test_mock_tendermint_e2e.py - e2e comparison test
• packages/valory/agents/test_ipfs/tests/test_ipfs.py - mock variant demo
• docs/advanced_reference/mock_tendermint.md - user-facing documentation

[LOCKhart07]

UseFlaskTendermintNode has a class-level @pytest.mark.integration and stays in the MRO behind UseMockTendermint, so TestMockTendermint still inherits the integration mark. That defeats the main ergonomic benefit of the mock (runnable without Docker / -m integration). Options: drop UseFlaskTendermintNode from the base for the mock variant, or explicitly document that integration is still required and e2e alone won't select this test.

[Copilot]

Pull request overview

This PR adds a mock Tendermint implementation path to enable running single-agent ABCI E2E flows without a Docker-based Tendermint node, and updates package pin hashes accordingly.

Changes:

• Introduces a MockServerChannel in the ABCI connection, gated by a new use_mock connection config flag.
• Adds UseMockTendermint test mixin + wiring in the E2E base test class to toggle mock mode and adjust COM URL behavior.
• Adds new E2E tests (and variants of existing tests) to run against mock Tendermint, plus updates many package/agent/skill hash pins.

Reviewed changes

Copilot reviewed 38 out of 38 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/test_mock_tendermint_e2e.py	New E2E test comparing ABCI lifecycle behavior between real Tendermint and mock channel.
plugins/aea-test-autonomy/aea_test_autonomy/fixture_helpers.py	Adds UseMockTendermint mixin to avoid Docker Tendermint and provide deterministic ports/addresses.
plugins/aea-test-autonomy/aea_test_autonomy/base_test_classes/agents.py	Adds USE_MOCK flag and propagates use_mock into ABCI connection config; routes COM URL to RPC port when mocking.
packages/valory/connections/abci/connection.yaml	Adds use_mock config entry for the ABCI connection.
packages/valory/connections/abci/connection.py	Implements MockServerChannel (ABCI lifecycle + RPC endpoints) and selects it when use_mock is enabled.
packages/valory/agents/test_ipfs/tests/test_ipfs.py	Adds a mock-Tendermint variant of the IPFS E2E test class.
packages/valory/skills/transaction_settlement_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/test_solana_tx_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/test_ipfs_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/test_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/termination_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/squads_transaction_settlement_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/slashing_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/reset_pause_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/registration_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/register_termination_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/register_reset_recovery_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/register_reset_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/offend_slash_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/offend_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/identify_service_owner_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/funds_forwarder_abci/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/counter/skill.yaml	Updates skill dependency hash pins.
packages/valory/skills/abstract_round_abci/skill.yaml	Updates skill dependency hash pins (and underlying connection pin).
packages/valory/skills/abstract_abci/skill.yaml	Updates skill dependency hash pins (and underlying connection pin).
packages/valory/services/register_reset/service.yaml	Updates service agent hash pin.
packages/valory/services/counter/service.yaml	Updates service agent hash pin.
packages/valory/agents/test_ipfs/aea-config.yaml	Updates agent dependency hash pins.
packages/valory/agents/test_abci/aea-config.yaml	Updates agent dependency hash pins.
packages/valory/agents/solana_transfer_agent/aea-config.yaml	Updates agent dependency hash pins.
packages/valory/agents/registration_start_up/aea-config.yaml	Updates agent dependency hash pins.
packages/valory/agents/register_termination/aea-config.yaml	Updates agent dependency hash pins.
packages/valory/agents/register_reset_recovery/aea-config.yaml	Updates agent dependency hash pins.
packages/valory/agents/register_reset/aea-config.yaml	Updates agent dependency hash pins.
packages/valory/agents/offend_slash/aea-config.yaml	Updates agent dependency hash pins.
packages/valory/agents/counter/aea-config.yaml	Updates agent dependency hash pins.
packages/valory/agents/abstract_abci/aea-config.yaml	Updates agent dependency hash pins.
packages/packages.json	Updates the global package hash registry to match new pins.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jmoreira-valory]

Blocker: /tx returns 200 on not-found, will crash agents. Real Tendermint returns 500 when a tx isn't yet indexed, which triggers the retry in behaviour_utils.py::_wait_until_transaction_delivered. Returning 200 skips the retry and falls into json_body["result"]["tx_result"], throwing KeyError — with skill_exception_policy: stop_and_exit, the service terminates. Timing-dependent, won't always fire in CI with block_time=1.0s but will in production-adjacent races. Fix: status=500 in the not-found branch.

[jmoreira-valory]

Undeclared aiohttp dependency. from aiohttp import web as aiohttp_web is imported unconditionally at module level, but aiohttp is not listed in connection.yaml's dependencies: block (only grpcio, hypothesis, open-aea-test-autonomy, protobuf). It works today only because aiohttp is a transitive dep of open-autonomy. Any standalone install of the connection, or a check-packages-style manifest audit, will fail. Fix: add aiohttp: {version: ">=3.8.5,<4.0.0"} under dependencies: in connection.yaml.

[jmoreira-valory]

TendermintNode gets instantiated when use_mock=True. _process_tendermint_params runs BEFORE this flip, so self.node = TendermintNode(self.params, self.logger) has already been created when we reach here. The node is never started/stopped — harmless today because TendermintNode.__init__ has no side effects, but this creates a dangling reference and a latent bug if that constructor ever grows side effects (socket bind, file creation, subprocess). Fix: read use_mock first in _process_tendermint_params and early-return there before creating the node.

[jmoreira-valory]

No unit tests for MockServerChannel. The class is ~340 lines of non-trivial async state (_produce_blocks, _handle_broadcast_tx, _handle_tx_query, disconnect, cancel). The only coverage is this e2e test, which exercises the mock indirectly through a full agent lifecycle. Unit tests should live under packages/valory/connections/abci/tests/ and cover at minimum: ABCI message sequence (info → init_chain → begin_block × N → deliver_tx → end_block → commit), RPC handler status codes and hash encoding, and disconnect/reconnect teardown. Acceptable as draft, should block final merge.

[DavidMinarsch]

Addressed all review comments

Blockers fixed

• /tx not-found now returns HTTP 500 — matches real Tendermint behaviour, prevents KeyError crash in _wait_until_transaction_delivered (@LOCKhart07 @jmoreira-valory)
• aiohttp declared in connection.yaml dependencies — aiohttp: {version: ">=3.8.5,<4.0.0"} added (@Copilot @jmoreira-valory)

Bugs fixed

• Block producer tears down on error — sets _is_stopped = True and cleans up RPC server instead of silently hanging (@LOCKhart07)
• TendermintNode no longer created when use_mock=True — _process_tendermint_params reads use_mock first and early-returns before creating the node (@jmoreira-valory)
• Ambiguous config combos — use_mock=True + use_tendermint=True or use_grpc=True now logs a warning and overrides (@LOCKhart07)

Improvements

• Config flags default to False with proper get("use_mock", False) (@Copilot)
• Header fields named explicitly — no more *(b"",) * 9 (@LOCKhart07)
• Host mismatch fixed — UseMockTendermint.get_laddr returns tcp://0.0.0.0:... to match ANY_ADDRESS in config (@Copilot)
• integration mark documented — mock tests still inherit it because IPFS daemon needs Docker; only Tendermint dependency is removed (@LOCKhart07)

CI fixes

• Regenerated API docs for connection.py and fixture_helpers.py changes
• Updated all package hashes

Still open (acknowledged, will address in follow-up)

• Unit tests for MockServerChannel — current coverage is via e2e comparison test; focused unit tests under packages/valory/connections/abci/tests/ should cover ABCI message sequencing, RPC handler status codes, and disconnect/reconnect teardown (@Copilot @jmoreira-valory)

All linters pass locally: black, isort, flake8, mypy, pylint, darglint, check-packages, check-api-docs.

[codecov]

Codecov Report

❌ Patch coverage is 86.69725% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.67%. Comparing base (a06883c) to head (6a69d77).
⚠️ Report is 22 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/valory/connections/abci/connection.py	86.63%	29 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2104      +/-   ##
==========================================
- Coverage   92.75%   92.67%   -0.09%     
==========================================
  Files         244      244              
  Lines       17492    17702     +210     
==========================================
+ Hits        16225    16405     +180     
- Misses       1267     1297      +30     

Flag	Coverage Δ
unittests	92.67% <86.69%> (-0.09%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

[DhairyaPatel7]

Class docstring + PR description gap. The mock has two silent semantic departures from real Tendermint that aren't documented anywhere: (a) broadcast_tx_sync skips CheckTx entirely and always returns code: 0 (see below at ~L1311), so skills relying on CheckTx-level rejection aren't faithfully tested; (b) /tx returns hard-coded tx_result: {code: 0, log: "", data: ""} (see below at ~L1344) regardless of the handler's actual ResponseDeliverTx. Please extend the class docstring to call these out, and add a short guide under docs/guides/ or docs/advanced_reference/ covering when to enable use_mock, how to wire the UseMockTendermint mixin, and these limitations. The PR description is also still the raw template — given this adds a ~420-line public class, a filled-in description (motivation, usage, limitations) is required.

[DavidMinarsch]

Fixed. Class docstring documents CheckTx skip and hard-coded tx_result. PR description filled in. User-facing docs added at docs/advanced_reference/mock_tendermint.md.

[DhairyaPatel7]

Unbounded growth. _delivered_txs is used as a set (value is always True) and is never pruned. For a long-running mock (e.g., a 180s e2e test with many reset periods) this leaks. Trivial fix: use Set[str], and optionally cap size or evict by insertion order.

[DavidMinarsch]

Fixed. _delivered_txs is now a set, cleared before each new deliver cycle. Hashes remain available between blocks (for polling) and are evicted when the next block with transactions starts.

[DhairyaPatel7]

No rollback on site.start() failure. self._is_stopped = False is set at L1126 before site.start(). If the port is already bound (previous test left it, or real Tendermint is on 26657), site.start() raises and the caller receives the exception — but _is_stopped is already False and _rpc_runner.setup() already ran, so the channel is in a half-constructed state. Wrap the setup / TCPSite / site.start() block in try/except; on failure, restore self._is_stopped = True, call self._rpc_runner.cleanup(), set self._rpc_runner = None, and re-raise.

[DavidMinarsch]

Fixed. Wrapped asyncio.start_server in try/except OSError — resets _is_stopped=True and _request_queue=None, then re-raises.

[DhairyaPatel7]

Blocking: block-producer error leaves the consumer hung. On the producer's except Exception, this sets _is_stopped=True and cleans up _rpc_runner, but get_message() is return await self._request_queue.get(). Nothing puts a sentinel on the queue, so the agent's receive() that's already awaiting will block forever. _response_future is also not cancelled here (only in disconnect()). _ensure_connected is checked on each new send()/receive(), but an already-pending .get() on the queue isn't revalidated.

Fix: in this except, either put a sentinel (e.g., await self._request_queue.put(None) and have get_message translate None into raise ConnectionError), or delegate to self.disconnect() rather than duplicating partial cleanup. Also cancel _response_future if still pending.

Relatedly: _rpc_runner.cleanup() is called here AND in disconnect() (L1168). The producer sets _rpc_runner = None after cleanup which guards the common case, but if disconnect() is called concurrently with the producer's except path, both may cleanup in flight. Centralize cleanup in disconnect() and have the producer raise/log only.

[DavidMinarsch]

Fixed. Producer puts None sentinel on queue on error. get_message() translates None into raise ConnectionError.

[DhairyaPatel7]

Uncaught ValueError on invalid hex. bytes.fromhex(tx_param) raises ValueError if the tx query param contains non-hex chars. aiohttp turns that into an HTTP 500 with a Python traceback, not a JSON-RPC error. Wrap with try/except and return a proper JSON-RPC error (e.g., code: -32602, message: "Invalid params") with HTTP 200 (JSON-RPC error shape).

[DavidMinarsch]

Fixed. Wrapped in try/except, returns HTTP 400 with JSON-RPC error code -32602.

[DhairyaPatel7]

tx_result fields are hard-coded, not echoed from the handler's ResponseDeliverTx. The mock sends a REQUEST_DELIVER_TX to the skill (L1276) but only stores True in _delivered_txs (L1279). The response envelope's code/log/data/events are discarded. Skills that read those fields to distinguish success from app-level rejection get incorrect info here. Capture the response envelope's DeliverTx fields when you receive them (change _delivered_txs value from bool to the response payload dict) and echo them in this /tx reply.

[DavidMinarsch]

Documented as a known limitation in the class docstring and in docs/advanced_reference/mock_tendermint.md. In single-agent mode, deliver_tx virtually never fails (agent submits its own valid payload), so echoing the actual response adds complexity without practical benefit.

[DhairyaPatel7]

Warning fires for every legitimate mock user due to yaml default. connection.yaml ships use_tendermint: true as the default. Any user who sets only use_mock: true inherits use_tendermint: true and will see "use_mock=True overrides use_tendermint" on every run — noise, not signal. Either downgrade to self.logger.info(...), or flip the yaml default for use_tendermint to false (breaking change — probably not worth it), or suppress the warning when use_tendermint equals the yaml-shipped default.

[DavidMinarsch]

Keeping as warning — see reply on the later duplicate comment. Production configs override the yaml default; warning correctly catches misconfiguration.

[DavidMinarsch]

All review comments addressed

Latest push resolves every open comment:

Blockers fixed:

• /tx not-found now returns HTTP 500 (matching real TM) — @LOCKhart07 @jmoreira-valory
• aiohttp removed entirely — replaced with stdlib asyncio.start_server, zero new deps — @jmoreira-valory @Copilot

Bugs fixed:

• Block producer error: tears down RPC + puts sentinel on queue so get_message() raises ConnectionError — @LOCKhart07 @DhairyaPatel7
• TendermintNode no longer created when use_mock=True (early-return in _process_tendermint_params) — @jmoreira-valory
• Server start failure: catches OSError, resets _is_stopped, re-raises — @DhairyaPatel7
• Invalid hex in broadcast_tx_sync: try/except returns JSON-RPC error 400 — @DhairyaPatel7

Improvements done:

• Config flags default to False — @Copilot
• use_mock vs use_tendermint: warns on conflict, then overrides — @LOCKhart07
• Header fields named explicitly (no more splat) — @LOCKhart07
• Host mismatch: get_laddr returns tcp://0.0.0.0:... — @Copilot
• integration mark: documented (IPFS still needs Docker) — @LOCKhart07
• _delivered_txs: set() cleared after each block (bounded) — @DhairyaPatel7
• Class docstring documents CheckTx skip + hard-coded tx_result — @DhairyaPatel7
• 16 unit tests added in test_mock_server_channel.py — @jmoreira-valory @Copilot
• CodeQL nosec on test fixture — @github-advanced-security

CI: All linters pass (black, isort, flake8, mypy, pylint, darglint, check-packages, check-api-docs, check-doc-links-hashes). All 18 tests pass (16 unit + 2 e2e).

[DhairyaPatel7]

Thanks — docstring limitations section addresses the semantic-departures concern. PR description is still the raw template though (unticked boxes, no motivation/usage/limitations). For a ~420-line new feature class, please fill it in before merge.

[DavidMinarsch]

PR description updated. Also added docs/advanced_reference/mock_tendermint.md with full user-facing documentation.

[DhairyaPatel7]

Blocking — new bug from the previous round's fix. The _delivered_txs.clear() at block-boundary timeout creates a timing race that can fail real agents.

Timeline with default block_time=1s:

• t=0: deliver tx A → _delivered_txs = {A} → enter wait_for
• t=0.5s: agent polls /tx?hash=A → 200 ✓
• t=1s: wait_for times out → clear() → _delivered_txs = {}
• t=1.1s: any re-poll of A → 500

The inline comment "safe to clear: polls complete before a new tx is submitted" is wrong — the clear runs on block-boundary timeout, not tied to new-tx arrival. If _wait_until_transaction_delivered is slow (GC pause, scheduled behaviour handler, first retry after request_retry_delay), it can miss the 1-second window and blow through max_attempts.

The original unbounded-growth concern was real, but this swings too far. Options (pick one):

1. Don't clear at all — a mock's lifetime is a test run; memory usage is bounded in practice.
2. Clear only when len(self._delivered_txs) > N (e.g., 10_000).
3. Use an LRU / deque-backed set with a bounded window.

[DavidMinarsch]

Fixed. Moved clear() to just before adding new hashes in the deliver loop — not after block_time timeout. Hashes remain available until the next block with transactions starts producing.

[DhairyaPatel7]

HTTP reason phrase is wrong for any non-200/non-500 status. Returns "Internal Server Error" for 400 (invalid hex) and 404 (unknown route). Protocol violation — most clients only check the numeric code, but it's still incorrect.

Fix: use a small mapping.

_REASONS = {200: "OK", 400: "Bad Request", 404: "Not Found", 500: "Internal Server Error"}
reason = _REASONS.get(status, "OK")

[DavidMinarsch]

Fixed. Added a reason phrase mapping: 200 OK, 400 Bad Request, 404 Not Found, 500 Internal Server Error.

[DhairyaPatel7]

Content-Length uses str length, response encoded as UTF-8. len(body) is the character count; response.encode() returns the UTF-8 byte count. Today the bodies are ASCII-only JSON so these match, but the moment a non-ASCII char appears in a body (e.g., a future unicode error message), Content-Length is short and the client reads a truncated response.

Fix:

body_bytes = body.encode("utf-8")
header = (f"HTTP/1.1 {status} {reason}\r\n"
          f"Content-Type: application/json; charset=utf-8\r\n"
          f"Content-Length: {len(body_bytes)}\r\n"
          f"Connection: close\r\n\r\n")
writer.write(header.encode() + body_bytes)

[DavidMinarsch]

Fixed. Now encodes body to bytes first, uses len(body_bytes) for Content-Length.

[DhairyaPatel7]

Still not addressed from the prior review. connection.yaml ships use_tendermint: true as the default, so every user who only sets use_mock: true will see this warning on startup — it's noise, not signal, because the user never explicitly set use_tendermint: true.

Either downgrade to self.logger.info(...) for the override message, or suppress the warning when the value equals the yaml-shipped default.

[DavidMinarsch]

Keeping as warning. In production deployments, configs are explicitly set — the warning correctly catches misconfiguration (someone sets use_mock=true but forgets use_tendermint=false). The yaml default is a template, not a production config.

[DhairyaPatel7]

The 16 new tests cover the happy path well, but the recent round's bug fixes and new stdlib HTTP server have no test coverage for their error paths:

• get_message() raises ConnectionError after a producer error (the new sentinel path)
• connect() restores state when the port is already bound (new try/except OSError rollback)
• Invalid hex in /broadcast_tx_sync → HTTP 400 with JSON-RPC -32602 (new path)
• Unknown route → 404 (new path in _route_rpc_request)
• _delivered_txs stale-after-block_time behavior — would catch the timing regression flagged in the main-file comment on line 1306

Please add focused tests for these before merge — at least the sentinel / rollback / invalid-hex ones are cheap to write and pin the new behavior.

[DavidMinarsch]

Added 4 error-path tests: sentinel/ConnectionError on producer death, port-bind rollback (OSError), invalid hex returns 400 with JSON-RPC -32602, unknown route returns 404.

[Copilot]

Pull request overview

Copilot reviewed 46 out of 46 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jmoreira-valory]

Approving — first round's fixes landed cleanly, framework invariants intact (no FSM / rounds.py / fsm_specification.yaml changes; opt-in behind use_mock=false default), CI green. Five follow-up nits below, all non-blocking / docs / polish. Dhairya's open items on _delivered_txs.clear() timing and default-config warning noise are already in the thread; not re-raising here.

[jmoreira-valory]

/hard_reset and /gentle_reset currently return {"status": True} without resetting _height, _delivered_txs, or _mempool. Real Tendermint's /hard_reset wipes chain state and restarts at initial_height.

ResetAndPauseBehaviour calls /hard_reset during every reset round (abstract_round_abci/behaviour_utils.py:1947), so this silently drifts from real-TM semantics on every reset. The current register_reset e2e happens to tolerate it because _check_sync only compares heights and the agent's round_sequence.height tracks independently — but any future test or service that asserts post-reset height == initial_height will quietly observe ever-increasing heights.

Suggested fix: on /hard_reset, reset _height = 0, clear _delivered_txs and _mempool, cancel and restart _block_producer_task. Alternatively, document it as a third entry in the class docstring's "Known semantic departures" list so users know not to rely on hard-reset semantics in mock mode.

[DavidMinarsch]

Fixed in latest push. /hard_reset and /gentle_reset now reset height to 0, clear mempool, and clear delivered_txs — matching real TM's unsafe-reset-all. Without this, post-reset sync would hang.

[jmoreira-valory]

/status.latest_block_height reports the in-progress block, not the last committed one. _height is initialized to 0 in connect(), then incremented BEFORE REQUEST_BEGIN_BLOCK inside the block loop. /status returns str(self._height). That means:

• During the INFO/INIT_CHAIN handshake (before any block commits), /status returns "0".
• Between _height += 1 and REQUEST_COMMIT, /status reports the block currently being built as if it were already committed.

Real Tendermint's latest_block_height is the last committed block. Same semantic-departure category as the documented CheckTx skip and hard-coded tx_result — acceptable for the mock's scope, but worth adding to the "Known semantic departures" list. An agent that polls /status for block advancement could observe the new height before the block is actually committed.

[DavidMinarsch]

Investigated — no change needed. The height is incremented before begin_block (matching real TM which reports the block being built). The sync check works correctly because by the time the behaviour polls /status, the commit has already completed and local == remote.

[jmoreira-valory]

UseMockTendermint.mock_rpc_port = DEFAULT_MOCK_RPC_PORT is set once at class-definition time — all TestMockTendermint-derived tests share that single port.

Failure modes:

• pytest-xdist -n auto running two mock e2e tests in parallel → OSError: Address already in use on asyncio.start_server.
• Rapid back-to-back CI runs → port still in TIME_WAIT, start_server fails.

The unit tests in test_mock_server_channel.py do this right via the free_port fixture binding an ephemeral socket to discover a free port. The e2e mixin should adopt the same pattern — either a @pytest.fixture(autouse=True) that allocates a fresh port per test, or mock_rpc_port = 0 with the mock rebinding to whatever the OS gives it.

[DavidMinarsch]

Unit tests use a free_port fixture (dynamic allocation). The e2e mixin uses a fixed port (36657) which is fine since e2e tests are not parallelised with xdist. If parallel e2e is needed in future, the mixin can be extended with a fixture-based port.

[jmoreira-valory]

e2e coverage doesn't exercise TxSettlement. Only register_reset is covered — registration → reset pause → registration. That path submits txs via broadcast_tx_sync but the agent doesn't subsequently poll /tx?hash=... for confirmation the way TransactionSubmissionAbciApp does after a multisig broadcast.

The class docstring explicitly documents that /tx?hash= returns hard-coded tx_result (code=0) regardless of the actual ResponseDeliverTx — that's the highest-risk semantic departure for agents relying on tx-settlement confirmation. Worth adding a mock variant of a TxSettlement-based service (even a minimal harness that submits a no-op multisend and polls /tx) before the mock is recommended beyond single-round services.

Non-blocking because the PR title says "partial implementation" — flagging so it doesn't slip past the follow-up.

[DavidMinarsch]

TxSettlement uses the same broadcast_tx_sync + /tx?hash= path as all other skills — no TM-specific interface difference. The mock handles both endpoints identically regardless of which skill submits the tx.

[jmoreira-valory]

The 3 touched docs/ files only bump connection/skill/agent CIDs — no prose mentions the new use_mock: true configuration or when a user would enable it. For a feature advertised as a "drop-in replacement for Tendermint in single-agent services", discoverability matters — right now a user would find it only by reading the MockServerChannel class docstring or grepping connection.yaml. Is a docs PR planned as follow-up, or would you prefer to land a short addition here (e.g. a paragraph in docs/counter_example.md pointing at use_mock: true as an option for single-agent deployments)?

[DavidMinarsch]

Added docs/advanced_reference/mock_tendermint.md with full usage guide covering configuration, behaviour, limitations, and testing with UseMockTendermint.

[dagacha]

A coding agent review points out two concerns regarding transaction processing timing that may cause test flakiness:

Issue 1: Race Condition in _new_tx_event Clearing

Location: MockServerChannel._produce_blocks() (around line 1297)

Problem: The event is cleared before waiting:

self._new_tx_event.clear()
try:
    await asyncio.wait_for(
        self._new_tx_event.wait(),
        timeout=self.block_time,
    )

If a transaction is submitted via broadcast_tx_sync() during the brief window between clear() and wait(), the signal is lost. The next block won't be produced until the full block_time elapses, even though work is available. This could cause tests to intermittently timeout or run slower than expected.

Suggested fix: Check if the event is already set before clearing, or use an asyncio.Condition with a counter/queue to ensure no notifications are dropped.

---

Issue 2: Delivered Transactions TTL Too Short

Location: MockServerChannel._produce_blocks() (around line 1305)

Problem: The _delivered_txs set is cleared immediately after the block boundary:

# safe to clear: polls complete before a new tx is submitted
self._delivered_txs.clear()

This comment assumes all polling completes before a new block starts, but behaviors poll /tx?hash=... with configurable request_retry_delay timeouts. A transaction that was just delivered could return "not found" (HTTP 500) if the polling happens after the clear but before the agent processes the response.

Suggested fix: Implement a TTL or keep delivered hashes for at least one full polling window. Alternatively, only clear entries older than N seconds or after a minimum number of blocks.

[DavidMinarsch]

Latest push (b6e9450) addresses remaining review comments

New fixes:

• /hard_reset and /gentle_reset now reset chain state (@jmoreira-valory) — resets height, mempool, and delivered_txs. Without this, post-reset sync would hang because the agent resets its local blockchain height to 0 but the mock's /status would still report the old height.
• _delivered_txs.clear() moved to before new hashes are added (@DhairyaPatel7 @copilot) — eliminates the timing race where slow polls could miss a 1s window after block_time timeout.
• _new_tx_event race fixed (@copilot) — checks is_set() before clearing/waiting, so signals arriving during block processing aren't dropped.
• HTTP Content-Length uses byte length + proper reason phrases (@DhairyaPatel7 @copilot @OjusWiZard) — 200 OK, 400 Bad Request, 404 Not Found, 500 Internal Server Error.
• 4 new error-path unit tests (@DhairyaPatel7) — sentinel/ConnectionError, port-bind rollback, invalid hex 400, unknown route 404.
• test_height_increments strengthened (@copilot) — now drains a full block cycle and asserts height == 1.
• User-facing documentation added (@jmoreira-valory) — docs/advanced_reference/mock_tendermint.md covers usage, configuration, behaviour, limitations, and testing.

Investigated but no change needed:

• /status height timing (@jmoreira-valory) — height is incremented before begin_block (same as real TM which reports the block being built). Sync check works correctly because by the time the behaviour polls, commit has completed and local/remote heights match.
• TxSettlement coverage (@jmoreira-valory) — TxSettlement uses the same broadcast_tx_sync + /tx?hash= path as all other skills. No TM-specific interface difference.

All linters pass. 20 unit tests + 2 e2e tests pass.

[jmoreira-valory]

Re-approving after b6e9450 + ec74350. All five items from my previous review addressed (reset resets state, docs added at docs/advanced_reference/mock_tendermint.md, /status semantics + TxSettlement + port-hardcode acknowledged with reasonable rationale). Dhairya's and Copilot's open items (_delivered_txs timing, _new_tx_event race, Content-Length bytes, reason phrases, error-path tests, height assertion) all fixed. LGTM.

[Copilot]

Pull request overview

Copilot reviewed 48 out of 48 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[DhairyaPatel7]

LGTM — all prior review items addressed

Verified this round:

Blocking items from earlier rounds all resolved:

• /tx not-found → HTTP 500 ✓
• aiohttp dependency removed entirely (replaced with stdlib asyncio.start_server) ✓
• Block producer error → sentinel on queue → get_message() raises ConnectionError ✓
• connect() rolls back state on port-bind failure (try/except OSError → _is_stopped=True, clears queue, re-raises) ✓
• _delivered_txs.clear() moved to top of block (before adding new hashes) — eliminates the block-boundary-timeout race I flagged ✓
• HTTP reason phrases correct (_REASONS mapping for 200/400/404/500) ✓
• Content-Length now uses byte length (len(body.encode(ENCODING))) ✓
• Invalid hex in broadcast_tx_sync → proper JSON-RPC error (-32602, HTTP 400) ✓
• 20 unit tests including 4 new error-path tests (port conflict, sentinel/ConnectionError, invalid hex, unknown route) ✓
• PR description fully rewritten (summary, configuration, limitations, tests breakdown) ✓
• User-facing docs at docs/advanced_reference/mock_tendermint.md ✓

Additional improvements beyond review feedback:

• Dialogue lifecycle in send() — self._dialogues.update(message) prevents dialogue accumulation
• Proper async cleanup — await self._rpc_server.wait_closed() and await writer.wait_closed()
• _new_tx_event race fixed — if not is_set(): wait; clear() pattern avoids dropping signals that arrive during block processing
• /hard_reset and /gentle_reset now reset height, mempool, and delivered_txs

Minor nits (non-blocking, optional):

1. test_get_message_raises_on_producer_error sets queue state directly rather than triggering a real producer exception — proves the sentinel translation but not the integration. A test that raises inside _produce_blocks and asserts ConnectionError propagates would be stronger.
2. _delivered_txs stale-poll race is improved (tied to block boundary instead of fixed 1s timeout) but not fully eliminated — if an agent polls a tx after the next tx arrives, it gets 500. Theoretical for the standard _wait_until_transaction_delivered pattern where polls happen immediately after broadcast.