Make the base fee configurable in ChannelConfig

Currently the base fee we apply is always the expected cost to
claim an HTLC on-chain in case of closure. This results in
significantly higher than market rate fees [1], and doesn't really
match the actual forwarding trust model anyway - as long as
channel counterparties are honest, our HTLCs shouldn't end up
on-chain no matter what the HTLC sender/recipient do.

While some users may wish to use a feerate that implies they will
not lose funds even if they go to chain (assuming no flood-and-loot
style attacks), they should do so by calculating fees themselves;
since they're already charging well above market-rate,
over-estimating some won't have a large impact.

This commit adds a configuration knob to set the base fee
explicitly, defaulting to 1 sat, which appears to be market-rate
today.

[1] Note that due to an msat-vs-sat bug we currently actually
    charge 1000x *less* than the calculated cost.

Author: TheBlueMatt

lightning-background-processor/src/lib.rs

@@ -239,7 +239,7 @@ mod tests {
 		let mut nodes = Vec::new();
 		for i in 0..num_nodes {
 			let tx_broadcaster = Arc::new(test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))});
-			let fee_estimator = Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 });
+			let fee_estimator = Arc::new(test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) });
 			let chain_source = Arc::new(test_utils::TestChainSource::new(Network::Testnet));
 			let logger = Arc::new(test_utils::TestLogger::with_id(format!("node {}", i)));
 			let persister = Arc::new(FilesystemPersister::new(format!("{}_persister_{}", persist_dir, i)));

lightning/src/chain/channelmonitor.rs

@@ -2846,7 +2846,7 @@ mod tests {
 		let secp_ctx = Secp256k1::new();
 		let logger = Arc::new(TestLogger::new());
 		let broadcaster = Arc::new(TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))});
-		let fee_estimator = Arc::new(TestFeeEstimator { sat_per_kw: 253 });
+		let fee_estimator = Arc::new(TestFeeEstimator { sat_per_kw: Mutex::new(253) });
 
 		let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 		let dummy_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() };

lightning/src/ln/channel.rs

@@ -455,7 +455,6 @@ struct CommitmentTxInfoCached {
 }
 
 pub const OUR_MAX_HTLCS: u16 = 50; //TODO
-const SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT: u64 = 79; // prevout: 36, nSequence: 4, script len: 1, witness lengths: (3+1)/4, sig: 73/4, if-selector: 1, redeemScript: (6 ops + 2*33 pubkeys + 1*2 delay)/4
 
 #[cfg(not(test))]
 const COMMITMENT_TX_BASE_WEIGHT: u64 = 724;
@@ -3477,24 +3476,8 @@ impl<Signer: Sign> Channel<Signer> {
 
 	/// Gets the fee we'd want to charge for adding an HTLC output to this Channel
 	/// Allowed in any state (including after shutdown)
-	pub fn get_holder_fee_base_msat<F: Deref>(&self, fee_estimator: &F) -> u32
-		where F::Target: FeeEstimator
-	{
-		// For lack of a better metric, we calculate what it would cost to consolidate the new HTLC
-		// output value back into a transaction with the regular channel output:
-
-		// the fee cost of the HTLC-Success/HTLC-Timeout transaction:
-		let mut res = self.feerate_per_kw as u64 * cmp::max(HTLC_TIMEOUT_TX_WEIGHT, HTLC_SUCCESS_TX_WEIGHT) / 1000;
-
-		if self.is_outbound() {
-			// + the marginal fee increase cost to us in the commitment transaction:
-			res += self.feerate_per_kw as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC / 1000;
-		}
-
-		// + the marginal cost of an input which spends the HTLC-Success/HTLC-Timeout output:
-		res += fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal) as u64 * SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT / 1000;
-
-		res as u32
+	pub fn get_holder_fee_base_msat(&self) -> u32 {
+		self.config.fee_base_msat
 	}
 
 	/// Returns true if we've ever received a message from the remote end for this Channel

lightning/src/ln/channelmanager.rs

@@ -1540,7 +1540,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 					if *amt_to_forward < chan.get_counterparty_htlc_minimum_msat() { // amount_below_minimum
 						break Some(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11, Some(self.get_channel_update(chan).unwrap())));
 					}
-					let fee = amt_to_forward.checked_mul(chan.get_fee_proportional_millionths() as u64).and_then(|prop_fee| { (prop_fee / 1000000).checked_add(chan.get_holder_fee_base_msat(&self.fee_estimator) as u64) });
+					let fee = amt_to_forward.checked_mul(chan.get_fee_proportional_millionths() as u64)
+						.and_then(|prop_fee| { (prop_fee / 1000000)
+						.checked_add(chan.get_holder_fee_base_msat() as u64) });
 					if fee.is_none() || msg.amount_msat < fee.unwrap() || (msg.amount_msat - fee.unwrap()) < *amt_to_forward { // fee_insufficient
 						break Some(("Prior hop has deviated from specified fees parameters or origin node has obsolete ones", 0x1000 | 12, Some(self.get_channel_update(chan).unwrap())));
 					}
@@ -1605,7 +1607,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 			cltv_expiry_delta: chan.get_cltv_expiry_delta(),
 			htlc_minimum_msat: chan.get_counterparty_htlc_minimum_msat(),
 			htlc_maximum_msat: OptionalField::Present(chan.get_announced_htlc_max_msat()),
-			fee_base_msat: chan.get_holder_fee_base_msat(&self.fee_estimator),
+			fee_base_msat: chan.get_holder_fee_base_msat(),
 			fee_proportional_millionths: chan.get_fee_proportional_millionths(),
 			excess_data: Vec::new(),
 		};

lightning/src/ln/functional_test_utils.rs

@@ -269,7 +269,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
 			// Check that if we serialize and then deserialize all our channel monitors we get the
 			// same set of outputs to watch for on chain as we have now. Note that if we write
 			// tests that fully close channels and remove the monitors at some point this may break.
-			let feeest = test_utils::TestFeeEstimator { sat_per_kw: 253 };
+			let feeest = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
 			let mut deserialized_monitors = Vec::new();
 			{
 				let old_monitors = self.chain_monitor.chain_monitor.monitors.read().unwrap();
@@ -295,7 +295,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
 				<(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut ::std::io::Cursor::new(w.0), ChannelManagerReadArgs {
 					default_config: *self.node.get_current_default_configuration(),
 					keys_manager: self.keys_manager,
-					fee_estimator: &test_utils::TestFeeEstimator { sat_per_kw: 253 },
+					fee_estimator: &test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) },
 					chain_monitor: self.chain_monitor,
 					tx_broadcaster: &test_utils::TestBroadcaster {
 						txn_broadcasted: Mutex::new(self.tx_broadcaster.txn_broadcasted.lock().unwrap().clone()),
@@ -1316,7 +1316,7 @@ pub fn create_chanmon_cfgs(node_count: usize) -> Vec<TestChanMonCfg> {
 			txn_broadcasted: Mutex::new(Vec::new()),
 			blocks: Arc::new(Mutex::new(vec![(genesis_block(Network::Testnet).header, 0)])),
 		};
-		let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 };
+		let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
 		let chain_source = test_utils::TestChainSource::new(Network::Testnet);
 		let logger = test_utils::TestLogger::with_id(format!("node {}", i));
 		let persister = test_utils::TestPersister::new();
@@ -1341,22 +1341,27 @@ pub fn create_node_cfgs<'a>(node_count: usize, chanmon_cfgs: &'a Vec<TestChanMon
 	nodes
 }
 
+pub fn test_default_channel_config() -> UserConfig {
+	let mut default_config = UserConfig::default();
+	// Set cltv_expiry_delta slightly lower to keep the final CLTV values inside one byte in our
+	// tests so that our script-length checks don't fail (see ACCEPTED_HTLC_SCRIPT_WEIGHT).
+	default_config.channel_options.cltv_expiry_delta = 6*6;
+	default_config.channel_options.announced_channel = true;
+	default_config.peer_channel_config_limits.force_announced_channel_preference = false;
+	default_config.own_channel_config.our_htlc_minimum_msat = 1000; // sanitization being done by the sender, to exerce receiver logic we need to lift of limit
+	default_config
+}
+
 pub fn create_node_chanmgrs<'a, 'b>(node_count: usize, cfgs: &'a Vec<NodeCfg<'b>>, node_config: &[Option<UserConfig>]) -> Vec<ChannelManager<EnforcingSigner, &'a TestChainMonitor<'b>, &'b test_utils::TestBroadcaster, &'a test_utils::TestKeysInterface, &'b test_utils::TestFeeEstimator, &'b test_utils::TestLogger>> {
 	let mut chanmgrs = Vec::new();
 	for i in 0..node_count {
-		let mut default_config = UserConfig::default();
-		// Set cltv_expiry_delta slightly lower to keep the final CLTV values inside one byte in our
-		// tests so that our script-length checks don't fail (see ACCEPTED_HTLC_SCRIPT_WEIGHT).
-		default_config.channel_options.cltv_expiry_delta = 6*6;
-		default_config.channel_options.announced_channel = true;
-		default_config.peer_channel_config_limits.force_announced_channel_preference = false;
-		default_config.own_channel_config.our_htlc_minimum_msat = 1000; // sanitization being done by the sender, to exerce receiver logic we need to lift of limit
 		let network = Network::Testnet;
 		let params = ChainParameters {
 			network,
 			best_block: BestBlock::from_genesis(network),
 		};
-		let node = ChannelManager::new(cfgs[i].fee_estimator, &cfgs[i].chain_monitor, cfgs[i].tx_broadcaster, cfgs[i].logger, cfgs[i].keys_manager, if node_config[i].is_some() { node_config[i].clone().unwrap() } else { default_config }, params);
+		let node = ChannelManager::new(cfgs[i].fee_estimator, &cfgs[i].chain_monitor, cfgs[i].tx_broadcaster, cfgs[i].logger, cfgs[i].keys_manager,
+			if node_config[i].is_some() { node_config[i].clone().unwrap() } else { test_default_channel_config() }, params);
 		chanmgrs.push(node);
 	}