Skip to content

Re-add support for non-zero-fee-anchors to chan_utils #1828

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
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
4 changes: 2 additions & 2 deletions fuzz/src/full_stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ impl KeysInterface for KeyProvider {
SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, ctr]).unwrap(),
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, ctr],
channel_value_satoshis,
[0; 32]
[0; 32],
)
} else {
InMemorySigner::new(
Expand All @@ -324,7 +324,7 @@ impl KeysInterface for KeyProvider {
SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 11, ctr]).unwrap(),
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 12, ctr],
channel_value_satoshis,
[0; 32]
[0; 32],
)
})
}
Expand Down
3 changes: 2 additions & 1 deletion lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4044,7 +4044,7 @@ mod tests {
SecretKey::from_slice(&[41; 32]).unwrap(),
[41; 32],
0,
[0; 32]
[0; 32],
);

let counterparty_pubkeys = ChannelPublicKeys {
Expand All @@ -4065,6 +4065,7 @@ mod tests {
}),
funding_outpoint: Some(funding_outpoint),
opt_anchors: None,
opt_non_zero_fee_anchors: None,
};
// Prune with one old state and a holder commitment tx holding a few overlaps with the
// old state.
Expand Down
8 changes: 5 additions & 3 deletions lightning/src/chain/keysinterface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,8 @@ impl InMemorySigner {
htlc_base_key: SecretKey,
commitment_seed: [u8; 32],
channel_value_satoshis: u64,
channel_keys_id: [u8; 32]) -> InMemorySigner {
channel_keys_id: [u8; 32],
) -> InMemorySigner {
let holder_channel_pubkeys =
InMemorySigner::make_holder_keys(secp_ctx, &funding_key, &revocation_base_key,
&payment_key, &delayed_payment_base_key,
Expand Down Expand Up @@ -704,7 +705,8 @@ impl BaseSign for InMemorySigner {

let mut htlc_sigs = Vec::with_capacity(commitment_tx.htlcs().len());
for htlc in commitment_tx.htlcs() {
let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_tx.feerate_per_kw(), self.holder_selected_contest_delay(), htlc, self.opt_anchors(), &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
let channel_parameters = self.get_channel_parameters();
let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_tx.feerate_per_kw(), self.holder_selected_contest_delay(), htlc, self.opt_anchors(), channel_parameters.opt_non_zero_fee_anchors.is_some(), &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, self.opt_anchors(), &keys);
let htlc_sighashtype = if self.opt_anchors() { EcdsaSighashType::SinglePlusAnyoneCanPay } else { EcdsaSighashType::All };
let htlc_sighash = hash_to_message!(&sighash::SighashCache::new(&htlc_tx).segwit_signature_hash(0, &htlc_redeemscript, htlc.amount_msat / 1000, htlc_sighashtype).unwrap()[..]);
Expand Down Expand Up @@ -1035,7 +1037,7 @@ impl KeysManager {
htlc_base_key,
commitment_seed,
channel_value_satoshis,
params.clone()
params.clone(),
)
}

Expand Down
33 changes: 26 additions & 7 deletions lightning/src/ln/chan_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ pub fn make_funding_redeemscript(broadcaster: &PublicKey, countersignatory: &Pub
///
/// Panics if htlc.transaction_output_index.is_none() (as such HTLCs do not appear in the
/// commitment transaction).
pub fn build_htlc_transaction(commitment_txid: &Txid, feerate_per_kw: u32, contest_delay: u16, htlc: &HTLCOutputInCommitment, opt_anchors: bool, broadcaster_delayed_payment_key: &PublicKey, revocation_key: &PublicKey) -> Transaction {
pub fn build_htlc_transaction(commitment_txid: &Txid, feerate_per_kw: u32, contest_delay: u16, htlc: &HTLCOutputInCommitment, opt_anchors: bool, use_non_zero_fee_anchors: bool, broadcaster_delayed_payment_key: &PublicKey, revocation_key: &PublicKey) -> Transaction {
let mut txins: Vec<TxIn> = Vec::new();
txins.push(TxIn {
previous_output: OutPoint {
Expand All @@ -677,7 +677,7 @@ pub fn build_htlc_transaction(commitment_txid: &Txid, feerate_per_kw: u32, conte
} else {
htlc_success_tx_weight(opt_anchors)
};
let output_value = if opt_anchors {
let output_value = if opt_anchors && !use_non_zero_fee_anchors {
htlc.amount_msat / 1000
} else {
let total_fee = feerate_per_kw as u64 * weight / 1000;
Expand Down Expand Up @@ -765,7 +765,11 @@ pub struct ChannelTransactionParameters {
pub funding_outpoint: Option<chain::transaction::OutPoint>,
/// Are anchors (zero fee HTLC transaction variant) used for this channel. Boolean is
/// serialization backwards-compatible.
pub opt_anchors: Option<()>
pub opt_anchors: Option<()>,
/// Are non-zero-fee anchors are enabled (used in conjuction with opt_anchors)
/// It is intended merely for backwards compatibility with signers that need it.
/// There is no support for this feature in LDK channel negotiation.
pub opt_non_zero_fee_anchors: Option<()>,
}

/// Late-bound per-channel counterparty data used to build transactions.
Expand Down Expand Up @@ -820,6 +824,7 @@ impl_writeable_tlv_based!(ChannelTransactionParameters, {
(6, counterparty_parameters, option),
(8, funding_outpoint, option),
(10, opt_anchors, option),
(12, opt_non_zero_fee_anchors, option),
});

/// Static channel fields used to build transactions given per-commitment fields, organized by
Expand Down Expand Up @@ -942,7 +947,8 @@ impl HolderCommitmentTransaction {
is_outbound_from_holder: false,
counterparty_parameters: Some(CounterpartyChannelTransactionParameters { pubkeys: channel_pubkeys.clone(), selected_contest_delay: 0 }),
funding_outpoint: Some(chain::transaction::OutPoint { txid: Txid::all_zeros(), index: 0 }),
opt_anchors: None
opt_anchors: None,
opt_non_zero_fee_anchors: None,
};
let mut htlcs_with_aux: Vec<(_, ())> = Vec::new();
let inner = CommitmentTransaction::new_with_auxiliary_htlc_data(0, 0, 0, false, dummy_key.clone(), dummy_key.clone(), keys, 0, &mut htlcs_with_aux, &channel_parameters.as_counterparty_broadcastable());
Expand Down Expand Up @@ -1160,6 +1166,8 @@ pub struct CommitmentTransaction {
htlcs: Vec<HTLCOutputInCommitment>,
// A boolean that is serialization backwards-compatible
opt_anchors: Option<()>,
// Whether non-zero-fee anchors should be used
opt_non_zero_fee_anchors: Option<()>,
// A cache of the parties' pubkeys required to construct the transaction, see doc for trust()
keys: TxCreationKeys,
// For access to the pre-built transaction, see doc for trust()
Expand Down Expand Up @@ -1193,6 +1201,7 @@ impl_writeable_tlv_based!(CommitmentTransaction, {
(10, built, required),
(12, htlcs, vec_type),
(14, opt_anchors, option),
(16, opt_non_zero_fee_anchors, option),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alternatively, I can create a const_false "TLV" directive for the macros, so that nothing is actually serialized for this field.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this either needs to be an odd type or an unwrap_or(false) such that we can read it on new versions that didn't previously serialize this field.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so? I think it should be even because old versions should reject it - technically this means a non-forwards-compatible change on VLS' end, but I assume that's okay, it shouldn't matter for LDK users.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am doing self.opt_non_zero_fee_anchors.is_some() instead of self.opt_non_zero_fee_anchors.unwrap_or(false), but they are equivalent.

if it was serialized by an old version, the field won't exist, and that will be interpreted as None.

so this should work as written

});

impl CommitmentTransaction {
Expand Down Expand Up @@ -1225,9 +1234,18 @@ impl CommitmentTransaction {
transaction,
txid
},
opt_non_zero_fee_anchors: None,
}
}

/// Use non-zero fee anchors
///
/// (C-not exported) due to move, and also not likely to be useful for binding users
pub fn with_non_zero_fee_anchors(mut self) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs: (C-not exported) since bindings don't support move semantics
cc @TheBlueMatt

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

self.opt_non_zero_fee_anchors = Some(());
self
}

fn internal_rebuild_transaction(&self, keys: &TxCreationKeys, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_funding_key: &PublicKey, countersignatory_funding_key: &PublicKey) -> Result<BuiltCommitmentTransaction, ()> {
let (obscured_commitment_transaction_number, txins) = Self::internal_build_inputs(self.commitment_number, channel_parameters);

Expand Down Expand Up @@ -1492,7 +1510,7 @@ impl<'a> TrustedCommitmentTransaction<'a> {

for this_htlc in inner.htlcs.iter() {
assert!(this_htlc.transaction_output_index.is_some());
let htlc_tx = build_htlc_transaction(&txid, inner.feerate_per_kw, channel_parameters.contest_delay(), &this_htlc, self.opt_anchors(), &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
let htlc_tx = build_htlc_transaction(&txid, inner.feerate_per_kw, channel_parameters.contest_delay(), &this_htlc, self.opt_anchors(), self.opt_non_zero_fee_anchors.is_some(), &keys.broadcaster_delayed_payment_key, &keys.revocation_key);

let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc, self.opt_anchors(), &keys.broadcaster_htlc_key, &keys.countersignatory_htlc_key, &keys.revocation_key);

Expand All @@ -1514,7 +1532,7 @@ impl<'a> TrustedCommitmentTransaction<'a> {
// Further, we should never be provided the preimage for an HTLC-Timeout transaction.
if this_htlc.offered && preimage.is_some() { unreachable!(); }

let mut htlc_tx = build_htlc_transaction(&txid, inner.feerate_per_kw, channel_parameters.contest_delay(), &this_htlc, self.opt_anchors(), &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
let mut htlc_tx = build_htlc_transaction(&txid, inner.feerate_per_kw, channel_parameters.contest_delay(), &this_htlc, self.opt_anchors(), self.opt_non_zero_fee_anchors.is_some(), &keys.broadcaster_delayed_payment_key, &keys.revocation_key);

let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc, self.opt_anchors(), &keys.broadcaster_htlc_key, &keys.countersignatory_htlc_key, &keys.revocation_key);

Expand Down Expand Up @@ -1614,7 +1632,8 @@ mod tests {
is_outbound_from_holder: false,
counterparty_parameters: Some(CounterpartyChannelTransactionParameters { pubkeys: counterparty_pubkeys.clone(), selected_contest_delay: 0 }),
funding_outpoint: Some(chain::transaction::OutPoint { txid: Txid::all_zeros(), index: 0 }),
opt_anchors: None
opt_anchors: None,
opt_non_zero_fee_anchors: None,
};

let mut htlcs_with_aux: Vec<(_, ())> = Vec::new();
Expand Down
10 changes: 6 additions & 4 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1038,6 +1038,7 @@ impl<Signer: Sign> Channel<Signer> {
counterparty_parameters: None,
funding_outpoint: None,
opt_anchors: if opt_anchors { Some(()) } else { None },
opt_non_zero_fee_anchors: None
},
funding_transaction: None,

Expand Down Expand Up @@ -1383,6 +1384,7 @@ impl<Signer: Sign> Channel<Signer> {
}),
funding_outpoint: None,
opt_anchors: if opt_anchors { Some(()) } else { None },
opt_non_zero_fee_anchors: None
},
funding_transaction: None,

Expand Down Expand Up @@ -3016,7 +3018,7 @@ impl<Signer: Sign> Channel<Signer> {
if let Some(_) = htlc.transaction_output_index {
let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_stats.feerate_per_kw,
self.get_counterparty_selected_contest_delay().unwrap(), &htlc, self.opt_anchors(),
&keys.broadcaster_delayed_payment_key, &keys.revocation_key);
false, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);

let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, self.opt_anchors(), &keys);
let htlc_sighashtype = if self.opt_anchors() { EcdsaSighashType::SinglePlusAnyoneCanPay } else { EcdsaSighashType::All };
Expand Down Expand Up @@ -5743,7 +5745,7 @@ impl<Signer: Sign> Channel<Signer> {

for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(htlcs) {
log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {} in channel {}",
encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_txid, commitment_stats.feerate_per_kw, self.get_holder_selected_contest_delay(), htlc, self.opt_anchors(), &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)),
encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_txid, commitment_stats.feerate_per_kw, self.get_holder_selected_contest_delay(), htlc, self.opt_anchors(), false, &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)),
encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, self.opt_anchors(), &counterparty_keys)),
log_bytes!(counterparty_keys.broadcaster_htlc_key.serialize()),
log_bytes!(htlc_sig.serialize_compact()[..]), log_bytes!(self.channel_id()));
Expand Down Expand Up @@ -7250,7 +7252,7 @@ mod tests {
// These aren't set in the test vectors:
[0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff],
10_000_000,
[0; 32]
[0; 32],
);

assert_eq!(signer.pubkeys().funding_pubkey.serialize()[..],
Expand Down Expand Up @@ -7368,7 +7370,7 @@ mod tests {
let ref htlc = htlcs[$htlc_idx];
let htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.feerate_per_kw,
chan.get_counterparty_selected_contest_delay().unwrap(),
&htlc, $opt_anchors, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
&htlc, $opt_anchors, false, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, $opt_anchors, &keys);
let htlc_sighashtype = if $opt_anchors { EcdsaSighashType::SinglePlusAnyoneCanPay } else { EcdsaSighashType::All };
let htlc_sighash = Message::from_slice(&sighash::SighashCache::new(&htlc_tx).segwit_signature_hash(0, &htlc_redeemscript, htlc.amount_msat / 1000, htlc_sighashtype).unwrap()[..]).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/util/enforcing_trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl BaseSign for EnforcingSigner {
for (this_htlc, sig) in trusted_tx.htlcs().iter().zip(&commitment_tx.counterparty_htlc_sigs) {
assert!(this_htlc.transaction_output_index.is_some());
let keys = trusted_tx.keys();
let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, trusted_tx.feerate_per_kw(), holder_csv, &this_htlc, self.opt_anchors(), &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, trusted_tx.feerate_per_kw(), holder_csv, &this_htlc, self.opt_anchors(), false, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);

let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&this_htlc, self.opt_anchors(), &keys);

Expand Down