tests: Add interoperability tests for BOLT12 (Offers) with CLN#905
Conversation
|
I've assigned @tnull as a reviewer! |
alexgrad42
force-pushed
the
feature/add_bolt12_scenario_coverage_to_interop_tests
branch
2 times, most recently
from
bf29202 to
bd238f7
Compare
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Thanks for looking into this! Some comments.
|
|
||
| /// Blocks execution until the channel with the specified peer becomes active (`is_usable == true`). | ||
| /// Maximum wait time is 15 seconds (30 attempts with a 500ms interval). | ||
| pub(crate) async fn wait_for_channel_usable(node: &Node, counterparty_node_id: PublicKey) { |
There was a problem hiding this comment.
We shouldn't use this manual sleep, we have events for that.
|
|
||
| /// Specialized version of `force_close_after_payment_scenario` for BOLT11 payments. | ||
| /// See [`force_close_after_payment_scenario`] for details. | ||
| pub(crate) async fn force_close_after_payment_bolt11_scenario<E: ElectrumApi>( |
There was a problem hiding this comment.
I don't understand why we need all this additional boilerplate.
There was a problem hiding this comment.
Removed. Note that BOLT11/BOLT12 scenarios are almost identical now. We can extract 11/12 step and run as prepayment->payment(this one id different)->postpayment chain on integration_tests_cln if you want.
| payment::send_bolt11_to_peer(node, peer, 5_000_000, "post-splice-bolt11").await; | ||
| }, | ||
| PaymentProtocol::Bolt12 => { | ||
| // Wait for Onion Message router updates the channel to usable state. |
There was a problem hiding this comment.
I don't think I understand what "Wait for Onion Message router updates the channel to usable state." means. Can you expand?
There was a problem hiding this comment.
Sorry, I think I've confused both of you and myself. My test were unstable, so I decided this is timing issue due to asynchronous nature of the peer-to-payer invoice delivery via messaging. In fact this seems to be my local WSL setup issue, and if it breaks splicing test, it does it for bolt11 too. Removed, please ignore
|
|
||
| /// Specialized version of `splice_in_scenario` for BOLT11 payments. | ||
| /// See [`splice_in_scenario`] for details. | ||
| pub(crate) async fn splice_in_bolt11_scenario<E: ElectrumApi>( |
There was a problem hiding this comment.
Please avoid adding helper methods that are less than 5 lines of code.
alexgrad42
force-pushed
the
feature/add_bolt12_scenario_coverage_to_interop_tests
branch
3 times, most recently
from
a41fc0a to
bb198e7
Compare
Activates scenarios for BOLT12 offer payments between ldk-node and Core Lightning (CLN) inside integration tests Fix lightningdevkit#856
alexgrad42
force-pushed
the
feature/add_bolt12_scenario_coverage_to_interop_tests
branch
from
bb198e7 to
a6eddd4
Compare
3 participants
Activates scenarios for BOLT12 offer payments between ldk-node and Core Lightning (CLN) inside integration tests
Fix #856