Conversation
…ario layers Adds a workspace crate housing an integration test framework built from independent layers: an environment layer that provisions a network and emits its partial sim.json config, a scenario layer that describes payment activity and config style, a runner that assembles config files and drives them through the same public entry points the sim-cli binary uses, and shared assertions over the observable output. Simulated networks run on virtual time so time-bounded scenarios complete instantly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Covers defined activity across the cross-product of node reference styles (alias/pubkey) and value shapes (scalar/range), random activity with and without exclusions, seeded determinism, count- and time-bounded runs, connector implementation inference from untagged config, and negative validation cases. Note that count-bounded assertions tolerate the loss of the final payment record: meeting a payment count shuts the simulation down in the same instant as the last dispatch, and the results consumer prefers the shutdown signal over draining pending results. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds a container-based environment provider that brings up a heterogeneous regtest network: bitcoind plus one node each of LND, CLN, Eclair and ldk-server (built from the upstream repository at the same rev the client dependency pins), connected in a ring of announced channels. All startup steps poll with capped exponential backoff, and container logs are dumped when a node fails to come up. Because some implementations broadcast funding transactions asynchronously after the open call returns, readiness polling keeps mining blocks until every channel is active rather than confirming once. The real-node test runs scenarios sequentially against one shared network: defined keysends across every directed ring edge with receipts verified against each destination's own books, a multi-hop route, alias references resolved from real node configuration, and random activity. It is marked ignored so it only runs when requested explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The simulated tier already runs under the existing build job's cargo test invocation; the new CI job covers the real-node tier, building the ldk-server image from the rev pinned in Cargo.lock with a docker layer cache so repeat runs reuse it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
only1dreamgene
left a comment
only1dreamgene
left a comment
There was a problem hiding this comment.
A few notes from going through the new integration-tests crate: two unchecked u64 subtractions that can panic on edge-case inputs, a non-idempotent retry pattern around channel-open RPCs that could double-open channels on a lost response, and some smaller flakiness/efficiency items inline.
| peer_host: &str, | ||
| peer_port: u16, | ||
| ) -> anyhow::Result<()> { | ||
| with_backoff("cln open channel", Backoff::slow(), || async { |
There was a problem hiding this comment.
open_channel wraps the whole connect_peer + fund_channel sequence in with_backoff, but channel-open RPCs aren't idempotent. If the response is lost or times out after the node already accepted the request (funding tx broadcast), a retry issues a second channel-open to the same peer. Same pattern in lnd.rs's open_channel_sync, eclair.rs's open, and ldk_server.rs's open_channel. Might be worth splitting the connect step (safe to retry) from the actual open call, or checking for an existing/pending channel before retrying.
| .copied() | ||
| .unwrap_or(0); | ||
| assert!( | ||
| (count - 1..=count).contains(&recorded), |
There was a problem hiding this comment.
count - 1 underflows if count == 0 (u64). Not hit by current call sites, but a future caller asserting zero payments between excluded pairs would panic here instead of getting a clear assertion failure. count.saturating_sub(1) would be safer.
| // Each ring destination's own node reports the keysends it received. The final payment of | ||
| // the run may still be settling (or its record lost to shutdown), hence the range. | ||
| for (_, dest) in RING { | ||
| let received = network.settled_keysend_count(dest).await? - receipts_before[dest]; |
There was a problem hiding this comment.
settled_keysend_count(dest).await? - receipts_before[dest] can underflow-panic if the post-scenario count is ever lower than the pre-scenario snapshot (e.g. a paginated list_payments race, or a restart mid-test). A saturating_sub plus a descriptive message would fail more usefully than a raw overflow panic.
| match op().await { | ||
| Ok(t) => return Ok(t), | ||
| Err(e) => { | ||
| if start.elapsed() + delay > backoff.timeout { |
There was a problem hiding this comment.
This check aborts before making the next attempt whenever elapsed + delay > timeout, even when the retry itself would still land within budget (e.g. elapsed=59s of a 60s timeout with delay=5s — aborts here even though the next poll would happen well before 60s). Could produce flaky 'not ready' failures for nodes that would've answered on the very next poll. Might be worth checking elapsed >= timeout after attempting, or clamping the final delay to the remaining budget instead of bailing early.
|
|
||
| /// The rev the workspace's ldk-server-client dependency pins; the server is built from the same | ||
| /// rev so client and server always match. | ||
| const LDK_SERVER_REV: &str = "8163f4fe139368613959bf4f10b19ee6a5b9b4ab"; |
There was a problem hiding this comment.
This is a second hardcoded copy of the ldk-server rev, separate from the git rev pinned for ldk-server-client in integration-tests/Cargo.toml. The CI workflow already derives its rev by grepping Cargo.lock rather than hardcoding it again — might be worth doing the same here so a future dependency bump can't silently build a server image at a different commit than the client expects.
|
|
||
| // The four nodes are independent of each other until channels open: start them in | ||
| // parallel since (particularly with cold image pulls) startup dominates runtime. | ||
| let (lnd, cln, eclair, ldk) = tokio::try_join!( |
There was a problem hiding this comment.
If one of these four futures errors while the others are still mid-initialization (containers already up, credentials being extracted), try_join! drops the still-running futures immediately. Cleanup then relies entirely on Drop/the testcontainers reaper rather than each harness's own dump_logs/error-context path, so a fast-failing node can leave the still-starting ones without a diagnostic log dump. A join_all with explicit error aggregation would let every harness reach its own failure/cleanup path.
| let count = 3u64; | ||
|
|
||
| let mut receipts_before = [0u64; 4]; | ||
| for (i, receipts) in receipts_before.iter_mut().enumerate() { |
There was a problem hiding this comment.
These four settled_keysend_count calls are awaited sequentially. They're independent RPCs to four different nodes (and Eclair's JVM-under-emulation responses are already the slow one) — a tokio::try_join! here would cut this baseline-read step down to the slowest single call instead of the sum of all four.
|
Thanks for the review @only1dreamgene ! This is an entirely-clauded idea that I haven't even read the code for, just thought it would be a good idea to throw up and deal with ✨ sometime ✨ but I never got around to it. Would you be interested in taking over this PR and addressing your own review? It's definitely something I'd like to get over the finish line. |
2 participants
Adds an integration test framework covering sim-ln's node backends, payment modalities and configuration surface, structured as independent layers:
integration-tests/src/env/): provisions a network and emits its part of the sim.json config. Two providers: a deterministic in-process simulated graph, and a dockerized heterogeneous regtest network (bitcoind + one node each of LND, CLN, Eclair and ldk-server in a ring of announced channels, via testcontainers). Knows nothing about payments.src/scenario.rs): describes activity (defined/random) and config style (alias vs pubkey references, scalar vs[min, max]values). Knows nothing about provisioning.src/runner.rs,src/asserts.rs): assembles real config files, drives them through the same public entry points the sim-cli binary uses, and asserts over parsing results, dispatched payments and the results CSV.Test tiers
Simulated (
tests/sim_matrix.rs, runs in the existing test CI job, ~0.2s on virtual time): the full cross-product of node reference styles and value shapes for defined activity, random activity with/without exclusions, seeded determinism, count- and time-bounded runs, untagged connector inference, and negative validation cases asserting specific errors.Real nodes (
tests/real_nodes.rs,#[ignore], new CI job /make integration-real, ~8 min): one shared network, scenarios run sequentially — defined keysends across every directed ring edge (each connector proves it sends and receives cross-implementation, with receipts checked against each destination node's own books), a multi-hop route, alias-referenced activities, and random activity.Error resistance
Every real-node startup step polls with capped exponential backoff and names the node/step in its failure. Container logs are dumped on startup or scenario failure. Readiness keeps mining while waiting for channels because some implementations (ldk-node) broadcast funding transactions asynchronously after the open call returns.
Notes
ldk-server-clientdependency already pins;SIMLN_LDK_SERVER_IMAGEoverrides.-Declair.allow-unsafe-startup=true(a static dev-build guard, nothing dynamic).testcontainersis pinned to=0.25.0, the last release whose MSRV fits the repo's Rust 1.85 toolchain;serde_with/timeare pinned in Cargo.lock for the same reason.select!prefers the shutdown listener over draining pending results. Possibly worth an upstream fix (drain the channel before exiting) — happy to file separately.Both tiers run green locally on macOS (Apple Silicon, Docker Desktop; eclair runs under Rosetta since its image is amd64-only).
🤖 Generated with Claude Code