Skip to content

Commit 4950f3c

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #3214 from TheBlueMatt/2024-07-chan-by-val
Make `funding_transaction_generated` take a `ChannelId` by value
2 parents 795887a + 1ff2495 commit 4950f3c

File tree

9 files changed

+55
-55
lines changed

9 files changed

+55
-55
lines changed

‎fuzz/src/chanmon_consistency.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -861,29 +861,29 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
861861
$source.handle_accept_channel(&$dest.get_our_node_id(), &accept_channel);
862862
let funding_output;
863863
{
864-
let events = $source.get_and_clear_pending_events();
864+
let mut events = $source.get_and_clear_pending_events();
865865
assert_eq!(events.len(), 1);
866866
if let events::Event::FundingGenerationReady {
867-
ref temporary_channel_id,
868-
ref channel_value_satoshis,
869-
ref output_script,
867+
temporary_channel_id,
868+
channel_value_satoshis,
869+
output_script,
870870
..
871-
} = events[0]
871+
} = events.pop().unwrap()
872872
{
873873
let tx = Transaction {
874874
version: Version($chan_id),
875875
lock_time: LockTime::ZERO,
876876
input: Vec::new(),
877877
output: vec![TxOut {
878-
value: Amount::from_sat(*channel_value_satoshis),
879-
script_pubkey: output_script.clone(),
878+
value: Amount::from_sat(channel_value_satoshis),
879+
script_pubkey: output_script,
880880
}],
881881
};
882882
funding_output = OutPoint { txid: tx.txid(), index: 0 };
883883
$source
884884
.funding_transaction_generated(
885-
&temporary_channel_id,
886-
&$dest.get_our_node_id(),
885+
temporary_channel_id,
886+
$dest.get_our_node_id(),
887887
tx.clone(),
888888
)
889889
.unwrap();

‎lightning-background-processor/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1676,8 +1676,8 @@ mod tests {
16761676
$node_a
16771677
.node
16781678
.funding_transaction_generated(
1679-
&temporary_channel_id,
1680-
&$node_b.node.get_our_node_id(),
1679+
temporary_channel_id,
1680+
$node_b.node.get_our_node_id(),
16811681
tx.clone(),
16821682
)
16831683
.unwrap();
@@ -2110,7 +2110,7 @@ mod tests {
21102110
.expect("FundingGenerationReady not handled within deadline");
21112111
nodes[0]
21122112
.node
2113-
.funding_transaction_generated(&temporary_channel_id, &node_1_id, funding_tx.clone())
2113+
.funding_transaction_generated(temporary_channel_id, node_1_id, funding_tx.clone())
21142114
.unwrap();
21152115
let msg_0 = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, node_1_id);
21162116
nodes[1].node.handle_funding_created(&node_0_id, &msg_0);

‎lightning/src/ln/async_signer_tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ fn test_async_commitment_signature_for_funding_created() {
4949
// message...
5050
let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42);
5151
nodes[0].disable_channel_signer_op(&nodes[1].node.get_our_node_id(), &temporary_channel_id, SignerOp::SignCounterpartyCommitment);
52-
nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
52+
nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
5353
check_added_monitors(&nodes[0], 0);
5454

5555
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
@@ -96,7 +96,7 @@ fn test_async_commitment_signature_for_funding_signed() {
9696

9797
// nodes[0] --- funding_created --> nodes[1]
9898
let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42);
99-
nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
99+
nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
100100
check_added_monitors(&nodes[0], 0);
101101

102102
let mut funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
@@ -236,7 +236,7 @@ fn test_async_commitment_signature_for_funding_signed_0conf() {
236236

237237
// nodes[0] --- funding_created --> nodes[1]
238238
let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42);
239-
nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
239+
nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
240240
check_added_monitors(&nodes[0], 0);
241241

242242
let mut funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());

‎lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1861,7 +1861,7 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf:
18611861

18621862
let (temporary_channel_id, funding_tx, funding_output) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 43);
18631863

1864-
nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap();
1864+
nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap();
18651865
check_added_monitors!(nodes[0], 0);
18661866

18671867
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
@@ -2795,7 +2795,7 @@ fn do_test_outbound_reload_without_init_mon(use_0conf: bool) {
27952795

27962796
let (temporary_channel_id, funding_tx, ..) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 43);
27972797

2798-
nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap();
2798+
nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap();
27992799
check_added_monitors!(nodes[0], 0);
28002800

28012801
let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
@@ -2886,7 +2886,7 @@ fn do_test_inbound_reload_without_init_mon(use_0conf: bool, lock_commitment: boo
28862886

28872887
let (temporary_channel_id, funding_tx, ..) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 43);
28882888

2889-
nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap();
2889+
nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), funding_tx.clone()).unwrap();
28902890
check_added_monitors!(nodes[0], 0);
28912891

28922892
let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());

‎lightning/src/ln/channelmanager.rs

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1452,7 +1452,7 @@ where
14521452
/// channel_value_satoshis, output_script
14531453
/// );
14541454
/// match channel_manager.funding_transaction_generated(
1455-
/// &temporary_channel_id, &counterparty_node_id, funding_transaction
1455+
/// temporary_channel_id, counterparty_node_id, funding_transaction
14561456
/// ) {
14571457
/// Ok(()) => println!("Funding channel {}", temporary_channel_id),
14581458
/// Err(e) => println!("Error funding channel {}: {:?}", temporary_channel_id, e),
@@ -4446,17 +4446,17 @@ where
44464446
/// Handles the generation of a funding transaction, optionally (for tests) with a function
44474447
/// which checks the correctness of the funding transaction given the associated channel.
44484448
fn funding_transaction_generated_intern<FundingOutput: FnMut(&OutboundV1Channel<SP>) -> Result<OutPoint, &'static str>>(
4449-
&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, funding_transaction: Transaction, is_batch_funding: bool,
4449+
&self, temporary_channel_id: ChannelId, counterparty_node_id: PublicKey, funding_transaction: Transaction, is_batch_funding: bool,
44504450
mut find_funding_output: FundingOutput, is_manual_broadcast: bool,
44514451
) -> Result<(), APIError> {
44524452
let per_peer_state = self.per_peer_state.read().unwrap();
4453-
let peer_state_mutex = per_peer_state.get(counterparty_node_id)
4453+
let peer_state_mutex = per_peer_state.get(&counterparty_node_id)
44544454
.ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) })?;
44554455

44564456
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
44574457
let peer_state = &mut *peer_state_lock;
44584458
let funding_txo;
4459-
let (mut chan, msg_opt) = match peer_state.channel_by_id.remove(temporary_channel_id) {
4459+
let (mut chan, msg_opt) = match peer_state.channel_by_id.remove(&temporary_channel_id) {
44604460
Some(ChannelPhase::UnfundedOutboundV1(mut chan)) => {
44614461
macro_rules! close_chan { ($err: expr, $api_err: expr, $chan: expr) => { {
44624462
let counterparty;
@@ -4492,7 +4492,7 @@ where
44924492
}
44934493
},
44944494
Some(phase) => {
4495-
peer_state.channel_by_id.insert(*temporary_channel_id, phase);
4495+
peer_state.channel_by_id.insert(temporary_channel_id, phase);
44964496
return Err(APIError::APIMisuseError {
44974497
err: format!(
44984498
"Channel with id {} for the passed counterparty node_id {} is not an unfunded, outbound V1 channel",
@@ -4542,7 +4542,7 @@ where
45424542
}
45434543

45444544
#[cfg(test)]
4545-
pub(crate) fn funding_transaction_generated_unchecked(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, funding_transaction: Transaction, output_index: u16) -> Result<(), APIError> {
4545+
pub(crate) fn funding_transaction_generated_unchecked(&self, temporary_channel_id: ChannelId, counterparty_node_id: PublicKey, funding_transaction: Transaction, output_index: u16) -> Result<(), APIError> {
45464546
let txid = funding_transaction.txid();
45474547
self.funding_transaction_generated_intern(temporary_channel_id, counterparty_node_id, funding_transaction, false, |_| {
45484548
Ok(OutPoint { txid, index: output_index })
@@ -4579,8 +4579,8 @@ where
45794579
///
45804580
/// [`Event::FundingGenerationReady`]: crate::events::Event::FundingGenerationReady
45814581
/// [`Event::ChannelClosed`]: crate::events::Event::ChannelClosed
4582-
pub fn funding_transaction_generated(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, funding_transaction: Transaction) -> Result<(), APIError> {
4583-
self.batch_funding_transaction_generated(&[(temporary_channel_id, counterparty_node_id)], funding_transaction)
4582+
pub fn funding_transaction_generated(&self, temporary_channel_id: ChannelId, counterparty_node_id: PublicKey, funding_transaction: Transaction) -> Result<(), APIError> {
4583+
self.batch_funding_transaction_generated(&[(&temporary_channel_id, &counterparty_node_id)], funding_transaction)
45844584
}
45854585

45864586

@@ -4611,10 +4611,10 @@ where
46114611
/// [`Event::FundingTxBroadcastSafe`]: crate::events::Event::FundingTxBroadcastSafe
46124612
/// [`Event::ChannelClosed`]: crate::events::Event::ChannelClosed
46134613
/// [`ChannelManager::funding_transaction_generated`]: crate::ln::channelmanager::ChannelManager::funding_transaction_generated
4614-
pub fn unsafe_manual_funding_transaction_generated(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, funding: OutPoint) -> Result<(), APIError> {
4614+
pub fn unsafe_manual_funding_transaction_generated(&self, temporary_channel_id: ChannelId, counterparty_node_id: PublicKey, funding: OutPoint) -> Result<(), APIError> {
46154615
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
46164616

4617-
let temporary_channels = &[(temporary_channel_id, counterparty_node_id)];
4617+
let temporary_channels = &[(&temporary_channel_id, &counterparty_node_id)];
46184618
return self.batch_funding_transaction_generated_intern(temporary_channels, FundingType::Unchecked(funding));
46194619

46204620
}
@@ -4688,8 +4688,8 @@ where
46884688
let is_manual_broadcast = funding.is_manual_broadcast();
46894689
for &(temporary_channel_id, counterparty_node_id) in temporary_channels {
46904690
result = result.and_then(|_| self.funding_transaction_generated_intern(
4691-
temporary_channel_id,
4692-
counterparty_node_id,
4691+
*temporary_channel_id,
4692+
*counterparty_node_id,
46934693
funding.transaction_or_dummy(),
46944694
is_batch_funding,
46954695
|chan| {
@@ -13348,7 +13348,7 @@ mod tests {
1334813348
assert_eq!(nodes[1].node.outpoint_to_peer.lock().unwrap().len(), 0);
1334913349
}
1335013350

13351-
nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
13351+
nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
1335213352
{
1335313353
// Assert that `nodes[0]`'s `outpoint_to_peer` map is populated with the channel as soon as
1335413354
// as it has the funding transaction.
@@ -13560,7 +13560,7 @@ mod tests {
1356013560
nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel);
1356113561
let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100_000, 42);
1356213562
funding_tx = Some(tx.clone());
13563-
nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx).unwrap();
13563+
nodes[0].node.funding_transaction_generated(temporary_channel_id, nodes[1].node.get_our_node_id(), tx).unwrap();
1356413564
let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
1356513565

1356613566
nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg);
@@ -14215,7 +14215,7 @@ pub mod bench {
1421514215
tx = Transaction { version: Version::TWO, lock_time: LockTime::ZERO, input: Vec::new(), output: vec![TxOut {
1421614216
value: Amount::from_sat(8_000_000), script_pubkey: output_script,
1421714217
}]};
14218-
node_a.funding_transaction_generated(&temporary_channel_id, &node_b.get_our_node_id(), tx.clone()).unwrap();
14218+
node_a.funding_transaction_generated(temporary_channel_id, node_b.get_our_node_id(), tx.clone()).unwrap();
1421914219
} else { panic!(); }
1422014220

1422114221
node_b.handle_funding_created(&node_a.get_our_node_id(), &get_event_msg!(node_a_holder, MessageSendEvent::SendFundingCreated, node_b.get_our_node_id()));

‎lightning/src/ln/functional_test_utils.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,7 +1218,7 @@ pub fn sign_funding_transaction<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &
12181218
let (temporary_channel_id, tx, funding_output) = create_funding_transaction(node_a, &node_b.node.get_our_node_id(), channel_value, 42);
12191219
assert_eq!(temporary_channel_id, expected_temporary_channel_id);
12201220

1221-
assert!(node_a.node.funding_transaction_generated(&temporary_channel_id, &node_b.node.get_our_node_id(), tx.clone()).is_ok());
1221+
assert!(node_a.node.funding_transaction_generated(temporary_channel_id, node_b.node.get_our_node_id(), tx.clone()).is_ok());
12221222
check_added_monitors!(node_a, 0);
12231223

12241224
let funding_created_msg = get_event_msg!(node_a, MessageSendEvent::SendFundingCreated, node_b.node.get_our_node_id());
@@ -1249,7 +1249,7 @@ pub fn sign_funding_transaction<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &
12491249
node_a.tx_broadcaster.txn_broadcasted.lock().unwrap().clear();
12501250

12511251
// Ensure that funding_transaction_generated is idempotent.
1252-
assert!(node_a.node.funding_transaction_generated(&temporary_channel_id, &node_b.node.get_our_node_id(), tx.clone()).is_err());
1252+
assert!(node_a.node.funding_transaction_generated(temporary_channel_id, node_b.node.get_our_node_id(), tx.clone()).is_err());
12531253
assert!(node_a.node.get_and_clear_pending_msg_events().is_empty());
12541254
check_added_monitors!(node_a, 0);
12551255

@@ -1279,7 +1279,7 @@ pub fn open_zero_conf_channel<'a, 'b, 'c, 'd>(initiator: &'a Node<'b, 'c, 'd>, r
12791279
initiator.node.handle_accept_channel(&receiver.node.get_our_node_id(), &accept_channel);
12801280

12811281
let (temporary_channel_id, tx, _) = create_funding_transaction(&initiator, &receiver.node.get_our_node_id(), 100_000, 42);
1282-
initiator.node.funding_transaction_generated(&temporary_channel_id, &receiver.node.get_our_node_id(), tx.clone()).unwrap();
1282+
initiator.node.funding_transaction_generated(temporary_channel_id, receiver.node.get_our_node_id(), tx.clone()).unwrap();
12831283
let funding_created = get_event_msg!(initiator, MessageSendEvent::SendFundingCreated, receiver.node.get_our_node_id());
12841284

12851285
receiver.node.handle_funding_created(&initiator.node.get_our_node_id(), &funding_created);
@@ -1456,7 +1456,7 @@ pub fn create_unannounced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &
14561456
nodes[a].node.handle_accept_channel(&nodes[b].node.get_our_node_id(), &accept_channel);
14571457

14581458
let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[a], &nodes[b].node.get_our_node_id(), channel_value, 42);
1459-
nodes[a].node.funding_transaction_generated(&temporary_channel_id, &nodes[b].node.get_our_node_id(), tx.clone()).unwrap();
1459+
nodes[a].node.funding_transaction_generated(temporary_channel_id, nodes[b].node.get_our_node_id(), tx.clone()).unwrap();
14601460
nodes[b].node.handle_funding_created(&nodes[a].node.get_our_node_id(), &get_event_msg!(nodes[a], MessageSendEvent::SendFundingCreated, nodes[b].node.get_our_node_id()));
14611461
check_added_monitors!(nodes[b], 1);
14621462

0 commit comments

Comments
 (0)