Skip to content

Misc (mostly-)feature cleanups #3250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 7 additions & 15 deletions lightning/src/ln/bolt11_payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,31 +91,22 @@ mod tests {
use crate::routing::router::Payee;
use bitcoin::hashes::sha256::Hash as Sha256;
use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
use core::time::Duration;
use lightning_invoice::{Currency, InvoiceBuilder};
#[cfg(feature = "std")]
use std::time::SystemTime;

fn duration_since_epoch() -> Duration {
#[cfg(feature = "std")]
let duration_since_epoch = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();
#[cfg(not(feature = "std"))]
let duration_since_epoch = Duration::from_secs(1234567);
duration_since_epoch
}

#[test]
fn invoice_test() {
let payment_hash = Sha256::hash(&[0; 32]);
let private_key = SecretKey::from_slice(&[42; 32]).unwrap();
let secp_ctx = Secp256k1::new();
let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);

let timestamp = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();
let invoice = InvoiceBuilder::new(Currency::Bitcoin)
.description("test".into())
.payment_hash(payment_hash)
.payment_secret(PaymentSecret([0; 32]))
.duration_since_epoch(duration_since_epoch())
.duration_since_epoch(timestamp)
.min_final_cltv_expiry_delta(144)
.amount_milli_satoshis(128)
.build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
Expand All @@ -142,11 +133,12 @@ mod tests {
let secp_ctx = Secp256k1::new();
let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);

let timestamp = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();
let invoice = InvoiceBuilder::new(Currency::Bitcoin)
.description("test".into())
.payment_hash(payment_hash)
.payment_secret(PaymentSecret([0; 32]))
.duration_since_epoch(duration_since_epoch())
.duration_since_epoch(timestamp)
.min_final_cltv_expiry_delta(144)
.build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
.unwrap();
Expand All @@ -167,12 +159,12 @@ mod tests {
}

#[test]
#[cfg(feature = "std")]
fn payment_metadata_end_to_end() {
use crate::events::Event;
use crate::ln::channelmanager::{PaymentId, Retry};
use crate::ln::functional_test_utils::*;
use crate::ln::msgs::ChannelMessageHandler;

// Test that a payment metadata read from an invoice passed to `pay_invoice` makes it all
// the way out through the `PaymentClaimable` event.
let chanmon_cfgs = create_chanmon_cfgs(2);
Expand All @@ -188,12 +180,12 @@ mod tests {

let secp_ctx = Secp256k1::new();
let node_secret = nodes[1].keys_manager.backing.get_node_secret_key();
let time = std::time::SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();
let timestamp = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();
let invoice = InvoiceBuilder::new(Currency::Bitcoin)
.description("test".into())
.payment_hash(Sha256::from_slice(&payment_hash.0).unwrap())
.payment_secret(payment_secret)
.duration_since_epoch(time)
.duration_since_epoch(timestamp)
.min_final_cltv_expiry_delta(144)
.amount_milli_satoshis(50_000)
.payment_metadata(payment_metadata.clone())
Expand Down
53 changes: 21 additions & 32 deletions lightning/src/ln/functional_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use crate::util::test_channel_signer::TestChannelSigner;
#[cfg(test)]
use crate::util::test_channel_signer::SignerOp;
use crate::util::test_utils;
use crate::util::test_utils::{panicking, TestChainMonitor, TestScorer, TestKeysInterface};
use crate::util::test_utils::{TestChainMonitor, TestScorer, TestKeysInterface};
use crate::util::ser::{ReadableArgs, Writeable};

use bitcoin::amount::Amount;
Expand Down Expand Up @@ -194,28 +194,23 @@ impl ConnectStyle {
}

fn random_style() -> ConnectStyle {
#[cfg(feature = "std")] {
use core::hash::{BuildHasher, Hasher};
// Get a random value using the only std API to do so - the DefaultHasher
let rand_val = std::collections::hash_map::RandomState::new().build_hasher().finish();
let res = match rand_val % 9 {
0 => ConnectStyle::BestBlockFirst,
1 => ConnectStyle::BestBlockFirstSkippingBlocks,
2 => ConnectStyle::BestBlockFirstReorgsOnlyTip,
3 => ConnectStyle::TransactionsFirst,
4 => ConnectStyle::TransactionsFirstSkippingBlocks,
5 => ConnectStyle::TransactionsDuplicativelyFirstSkippingBlocks,
6 => ConnectStyle::HighlyRedundantTransactionsFirstSkippingBlocks,
7 => ConnectStyle::TransactionsFirstReorgsOnlyTip,
8 => ConnectStyle::FullBlockViaListen,
_ => unreachable!(),
};
eprintln!("Using Block Connection Style: {:?}", res);
res
}
#[cfg(not(feature = "std"))] {
ConnectStyle::FullBlockViaListen
}
use core::hash::{BuildHasher, Hasher};
// Get a random value using the only std API to do so - the DefaultHasher
let rand_val = std::collections::hash_map::RandomState::new().build_hasher().finish();
let res = match rand_val % 9 {
0 => ConnectStyle::BestBlockFirst,
1 => ConnectStyle::BestBlockFirstSkippingBlocks,
2 => ConnectStyle::BestBlockFirstReorgsOnlyTip,
3 => ConnectStyle::TransactionsFirst,
4 => ConnectStyle::TransactionsFirstSkippingBlocks,
5 => ConnectStyle::TransactionsDuplicativelyFirstSkippingBlocks,
6 => ConnectStyle::HighlyRedundantTransactionsFirstSkippingBlocks,
7 => ConnectStyle::TransactionsFirstReorgsOnlyTip,
8 => ConnectStyle::FullBlockViaListen,
_ => unreachable!(),
};
eprintln!("Using Block Connection Style: {:?}", res);
res
}
}

Expand Down Expand Up @@ -270,9 +265,7 @@ fn do_connect_block_with_consistency_checks<'a, 'b, 'c, 'd>(node: &'a Node<'b, '

fn do_connect_block_without_consistency_checks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: Block, skip_intermediaries: bool) {
let height = node.best_block_info().1 + 1;
#[cfg(feature = "std")] {
eprintln!("Connecting block using Block Connection Style: {:?}", *node.connect_style.borrow());
}
eprintln!("Connecting block using Block Connection Style: {:?}", *node.connect_style.borrow());
// Update the block internally before handing it over to LDK, to ensure our assertions regarding
// transaction broadcast are correct.
node.blocks.lock().unwrap().push((block.clone(), height));
Expand Down Expand Up @@ -340,9 +333,7 @@ fn do_connect_block_without_consistency_checks<'a, 'b, 'c, 'd>(node: &'a Node<'b

pub fn disconnect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, count: u32) {
call_claimable_balances(node);
#[cfg(feature = "std")] {
eprintln!("Disconnecting {} blocks using Block Connection Style: {:?}", count, *node.connect_style.borrow());
}
eprintln!("Disconnecting {} blocks using Block Connection Style: {:?}", count, *node.connect_style.borrow());
for i in 0..count {
let orig = node.blocks.lock().unwrap().pop().unwrap();
assert!(orig.1 > 0); // Cannot disconnect genesis
Expand Down Expand Up @@ -471,9 +462,7 @@ impl<'a, 'b, 'c> Node<'a, 'b, 'c> {
}
}

#[cfg(feature = "std")]
impl<'a, 'b, 'c> std::panic::UnwindSafe for Node<'a, 'b, 'c> {}
#[cfg(feature = "std")]
impl<'a, 'b, 'c> std::panic::RefUnwindSafe for Node<'a, 'b, 'c> {}
impl<'a, 'b, 'c> Node<'a, 'b, 'c> {
pub fn best_block_hash(&self) -> BlockHash {
Expand Down Expand Up @@ -620,7 +609,7 @@ impl<'a, 'b: 'a, 'c: 'b> NodeHolder for Node<'a, 'b, 'c> {

impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
fn drop(&mut self) {
if !panicking() {
if !std::thread::panicking() {
// Check that we processed all pending events
let msg_events = self.node.get_and_clear_pending_msg_events();
if !msg_events.is_empty() {
Expand Down
13 changes: 0 additions & 13 deletions lightning/src/ln/invoice_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,6 @@ mod test {
use crate::sign::PhantomKeysManager;
use crate::events::{MessageSendEvent, MessageSendEventsProvider};
use crate::ln::types::PaymentHash;
#[cfg(feature = "std")]
use crate::ln::types::PaymentPreimage;
use crate::ln::channelmanager::{PhantomRouteHints, MIN_FINAL_CLTV_EXPIRY_DELTA, PaymentId, RecipientOnionFields, Retry};
use crate::ln::functional_test_utils::*;
Expand Down Expand Up @@ -1281,13 +1280,11 @@ mod test {
}

#[test]
#[cfg(feature = "std")]
fn test_multi_node_receive() {
do_test_multi_node_receive(true);
do_test_multi_node_receive(false);
}

#[cfg(feature = "std")]
fn do_test_multi_node_receive(user_generated_pmt_hash: bool) {
use crate::events::{Event, EventsProvider};
use core::cell::RefCell;
Expand Down Expand Up @@ -1395,7 +1392,6 @@ mod test {
}

#[test]
#[cfg(feature = "std")]
fn test_multi_node_hints_has_htlc_min_max_values() {
let mut chanmon_cfgs = create_chanmon_cfgs(3);
let seed_1 = [42u8; 32];
Expand Down Expand Up @@ -1432,7 +1428,6 @@ mod test {
}

#[test]
#[cfg(feature = "std")]
fn test_create_phantom_invoice_with_description_hash() {
let chanmon_cfgs = create_chanmon_cfgs(3);
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
Expand Down Expand Up @@ -1462,7 +1457,6 @@ mod test {
}

#[test]
#[cfg(feature = "std")]
fn create_phantom_invoice_with_custom_payment_hash_and_custom_min_final_cltv_delta() {
let chanmon_cfgs = create_chanmon_cfgs(3);
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
Expand All @@ -1489,7 +1483,6 @@ mod test {
}

#[test]
#[cfg(feature = "std")]
fn test_multi_node_hints_includes_single_channels_to_participating_nodes() {
let mut chanmon_cfgs = create_chanmon_cfgs(3);
let seed_1 = [42u8; 32];
Expand Down Expand Up @@ -1518,7 +1511,6 @@ mod test {
}

#[test]
#[cfg(feature = "std")]
fn test_multi_node_hints_includes_one_channel_of_each_counterparty_nodes_per_participating_node() {
let mut chanmon_cfgs = create_chanmon_cfgs(4);
let seed_1 = [42u8; 32];
Expand Down Expand Up @@ -1549,7 +1541,6 @@ mod test {
}

#[test]
#[cfg(feature = "std")]
fn test_multi_node_forwarding_info_not_assigned_channel_excluded_from_hints() {
let mut chanmon_cfgs = create_chanmon_cfgs(4);
let seed_1 = [42u8; 32];
Expand Down Expand Up @@ -1607,7 +1598,6 @@ mod test {
}

#[test]
#[cfg(feature = "std")]
fn test_multi_node_with_only_public_channels_hints_includes_only_phantom_route() {
let mut chanmon_cfgs = create_chanmon_cfgs(3);
let seed_1 = [42u8; 32];
Expand Down Expand Up @@ -1640,7 +1630,6 @@ mod test {
}

#[test]
#[cfg(feature = "std")]
fn test_multi_node_with_mixed_public_and_private_channel_hints_includes_only_phantom_route() {
let mut chanmon_cfgs = create_chanmon_cfgs(4);
let seed_1 = [42u8; 32];
Expand Down Expand Up @@ -1674,7 +1663,6 @@ mod test {
}

#[test]
#[cfg(feature = "std")]
fn test_multi_node_hints_has_only_lowest_inbound_channel_above_minimum() {
let mut chanmon_cfgs = create_chanmon_cfgs(3);
let seed_1 = [42u8; 32];
Expand Down Expand Up @@ -1705,7 +1693,6 @@ mod test {
}

#[test]
#[cfg(feature = "std")]
fn test_multi_node_channels_inbound_capacity_lower_than_invoice_amt_filtering() {
let mut chanmon_cfgs = create_chanmon_cfgs(4);
let seed_1 = [42u8; 32];
Expand Down
40 changes: 11 additions & 29 deletions lightning/src/ln/outbound_payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@ use crate::routing::router::{BlindedTail, InFlightHtlcs, Path, PaymentParameters
use crate::sign::{EntropySource, NodeSigner, Recipient};
use crate::util::errors::APIError;
use crate::util::logger::Logger;
use crate::util::time::Time;
#[cfg(all(feature = "std", test))]
use crate::util::time::tests::SinceEpoch;
#[cfg(feature = "std")]
use crate::util::time::Instant;
use crate::util::ser::ReadableArgs;

use core::fmt::{self, Display, Formatter};
Expand Down Expand Up @@ -319,12 +318,9 @@ impl Retry {
(Retry::Attempts(max_retry_count), PaymentAttempts { count, .. }) => {
max_retry_count > count
},
#[cfg(all(feature = "std", not(test)))]
(Retry::Timeout(max_duration), PaymentAttempts { first_attempted_at, .. }) =>
*max_duration >= crate::util::time::MonotonicTime::now().duration_since(*first_attempted_at),
#[cfg(all(feature = "std", test))]
#[cfg(feature = "std")]
(Retry::Timeout(max_duration), PaymentAttempts { first_attempted_at, .. }) =>
*max_duration >= SinceEpoch::now().duration_since(*first_attempted_at),
*max_duration >= Instant::now().duration_since(*first_attempted_at),
}
}
}
Expand All @@ -339,42 +335,28 @@ pub(super) fn has_expired(route_params: &RouteParameters) -> bool {
false
}

pub(crate) type PaymentAttempts = PaymentAttemptsUsingTime<ConfiguredTime>;

/// Storing minimal payment attempts information required for determining if a outbound payment can
/// be retried.
pub(crate) struct PaymentAttemptsUsingTime<T: Time> {
pub(crate) struct PaymentAttempts {
/// This count will be incremented only after the result of the attempt is known. When it's 0,
/// it means the result of the first attempt is not known yet.
pub(crate) count: u32,
/// This field is only used when retry is `Retry::Timeout` which is only build with feature std
#[cfg(feature = "std")]
first_attempted_at: T,
#[cfg(not(feature = "std"))]
phantom: core::marker::PhantomData<T>,

first_attempted_at: Instant,
}

#[cfg(not(feature = "std"))]
type ConfiguredTime = crate::util::time::Eternity;
#[cfg(all(feature = "std", not(test)))]
type ConfiguredTime = crate::util::time::MonotonicTime;
#[cfg(all(feature = "std", test))]
type ConfiguredTime = SinceEpoch;

impl<T: Time> PaymentAttemptsUsingTime<T> {
impl PaymentAttempts {
pub(crate) fn new() -> Self {
PaymentAttemptsUsingTime {
PaymentAttempts {
count: 0,
#[cfg(feature = "std")]
first_attempted_at: T::now(),
#[cfg(not(feature = "std"))]
phantom: core::marker::PhantomData,
first_attempted_at: Instant::now(),
}
}
}

impl<T: Time> Display for PaymentAttemptsUsingTime<T> {
impl Display for PaymentAttempts {
fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> {
#[cfg(not(feature = "std"))]
return write!(f, "attempts: {}", self.count);
Expand All @@ -383,7 +365,7 @@ impl<T: Time> Display for PaymentAttemptsUsingTime<T> {
f,
"attempts: {}, duration: {}s",
self.count,
T::now().duration_since(self.first_attempted_at).as_secs()
Instant::now().duration_since(self.first_attempted_at).as_secs()
);
}
}
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/payment_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use crate::routing::gossip::NodeId;

#[cfg(feature = "std")]
use {
crate::util::time::tests::SinceEpoch,
crate::util::time::Instant as TestTime,
std::time::{SystemTime, Instant, Duration},
};

Expand Down Expand Up @@ -2340,7 +2340,7 @@ fn do_automatic_retries(test: AutoRetry) {
pass_failed_attempt_with_retry_along_path!(channel_id_2, true);

// Advance the time so the second attempt fails due to timeout.
SinceEpoch::advance(Duration::from_secs(61));
TestTime::advance(Duration::from_secs(61));

// Make sure we don't retry again.
nodes[0].node.process_pending_htlc_forwards();
Expand Down
Loading
Loading