Replace config max counterpary dust_limit_satoshis by a constant.

Current Bitcoin Core's policy will reject a p2wsh as a dust if it's
under 330 satoshis. A typical p2wsh output is 43 bytes big to which
Core's `GetDustThreshold()` sums up a minimal spend of 67 bytes (even
if a p2wsh witnessScript might be smaller). `dustRelayFee` is set
to 3000 sat/kb, thus 110 * 3000 / 1000 = 330. As all time-sensitive
outputs are p2wsh, a value of 330 sat is the lower bound desired
to ensure good propagation of transactions. We give a bit margin to
our counterparty and pick up 660 satoshis as an accepted
`dust_limit_satoshis` upper bound.

As this reasoning is tricky and error-prone we hardcode it instead of
letting the user picking up a non-sense value.

Further, this lower bound of 330 sats is also hardcoded as another constant
(MIN_DUST_LIMIT_SATOSHIS) instead of being dynamically computed on
feerate (derive_holder_dust_limit_satoshis`). Reducing risks of
non-propagating transactions in casee of failing fee festimation.

Author: Antoine Riard

fuzz/src/chanmon_consistency.rs

@@ -319,7 +319,6 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 			let mut config = UserConfig::default();
 			config.channel_options.fee_proportional_millionths = 0;
 			config.channel_options.announced_channel = true;
-			config.peer_channel_config_limits.min_dust_limit_satoshis = 0;
 			let network = Network::Bitcoin;
 			let params = ChainParameters {
 				network,
@@ -339,7 +338,6 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 			let mut config = UserConfig::default();
 			config.channel_options.fee_proportional_millionths = 0;
 			config.channel_options.announced_channel = true;
-			config.peer_channel_config_limits.min_dust_limit_satoshis = 0;
 
 			let mut monitors = HashMap::new();
 			let mut old_monitors = $old_monitors.latest_monitors.lock().unwrap();

fuzz/src/full_stack.rs

@@ -438,7 +438,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
-const B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT: u64 = 104; // prevout: 40, nSequence: 4, script len: 1, witness lengths: 3/4, sig: 73/4, pubkey: 33/4, output: 31 (TODO: Wrong? Useless?)
 
 #[cfg(not(test))]
 const COMMITMENT_TX_BASE_WEIGHT: u64 = 724;
@@ -453,6 +452,22 @@ pub const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172;
 /// it's 2^24.
 pub const MAX_FUNDING_SATOSHIS: u64 = 1 << 24;
 
+/// Maximum counterparty `dust_limit_satoshis` allowed. 2 * standard dust threshold on p2wsh output
+/// Scales up on Bitcoin Core's proceeding policy with dust outputs. A typical p2wsh output is 43
+/// bytes big to which Core's `GetDustThreshold()` sums up a minimal spend of 67 bytes (even if
+/// a p2wsh witnessScript might be *effectively* smaller), `dustRelayFee` is set to 3000sat/kb, thus
+/// 110 * 3000 / 1000 = 330. Per-protocol rules, all time-sensitive outputs are p2wsh, a value of
+/// 330 sats is the lower bound desired to ensure good propagation of transactions. We give a bit
+/// of margin to our counterparty and pick up 660 satoshis as an accepted `dust_limit_satoshis`
+/// upper bound to avoid negotiation conflicts with other implementations.
+pub const MAX_DUST_LIMIT_SATOSHIS: u64 = 2 * 330;
+
+/// A typical p2wsh output is 43 bytes big to which Core's `GetDustThreshold()` sums up a minimal
+/// spend of 67 bytes (even if a p2wsh witnessScript might be *effectively* smaller), `dustRelayFee`
+/// is set to 3000sat/kb, thus 110 * 3000 / 1000 = 330. Per-protocol rules, all time-sensitive outputs
+/// are p2wsh, a value of 330 sats is the lower bound desired to ensure good propagation of transactions.
+pub const MIN_DUST_LIMIT_SATOSHIS: u64 = 330;
+
 /// Used to return a simple Error back to ChannelManager. Will get converted to a
 /// msgs::ErrorAction::SendErrorMessage or msgs::ErrorAction::IgnoreError as appropriate with our
 /// channel_id in ChannelManager.
@@ -496,10 +511,6 @@ impl<Signer: Sign> Channel<Signer> {
 		cmp::min(channel_value_satoshis, cmp::max(q, 1000)) //TODO
 	}
 
-	fn derive_holder_dust_limit_satoshis(at_open_background_feerate: u32) -> u64 {
-		cmp::max(at_open_background_feerate as u64 * B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT / 1000, 546) //TODO
-	}
-
 	// Constructors:
 	pub fn new_outbound<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig) -> Result<Channel<Signer>, APIError>
 	where K::Target: KeysInterface<Signer = Signer>,
@@ -519,9 +530,9 @@ impl<Signer: Sign> Channel<Signer> {
 		if holder_selected_contest_delay < BREAKDOWN_TIMEOUT {
 			return Err(APIError::APIMisuseError {err: format!("Configured with an unreasonable our_to_self_delay ({}) putting user funds at risks", holder_selected_contest_delay)});
 		}
-		let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
-		if Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(channel_value_satoshis) < Channel::<Signer>::derive_holder_dust_limit_satoshis(background_feerate) {
-			return Err(APIError::FeeRateTooHigh{err: format!("Not enough reserve above dust limit can be found at current fee rate({})", background_feerate), feerate: background_feerate});
+		let holder_selected_channel_reserve_satoshis = Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(channel_value_satoshis);
+		if holder_selected_channel_reserve_satoshis < MIN_DUST_LIMIT_SATOSHIS {
+			return Err(APIError::APIMisuseError { err: format!("Holder selected channel  reserve below implemention limit dust_limit_satoshis {}", holder_selected_channel_reserve_satoshis) });
 		}
 
 		let feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
@@ -578,7 +589,7 @@ impl<Signer: Sign> Channel<Signer> {
 
 			feerate_per_kw: feerate,
 			counterparty_dust_limit_satoshis: 0,
-			holder_dust_limit_satoshis: Channel::<Signer>::derive_holder_dust_limit_satoshis(background_feerate),
+			holder_dust_limit_satoshis: MIN_DUST_LIMIT_SATOSHIS,
 			counterparty_max_htlc_value_in_flight_msat: 0,
 			counterparty_selected_channel_reserve_satoshis: 0,
 			counterparty_htlc_minimum_msat: 0,
@@ -699,11 +710,11 @@ impl<Signer: Sign> Channel<Signer> {
 		if msg.max_accepted_htlcs < config.peer_channel_config_limits.min_max_accepted_htlcs {
 			return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.max_accepted_htlcs, config.peer_channel_config_limits.min_max_accepted_htlcs)));
 		}
-		if msg.dust_limit_satoshis < config.peer_channel_config_limits.min_dust_limit_satoshis {
-			return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the user specified limit ({})", msg.dust_limit_satoshis, config.peer_channel_config_limits.min_dust_limit_satoshis)));
+		if msg.dust_limit_satoshis < MIN_DUST_LIMIT_SATOSHIS {
+			return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the implementation limit ({})", msg.dust_limit_satoshis, MIN_DUST_LIMIT_SATOSHIS)));
 		}
-		if msg.dust_limit_satoshis > config.peer_channel_config_limits.max_dust_limit_satoshis {
-			return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the user specified limit ({})", msg.dust_limit_satoshis, config.peer_channel_config_limits.max_dust_limit_satoshis)));
+		if msg.dust_limit_satoshis >  MAX_DUST_LIMIT_SATOSHIS {
+			return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the implementation limit ({})", msg.dust_limit_satoshis, MAX_DUST_LIMIT_SATOSHIS)));
 		}
 
 		// Convert things into internal flags and prep our state:
@@ -719,13 +730,12 @@ impl<Signer: Sign> Channel<Signer> {
 
 		let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
 
-		let holder_dust_limit_satoshis = Channel::<Signer>::derive_holder_dust_limit_satoshis(background_feerate);
 		let holder_selected_channel_reserve_satoshis = Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(msg.funding_satoshis);
-		if holder_selected_channel_reserve_satoshis < holder_dust_limit_satoshis {
-			return Err(ChannelError::Close(format!("Suitable channel reserve not found. remote_channel_reserve was ({}). dust_limit_satoshis is ({}).", holder_selected_channel_reserve_satoshis, holder_dust_limit_satoshis)));
+		if holder_selected_channel_reserve_satoshis < MIN_DUST_LIMIT_SATOSHIS {
+			return Err(ChannelError::Close(format!("Suitable channel reserve not found. remote_channel_reserve was ({}). dust_limit_satoshis is ({}).", holder_selected_channel_reserve_satoshis, MIN_DUST_LIMIT_SATOSHIS)));
 		}
-		if msg.channel_reserve_satoshis < holder_dust_limit_satoshis {
-			return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is smaller than our dust limit ({})", msg.channel_reserve_satoshis, holder_dust_limit_satoshis)));
+		if msg.channel_reserve_satoshis < MIN_DUST_LIMIT_SATOSHIS {
+			return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is smaller than our dust limit ({})", msg.channel_reserve_satoshis, MIN_DUST_LIMIT_SATOSHIS)));
 		}
 		if holder_selected_channel_reserve_satoshis < msg.dust_limit_satoshis {
 			return Err(ChannelError::Close(format!("Dust limit ({}) too high for the channel reserve we require the remote to keep ({})", msg.dust_limit_satoshis, holder_selected_channel_reserve_satoshis)));
@@ -817,7 +827,7 @@ impl<Signer: Sign> Channel<Signer> {
 			feerate_per_kw: msg.feerate_per_kw,
 			channel_value_satoshis: msg.funding_satoshis,
 			counterparty_dust_limit_satoshis: msg.dust_limit_satoshis,
-			holder_dust_limit_satoshis,
+			holder_dust_limit_satoshis: MIN_DUST_LIMIT_SATOSHIS,
 			counterparty_max_htlc_value_in_flight_msat: cmp::min(msg.max_htlc_value_in_flight_msat, msg.funding_satoshis * 1000),
 			counterparty_selected_channel_reserve_satoshis: msg.channel_reserve_satoshis,
 			counterparty_htlc_minimum_msat: msg.htlc_minimum_msat,
@@ -1441,11 +1451,11 @@ impl<Signer: Sign> Channel<Signer> {
 		if msg.max_accepted_htlcs < config.peer_channel_config_limits.min_max_accepted_htlcs {
 			return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.max_accepted_htlcs, config.peer_channel_config_limits.min_max_accepted_htlcs)));
 		}
-		if msg.dust_limit_satoshis < config.peer_channel_config_limits.min_dust_limit_satoshis {
-			return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the user specified limit ({})", msg.dust_limit_satoshis, config.peer_channel_config_limits.min_dust_limit_satoshis)));
+		if msg.dust_limit_satoshis < MIN_DUST_LIMIT_SATOSHIS {
+			return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the implementation limit ({})", msg.dust_limit_satoshis, MIN_DUST_LIMIT_SATOSHIS)));
 		}
-		if msg.dust_limit_satoshis > config.peer_channel_config_limits.max_dust_limit_satoshis {
-			return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the user specified limit ({})", msg.dust_limit_satoshis, config.peer_channel_config_limits.max_dust_limit_satoshis)));
+		if msg.dust_limit_satoshis > MAX_DUST_LIMIT_SATOSHIS {
+			return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the implementation limit ({})", msg.dust_limit_satoshis, MAX_DUST_LIMIT_SATOSHIS)));
 		}
 		if msg.minimum_depth > config.peer_channel_config_limits.max_minimum_depth {
 			return Err(ChannelError::Close(format!("We consider the minimum depth to be unreasonably large. Expected minimum: ({}). Actual: ({})", config.peer_channel_config_limits.max_minimum_depth, msg.minimum_depth)));
@@ -4935,14 +4945,14 @@ mod tests {
 		// Create Node B's channel by receiving Node A's open_channel message
 		// Make sure A's dust limit is as we expect.
 		let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash());
-		assert_eq!(open_channel_msg.dust_limit_satoshis, 1560);
 		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
 		let node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), &open_channel_msg, 7, &config).unwrap();
 
 		// Node B --> Node A: accept channel, explicitly setting B's dust limit.
 		let mut accept_channel_msg = node_b_chan.get_accept_channel();
 		accept_channel_msg.dust_limit_satoshis = 546;
 		node_a_chan.accept_channel(&accept_channel_msg, &config, InitFeatures::known()).unwrap();
+		node_a_chan.holder_dust_limit_satoshis = 1560;
 
 		// Put some inbound and outbound HTLCs in A's channel.
 		let htlc_amount_msat = 11_092_000; // put an amount below A's effective dust limit but above B's.

lightning/src/ln/channel.rs

@@ -1792,7 +1792,7 @@ fn test_chan_reserve_dust_inbound_htlcs_outbound_chan() {
 	// transaction fee with 0 HTLCs (183 sats)).
 	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 98817000, InitFeatures::known(), InitFeatures::known());
 
-	let dust_amt = 546000; // Dust amount
+	let dust_amt = 329000; // Dust amount
 	// In the previous code, routing this dust payment would cause nodes[0] to perceive a channel
 	// reserve violation even though it's a dust HTLC and therefore shouldn't count towards the
 	// commitment transaction fee.

lightning/src/ln/functional_tests.rs

@@ -98,22 +98,6 @@ pub struct ChannelHandshakeLimits {
 	///
 	/// Default value: 0.
 	pub min_max_accepted_htlcs: u16,
-	/// Outputs below a certain value will not be added to on-chain transactions. The dust value is
-	/// required to always be higher than this value so this only applies to HTLC outputs (and
-	/// potentially to-self outputs before any payments have been made).
-	/// Thus, HTLCs below this amount plus HTLC transaction fees are not enforceable on-chain.
-	/// This setting allows you to set a minimum dust limit for their commitment transactions,
-	/// reflecting the reality that tiny outputs are not considered standard transactions and will
-	/// not propagate through the Bitcoin network.
-	///
-	/// Default value: 546, the current dust limit on the Bitcoin network.
-	pub min_dust_limit_satoshis: u64,
-	/// Maximum allowed threshold above which outputs will not be generated in their commitment
-	/// transactions.
-	/// HTLCs below this amount plus HTLC transaction fees are not enforceable on-chain.
-	///
-	/// Default value: u64::max_value.
-	pub max_dust_limit_satoshis: u64,
 	/// Before a channel is usable the funding transaction will need to be confirmed by at least a
 	/// certain number of blocks, specified by the node which is not the funder (as the funder can
 	/// assume they aren't going to double-spend themselves).
@@ -145,8 +129,6 @@ impl Default for ChannelHandshakeLimits {
 			min_max_htlc_value_in_flight_msat: 0,
 			max_channel_reserve_satoshis: <u64>::max_value(),
 			min_max_accepted_htlcs: 0,
-			min_dust_limit_satoshis: 546,
-			max_dust_limit_satoshis: <u64>::max_value(),
 			max_minimum_depth: 144,
 			force_announced_channel_preference: true,
 			their_to_self_delay: MAX_LOCAL_BREAKDOWN_TIMEOUT,