Survive a dropped Electrum connection, and fall back to other servers
A dropped connection used to hang the whole platform permanently, and three defects composed to do it (BUGS.md B-01): The read loop's death was invisible. When the socket closed, _read_loop broke out and finished, but _run_once was blocked on gather() over two notification consumers waiting on queues nobody would ever fill again — it never returned and never raised, so the reconnect-with-backoff logic was unreachable. client.wait_closed() now resolves when the loop ends for any reason, and _run_once races it against the consumers and a keepalive with asyncio.wait(FIRST_COMPLETED). Nothing had a timeout. request() registered a future, wrote to a half-closed socket (drain() often doesn't raise) and awaited a reply that would never come. That hung a POST /bets *while holding the per-user lock*, and could stop the confirmation poller for good. Every request is now bounded at 15s, and a timeout tears the connection down rather than leaving a server that owes us a reply in rotation. There was no keepalive, so on a quiet instance the normal way this connection dies is an idle-timeout drop by the server (~10 minutes for many). A server.ping every 60s makes that observable within a minute. listener.client is also cleared before reconnecting, so callers stop treating a dead connection as live. On top of the finding, the listener now rotates over a list of servers: ELECTRUM_FALLBACK_SERVERS holds comma-separated host:port[:notls] extras, tried after the primary. Everything the platform does goes through this one connection — deposit credits, broadcasts, confirmations, the chain tip the draw waits on — which made a single hardcoded server its biggest point of failure. A failed or dropped session moves to the next server immediately and only sleeps on the backoff once every server has had a turn, so one dead server costs one attempt instead of an outage, while a genuinely offline network still backs off. A malformed entry fails at startup, not during the outage when the fallback is what you need. Also fixes B-19: header handling refuses a height below the current tip and applies height and hex together, since _wait_for_next_block waits for tip_height > tip_at_close (a regression silently added a block to the draw's wait) and that hex is the draw's entropy source, so a mismatched pair would be worse than a stale one. Verified in the live deployment: the log shows the endpoint list, then "Electrum connected to santantonio.sytes.net:50002", and the connection holds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -75,3 +75,83 @@ async def test_notification_delivered_to_subscription_queue():
|
||||
assert params == ["abcd", "newstatus"]
|
||||
|
||||
client._read_task.cancel()
|
||||
|
||||
|
||||
async def test_request_times_out_instead_of_hanging_forever(monkeypatch):
|
||||
"""B-01: a server that owes us a reply and never sends one used to hang the
|
||||
caller permanently — which meant a POST /bets could hang while holding the
|
||||
per-user lock, and the confirmation poller could stop polling for good."""
|
||||
from app.electrum import client as client_module
|
||||
|
||||
monkeypatch.setattr(client_module, "_REQUEST_TIMEOUT_SECONDS", 0.05)
|
||||
client, reader, writer = await _client_with_fake_transport()
|
||||
|
||||
try:
|
||||
await client.request("blockchain.transaction.get", ["deadbeef"])
|
||||
assert False, "expected ElectrumError"
|
||||
except ElectrumError as exc:
|
||||
assert "timed out" in str(exc)
|
||||
|
||||
# The connection is torn down, so callers stop reusing a server that owes us.
|
||||
assert client._closed.is_set()
|
||||
client._read_task.cancel()
|
||||
|
||||
|
||||
async def test_wait_closed_resolves_when_the_read_loop_dies():
|
||||
"""B-01: the read loop dying used to be invisible — the listener sat on its
|
||||
notification queues forever and never reconnected."""
|
||||
client, reader, writer = await _client_with_fake_transport()
|
||||
|
||||
waiter = asyncio.create_task(client.wait_closed())
|
||||
reader.feed_eof() # peer closed the connection
|
||||
|
||||
await asyncio.wait_for(waiter, timeout=1)
|
||||
client._read_task.cancel()
|
||||
|
||||
|
||||
async def test_pending_request_fails_when_the_connection_drops():
|
||||
client, reader, writer = await _client_with_fake_transport()
|
||||
|
||||
task = asyncio.create_task(client.request("server.ping"))
|
||||
await asyncio.sleep(0)
|
||||
reader.feed_eof()
|
||||
|
||||
try:
|
||||
await asyncio.wait_for(task, timeout=1)
|
||||
assert False, "expected ElectrumError"
|
||||
except ElectrumError:
|
||||
pass
|
||||
|
||||
|
||||
def test_parse_endpoints_puts_the_primary_first_and_dedupes():
|
||||
from app.electrum.client import ElectrumEndpoint, parse_endpoints
|
||||
|
||||
endpoints = parse_endpoints(
|
||||
"primary.example", 50002, True, "second.example:50002, third.example:50001:notls, primary.example:50002"
|
||||
)
|
||||
assert endpoints == [
|
||||
ElectrumEndpoint("primary.example", 50002, True),
|
||||
ElectrumEndpoint("second.example", 50002, True),
|
||||
ElectrumEndpoint("third.example", 50001, False),
|
||||
]
|
||||
|
||||
|
||||
def test_parse_endpoints_handles_an_empty_fallback_list():
|
||||
from app.electrum.client import ElectrumEndpoint, parse_endpoints
|
||||
|
||||
assert parse_endpoints("only.example", 50002, True, "") == [ElectrumEndpoint("only.example", 50002, True)]
|
||||
assert parse_endpoints("only.example", 50002, True, " , ") == [
|
||||
ElectrumEndpoint("only.example", 50002, True)
|
||||
]
|
||||
|
||||
|
||||
def test_parse_endpoints_rejects_a_malformed_entry():
|
||||
"""A typo in a fallback server must fail at startup, not during the outage when
|
||||
the fallback is the thing that's needed."""
|
||||
import pytest
|
||||
|
||||
from app.electrum.client import parse_endpoints
|
||||
|
||||
for bad in ["nohost:", "host:notaport", "host", "host:50002:weird"]:
|
||||
with pytest.raises(ValueError):
|
||||
parse_endpoints("primary.example", 50002, True, bad)
|
||||
|
||||
@@ -0,0 +1,142 @@
|
||||
"""Listener-level behaviour: server rotation on failure (the fallback-servers
|
||||
feature), and the chain-tip monotonicity guard (B-19).
|
||||
|
||||
The reconnect loop itself (B-01) is covered from the client side in
|
||||
test_electrum_client.py — what's asserted here is that the listener *acts* on a
|
||||
dead connection by moving to the next server instead of retrying the same one.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
|
||||
import pytest
|
||||
from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
|
||||
|
||||
from app.db.base import Base
|
||||
from app.electrum.client import ElectrumEndpoint
|
||||
from app.electrum.listener import ElectrumListener
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def session_factory():
|
||||
engine = create_async_engine("sqlite+aiosqlite:///:memory:")
|
||||
async with engine.begin() as conn:
|
||||
await conn.run_sync(Base.metadata.create_all)
|
||||
yield async_sessionmaker(engine, expire_on_commit=False)
|
||||
await engine.dispose()
|
||||
|
||||
|
||||
_ENDPOINTS = [
|
||||
ElectrumEndpoint("first.example", 50002, True),
|
||||
ElectrumEndpoint("second.example", 50002, True),
|
||||
ElectrumEndpoint("third.example", 50001, False),
|
||||
]
|
||||
|
||||
|
||||
async def test_rotates_to_the_next_server_after_a_failed_session(session_factory):
|
||||
"""One unreachable server should cost a single attempt, not an outage: every
|
||||
deposit credit, broadcast and confirmation goes through this one connection."""
|
||||
attempted: list[str] = []
|
||||
|
||||
listener = ElectrumListener(lambda endpoint: None, session_factory, _ENDPOINTS)
|
||||
|
||||
async def failing_run_once(endpoint):
|
||||
attempted.append(endpoint.host)
|
||||
if len(attempted) >= 5:
|
||||
raise asyncio.CancelledError # stop the loop
|
||||
raise ConnectionRefusedError("nope")
|
||||
|
||||
listener._run_once = failing_run_once
|
||||
|
||||
with pytest.raises(asyncio.CancelledError):
|
||||
await listener.run()
|
||||
|
||||
# Round-robin over all three, wrapping around — never the same one twice in a row.
|
||||
assert attempted == [
|
||||
"first.example",
|
||||
"second.example",
|
||||
"third.example",
|
||||
"first.example",
|
||||
"second.example",
|
||||
]
|
||||
|
||||
|
||||
async def test_backoff_only_sleeps_after_every_server_has_been_tried(session_factory, monkeypatch):
|
||||
"""A genuinely offline network must back off, but not before the alternatives have
|
||||
had their turn."""
|
||||
sleeps: list[float] = []
|
||||
|
||||
async def fake_sleep(seconds):
|
||||
sleeps.append(seconds)
|
||||
|
||||
monkeypatch.setattr(asyncio, "sleep", fake_sleep)
|
||||
|
||||
listener = ElectrumListener(lambda endpoint: None, session_factory, _ENDPOINTS)
|
||||
attempts = {"n": 0}
|
||||
|
||||
async def failing_run_once(endpoint):
|
||||
attempts["n"] += 1
|
||||
if attempts["n"] > 6:
|
||||
raise asyncio.CancelledError
|
||||
raise ConnectionRefusedError("nope")
|
||||
|
||||
listener._run_once = failing_run_once
|
||||
|
||||
with pytest.raises(asyncio.CancelledError):
|
||||
await listener.run()
|
||||
|
||||
# 6 failures over 3 servers = 2 completed cycles = 2 sleeps, growing.
|
||||
assert sleeps == [1, 2]
|
||||
|
||||
|
||||
async def test_a_connected_session_resets_the_backoff(session_factory, monkeypatch):
|
||||
sleeps: list[float] = []
|
||||
|
||||
async def fake_sleep(seconds):
|
||||
sleeps.append(seconds)
|
||||
|
||||
monkeypatch.setattr(asyncio, "sleep", fake_sleep)
|
||||
|
||||
listener = ElectrumListener(lambda endpoint: None, session_factory, _ENDPOINTS)
|
||||
outcomes = iter([False, False, False, True, False, False, False])
|
||||
|
||||
async def run_once(endpoint):
|
||||
try:
|
||||
connected = next(outcomes)
|
||||
except StopIteration:
|
||||
raise asyncio.CancelledError from None
|
||||
if not connected:
|
||||
raise ConnectionRefusedError("nope")
|
||||
return True # connected, then dropped
|
||||
|
||||
listener._run_once = run_once
|
||||
|
||||
with pytest.raises(asyncio.CancelledError):
|
||||
await listener.run()
|
||||
|
||||
# First cycle of 3 failures sleeps 1s; the successful connection resets the
|
||||
# counter, so the next 3 failures sleep 1s again rather than 2s.
|
||||
assert sleeps == [1, 1]
|
||||
|
||||
|
||||
async def test_listener_with_no_endpoints_gives_up_loudly(session_factory):
|
||||
listener = ElectrumListener(lambda endpoint: None, session_factory, [])
|
||||
await listener.run() # returns instead of spinning or crashing
|
||||
assert listener.current_endpoint is None
|
||||
|
||||
|
||||
def test_tip_never_moves_backwards(session_factory):
|
||||
"""B-19: `self.tip_height = header["height"]` accepted a lower height, and
|
||||
_wait_for_next_block waits for tip_height > tip_at_close — so a regression
|
||||
silently added a block to the draw's wait. The hash must not move either: it's
|
||||
the draw's entropy source, and a mismatched height/hash pair would be worse than
|
||||
a stale one."""
|
||||
listener = ElectrumListener(lambda endpoint: None, session_factory, _ENDPOINTS)
|
||||
|
||||
listener._apply_header({"height": 100, "hex": "aa"})
|
||||
assert (listener.tip_height, listener.tip_header_hex) == (100, "aa")
|
||||
|
||||
listener._apply_header({"height": 99, "hex": "bb"}) # reorg, or a server switch
|
||||
assert (listener.tip_height, listener.tip_header_hex) == (100, "aa")
|
||||
|
||||
listener._apply_header({"height": 101, "hex": "cc"})
|
||||
assert (listener.tip_height, listener.tip_header_hex) == (101, "cc")
|
||||
@@ -42,7 +42,7 @@ async def client(monkeypatch, tmp_path):
|
||||
app = FastAPI()
|
||||
app.include_router(auth_router)
|
||||
app.include_router(rounds_router)
|
||||
app.state.electrum_listener = ElectrumListener(lambda: None, db_base.AsyncSessionLocal)
|
||||
app.state.electrum_listener = ElectrumListener(lambda endpoint: None, db_base.AsyncSessionLocal)
|
||||
|
||||
transport = ASGITransport(app=app)
|
||||
async with AsyncClient(transport=transport, base_url="http://test") as ac:
|
||||
|
||||
Reference in New Issue
Block a user