address code review issues

added fee_proportional_millionths and annouce_channel  to config file

Author: SWvheerden

fuzz/fuzz_targets/full_stack_target.rs

@@ -22,6 +22,7 @@ use lightning::util::events::{EventsProvider,Event};
 use lightning::util::reset_rng_state;
 use lightning::util::logger::Logger;
 use lightning::util::sha2::Sha256;
+use lightning::util::UserConfigurations;
 
 mod utils;
 
@@ -235,8 +236,8 @@ pub fn do_test(data: &[u8], logger: &Arc<Logger>) {
 	let watch = Arc::new(ChainWatchInterfaceUtil::new(Network::Bitcoin, Arc::clone(&logger)));
 	let broadcast = Arc::new(TestBroadcaster{});
 	let monitor = channelmonitor::SimpleManyChannelMonitor::new(watch.clone(), broadcast.clone());
-
-	let channelmanager = ChannelManager::new(our_network_key, slice_to_be32(get_slice!(4)), get_slice!(1)[0] != 0, Network::Bitcoin, fee_est.clone(), monitor.clone(), watch.clone(), broadcast.clone(), Arc::clone(&logger)).unwrap();
+	let config = UserConfigurations::new();
+	let channelmanager = ChannelManager::new(our_network_key, slice_to_be32(get_slice!(4)), get_slice!(1)[0] != 0, Network::Bitcoin, fee_est.clone(), monitor.clone(), watch.clone(), broadcast.clone(), Arc::clone(&logger), config).unwrap();
 	let router = Arc::new(Router::new(PublicKey::from_secret_key(&secp_ctx, &our_network_key), watch.clone(), Arc::clone(&logger)));
 
 	let peers = RefCell::new([false; 256]);

src/ln/channel.rs

@@ -266,8 +266,10 @@ const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
 // calling channel_id() before we're set up or things like get_outbound_funding_signed on an
 // inbound channel.
 pub struct Channel {
-
-	config : UserConfigurations,
+	///TODO: split limits and runtime setting here
+	/// channel limits cant change during channel existence
+	/// but other settings can, but they need an update broadcast before they can change. 
+	config : UserConfigurations, 
 	user_id: u64,
 
 	channel_id: [u8; 32],
@@ -613,7 +615,7 @@ impl Channel {
 
 		let mut chan = Channel {
 			user_id: user_id,
-			config: (*configurations).clone(),
+			config: (*configurations).clone(), 
 			channel_id: msg.temporary_channel_id,
 			channel_state: (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32),
 			channel_outbound: false,

src/ln/channelmanager.rs

@@ -256,7 +256,6 @@ pub struct ChannelManager {
 	tx_broadcaster: Arc<BroadcasterInterface>,
 
 	announce_channels_publicly: bool,
-	fee_proportional_millionths: u32,
 	latest_block_height: AtomicUsize,
 	secp_ctx: Secp256k1<secp256k1::All>,
 
@@ -307,22 +306,19 @@ pub struct ChannelDetails {
 impl ChannelManager {
 	/// Constructs a new ChannelManager to hold several channels and route between them. This is
 	/// the main "logic hub" for all channel-related actions, and implements ChannelMessageHandler.
-	/// fee_proportional_millionths is an optional fee to charge any payments routed through us.
 	/// Non-proportional fees are fixed according to our risk using the provided fee estimator.
 	/// panics if channel_value_satoshis is >= `MAX_FUNDING_SATOSHIS`!
-	pub fn new(our_network_key: SecretKey, fee_proportional_millionths: u32, announce_channels_publicly: bool, network: Network, feeest: Arc<FeeEstimator>, monitor: Arc<ManyChannelMonitor>, chain_monitor: Arc<ChainWatchInterface>, tx_broadcaster: Arc<BroadcasterInterface>, logger: Arc<Logger>) -> Result<Arc<ChannelManager>, secp256k1::Error> {
+	pub fn new(our_network_key: SecretKey, fee_proportional_millionths: u32, announce_channels_publicly: bool, network: Network, feeest: Arc<FeeEstimator>, monitor: Arc<ManyChannelMonitor>, chain_monitor: Arc<ChainWatchInterface>, tx_broadcaster: Arc<BroadcasterInterface>, logger: Arc<Logger>, config : UserConfigurations) -> Result<Arc<ChannelManager>, secp256k1::Error> {
 		let secp_ctx = Secp256k1::new();
-
 		let res = Arc::new(ChannelManager {
-			configuration : UserConfigurations::new(),
+			configuration : config,
 			genesis_hash: genesis_block(network).header.bitcoin_hash(),
 			fee_estimator: feeest.clone(),
 			monitor: monitor.clone(),
 			chain_monitor,
 			tx_broadcaster,
 
-			announce_channels_publicly,
-			fee_proportional_millionths,
+			announce_channels_publicly,			
 			latest_block_height: AtomicUsize::new(0), //TODO: Get an init value (generally need to replay recent chain on chain_monitor registration)
 			secp_ctx,
 
@@ -917,7 +913,7 @@ impl ChannelManager {
 					if !chan.is_live() {
 						Some(("Forwarding channel is not in a ready state.", 0x1000 | 7, self.get_channel_update(chan).unwrap()))
 					} else {
-						let fee = amt_to_forward.checked_mul(self.fee_proportional_millionths as u64).and_then(|prop_fee| { (prop_fee / 1000000).checked_add(chan.get_our_fee_base_msat(&*self.fee_estimator) as u64) });
+						let fee = amt_to_forward.checked_mul(self.configuration.channel_options.fee_proportional_millionths as u64).and_then(|prop_fee| { (prop_fee / 1000000).checked_add(chan.get_our_fee_base_msat(&*self.fee_estimator) as u64) });
 						if fee.is_none() || msg.amount_msat < fee.unwrap() || (msg.amount_msat - fee.unwrap()) < *amt_to_forward {
 							Some(("Prior hop has deviated from specified fees parameters or origin node has obsolete ones", 0x1000 | 12, self.get_channel_update(chan).unwrap()))
 						} else {
@@ -954,7 +950,7 @@ impl ChannelManager {
 			cltv_expiry_delta: CLTV_EXPIRY_DELTA,
 			htlc_minimum_msat: chan.get_our_htlc_minimum_msat(),
 			fee_base_msat: chan.get_our_fee_base_msat(&*self.fee_estimator),
-			fee_proportional_millionths: self.fee_proportional_millionths,
+			fee_proportional_millionths: self.configuration.channel_options.fee_proportional_millionths,
 			excess_data: Vec::new(),
 		};
 
@@ -1486,7 +1482,8 @@ impl ChannelManager {
 		pending_events.push(events::Event::FundingGenerationReady {
 			temporary_channel_id: msg.temporary_channel_id,
 			channel_value_satoshis: value,
-			output_script: output_script,			user_channel_id: user_id,
+			output_script: output_script,
+			user_channel_id: user_id,
 		});
 		Ok(())
 	}
@@ -2996,6 +2993,8 @@ mod tests {
 	}
 
 	fn create_network(node_count: usize) -> Vec<Node> {
+		use util::UserConfigurations;
+		
 		let mut nodes = Vec::new();
 		let mut rng = thread_rng();
 		let secp_ctx = Secp256k1::new();
@@ -3014,7 +3013,8 @@ mod tests {
 				rng.fill_bytes(&mut key_slice);
 				SecretKey::from_slice(&secp_ctx, &key_slice).unwrap()
 			};
-			let node = ChannelManager::new(node_id.clone(), 0, true, Network::Testnet, feeest.clone(), chan_monitor.clone(), chain_monitor.clone(), tx_broadcaster.clone(), Arc::clone(&logger)).unwrap();
+			let config = UserConfigurations::new();
+			let node = ChannelManager::new(node_id.clone(), 0, true, Network::Testnet, feeest.clone(), chan_monitor.clone(), chain_monitor.clone(), tx_broadcaster.clone(), Arc::clone(&logger), config).unwrap();
 			let router = Router::new(PublicKey::from_secret_key(&secp_ctx, &node_id), chain_monitor.clone(), Arc::clone(&logger));
 			nodes.push(Node { chain_monitor, tx_broadcaster, chan_monitor, node, router,
 				network_payment_count: payment_count.clone(),

src/ln/router.rs

@@ -71,8 +71,6 @@ impl std::fmt::Display for ChannelInfo {
 	}
 }
 
-
-
 struct NodeInfo {
 	#[cfg(feature = "non_bitcoin_chain_hash_routing")]
 	channels: Vec<(u64, Sha256dHash)>,

src/util/configurations.rs

@@ -1,29 +1,43 @@
-#[derive(Copy, Clone)]
+/// This is the main user configuration
+/// This struct should contain all user customizable options as this is passed to the channel to be accessed
+#[derive(Copy, Clone)] 
 pub struct UserConfigurations{
+	/// optional user spesefied channel limits
+	/// These should stay the same for a channel and cannot change during the life of a channel
     pub channel_limits : ChannelLimits,
+	/// Channel options
+	pub channel_options : ChannelOptions,
 }
 
 impl UserConfigurations {
     pub fn new() -> Self{
         UserConfigurations {
             channel_limits : ChannelLimits::new(),
+			channel_options : ChannelOptions::new(),
         }
     }
 }
 
+/// This struct contains all the optional bolt 2 channel limits.
+/// If the user wants to check a value, the value needs to be filled in, as by default they are not checked
 #[derive(Copy, Clone)]
-pub struct ChannelLimits
-{
+pub struct ChannelLimits{
+	/// minimum allowed funding_satoshis
 	pub funding_satoshis :u64,
+	/// maximum allowed htlc_minimum_msat
 	pub htlc_minimum_msat : u64,
+	/// min allowed max_htlc_value_in_flight_msat
 	pub max_htlc_value_in_flight_msat : u64,
+	/// max allowed channel_reserve_satashis
 	pub channel_reserve_satoshis : u64,
+	/// min allowed max_accepted_htlcs
 	pub max_accepted_htlcs : u16,
+	/// min allowed dust_limit_satashis
 	pub dust_limit_satoshis : u64,
 }
 
 impl ChannelLimits {
-	//creating max and min possible values because if they are not set, means we should not check them. 
+//creating max and min possible values because if they are not set, means we should not check them.
 	pub fn new() -> Self{
 		ChannelLimits {
 			funding_satoshis : 0,
@@ -34,4 +48,23 @@ impl ChannelLimits {
 			dust_limit_satoshis : 0,
 		}
 	}
+}
+
+/// This struct contains all the custom channel options.
+#[derive(Copy, Clone)]
+pub struct ChannelOptions{
+	/// Amount (in millionths of a satoshi) channel will charge per transferred satoshi.
+	pub fee_proportional_millionths : u32, 	
+	///should the channel be annouced or not
+	pub annouce_channel : bool,
+}
+impl ChannelOptions {
+	/// creating a struct with values. 
+	/// fee_proportional_millionths should be changed afterwords 
+	pub fn new() -> Self{
+		ChannelOptions {
+			fee_proportional_millionths : 0,
+			annouce_channel : true,
+		}
+	}
 }

src/util/mod.rs

@@ -27,5 +27,5 @@ pub use self::rng::reset_rng_state;
 #[cfg(test)]
 pub(crate) mod test_utils;
 
-pub use self::configurations::{UserConfigurations, ChannelLimits};
-pub mod configurations;
+pub use self::configurations::UserConfigurations;
+pub mod configurations;