Skip to content

Remove unnecessary heap allocations in log-entry-matching tests #2118

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
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
2 changes: 1 addition & 1 deletion lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4012,7 +4012,7 @@ mod tests {
use crate::ln::functional_test_utils::*;
use crate::ln::script::ShutdownScript;
use crate::util::errors::APIError;
use crate::util::events::{ClosureReason, MessageSendEventsProvider};
use crate::util::events::ClosureReason;
use crate::util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator};
use crate::util::ser::{ReadableArgs, Writeable};
use crate::sync::{Arc, Mutex};
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/chanmon_update_fail_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ fn test_monitor_and_persister_update_fail() {
// because the update is bogus, ultimately the error that's returned
// should be a PermanentFailure.
if let ChannelMonitorUpdateStatus::PermanentFailure = chain_mon.chain_monitor.update_channel(outpoint, &update) {} else { panic!("Expected monitor error to be permanent"); }
logger.assert_log_regex("lightning::chain::chainmonitor".to_string(), regex::Regex::new("Persistence of ChannelMonitorUpdate for channel [0-9a-f]* in progress").unwrap(), 1);
logger.assert_log_regex("lightning::chain::chainmonitor", regex::Regex::new("Persistence of ChannelMonitorUpdate for channel [0-9a-f]* in progress").unwrap(), 1);
assert_eq!(nodes[0].chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::Completed);
} else { assert!(false); }
}
Expand Down
9 changes: 5 additions & 4 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8195,7 +8195,7 @@ mod tests {
assert!(updates.update_fee.is_none());
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);

nodes[1].logger.assert_log_contains("lightning::ln::channelmanager".to_string(), "Payment preimage didn't match payment hash".to_string(), 1);
nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Payment preimage didn't match payment hash", 1);
}

#[test]
Expand Down Expand Up @@ -8238,7 +8238,7 @@ mod tests {
assert!(updates.update_fee.is_none());
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);

nodes[1].logger.assert_log_contains("lightning::ln::channelmanager".to_string(), "We don't support MPP keysend payments".to_string(), 1);
nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "We don't support MPP keysend payments", 1);
}

#[test]
Expand Down Expand Up @@ -8266,7 +8266,8 @@ mod tests {

match nodes[0].node.send_payment(&route, payment_hash, &None, PaymentId(payment_hash.0)).unwrap_err() {
PaymentSendFailure::ParameterError(APIError::APIMisuseError { ref err }) => {
assert!(regex::Regex::new(r"Payment secret is required for multi-path payments").unwrap().is_match(err)) },
assert!(regex::Regex::new(r"Payment secret is required for multi-path payments").unwrap().is_match(err))
},
_ => panic!("unexpected error")
}
}
Expand Down Expand Up @@ -8326,7 +8327,7 @@ mod tests {
match inbound_payment::verify(bad_payment_hash, &payment_data, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger) {
Ok(_) => panic!("Unexpected ok"),
Err(()) => {
nodes[0].logger.assert_log_contains("lightning::ln::inbound_payment".to_string(), "Failing HTLC with user-generated payment_hash".to_string(), 1);
nodes[0].logger.assert_log_contains("lightning::ln::inbound_payment", "Failing HTLC with user-generated payment_hash", 1);
}
}

Expand Down
20 changes: 10 additions & 10 deletions lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ fn test_insane_channel_opens() {
if let MessageSendEvent::HandleError { ref action, .. } = msg_events[0] {
match action {
&ErrorAction::SendErrorMessage { .. } => {
nodes[1].logger.assert_log_regex("lightning::ln::channelmanager".to_string(), expected_regex, 1);
nodes[1].logger.assert_log_regex("lightning::ln::channelmanager", expected_regex, 1);
},
_ => panic!("unexpected event!"),
}
Expand Down Expand Up @@ -1118,7 +1118,7 @@ fn holding_cell_htlc_counting() {
unwrap_send_err!(nodes[1].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1), PaymentId(payment_hash_1.0)), true, APIError::ChannelUnavailable { ref err },
assert!(regex::Regex::new(r"Cannot push more than their max accepted HTLCs \(\d+\)").unwrap().is_match(err)));
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
nodes[1].logger.assert_log_contains("lightning::ln::channelmanager".to_string(), "Cannot push more than their max accepted HTLCs".to_string(), 1);
nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot push more than their max accepted HTLCs", 1);
}

// This should also be true if we try to forward a payment.
Expand Down Expand Up @@ -1346,7 +1346,7 @@ fn test_basic_channel_reserve() {
_ => panic!("Unexpected error variant"),
}
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager".to_string(), "Cannot send value that would put our balance under counterparty-announced channel reserve value".to_string(), 1);
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send value that would put our balance under counterparty-announced channel reserve value", 1);

send_payment(&nodes[0], &vec![&nodes[1]], max_can_send);
}
Expand Down Expand Up @@ -1811,7 +1811,7 @@ fn test_channel_reserve_holding_cell_htlcs() {
unwrap_send_err!(nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret), PaymentId(our_payment_hash.0)), true, APIError::ChannelUnavailable { ref err },
assert!(regex::Regex::new(r"Cannot send value that would put us over the max HTLC value in flight our peer will accept \(\d+\)").unwrap().is_match(err)));
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us over the max HTLC value in flight our peer will accept".to_string(), 1);
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send value that would put us over the max HTLC value in flight our peer will accept", 1);
}

// channel reserve is bigger than their_max_htlc_value_in_flight_msat so loop to deplete
Expand Down Expand Up @@ -1906,7 +1906,7 @@ fn test_channel_reserve_holding_cell_htlcs() {
unwrap_send_err!(nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret), PaymentId(our_payment_hash.0)), true, APIError::ChannelUnavailable { ref err },
assert!(regex::Regex::new(r"Cannot send value that would put our balance under counterparty-announced channel reserve value \(\d+\)").unwrap().is_match(err)));
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager".to_string(), "Cannot send value that would put our balance under counterparty-announced channel reserve value".to_string(), 2);
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send value that would put our balance under counterparty-announced channel reserve value", 2);
}

let (route_22, our_payment_hash_22, our_payment_preimage_22, our_payment_secret_22) = get_route_and_payment_hash!(nodes[0], nodes[2], recv_value_22);
Expand Down Expand Up @@ -5987,7 +5987,7 @@ fn test_update_add_htlc_bolt2_sender_value_below_minimum_msat() {
unwrap_send_err!(nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret), PaymentId(our_payment_hash.0)), true, APIError::ChannelUnavailable { ref err },
assert!(regex::Regex::new(r"Cannot send less than their minimum HTLC value \(\d+\)").unwrap().is_match(err)));
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager".to_string(), "Cannot send less than their minimum HTLC value".to_string(), 1);
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send less than their minimum HTLC value", 1);
}

#[test]
Expand All @@ -6005,7 +6005,7 @@ fn test_update_add_htlc_bolt2_sender_zero_value_msat() {
assert_eq!(err, "Cannot send 0-msat HTLC"));

assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager".to_string(), "Cannot send 0-msat HTLC".to_string(), 1);
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send 0-msat HTLC", 1);
}

#[test]
Expand Down Expand Up @@ -6088,7 +6088,7 @@ fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_num_and_htlc_id_increment()
assert!(regex::Regex::new(r"Cannot push more than their max accepted HTLCs \(\d+\)").unwrap().is_match(err)));

assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager".to_string(), "Cannot push more than their max accepted HTLCs".to_string(), 1);
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot push more than their max accepted HTLCs", 1);
}

#[test]
Expand All @@ -6112,7 +6112,7 @@ fn test_update_add_htlc_bolt2_sender_exceed_max_htlc_value_in_flight() {
assert!(regex::Regex::new(r"Cannot send value that would put us over the max HTLC value in flight our peer will accept \(\d+\)").unwrap().is_match(err)));

assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us over the max HTLC value in flight our peer will accept".to_string(), 1);
nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot send value that would put us over the max HTLC value in flight our peer will accept", 1);

send_payment(&nodes[0], &[&nodes[1]], max_in_flight);
}
Expand Down Expand Up @@ -9502,7 +9502,7 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
}
nodes[0].node.timer_tick_occurred();
check_added_monitors!(nodes[0], 1);
nodes[0].logger.assert_log_contains("lightning::ln::channel".to_string(), "Cannot afford to send new feerate at 2530 without infringing max dust htlc exposure".to_string(), 1);
nodes[0].logger.assert_log_contains("lightning::ln::channel", "Cannot afford to send new feerate at 2530 without infringing max dust htlc exposure", 1);
}

let _ = nodes[0].node.get_and_clear_pending_msg_events();
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/priv_short_conf_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ fn test_inbound_scid_privacy() {
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, true, true);

nodes[1].logger.assert_log_regex("lightning::ln::channelmanager".to_string(), regex::Regex::new(r"Refusing to forward over real channel SCID as our counterparty requested").unwrap(), 1);
nodes[1].logger.assert_log_regex("lightning::ln::channelmanager", regex::Regex::new(r"Refusing to forward over real channel SCID as our counterparty requested").unwrap(), 1);

let mut updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
Expand Down
12 changes: 6 additions & 6 deletions lightning/src/onion_message/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ fn pass_along_path(path: &Vec<MessengerNode>, expected_path_id: Option<[u8; 32]>
node.messenger.handle_onion_message(&prev_node.get_node_pk(), &onion_msg);
if idx == num_nodes - 1 {
node.logger.assert_log_contains(
"lightning::onion_message::messenger".to_string(),
format!("Received an onion message with path_id: {:02x?}", expected_path_id).to_string(), 1);
"lightning::onion_message::messenger",
&format!("Received an onion message with path_id: {:02x?}", expected_path_id), 1);
}
prev_node = node;
}
Expand Down Expand Up @@ -218,8 +218,8 @@ fn reply_path() {
pass_along_path(&nodes, None);
// Make sure the last node successfully decoded the reply path.
nodes[3].logger.assert_log_contains(
"lightning::onion_message::messenger".to_string(),
format!("Received an onion message with path_id None and a reply_path").to_string(), 1);
"lightning::onion_message::messenger",
&format!("Received an onion message with path_id None and a reply_path"), 1);

// Destination::BlindedPath
let blinded_path = BlindedPath::new(&[nodes[1].get_node_pk(), nodes[2].get_node_pk(), nodes[3].get_node_pk()], &*nodes[3].keys_manager, &secp_ctx).unwrap();
Expand All @@ -228,8 +228,8 @@ fn reply_path() {
nodes[0].messenger.send_onion_message(&[], Destination::BlindedPath(blinded_path), OnionMessageContents::Custom(test_msg), Some(reply_path)).unwrap();
pass_along_path(&nodes, None);
nodes[3].logger.assert_log_contains(
"lightning::onion_message::messenger".to_string(),
format!("Received an onion message with path_id None and a reply_path").to_string(), 2);
"lightning::onion_message::messenger",
&format!("Received an onion message with path_id None and a reply_path"), 2);
}

#[test]
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/util/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,10 +642,10 @@ impl TestLogger {
/// 1. belongs to the specified module and
/// 2. contains `line` in it.
/// And asserts if the number of occurrences is the same with the given `count`
pub fn assert_log_contains(&self, module: String, line: String, count: usize) {
pub fn assert_log_contains(&self, module: &str, line: &str, count: usize) {
let log_entries = self.lines.lock().unwrap();
let l: usize = log_entries.iter().filter(|&(&(ref m, ref l), _c)| {
m == &module && l.contains(line.as_str())
m == module && l.contains(line)
}).map(|(_, c) | { c }).sum();
assert_eq!(l, count)
}
Expand All @@ -654,10 +654,10 @@ impl TestLogger {
/// 1. belong to the specified module and
/// 2. match the given regex pattern.
/// Assert that the number of occurrences equals the given `count`
pub fn assert_log_regex(&self, module: String, pattern: regex::Regex, count: usize) {
pub fn assert_log_regex(&self, module: &str, pattern: regex::Regex, count: usize) {
let log_entries = self.lines.lock().unwrap();
let l: usize = log_entries.iter().filter(|&(&(ref m, ref l), _c)| {
m == &module && pattern.is_match(&l)
m == module && pattern.is_match(&l)
}).map(|(_, c) | { c }).sum();
assert_eq!(l, count)
}
Expand Down