Skip to content

Commit 601669b

Browse files
TheBlueMattTheBlueMatt
committed
Introduce traits to make test utils generic across the CM Holder
In our test utilities, we generally refer to a `Node` struct which holds a `ChannelManager` and a number of other structs. However, we use the same utilities in benchmarking, where we have a different `Node`-like struct. This made moving from macros to functions entirely impossible, as we end up needing multiple types in a given context. Thus, here, we take the pain and introduce some wrapper traits which encapsulte what we need from `Node`, swapping some of our macros to functions.
1 parent 217c3e0 commit 601669b

File tree

2 files changed

+148
-55
lines changed

2 files changed

+148
-55
lines changed

‎lightning/src/ln/channelmanager.rs

Lines changed: 84 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,61 @@ pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<
595595
/// (C-not exported) as Arcs don't make sense in bindings
596596
pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> = ChannelManager<&'a M, &'b T, &'c KeysManager, &'c KeysManager, &'c KeysManager, &'d F, &'e DefaultRouter<&'f NetworkGraph<&'g L>, &'g L, &'h Mutex<ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L>>>, &'g L>;
597597

598+
/// A trivial trait which describes any [`ChannelManager`] used in testing.
599+
#[cfg(any(test, feature = "_test_utils"))]
600+
pub trait AChannelManager {
601+
type Watch: chain::Watch<Self::Signer>;
602+
type M: Deref<Target = Self::Watch>;
603+
type Broadcaster: BroadcasterInterface;
604+
type T: Deref<Target = Self::Broadcaster>;
605+
type EntropySource: EntropySource;
606+
type ES: Deref<Target = Self::EntropySource>;
607+
type NodeSigner: NodeSigner;
608+
type NS: Deref<Target = Self::NodeSigner>;
609+
type Signer: WriteableEcdsaChannelSigner;
610+
type SignerProvider: SignerProvider<Signer = Self::Signer>;
611+
type SP: Deref<Target = Self::SignerProvider>;
612+
type FeeEstimator: FeeEstimator;
613+
type F: Deref<Target = Self::FeeEstimator>;
614+
type Router: Router;
615+
type R: Deref<Target = Self::Router>;
616+
type Logger: Logger;
617+
type L: Deref<Target = Self::Logger>;
618+
fn get_cm(&self) -> &ChannelManager<Self::M, Self::T, Self::ES, Self::NS, Self::SP, Self::F, Self::R, Self::L>;
619+
}
620+
#[cfg(any(test, feature = "_test_utils"))]
621+
impl<M: Deref, T: Deref, ES: Deref, NS: Deref, SP: Deref, F: Deref, R: Deref, L: Deref> AChannelManager
622+
for ChannelManager<M, T, ES, NS, SP, F, R, L>
623+
where
624+
M::Target: chain::Watch<<SP::Target as SignerProvider>::Signer> + Sized,
625+
T::Target: BroadcasterInterface + Sized,
626+
ES::Target: EntropySource + Sized,
627+
NS::Target: NodeSigner + Sized,
628+
SP::Target: SignerProvider + Sized,
629+
F::Target: FeeEstimator + Sized,
630+
R::Target: Router + Sized,
631+
L::Target: Logger + Sized,
632+
{
633+
type Watch = M::Target;
634+
type M = M;
635+
type Broadcaster = T::Target;
636+
type T = T;
637+
type EntropySource = ES::Target;
638+
type ES = ES;
639+
type NodeSigner = NS::Target;
640+
type NS = NS;
641+
type Signer = <SP::Target as SignerProvider>::Signer;
642+
type SignerProvider = SP::Target;
643+
type SP = SP;
644+
type FeeEstimator = F::Target;
645+
type F = F;
646+
type Router = R::Target;
647+
type R = R;
648+
type Logger = L::Target;
649+
type L = L;
650+
fn get_cm(&self) -> &ChannelManager<M, T, ES, NS, SP, F, R, L> { self }
651+
}
652+
598653
/// Manager which keeps track of a number of channels and sends messages to the appropriate
599654
/// channel, also tracking HTLC preimages and forwarding onion packets appropriately.
600655
///
@@ -8754,7 +8809,7 @@ pub mod bench {
87548809
use crate::chain::Listen;
87558810
use crate::chain::chainmonitor::{ChainMonitor, Persist};
87568811
use crate::chain::keysinterface::{EntropySource, KeysManager, InMemorySigner};
8757-
use crate::ln::channelmanager::{self, BestBlock, ChainParameters, ChannelManager, PaymentHash, PaymentPreimage, PaymentId};
8812+
use crate::ln::channelmanager::{BestBlock, ChainParameters, ChannelManager, PaymentHash, PaymentPreimage, PaymentId};
87588813
use crate::ln::functional_test_utils::*;
87598814
use crate::ln::msgs::{ChannelMessageHandler, Init};
87608815
use crate::routing::gossip::NetworkGraph;
@@ -8771,14 +8826,24 @@ pub mod bench {
87718826

87728827
use test::Bencher;
87738828

8774-
struct NodeHolder<'a, P: Persist<InMemorySigner>> {
8775-
node: &'a ChannelManager<
8776-
&'a ChainMonitor<InMemorySigner, &'a test_utils::TestChainSource,
8777-
&'a test_utils::TestBroadcaster, &'a test_utils::TestFeeEstimator,
8778-
&'a test_utils::TestLogger, &'a P>,
8779-
&'a test_utils::TestBroadcaster, &'a KeysManager, &'a KeysManager, &'a KeysManager,
8780-
&'a test_utils::TestFeeEstimator, &'a test_utils::TestRouter<'a>,
8781-
&'a test_utils::TestLogger>,
8829+
#[allow(type_alias_bounds)] // rustc warns us about unapplied bounds, but errors without...
8830+
type Manager<'a, P: Persist<InMemorySigner>> = ChannelManager<
8831+
&'a ChainMonitor<InMemorySigner, &'a test_utils::TestChainSource,
8832+
&'a test_utils::TestBroadcaster, &'a test_utils::TestFeeEstimator,
8833+
&'a test_utils::TestLogger, &'a P>,
8834+
&'a test_utils::TestBroadcaster, &'a KeysManager, &'a KeysManager, &'a KeysManager,
8835+
&'a test_utils::TestFeeEstimator, &'a test_utils::TestRouter<'a>,
8836+
&'a test_utils::TestLogger>;
8837+
8838+
struct ANodeHolder<'a, P: Persist<InMemorySigner>> {
8839+
node: &'a Manager<'a, P>,
8840+
}
8841+
impl<'a, P: Persist<InMemorySigner>> NodeHolder for ANodeHolder<'a, P> {
8842+
type CM = Manager<'a, P>;
8843+
#[inline]
8844+
fn node(&self) -> &Manager<'a, P> { self.node }
8845+
#[inline]
8846+
fn chain_monitor(&self) -> Option<&test_utils::TestChainMonitor> { None }
87828847
}
87838848

87848849
#[cfg(test)]
@@ -8809,7 +8874,7 @@ pub mod bench {
88098874
network,
88108875
best_block: BestBlock::from_network(network),
88118876
});
8812-
let node_a_holder = NodeHolder { node: &node_a };
8877+
let node_a_holder = ANodeHolder { node: &node_a };
88138878

88148879
let logger_b = test_utils::TestLogger::with_id("node a".to_owned());
88158880
let chain_monitor_b = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_b);
@@ -8819,7 +8884,7 @@ pub mod bench {
88198884
network,
88208885
best_block: BestBlock::from_network(network),
88218886
});
8822-
let node_b_holder = NodeHolder { node: &node_b };
8887+
let node_b_holder = ANodeHolder { node: &node_b };
88238888

88248889
node_a.peer_connected(&node_b.get_our_node_id(), &Init { features: node_b.init_features(), remote_network_address: None }, true).unwrap();
88258890
node_b.peer_connected(&node_a.get_our_node_id(), &Init { features: node_a.init_features(), remote_network_address: None }, false).unwrap();
@@ -8905,15 +8970,15 @@ pub mod bench {
89058970
let payment_event = SendEvent::from_event($node_a.get_and_clear_pending_msg_events().pop().unwrap());
89068971
$node_b.handle_update_add_htlc(&$node_a.get_our_node_id(), &payment_event.msgs[0]);
89078972
$node_b.handle_commitment_signed(&$node_a.get_our_node_id(), &payment_event.commitment_msg);
8908-
let (raa, cs) = do_get_revoke_commit_msgs!(NodeHolder { node: &$node_b }, &$node_a.get_our_node_id());
8973+
let (raa, cs) = get_revoke_commit_msgs(&ANodeHolder { node: &$node_b }, &$node_a.get_our_node_id());
89098974
$node_a.handle_revoke_and_ack(&$node_b.get_our_node_id(), &raa);
89108975
$node_a.handle_commitment_signed(&$node_b.get_our_node_id(), &cs);
8911-
$node_b.handle_revoke_and_ack(&$node_a.get_our_node_id(), &get_event_msg!(NodeHolder { node: &$node_a }, MessageSendEvent::SendRevokeAndACK, $node_b.get_our_node_id()));
8976+
$node_b.handle_revoke_and_ack(&$node_a.get_our_node_id(), &get_event_msg!(ANodeHolder { node: &$node_a }, MessageSendEvent::SendRevokeAndACK, $node_b.get_our_node_id()));
89128977

8913-
expect_pending_htlcs_forwardable!(NodeHolder { node: &$node_b });
8914-
expect_payment_claimable!(NodeHolder { node: &$node_b }, payment_hash, payment_secret, 10_000);
8978+
expect_pending_htlcs_forwardable!(ANodeHolder { node: &$node_b });
8979+
expect_payment_claimable!(ANodeHolder { node: &$node_b }, payment_hash, payment_secret, 10_000);
89158980
$node_b.claim_funds(payment_preimage);
8916-
expect_payment_claimed!(NodeHolder { node: &$node_b }, payment_hash, 10_000);
8981+
expect_payment_claimed!(ANodeHolder { node: &$node_b }, payment_hash, 10_000);
89178982

89188983
match $node_b.get_and_clear_pending_msg_events().pop().unwrap() {
89198984
MessageSendEvent::UpdateHTLCs { node_id, updates } => {
@@ -8924,12 +8989,12 @@ pub mod bench {
89248989
_ => panic!("Failed to generate claim event"),
89258990
}
89268991

8927-
let (raa, cs) = do_get_revoke_commit_msgs!(NodeHolder { node: &$node_a }, &$node_b.get_our_node_id());
8992+
let (raa, cs) = get_revoke_commit_msgs(&ANodeHolder { node: &$node_a }, &$node_b.get_our_node_id());
89288993
$node_b.handle_revoke_and_ack(&$node_a.get_our_node_id(), &raa);
89298994
$node_b.handle_commitment_signed(&$node_a.get_our_node_id(), &cs);
8930-
$node_a.handle_revoke_and_ack(&$node_b.get_our_node_id(), &get_event_msg!(NodeHolder { node: &$node_b }, MessageSendEvent::SendRevokeAndACK, $node_a.get_our_node_id()));
8995+
$node_a.handle_revoke_and_ack(&$node_b.get_our_node_id(), &get_event_msg!(ANodeHolder { node: &$node_b }, MessageSendEvent::SendRevokeAndACK, $node_a.get_our_node_id()));
89318996

8932-
expect_payment_sent!(NodeHolder { node: &$node_a }, payment_preimage);
8997+
expect_payment_sent!(ANodeHolder { node: &$node_a }, payment_preimage);
89338998
}
89348999
}
89359000

‎lightning/src/ln/functional_test_utils.rs

Lines changed: 64 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use crate::chain::{BestBlock, ChannelMonitorUpdateStatus, Confirm, Listen, Watch
1414
use crate::chain::channelmonitor::ChannelMonitor;
1515
use crate::chain::transaction::OutPoint;
1616
use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
17-
use crate::ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, PaymentId, MIN_CLTV_EXPIRY_DELTA};
17+
use crate::ln::channelmanager::{AChannelManager, ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, PaymentId, MIN_CLTV_EXPIRY_DELTA};
1818
use crate::routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate};
1919
use crate::routing::router::{self, PaymentParameters, Route};
2020
use crate::ln::features::InitFeatures;
@@ -322,14 +322,16 @@ pub struct NodeCfg<'a> {
322322
pub override_init_features: Rc<RefCell<Option<InitFeatures>>>,
323323
}
324324

325+
type TestChannelManager<'a, 'b, 'c> = ChannelManager<&'b TestChainMonitor<'c>, &'c test_utils::TestBroadcaster, &'b test_utils::TestKeysInterface, &'b test_utils::TestKeysInterface, &'b test_utils::TestKeysInterface, &'c test_utils::TestFeeEstimator, &'b test_utils::TestRouter<'c>, &'c test_utils::TestLogger>;
326+
325327
pub struct Node<'a, 'b: 'a, 'c: 'b> {
326328
pub chain_source: &'c test_utils::TestChainSource,
327329
pub tx_broadcaster: &'c test_utils::TestBroadcaster,
328330
pub fee_estimator: &'c test_utils::TestFeeEstimator,
329331
pub router: &'b test_utils::TestRouter<'c>,
330332
pub chain_monitor: &'b test_utils::TestChainMonitor<'c>,
331333
pub keys_manager: &'b test_utils::TestKeysInterface,
332-
pub node: &'a ChannelManager<&'b TestChainMonitor<'c>, &'c test_utils::TestBroadcaster, &'b test_utils::TestKeysInterface, &'b test_utils::TestKeysInterface, &'b test_utils::TestKeysInterface, &'c test_utils::TestFeeEstimator, &'b test_utils::TestRouter<'c>, &'c test_utils::TestLogger>,
334+
pub node: &'a TestChannelManager<'a, 'b, 'c>,
333335
pub network_graph: &'a NetworkGraph<&'c test_utils::TestLogger>,
334336
pub gossip_sync: P2PGossipSync<&'b NetworkGraph<&'c test_utils::TestLogger>, &'c test_utils::TestChainSource, &'c test_utils::TestLogger>,
335337
pub node_seed: [u8; 32],
@@ -365,6 +367,39 @@ impl NodePtr {
365367
unsafe impl Send for NodePtr {}
366368
unsafe impl Sync for NodePtr {}
367369

370+
371+
pub trait NodeHolder {
372+
type CM: AChannelManager;
373+
fn node(&self) -> &ChannelManager<
374+
<Self::CM as AChannelManager>::M,
375+
<Self::CM as AChannelManager>::T,
376+
<Self::CM as AChannelManager>::ES,
377+
<Self::CM as AChannelManager>::NS,
378+
<Self::CM as AChannelManager>::SP,
379+
<Self::CM as AChannelManager>::F,
380+
<Self::CM as AChannelManager>::R,
381+
<Self::CM as AChannelManager>::L>;
382+
fn chain_monitor(&self) -> Option<&test_utils::TestChainMonitor>;
383+
}
384+
impl<H: NodeHolder> NodeHolder for &H {
385+
type CM = H::CM;
386+
fn node(&self) -> &ChannelManager<
387+
<Self::CM as AChannelManager>::M,
388+
<Self::CM as AChannelManager>::T,
389+
<Self::CM as AChannelManager>::ES,
390+
<Self::CM as AChannelManager>::NS,
391+
<Self::CM as AChannelManager>::SP,
392+
<Self::CM as AChannelManager>::F,
393+
<Self::CM as AChannelManager>::R,
394+
<Self::CM as AChannelManager>::L> { (*self).node() }
395+
fn chain_monitor(&self) -> Option<&test_utils::TestChainMonitor> { (*self).chain_monitor() }
396+
}
397+
impl<'a, 'b: 'a, 'c: 'b> NodeHolder for Node<'a, 'b, 'c> {
398+
type CM = TestChannelManager<'a, 'b, 'c>;
399+
fn node(&self) -> &TestChannelManager<'a, 'b, 'c> { &self.node }
400+
fn chain_monitor(&self) -> Option<&test_utils::TestChainMonitor> { Some(self.chain_monitor) }
401+
}
402+
368403
impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
369404
fn drop(&mut self) {
370405
if !panicking() {
@@ -484,36 +519,27 @@ pub fn create_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(node_a: &'a Node<'b,
484519
}
485520

486521
/// Gets an RAA and CS which were sent in response to a commitment update
487-
///
488-
/// Should only be used directly when the `$node` is not actually a [`Node`].
489-
macro_rules! do_get_revoke_commit_msgs {
490-
($node: expr, $recipient: expr) => { {
491-
let events = $node.node.get_and_clear_pending_msg_events();
492-
assert_eq!(events.len(), 2);
493-
(match events[0] {
494-
MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
495-
assert_eq!(node_id, $recipient);
496-
(*msg).clone()
497-
},
498-
_ => panic!("Unexpected event"),
499-
}, match events[1] {
500-
MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
501-
assert_eq!(node_id, $recipient);
502-
assert!(updates.update_add_htlcs.is_empty());
503-
assert!(updates.update_fulfill_htlcs.is_empty());
504-
assert!(updates.update_fail_htlcs.is_empty());
505-
assert!(updates.update_fail_malformed_htlcs.is_empty());
506-
assert!(updates.update_fee.is_none());
507-
updates.commitment_signed.clone()
508-
},
509-
_ => panic!("Unexpected event"),
510-
})
511-
} }
512-
}
513-
514-
/// Gets an RAA and CS which were sent in response to a commitment update
515-
pub fn get_revoke_commit_msgs(node: &Node, recipient: &PublicKey) -> (msgs::RevokeAndACK, msgs::CommitmentSigned) {
516-
do_get_revoke_commit_msgs!(node, recipient)
522+
pub fn get_revoke_commit_msgs<CM: AChannelManager, H: NodeHolder<CM=CM>>(node: &H, recipient: &PublicKey) -> (msgs::RevokeAndACK, msgs::CommitmentSigned) {
523+
let events = node.node().get_and_clear_pending_msg_events();
524+
assert_eq!(events.len(), 2);
525+
(match events[0] {
526+
MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
527+
assert_eq!(node_id, recipient);
528+
(*msg).clone()
529+
},
530+
_ => panic!("Unexpected event"),
531+
}, match events[1] {
532+
MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
533+
assert_eq!(node_id, recipient);
534+
assert!(updates.update_add_htlcs.is_empty());
535+
assert!(updates.update_fulfill_htlcs.is_empty());
536+
assert!(updates.update_fail_htlcs.is_empty());
537+
assert!(updates.update_fail_malformed_htlcs.is_empty());
538+
assert!(updates.update_fee.is_none());
539+
updates.commitment_signed.clone()
540+
},
541+
_ => panic!("Unexpected event"),
542+
})
517543
}
518544

519545
#[macro_export]
@@ -772,10 +798,12 @@ macro_rules! unwrap_send_err {
772798
}
773799

774800
/// Check whether N channel monitor(s) have been added.
775-
pub fn check_added_monitors(node: &Node, count: usize) {
776-
let mut added_monitors = node.chain_monitor.added_monitors.lock().unwrap();
777-
assert_eq!(added_monitors.len(), count);
778-
added_monitors.clear();
801+
pub fn check_added_monitors<CM: AChannelManager, H: NodeHolder<CM=CM>>(node: &H, count: usize) {
802+
if let Some(chain_monitor) = node.chain_monitor() {
803+
let mut added_monitors = chain_monitor.added_monitors.lock().unwrap();
804+
assert_eq!(added_monitors.len(), count);
805+
added_monitors.clear();
806+
}
779807
}
780808

781809
/// Check whether N channel monitor(s) have been added.

0 commit comments

Comments
 (0)