Skip to content

Migrate some inner structs to TLVs #928

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
merged 9 commits into from
May 27, 2021
8 changes: 4 additions & 4 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,13 @@ jobs:
id: cache-graph
uses: actions/cache@v2
with:
path: lightning/net_graph-2021-02-12.bin
key: ldk-net_graph-05f0c5a0d772-2020-02-12.bin
path: lightning/net_graph-2021-05-27.bin
key: ldk-net_graph-45d86ead641d-2021-05-27.bin
- name: Fetch routing graph snapshot
if: steps.cache-graph.outputs.cache-hit != 'true'
run: |
wget -O lightning/net_graph-2021-02-12.bin https://bitcoin.ninja/ldk-net_graph-05f0c5a0d772-2020-02-12.bin
if [ "$(sha256sum lightning/net_graph-2021-02-12.bin | awk '{ print $1 }')" != "7116fca78551fedc714a604cec0ad1ca66caa77bb4d0051290258e7a10e0c6e7" ]; then
wget -O lightning/net_graph-2021-05-27.bin https://bitcoin.ninja/ldk-net_graph-45d86ead641d-2021-05-27.bin
if [ "$(sha256sum lightning/net_graph-2021-05-27.bin | awk '{ print $1 }')" != "3d6261187cfa583255d978efb908b51c2f4dc4ad9a7160cd2c5263c9a4830121" ]; then
echo "Bad hash"
exit 1
fi
Expand Down
101 changes: 54 additions & 47 deletions lightning/src/chain/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use bitcoin::hash_types::Txid;
use bitcoin::secp256k1::key::{SecretKey,PublicKey};

use ln::PaymentPreimage;
use ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment, HTLC_OUTPUT_IN_COMMITMENT_SIZE};
use ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment};
use ln::chan_utils;
use ln::msgs::DecodeError;
use chain::chaininterface::{FeeEstimator, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT};
Expand Down Expand Up @@ -86,15 +86,15 @@ impl RevokedOutput {
}
}

impl_writeable!(RevokedOutput, 33*3 + 32 + 8 + 8 + 2, {
per_commitment_point,
counterparty_delayed_payment_base_key,
counterparty_htlc_base_key,
per_commitment_key,
weight,
amount,
on_counterparty_tx_csv
});
impl_writeable_tlv_based!(RevokedOutput, {
(0, per_commitment_point),
(2, counterparty_delayed_payment_base_key),
(4, counterparty_htlc_base_key),
(6, per_commitment_key),
(8, weight),
(10, amount),
(12, on_counterparty_tx_csv),
}, {}, {});

/// A struct to describe a revoked offered output and corresponding information to generate a
/// solving witness.
Expand Down Expand Up @@ -130,15 +130,15 @@ impl RevokedHTLCOutput {
}
}

impl_writeable!(RevokedHTLCOutput, 33*3 + 32 + 8 + 8 + HTLC_OUTPUT_IN_COMMITMENT_SIZE, {
per_commitment_point,
counterparty_delayed_payment_base_key,
counterparty_htlc_base_key,
per_commitment_key,
weight,
amount,
htlc
});
impl_writeable_tlv_based!(RevokedHTLCOutput, {
(0, per_commitment_point),
(2, counterparty_delayed_payment_base_key),
(4, counterparty_htlc_base_key),
(6, per_commitment_key),
(8, weight),
(10, amount),
(12, htlc),
}, {}, {});

/// A struct to describe a HTLC output on a counterparty commitment transaction.
///
Expand Down Expand Up @@ -167,13 +167,13 @@ impl CounterpartyOfferedHTLCOutput {
}
}

impl_writeable!(CounterpartyOfferedHTLCOutput, 33*3 + 32 + HTLC_OUTPUT_IN_COMMITMENT_SIZE, {
per_commitment_point,
counterparty_delayed_payment_base_key,
counterparty_htlc_base_key,
preimage,
htlc
});
impl_writeable_tlv_based!(CounterpartyOfferedHTLCOutput, {
(0, per_commitment_point),
(2, counterparty_delayed_payment_base_key),
(4, counterparty_htlc_base_key),
(6, preimage),
(8, htlc),
}, {}, {});

/// A struct to describe a HTLC output on a counterparty commitment transaction.
///
Expand All @@ -198,12 +198,12 @@ impl CounterpartyReceivedHTLCOutput {
}
}

impl_writeable!(CounterpartyReceivedHTLCOutput, 33*3 + HTLC_OUTPUT_IN_COMMITMENT_SIZE, {
per_commitment_point,
counterparty_delayed_payment_base_key,
counterparty_htlc_base_key,
htlc
});
impl_writeable_tlv_based!(CounterpartyReceivedHTLCOutput, {
(0, per_commitment_point),
(2, counterparty_delayed_payment_base_key),
(4, counterparty_htlc_base_key),
(6, htlc),
}, {}, {});

/// A struct to describe a HTLC output on holder commitment transaction.
///
Expand All @@ -224,10 +224,11 @@ impl HolderHTLCOutput {
}
}

impl_writeable!(HolderHTLCOutput, 0, {
preimage,
amount
});
impl_writeable_tlv_based!(HolderHTLCOutput, {
(0, amount),
}, {
(2, preimage),
}, {});

/// A struct to describe the channel output on the funding transaction.
///
Expand All @@ -245,9 +246,9 @@ impl HolderFundingOutput {
}
}

impl_writeable!(HolderFundingOutput, 0, {
funding_redeemscript
});
impl_writeable_tlv_based!(HolderFundingOutput, {
(0, funding_redeemscript),
}, {}, {});

/// A wrapper encapsulating all in-protocol differing outputs types.
///
Expand Down Expand Up @@ -703,10 +704,11 @@ impl Writeable for PackageTemplate {
outpoint.write(writer)?;
rev_outp.write(writer)?;
}
self.soonest_conf_deadline.write(writer)?;
self.feerate_previous.write(writer)?;
self.height_timer.write(writer)?;
self.height_original.write(writer)?;
write_tlv_fields!(writer, {
(0, self.soonest_conf_deadline),
(2, self.feerate_previous),
(4, self.height_original),
}, { (6, self.height_timer) });
Ok(())
}
}
Expand All @@ -730,10 +732,15 @@ impl Readable for PackageTemplate {
PackageSolvingData::HolderFundingOutput(..) => { (PackageMalleability::Untractable, false) },
}
} else { return Err(DecodeError::InvalidValue); };
let soonest_conf_deadline = Readable::read(reader)?;
let feerate_previous = Readable::read(reader)?;
let height_timer = Readable::read(reader)?;
let height_original = Readable::read(reader)?;
let mut soonest_conf_deadline = 0;
let mut feerate_previous = 0;
let mut height_timer = None;
let mut height_original = 0;
read_tlv_fields!(reader, {
(0, soonest_conf_deadline),
(2, feerate_previous),
(4, height_original)
}, { (6, height_timer) });
Ok(PackageTemplate {
inputs,
malleability,
Expand Down
8 changes: 8 additions & 0 deletions lightning/src/chain/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ impl OutPoint {
vout: self.index as u32,
}
}

/// Creates a dummy BitcoinOutPoint, useful for deserializing into.
pub(crate) fn null() -> Self {
Self {
txid: Default::default(),
index: 0
}
}
Comment on lines +78 to +83
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we consider simply implementing Default? I guess it exposes it outside the crate. Would be nice if traits could be implemented internally only.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea, so initially I went down a rabbit hole on creating a whole trait for this stuff but I eventually just gave up and created a wrapper that deserializes into Options. This one is left because its used outside of the impl_writeable_tlv_based macro, so its kinda a one-off.

Copy link
Contributor

Choose a reason for hiding this comment

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

If a one-off, consider inlining.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I found it to be a bit cleaner, but I'm happy to inline in channelmanager.rs if you prefer?

}

impl_writeable!(OutPoint, 0, { txid, index });
Expand Down
99 changes: 55 additions & 44 deletions lightning/src/ln/chan_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ impl Writeable for CounterpartyCommitmentSecrets {
writer.write_all(secret)?;
writer.write_all(&byte_utils::be64_to_array(*idx))?;
}
write_tlv_fields!(writer, {}, {});
Ok(())
}
}
Expand All @@ -188,7 +189,7 @@ impl Readable for CounterpartyCommitmentSecrets {
*secret = Readable::read(reader)?;
*idx = Readable::read(reader)?;
}

read_tlv_fields!(reader, {}, {});
Ok(Self { old_secrets })
}
}
Expand Down Expand Up @@ -322,8 +323,13 @@ pub struct TxCreationKeys {
pub broadcaster_delayed_payment_key: PublicKey,
}

impl_writeable!(TxCreationKeys, 33*5,
{ per_commitment_point, revocation_key, broadcaster_htlc_key, countersignatory_htlc_key, broadcaster_delayed_payment_key });
impl_writeable_tlv_based!(TxCreationKeys, {
(0, per_commitment_point),
(2, revocation_key),
(4, broadcaster_htlc_key),
(6, countersignatory_htlc_key),
(8, broadcaster_delayed_payment_key),
}, {}, {});

/// One counterparty's public keys which do not change over the life of a channel.
#[derive(Clone, PartialEq)]
Expand All @@ -349,14 +355,13 @@ pub struct ChannelPublicKeys {
pub htlc_basepoint: PublicKey,
}

impl_writeable!(ChannelPublicKeys, 33*5, {
funding_pubkey,
revocation_basepoint,
payment_point,
delayed_payment_basepoint,
htlc_basepoint
});

impl_writeable_tlv_based!(ChannelPublicKeys, {
(0, funding_pubkey),
(2, revocation_basepoint),
(4, payment_point),
(6, delayed_payment_basepoint),
(8, htlc_basepoint),
}, {}, {});

impl TxCreationKeys {
/// Create per-state keys from channel base points and the per-commitment point.
Expand Down Expand Up @@ -429,16 +434,14 @@ pub struct HTLCOutputInCommitment {
pub transaction_output_index: Option<u32>,
}

impl_writeable_len_match!(HTLCOutputInCommitment, {
{ HTLCOutputInCommitment { transaction_output_index: None, .. }, HTLC_OUTPUT_IN_COMMITMENT_SIZE - 4 },
{ _, HTLC_OUTPUT_IN_COMMITMENT_SIZE }
}, {
offered,
amount_msat,
cltv_expiry,
payment_hash,
transaction_output_index
});
impl_writeable_tlv_based!(HTLCOutputInCommitment, {
(0, offered),
(2, amount_msat),
(4, cltv_expiry),
(6, payment_hash),
}, {
(8, transaction_output_index)
}, {});

#[inline]
pub(crate) fn get_htlc_redeemscript_with_explicit_keys(htlc: &HTLCOutputInCommitment, broadcaster_htlc_key: &PublicKey, countersignatory_htlc_key: &PublicKey, revocation_key: &PublicKey) -> Script {
Expand Down Expand Up @@ -622,18 +625,19 @@ impl ChannelTransactionParameters {
}
}

impl_writeable!(CounterpartyChannelTransactionParameters, 0, {
pubkeys,
selected_contest_delay
});
impl_writeable_tlv_based!(CounterpartyChannelTransactionParameters, {
(0, pubkeys),
(2, selected_contest_delay),
}, {}, {});

impl_writeable!(ChannelTransactionParameters, 0, {
holder_pubkeys,
holder_selected_contest_delay,
is_outbound_from_holder,
counterparty_parameters,
funding_outpoint
});
impl_writeable_tlv_based!(ChannelTransactionParameters, {
(0, holder_pubkeys),
(2, holder_selected_contest_delay),
(4, is_outbound_from_holder),
}, {
(6, counterparty_parameters),
(8, funding_outpoint),
}, {});

/// Static channel fields used to build transactions given per-commitment fields, organized by
/// broadcaster/countersignatory.
Expand Down Expand Up @@ -715,8 +719,12 @@ impl PartialEq for HolderCommitmentTransaction {
}
}

impl_writeable!(HolderCommitmentTransaction, 0, {
inner, counterparty_sig, counterparty_htlc_sigs, holder_sig_first
impl_writeable_tlv_based!(HolderCommitmentTransaction, {
(0, inner),
(2, counterparty_sig),
(4, holder_sig_first),
}, {}, {
(6, counterparty_htlc_sigs),
});

impl HolderCommitmentTransaction {
Expand Down Expand Up @@ -800,7 +808,10 @@ pub struct BuiltCommitmentTransaction {
pub txid: Txid,
}

impl_writeable!(BuiltCommitmentTransaction, 0, { transaction, txid });
impl_writeable_tlv_based!(BuiltCommitmentTransaction, {
(0, transaction),
(2, txid)
}, {}, {});

impl BuiltCommitmentTransaction {
/// Get the SIGHASH_ALL sighash value of the transaction.
Expand Down Expand Up @@ -883,15 +894,15 @@ impl Readable for Vec<HTLCOutputInCommitment> {
}
}

impl_writeable!(CommitmentTransaction, 0, {
commitment_number,
to_broadcaster_value_sat,
to_countersignatory_value_sat,
feerate_per_kw,
htlcs,
keys,
built
});
impl_writeable_tlv_based!(CommitmentTransaction, {
(0, commitment_number),
(2, to_broadcaster_value_sat),
(4, to_countersignatory_value_sat),
(6, feerate_per_kw),
(8, htlcs),
(10, keys),
(12, built),
}, {}, {});

impl CommitmentTransaction {
/// Construct an object of the class while assigning transaction output indices to HTLCs.
Expand Down
Loading