Skip to content

Commit

Permalink
Remove OptionalField and move shutdown_scriptpubkey into TLV stream
Browse files Browse the repository at this point in the history 
As pointed out in lightning/bolts@6656b70,
we can move the `shutdown_scriptpubkey` field into the TLV streams of
`OpenChannel` and `AcceptChannel` without affecting the resulting encoding.

We use `option_without_length` here to ensure that we do not encode a
length prefix along with `Script` as is normally the case.
  • Loading branch information
@dunxen
dunxen committed May 1, 2023
1 parent a348209 commit 64aa89c
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 82 deletions.
14 changes: 7 additions & 7 deletions lightning/src/ln/channel.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use bitcoin::secp256k1;
use crate::ln::{PaymentPreimage, PaymentHash};
use crate::ln::features::{ChannelTypeFeatures, InitFeatures};
use crate::ln::msgs;
use crate::ln::msgs::{DecodeError, OptionalField};
use crate::ln::msgs::DecodeError;
use crate::ln::script::{self, ShutdownScript};
use crate::ln::channelmanager::{self, CounterpartyForwardingInfo, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
use crate::ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight, htlc_timeout_tx_weight, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor, ClosingTransaction};
Expand Down Expand Up @@ -1314,7 +1314,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {

let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
match &msg.shutdown_scriptpubkey {
&OptionalField::Present(ref script) => {
&Some(ref script) => {
// Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything
if script.len() == 0 {
None
Expand All @@ -1326,7 +1326,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
}
},
// Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel
&OptionalField::Absent => {
&None => {
return Err(ChannelError::Close("Peer is signaling upfront_shutdown but we don't get any script. Use 0-length script to opt-out".to_owned()));
}
}
Expand Down Expand Up @@ -2191,7 +2191,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {

let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
match &msg.shutdown_scriptpubkey {
&OptionalField::Present(ref script) => {
&Some(ref script) => {
// Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything
if script.len() == 0 {
None
Expand All @@ -2203,7 +2203,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
}
},
// Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel
&OptionalField::Absent => {
&None => {
return Err(ChannelError::Close("Peer is signaling upfront_shutdown but we don't get any script. Use 0-length script to opt-out".to_owned()));
}
}
Expand Down Expand Up @@ -5318,7 +5318,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
htlc_basepoint: keys.htlc_basepoint,
first_per_commitment_point,
channel_flags: if self.config.announced_channel {1} else {0},
shutdown_scriptpubkey: OptionalField::Present(match &self.shutdown_scriptpubkey {
shutdown_scriptpubkey: Some(match &self.shutdown_scriptpubkey {
Some(script) => script.clone().into_inner(),
None => Builder::new().into_script(),
}),
Expand Down Expand Up @@ -5384,7 +5384,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
delayed_payment_basepoint: keys.delayed_payment_basepoint,
htlc_basepoint: keys.htlc_basepoint,
first_per_commitment_point,
shutdown_scriptpubkey: OptionalField::Present(match &self.shutdown_scriptpubkey {
shutdown_scriptpubkey: Some(match &self.shutdown_scriptpubkey {
Some(script) => script.clone().into_inner(),
None => Builder::new().into_script(),
}),
Expand Down
81 changes: 10 additions & 71 deletions lightning/src/ln/msgs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ pub struct OpenChannel {
pub first_per_commitment_point: PublicKey,
/// The channel flags to be used
pub channel_flags: u8,
/// Optionally, a request to pre-set the to-sender output's `scriptPubkey` for when we collaboratively close
pub shutdown_scriptpubkey: OptionalField<Script>,
/// A request to pre-set the to-sender output's `scriptPubkey` for when we collaboratively close
pub shutdown_scriptpubkey: Option<Script>,
/// The channel type that this channel will represent
///
/// If this is `None`, we derive the channel type from the intersection of our
Expand Down Expand Up @@ -241,8 +241,8 @@ pub struct AcceptChannel {
pub htlc_basepoint: PublicKey,
/// The first to-be-broadcast-by-sender transaction's per commitment point
pub first_per_commitment_point: PublicKey,
/// Optionally, a request to pre-set the to-sender output's scriptPubkey for when we collaboratively close
pub shutdown_scriptpubkey: OptionalField<Script>,
/// A request to pre-set the to-sender output's scriptPubkey for when we collaboratively close
pub shutdown_scriptpubkey: Option<Script>,
/// The channel type that this channel will represent.
///
/// If this is `None`, we derive the channel type from the intersection of
Expand Down Expand Up @@ -946,20 +946,6 @@ pub struct CommitmentUpdate {
pub commitment_signed: CommitmentSigned,
}

/// Messages could have optional fields to use with extended features
/// As we wish to serialize these differently from `Option<T>`s (`Options` get a tag byte, but
/// [`OptionalField`] simply gets `Present` if there are enough bytes to read into it), we have a
/// separate enum type for them.
///
/// This is not exported to bindings users due to a free generic in `T`
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum OptionalField<T> {
/// Optional field is included in message
Present(T),
/// Optional field is absent in message
Absent
}

/// A trait to describe an object which can receive channel messages.
///
/// Messages MAY be called in parallel when they originate from different `their_node_ids`, however
Expand Down Expand Up @@ -1255,53 +1241,6 @@ impl From<io::Error> for DecodeError {
}
}

impl Writeable for OptionalField<Script> {
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
match *self {
OptionalField::Present(ref script) => {
// Note that Writeable for script includes the 16-bit length tag for us
script.write(w)?;
},
OptionalField::Absent => {}
}
Ok(())
}
}

impl Readable for OptionalField<Script> {
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
match <u16 as Readable>::read(r) {
Ok(len) => {
let mut buf = vec![0; len as usize];
r.read_exact(&mut buf)?;
Ok(OptionalField::Present(Script::from(buf)))
},
Err(DecodeError::ShortRead) => Ok(OptionalField::Absent),
Err(e) => Err(e)
}
}
}

impl Writeable for OptionalField<u64> {
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
match *self {
OptionalField::Present(ref value) => {
value.write(w)?;
},
OptionalField::Absent => {}
}
Ok(())
}
}

impl Readable for OptionalField<u64> {
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
let value: u64 = Readable::read(r)?;
Ok(OptionalField::Present(value))
}
}

#[cfg(not(taproot))]
impl_writeable_msg!(AcceptChannel, {
temporary_channel_id,
dust_limit_satoshis,
Expand All @@ -1317,8 +1256,8 @@ impl_writeable_msg!(AcceptChannel, {
delayed_payment_basepoint,
htlc_basepoint,
first_per_commitment_point,
shutdown_scriptpubkey
}, {
(0, shutdown_scriptpubkey, option_without_length),
(1, channel_type, option),
});

Expand All @@ -1338,8 +1277,8 @@ impl_writeable_msg!(AcceptChannel, {
delayed_payment_basepoint,
htlc_basepoint,
first_per_commitment_point,
shutdown_scriptpubkey
}, {
(0, shutdown_scriptpubkey, option_without_length),
(1, channel_type, option),
(4, next_local_nonce, option),
});
Expand Down Expand Up @@ -1477,8 +1416,8 @@ impl_writeable_msg!(OpenChannel, {
htlc_basepoint,
first_per_commitment_point,
channel_flags,
shutdown_scriptpubkey
}, {
(0, shutdown_scriptpubkey, option_without_length), // We don't want to encode length twice.
(1, channel_type, option),
});

Expand Down Expand Up @@ -2103,7 +2042,7 @@ mod tests {
use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
use crate::ln::msgs;
use crate::ln::msgs::{FinalOnionHopData, OptionalField, OnionErrorPacket, OnionHopDataFormat};
use crate::ln::msgs::{FinalOnionHopData, OnionErrorPacket, OnionHopDataFormat};
use crate::routing::gossip::{NodeAlias, NodeId};
use crate::util::ser::{Writeable, Readable, Hostname};

Expand Down Expand Up @@ -2422,7 +2361,7 @@ mod tests {
htlc_basepoint: pubkey_5,
first_per_commitment_point: pubkey_6,
channel_flags: if random_bit { 1 << 5 } else { 0 },
shutdown_scriptpubkey: if shutdown { OptionalField::Present(Address::p2pkh(&::bitcoin::PublicKey{compressed: true, inner: pubkey_1}, Network::Testnet).script_pubkey()) } else { OptionalField::Absent },
shutdown_scriptpubkey: if shutdown { Some(Address::p2pkh(&::bitcoin::PublicKey{compressed: true, inner: pubkey_1}, Network::Testnet).script_pubkey()) } else { None },
channel_type: if incl_chan_type { Some(ChannelTypeFeatures::empty()) } else { None },
};
let encoded_value = open_channel.encode();
Expand Down Expand Up @@ -2478,7 +2417,7 @@ mod tests {
delayed_payment_basepoint: pubkey_4,
htlc_basepoint: pubkey_5,
first_per_commitment_point: pubkey_6,
shutdown_scriptpubkey: if shutdown { OptionalField::Present(Address::p2pkh(&::bitcoin::PublicKey{compressed: true, inner: pubkey_1}, Network::Testnet).script_pubkey()) } else { OptionalField::Absent },
shutdown_scriptpubkey: if shutdown { Some(Address::p2pkh(&::bitcoin::PublicKey{compressed: true, inner: pubkey_1}, Network::Testnet).script_pubkey()) } else { None },
channel_type: None,
#[cfg(taproot)]
next_local_nonce: None,
Expand Down
7 changes: 3 additions & 4 deletions lightning/src/ln/shutdown_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ use regex;
use core::default::Default;

use crate::ln::functional_test_utils::*;
use crate::ln::msgs::OptionalField::Present;

#[test]
fn pre_funding_lock_shutdown_test() {
Expand Down Expand Up @@ -517,7 +516,7 @@ fn test_unsupported_anysegwit_upfront_shutdown_script() {
// Check script when handling an open_channel message
nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, None).unwrap();
let mut open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
open_channel.shutdown_scriptpubkey = Present(anysegwit_shutdown_script.clone());
open_channel.shutdown_scriptpubkey = Some(anysegwit_shutdown_script.clone());
nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel);

let events = nodes[1].node.get_and_clear_pending_msg_events();
Expand All @@ -542,7 +541,7 @@ fn test_unsupported_anysegwit_upfront_shutdown_script() {
let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel);
let mut accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
accept_channel.shutdown_scriptpubkey = Present(anysegwit_shutdown_script.clone());
accept_channel.shutdown_scriptpubkey = Some(anysegwit_shutdown_script.clone());
nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel);

let events = nodes[0].node.get_and_clear_pending_msg_events();
Expand All @@ -568,7 +567,7 @@ fn test_invalid_upfront_shutdown_script() {

// Use a segwit v0 script with an unsupported witness program
let mut open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
open_channel.shutdown_scriptpubkey = Present(Builder::new().push_int(0)
open_channel.shutdown_scriptpubkey = Some(Builder::new().push_int(0)
.push_slice(&[0, 0])
.into_script());
nodes[0].node.handle_open_channel(&nodes[1].node.get_our_node_id(), &open_channel);
Expand Down

0 comments on commit 64aa89c

Please sign in to comment.