Skip to content

Commit 12c401b

Browse files
chenyukangchenyukang
committed
more on atomic mpp
1 parent 6f404e1 commit 12c401b

File tree

8 files changed

+106
-34
lines changed

8 files changed

+106
-34
lines changed

‎crates/fiber-lib/src/fiber/channel.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -928,7 +928,7 @@ where
928928
error!("invoice already paid, ignore");
929929
return;
930930
}
931-
_ if invoice.allow_mpp() => {
931+
_ if invoice.basic_mpp() => {
932932
// add to pending settlement tlc set
933933
// the tlc set will be settled by network actor
934934
state
@@ -1081,7 +1081,7 @@ where
10811081
tlc.total_amount = Some(record.total_amount);
10821082
}
10831083
(Some(invoice), None) => {
1084-
if invoice.allow_mpp() {
1084+
if invoice.basic_mpp() {
10851085
// FIXME: whether we allow MPP without MPP records in onion packet?
10861086
// currently we allow it pay with enough amount
10871087
// TODO: add a unit test of using single path payment pay MPP invoice successfully

‎crates/fiber-lib/src/fiber/network.rs‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,8 @@ pub struct SendPaymentData {
494494
pub allow_self_payment: bool,
495495
pub hop_hints: Vec<HopHint>,
496496
pub router: Vec<RouterHop>,
497-
pub allow_mpp: bool,
497+
pub allow_basic_mpp: bool,
498+
pub allow_atomic_mpp: bool,
498499
pub dry_run: bool,
499500
#[serde(skip)]
500501
pub channel_stats: GraphChannelStat,
@@ -637,14 +638,22 @@ impl SendPaymentData {
637638

638639
let hop_hints = command.hop_hints.unwrap_or_default();
639640

640-
let allow_mpp = invoice.as_ref().is_some_and(|inv| inv.allow_mpp());
641+
let basic_mpp = invoice.as_ref().is_some_and(|inv| inv.basic_mpp());
642+
let atomic_mpp = invoice.as_ref().is_some_and(|inv| inv.atomic_mpp());
643+
let is_mpp_payment = basic_mpp || atomic_mpp;
644+
645+
if basic_mpp && atomic_mpp {
646+
// invoice rpc already make sure this
647+
return Err("basic_mpp and atomic_mpp cannot be both true".to_string());
648+
}
649+
641650
let payment_secret = invoice
642651
.as_ref()
643652
.and_then(|inv| inv.payment_secret().cloned());
644-
if allow_mpp && payment_secret.is_none() {
653+
if is_mpp_payment && payment_secret.is_none() {
645654
return Err("payment secret is required for multi-path payment".to_string());
646655
}
647-
if allow_mpp
656+
if is_mpp_payment
648657
&& command
649658
.max_parts
650659
.is_some_and(|max_parts| max_parts <= 1 || max_parts > PAYMENT_MAX_PARTS_LIMIT)
@@ -700,7 +709,8 @@ impl SendPaymentData {
700709
custom_records,
701710
allow_self_payment: command.allow_self_payment,
702711
hop_hints,
703-
allow_mpp,
712+
allow_basic_mpp: basic_mpp,
713+
allow_atomic_mpp: atomic_mpp,
704714
router: vec![],
705715
dry_run: command.dry_run,
706716
channel_stats: Default::default(),
@@ -713,7 +723,7 @@ impl SendPaymentData {
713723

714724
pub fn allow_mpp(&self) -> bool {
715725
// only allow mpp if max_parts is greater than 1 and not keysend
716-
self.allow_mpp && self.max_parts() > 1 && !self.keysend
726+
(self.allow_basic_mpp || self.allow_atomic_mpp) && self.max_parts() > 1 && !self.keysend
717727
}
718728
}
719729

‎crates/fiber-lib/src/fiber/tests/graph.rs‎

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,8 @@ fn test_graph_build_router_is_ok_with_fee_rate() {
623623
dry_run: false,
624624
custom_records: None,
625625
router: vec![],
626-
allow_mpp: false,
626+
allow_basic_mpp: false,
627+
allow_atomic_mpp: false,
627628
channel_stats: Default::default(),
628629
};
629630
let route = network
@@ -673,7 +674,8 @@ fn test_graph_build_router_fee_rate_optimize() {
673674
dry_run: false,
674675
custom_records: None,
675676
router: vec![],
676-
allow_mpp: false,
677+
allow_basic_mpp: false,
678+
allow_atomic_mpp: false,
677679
channel_stats: Default::default(),
678680
};
679681

@@ -715,7 +717,8 @@ fn test_graph_build_router_no_fee_with_direct_pay() {
715717
dry_run: false,
716718
custom_records: None,
717719
router: vec![],
718-
allow_mpp: false,
720+
allow_basic_mpp: false,
721+
allow_atomic_mpp: false,
719722
channel_stats: Default::default(),
720723
};
721724
let route = network
@@ -867,7 +870,8 @@ fn test_graph_build_route_three_nodes_amount() {
867870
dry_run: false,
868871
custom_records: None,
869872
router: vec![],
870-
allow_mpp: false,
873+
allow_basic_mpp: false,
874+
allow_atomic_mpp: false,
871875
channel_stats: Default::default(),
872876
};
873877
let route = network
@@ -924,7 +928,8 @@ fn do_test_graph_build_route_expiry(n_nodes: usize) {
924928
dry_run: false,
925929
custom_records: None,
926930
router: vec![],
927-
allow_mpp: false,
931+
allow_basic_mpp: false,
932+
allow_atomic_mpp: false,
928933
channel_stats: Default::default(),
929934
};
930935
let route = network
@@ -1014,7 +1019,8 @@ fn test_graph_build_route_below_min_tlc_value() {
10141019
dry_run: false,
10151020
custom_records: None,
10161021
router: vec![],
1017-
allow_mpp: false,
1022+
allow_basic_mpp: false,
1023+
allow_atomic_mpp: false,
10181024
channel_stats: Default::default(),
10191025
};
10201026
let route = network
@@ -1051,7 +1057,8 @@ fn test_graph_build_route_select_edge_with_latest_timestamp() {
10511057
dry_run: false,
10521058
custom_records: None,
10531059
router: vec![],
1054-
allow_mpp: false,
1060+
allow_basic_mpp: false,
1061+
allow_atomic_mpp: false,
10551062
channel_stats: Default::default(),
10561063
};
10571064
let route = network
@@ -1096,7 +1103,8 @@ fn test_graph_build_route_select_edge_with_large_capacity() {
10961103
dry_run: false,
10971104
custom_records: None,
10981105
router: vec![],
1099-
allow_mpp: false,
1106+
allow_basic_mpp: false,
1107+
allow_atomic_mpp: false,
11001108
channel_stats: Default::default(),
11011109
};
11021110
let route = network
@@ -1160,7 +1168,8 @@ fn test_graph_mark_failed_channel() {
11601168
dry_run: false,
11611169
custom_records: None,
11621170
router: vec![],
1163-
allow_mpp: false,
1171+
allow_basic_mpp: false,
1172+
allow_atomic_mpp: false,
11641173
channel_stats: Default::default(),
11651174
};
11661175
let route = network
@@ -1190,7 +1199,8 @@ fn test_graph_mark_failed_channel() {
11901199
dry_run: false,
11911200
custom_records: None,
11921201
router: vec![],
1193-
allow_mpp: false,
1202+
allow_basic_mpp: false,
1203+
allow_atomic_mpp: false,
11941204
channel_stats: Default::default(),
11951205
};
11961206
let route = network
@@ -1231,7 +1241,8 @@ fn test_graph_session_router() {
12311241
dry_run: false,
12321242
custom_records: None,
12331243
router: vec![],
1234-
allow_mpp: false,
1244+
allow_basic_mpp: false,
1245+
allow_atomic_mpp: false,
12351246
channel_stats: Default::default(),
12361247
};
12371248
let route = network
@@ -1285,7 +1296,8 @@ fn test_graph_mark_failed_node() {
12851296
dry_run: false,
12861297
custom_records: None,
12871298
router: vec![],
1288-
allow_mpp: false,
1299+
allow_basic_mpp: false,
1300+
allow_atomic_mpp: false,
12891301
channel_stats: Default::default(),
12901302
};
12911303
let route = network
@@ -1313,7 +1325,8 @@ fn test_graph_mark_failed_node() {
13131325
dry_run: false,
13141326
custom_records: None,
13151327
router: vec![],
1316-
allow_mpp: false,
1328+
allow_basic_mpp: false,
1329+
allow_atomic_mpp: false,
13171330
channel_stats: Default::default(),
13181331
};
13191332
let route = network
@@ -1343,7 +1356,8 @@ fn test_graph_mark_failed_node() {
13431356
dry_run: false,
13441357
custom_records: None,
13451358
router: vec![],
1346-
allow_mpp: false,
1359+
allow_basic_mpp: false,
1360+
allow_atomic_mpp: false,
13471361
channel_stats: Default::default(),
13481362
};
13491363
let route = network
@@ -1370,7 +1384,8 @@ fn test_graph_mark_failed_node() {
13701384
dry_run: false,
13711385
custom_records: None,
13721386
router: vec![],
1373-
allow_mpp: false,
1387+
allow_basic_mpp: false,
1388+
allow_atomic_mpp: false,
13741389
channel_stats: Default::default(),
13751390
};
13761391
let route = network

‎crates/fiber-lib/src/fiber/tests/payment.rs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6334,3 +6334,36 @@ async fn test_send_payment_3_nodes_failed_last_hop() {
63346334
.wait_until_success(payment.unwrap().payment_hash)
63356335
.await;
63366336
}
6337+
6338+
#[tokio::test]
6339+
async fn test_send_payment_mpp_can_not_be_keysend() {
6340+
init_tracing();
6341+
6342+
let (nodes, _channels) = create_n_nodes_network(
6343+
&[
6344+
((0, 1), (MIN_RESERVED_CKB + 10000000000, MIN_RESERVED_CKB)),
6345+
((1, 2), (MIN_RESERVED_CKB + 10000000000, MIN_RESERVED_CKB)),
6346+
],
6347+
3,
6348+
)
6349+
.await;
6350+
let [node_0, _node_1, mut node_2] = nodes.try_into().expect("3 nodes");
6351+
let node_2_pubkey = node_2.pubkey;
6352+
let res = node_0
6353+
.send_mpp_payment_with_command(
6354+
&mut node_2,
6355+
100,
6356+
SendPaymentCommand {
6357+
target_pubkey: Some(node_2_pubkey),
6358+
keysend: Some(true),
6359+
allow_self_payment: false,
6360+
dry_run: false,
6361+
..Default::default()
6362+
},
6363+
)
6364+
.await;
6365+
eprintln!("res: {:?}", res);
6366+
assert!(res
6367+
.unwrap_err()
6368+
.contains("keysend payment should not have invoice"));
6369+
}

‎crates/fiber-lib/src/invoice/invoice_impl.rs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,11 +348,22 @@ impl CkbInvoice {
348348
attr_getter!(hash_algorithm, HashAlgorithm, HashAlgorithm);
349349
attr_getter!(payment_secret, PaymentSecret, Hash256);
350350

351-
pub fn allow_mpp(&self) -> bool {
351+
fn has_feature<F>(&self, feature_check: F) -> bool
352+
where
353+
F: Fn(&FeatureVector) -> bool,
354+
{
352355
self.data
353356
.attrs
354357
.iter()
355-
.any(|attr| matches!(attr, Attribute::Feature(feature) if feature.supports_basic_mpp()))
358+
.any(|attr| matches!(attr, Attribute::Feature(feature) if feature_check(feature)))
359+
}
360+
361+
pub fn basic_mpp(&self) -> bool {
362+
self.has_feature(|feature| feature.supports_basic_mpp())
363+
}
364+
365+
pub fn atomic_mpp(&self) -> bool {
366+
self.has_feature(|feature| feature.supports_atomic_mpp())
356367
}
357368
}
358369

‎crates/fiber-lib/src/invoice/tests/invoice_impl.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ fn test_invoice_with_mpp_option() {
575575
.build_with_sign(|hash| Secp256k1::new().sign_ecdsa_recoverable(hash, &private_key))
576576
.unwrap();
577577

578-
assert!(!invoice.allow_mpp());
578+
assert!(!invoice.basic_mpp());
579579

580580
let invoice = InvoiceBuilder::new(Currency::Fibb)
581581
.amount(Some(1280))
@@ -585,21 +585,21 @@ fn test_invoice_with_mpp_option() {
585585
.build_with_sign(|hash| Secp256k1::new().sign_ecdsa_recoverable(hash, &private_key))
586586
.unwrap();
587587

588-
assert!(invoice.allow_mpp());
588+
assert!(invoice.basic_mpp());
589589

590590
let serialized_invoice = serde_json::to_string(&invoice).unwrap();
591591
eprintln!("Serialized invoice: {}", serialized_invoice);
592592
let deserialized_invoice: CkbInvoice =
593593
serde_json::from_str(&serialized_invoice).expect("Failed to deserialize invoice");
594594
assert_eq!(deserialized_invoice, invoice);
595-
assert!(deserialized_invoice.allow_mpp());
595+
assert!(deserialized_invoice.basic_mpp());
596596

597597
let human_readable_invoice = invoice.to_string();
598598
let parsed_invoice: CkbInvoice = human_readable_invoice
599599
.parse()
600600
.expect("Failed to parse invoice");
601601
assert_eq!(parsed_invoice, invoice);
602-
assert!(parsed_invoice.allow_mpp());
602+
assert!(parsed_invoice.basic_mpp());
603603
assert!(parsed_invoice.payment_secret().is_some());
604604

605605
let invoice = InvoiceBuilder::new(Currency::Fibb)
@@ -609,7 +609,7 @@ fn test_invoice_with_mpp_option() {
609609
.build_with_sign(|hash| Secp256k1::new().sign_ecdsa_recoverable(hash, &private_key))
610610
.unwrap();
611611

612-
assert!(!invoice.allow_mpp());
612+
assert!(!invoice.basic_mpp());
613613
let payment_secret = invoice.payment_secret();
614614
assert!(payment_secret.is_none());
615615
}

‎crates/fiber-lib/src/store/.schema.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
"RevocationNonce": "55812ed9f87d6495f0d9d06935bc0b9084a4a8a58fc53d627e3e494d1d972285",
5151
"RevokeAndAck": "8b2a0d7a780fe10a8245e97815743c9769774ff3d4834e601dea2fdde2d56702",
5252
"RouterHop": "10cc5df6c304bc3b2242add0b6a6f297015a09fd7f453667bdf65a5e4e872cd3",
53-
"SendPaymentData": "43b38b50aa4c26407a7b92fa89932acb9365e0420048512dec7f34e48c55b4fc",
53+
"SendPaymentData": "5bcc6938037f5ebf721bc1afc3b52fdb721c842036fdef6146bc7b313f702c3a",
5454
"SessionRoute": "17ca3487423c528df34fd906b573af20fa2cd3f35c8ecaa9ca8f3138a7b7541f",
5555
"SessionRouteNode": "3905d18b71ffcaac5abf78441b70b445bf406dd6582aeaa7db354049c05f4395",
5656
"SettlementData": "11de001a0a8aea2eeb6ac571dd35f91c9ce2ceade91313d0724fe04e3fd02abe",

‎crates/fiber-lib/src/store/tests/store.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,8 @@ fn test_store_payment_session() {
686686
dry_run: false,
687687
custom_records: None,
688688
router: vec![],
689-
allow_mpp: false,
689+
allow_basic_mpp: false,
690+
allow_atomic_mpp: false,
690691
channel_stats: Default::default(),
691692
};
692693
let payment_session = PaymentSession::new(&store, payment_data.clone(), 10);
@@ -720,7 +721,8 @@ fn test_store_payment_sessions_with_status() {
720721
dry_run: false,
721722
custom_records: None,
722723
router: vec![],
723-
allow_mpp: false,
724+
allow_basic_mpp: false,
725+
allow_atomic_mpp: false,
724726
channel_stats: Default::default(),
725727
};
726728
let payment_session = PaymentSession::new(&store, payment_data.clone(), 10);
@@ -745,7 +747,8 @@ fn test_store_payment_sessions_with_status() {
745747
dry_run: false,
746748
custom_records: None,
747749
router: vec![],
748-
allow_mpp: false,
750+
allow_basic_mpp: false,
751+
allow_atomic_mpp: false,
749752
channel_stats: Default::default(),
750753
};
751754
let mut payment_session = PaymentSession::new(&store, payment_data.clone(), 10);

0 commit comments

Comments
 (0)