Conversation
Exercise a PostgreSQL-backed server between two SQLite-backed peers, including direct and routed payments, process-kill recovery, persisted wallet/channel/payment/forwarding state, and cooperative channel closure. Cover force-close recovery across restart while funds are timelocked, then confirm sweeps and reconcile wallet balances with confirmed onchain payment records through the public APIs. Compare each node's pre-close onchain plus Lightning balance with its settled onchain balance after cooperative and force closes, allowing a 5,000-sat difference for fees. Verify final balances and payment history survive another restart. Verify two concurrent PostgreSQL-backed servers sharing a database use separate configured tables and retain independent wallet balances and payment histories across restart. Add restart and PostgreSQL configuration support to the e2e harness. Run the PostgreSQL tests explicitly in CI using postgres:latest, matching LDK Node. AI assistance: OpenAI Codex.
Explain that certificate_path is required to enable PostgreSQL TLS and omitting it leaves sslmode=prefer connections unencrypted. Document the sslmode=require failure without a CA file and the sslmode=disable conflict when one is supplied in the configuration guide and example. Clarify the CA requirement and plaintext default in CLI help. AI assistance: OpenAI Codex.
|
👋 Thanks for assigning @benthecarman as a reviewer! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| services: | ||
| postgres: | ||
| image: postgres:latest |
The test expected exactly one successful payment after sending the Lightning payment. However, the successful-payment count also includes the deposit and channel funding transaction. Depending on timing, the test could pass because an onchain payment succeeded, or time out because the count had already exceeded one. Confirm both onchain payments and wait for their metrics before sending the Lightning payment. Then require three successful payments with none pending or failed, and include current metrics in the timeout diagnostic. AI assistance: OpenAI Codex.
|
Can you add a description |
benthecarman
left a comment
benthecarman
left a comment
There was a problem hiding this comment.
Thanks, didn't want to retest anything in ldk-node but these look reasonable.
some claude review
| } | ||
| // Have SQLite discover the unilateral close onchain, ensuring PostgreSQL's commitment | ||
| // confirms without a competing commitment broadcast in response to a peer error message. | ||
| postgres |
There was a problem hiding this comment.
C can still reconnect to B during the force-close window. The force channel is inbound on C, so ldk-node re-adds B to C's peer store on ChannelPending. B's disconnect_peer only clears B's own store. If C's 60s reconnect tick lands before B's commitment confirms, B answers with an error and C force-closes with broadcast, producing a competing commitment. Low probability, but calling disconnect_peer from C's side as well removes the risk cheaply.
| let server = LdkServerHandle::start_with_config(bitcoind, |params| { | ||
| // Each server gets its own table, even when sharing the same test database. | ||
| let table_name = format!( | ||
| "node_{}", |
There was a problem hiding this comment.
derives the table name by stripping a leading dot from the tempdir basename. That works only because tempfile uses a .tmp prefix and ldk-node splits identifiers on dots. format!("node_{}", params.grpc_port) is already unique and obviously valid.
| } | ||
| } | ||
|
|
||
| async fn close_channel(initiator: &LdkServerHandle, peer: &LdkServerHandle, user_channel_id: &str) { |
There was a problem hiding this comment.
close_channel retries any LightningError for a full 60s in close_channel. That masks a real close failure as a generic timeout. Print the first error, or cap the retry to a few seconds.
Disconnect SQLite C from PostgreSQL B before force-closing, clearing the peer stores on both sides. C stores B when the inbound channel becomes pending, so disconnecting only B still allowed C to reconnect and broadcast a competing commitment before B's commitment confirmed. AI assistance: OpenAI Codex.
Use node_<grpc_port> for each test server's table name, removing the dependency on tempfile basename formatting and leading-dot stripping. AI assistance: OpenAI Codex.
Print the channel ID and full first close error in the PostgreSQL e2e helper so retries do not obscure the original failure in test output. Use a helper-local five-second timeout for the transient monitor-update race so persistent close failures surface promptly. AI assistance: OpenAI Codex.
thanks yea tried to avoid too much duplication against ldk-node, seems a standalone e2e test in ldk-server could be useful to make sure we don't forget anything. |
| server | ||
| } | ||
|
|
||
| async fn channels(server: &LdkServerHandle, count: usize) -> Vec<Channel> { |
There was a problem hiding this comment.
all the helpers in this file might be useful for other tests, can you see if there's any where we can use them and put them in the lib.rs instead of just this file
4 participants
Follow-ups to PR #243