Skip to content

Commit

Permalink
Merge pull request #3059 from jkczyz/2024-05-drop-writeable-signer
Browse files Browse the repository at this point in the history
Remove `WriteableEcdsaChannelSigner` trait
  • Loading branch information
TheBlueMatt authored May 11, 2024
2 parents 0ffa4b3 + efc7579 commit d3cd080
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 84 deletions.
24 changes: 12 additions & 12 deletions lightning/src/chain/chainmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, Balance, MonitorEvent, TransactionOutputs, WithChannelMonitor};
use crate::chain::transaction::{OutPoint, TransactionData};
use crate::ln::types::ChannelId;
use crate::sign::ecdsa::WriteableEcdsaChannelSigner;
use crate::sign::ecdsa::EcdsaChannelSigner;
use crate::events;
use crate::events::{Event, EventHandler};
use crate::util::logger::{Logger, WithContext};
Expand Down Expand Up @@ -101,7 +101,7 @@ use bitcoin::secp256k1::PublicKey;
///
/// [`TrustedCommitmentTransaction::revokeable_output_index`]: crate::ln::chan_utils::TrustedCommitmentTransaction::revokeable_output_index
/// [`TrustedCommitmentTransaction::build_to_local_justice_tx`]: crate::ln::chan_utils::TrustedCommitmentTransaction::build_to_local_justice_tx
pub trait Persist<ChannelSigner: WriteableEcdsaChannelSigner> {
pub trait Persist<ChannelSigner: EcdsaChannelSigner> {
/// Persist a new channel's data in response to a [`chain::Watch::watch_channel`] call. This is
/// called by [`ChannelManager`] for new channels, or may be called directly, e.g. on startup.
///
Expand Down Expand Up @@ -163,7 +163,7 @@ pub trait Persist<ChannelSigner: WriteableEcdsaChannelSigner> {
fn archive_persisted_channel(&self, channel_funding_outpoint: OutPoint);
}

struct MonitorHolder<ChannelSigner: WriteableEcdsaChannelSigner> {
struct MonitorHolder<ChannelSigner: EcdsaChannelSigner> {
monitor: ChannelMonitor<ChannelSigner>,
/// The full set of pending monitor updates for this Channel.
///
Expand All @@ -174,7 +174,7 @@ struct MonitorHolder<ChannelSigner: WriteableEcdsaChannelSigner> {
pending_monitor_updates: Mutex<Vec<u64>>,
}

impl<ChannelSigner: WriteableEcdsaChannelSigner> MonitorHolder<ChannelSigner> {
impl<ChannelSigner: EcdsaChannelSigner> MonitorHolder<ChannelSigner> {
fn has_pending_updates(&self, pending_monitor_updates_lock: &MutexGuard<Vec<u64>>) -> bool {
!pending_monitor_updates_lock.is_empty()
}
Expand All @@ -184,12 +184,12 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> MonitorHolder<ChannelSigner> {
///
/// Note that this holds a mutex in [`ChainMonitor`] and may block other events until it is
/// released.
pub struct LockedChannelMonitor<'a, ChannelSigner: WriteableEcdsaChannelSigner> {
pub struct LockedChannelMonitor<'a, ChannelSigner: EcdsaChannelSigner> {
lock: RwLockReadGuard<'a, HashMap<OutPoint, MonitorHolder<ChannelSigner>>>,
funding_txo: OutPoint,
}

impl<ChannelSigner: WriteableEcdsaChannelSigner> Deref for LockedChannelMonitor<'_, ChannelSigner> {
impl<ChannelSigner: EcdsaChannelSigner> Deref for LockedChannelMonitor<'_, ChannelSigner> {
type Target = ChannelMonitor<ChannelSigner>;
fn deref(&self) -> &ChannelMonitor<ChannelSigner> {
&self.lock.get(&self.funding_txo).expect("Checked at construction").monitor
Expand All @@ -212,7 +212,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> Deref for LockedChannelMonitor<
/// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
/// [module-level documentation]: crate::chain::chainmonitor
/// [`rebroadcast_pending_claims`]: Self::rebroadcast_pending_claims
pub struct ChainMonitor<ChannelSigner: WriteableEcdsaChannelSigner, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref>
pub struct ChainMonitor<ChannelSigner: EcdsaChannelSigner, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref>
where C::Target: chain::Filter,
T::Target: BroadcasterInterface,
F::Target: FeeEstimator,
Expand All @@ -236,7 +236,7 @@ pub struct ChainMonitor<ChannelSigner: WriteableEcdsaChannelSigner, C: Deref, T:
event_notifier: Notifier,
}

impl<ChannelSigner: WriteableEcdsaChannelSigner, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> ChainMonitor<ChannelSigner, C, T, F, L, P>
impl<ChannelSigner: EcdsaChannelSigner, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> ChainMonitor<ChannelSigner, C, T, F, L, P>
where C::Target: chain::Filter,
T::Target: BroadcasterInterface,
F::Target: FeeEstimator,
Expand Down Expand Up @@ -623,7 +623,7 @@ where C::Target: chain::Filter,
}
}

impl<ChannelSigner: WriteableEcdsaChannelSigner, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref>
impl<ChannelSigner: EcdsaChannelSigner, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref>
chain::Listen for ChainMonitor<ChannelSigner, C, T, F, L, P>
where
C::Target: chain::Filter,
Expand Down Expand Up @@ -652,7 +652,7 @@ where
}
}

impl<ChannelSigner: WriteableEcdsaChannelSigner, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref>
impl<ChannelSigner: EcdsaChannelSigner, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref>
chain::Confirm for ChainMonitor<ChannelSigner, C, T, F, L, P>
where
C::Target: chain::Filter,
Expand Down Expand Up @@ -706,7 +706,7 @@ where
}
}

impl<ChannelSigner: WriteableEcdsaChannelSigner, C: Deref , T: Deref , F: Deref , L: Deref , P: Deref >
impl<ChannelSigner: EcdsaChannelSigner, C: Deref , T: Deref , F: Deref , L: Deref , P: Deref >
chain::Watch<ChannelSigner> for ChainMonitor<ChannelSigner, C, T, F, L, P>
where C::Target: chain::Filter,
T::Target: BroadcasterInterface,
Expand Down Expand Up @@ -841,7 +841,7 @@ where C::Target: chain::Filter,
}
}

impl<ChannelSigner: WriteableEcdsaChannelSigner, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> events::EventsProvider for ChainMonitor<ChannelSigner, C, T, F, L, P>
impl<ChannelSigner: EcdsaChannelSigner, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> events::EventsProvider for ChainMonitor<ChannelSigner, C, T, F, L, P>
where C::Target: chain::Filter,
T::Target: BroadcasterInterface,
F::Target: FeeEstimator,
Expand Down
30 changes: 15 additions & 15 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use crate::chain;
use crate::chain::{BestBlock, WatchedOutput};
use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator};
use crate::chain::transaction::{OutPoint, TransactionData};
use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, ecdsa::WriteableEcdsaChannelSigner, SignerProvider, EntropySource};
use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, ecdsa::EcdsaChannelSigner, SignerProvider, EntropySource};
use crate::chain::onchaintx::{ClaimEvent, FeerateStrategy, OnchainTxHandler};
use crate::chain::package::{CounterpartyOfferedHTLCOutput, CounterpartyReceivedHTLCOutput, HolderFundingOutput, HolderHTLCOutput, PackageSolvingData, PackageTemplate, RevokedOutput, RevokedHTLCOutput};
use crate::chain::Filter;
Expand Down Expand Up @@ -774,22 +774,22 @@ impl Readable for IrrevocablyResolvedHTLC {
/// the "reorg path" (ie disconnecting blocks until you find a common ancestor from both the
/// returned block hash and the the current chain and then reconnecting blocks to get to the
/// best chain) upon deserializing the object!
pub struct ChannelMonitor<Signer: WriteableEcdsaChannelSigner> {
pub struct ChannelMonitor<Signer: EcdsaChannelSigner> {
#[cfg(test)]
pub(crate) inner: Mutex<ChannelMonitorImpl<Signer>>,
#[cfg(not(test))]
pub(super) inner: Mutex<ChannelMonitorImpl<Signer>>,
}

impl<Signer: WriteableEcdsaChannelSigner> Clone for ChannelMonitor<Signer> where Signer: Clone {
impl<Signer: EcdsaChannelSigner> Clone for ChannelMonitor<Signer> where Signer: Clone {
fn clone(&self) -> Self {
let inner = self.inner.lock().unwrap().clone();
ChannelMonitor::from_impl(inner)
}
}

#[derive(Clone, PartialEq)]
pub(crate) struct ChannelMonitorImpl<Signer: WriteableEcdsaChannelSigner> {
pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
latest_update_id: u64,
commitment_transaction_number_obscure_factor: u64,

Expand Down Expand Up @@ -943,7 +943,7 @@ pub(crate) struct ChannelMonitorImpl<Signer: WriteableEcdsaChannelSigner> {
/// Transaction outputs to watch for on-chain spends.
pub type TransactionOutputs = (Txid, Vec<(u32, TxOut)>);

impl<Signer: WriteableEcdsaChannelSigner> PartialEq for ChannelMonitor<Signer> where Signer: PartialEq {
impl<Signer: EcdsaChannelSigner> PartialEq for ChannelMonitor<Signer> where Signer: PartialEq {
fn eq(&self, other: &Self) -> bool {
// We need some kind of total lockorder. Absent a better idea, we sort by position in
// memory and take locks in that order (assuming that we can't move within memory while a
Expand All @@ -955,7 +955,7 @@ impl<Signer: WriteableEcdsaChannelSigner> PartialEq for ChannelMonitor<Signer> w
}
}

impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitor<Signer> {
impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitor<Signer> {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
self.inner.lock().unwrap().write(writer)
}
Expand All @@ -965,7 +965,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitor<Signer> {
const SERIALIZATION_VERSION: u8 = 1;
const MIN_SERIALIZATION_VERSION: u8 = 1;

impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);

Expand Down Expand Up @@ -1208,11 +1208,11 @@ impl<'a, L: Deref> Logger for WithChannelMonitor<'a, L> where L::Target: Logger
}

impl<'a, L: Deref> WithChannelMonitor<'a, L> where L::Target: Logger {
pub(crate) fn from<S: WriteableEcdsaChannelSigner>(logger: &'a L, monitor: &ChannelMonitor<S>, payment_hash: Option<PaymentHash>) -> Self {
pub(crate) fn from<S: EcdsaChannelSigner>(logger: &'a L, monitor: &ChannelMonitor<S>, payment_hash: Option<PaymentHash>) -> Self {
Self::from_impl(logger, &*monitor.inner.lock().unwrap(), payment_hash)
}

pub(crate) fn from_impl<S: WriteableEcdsaChannelSigner>(logger: &'a L, monitor_impl: &ChannelMonitorImpl<S>, payment_hash: Option<PaymentHash>) -> Self {
pub(crate) fn from_impl<S: EcdsaChannelSigner>(logger: &'a L, monitor_impl: &ChannelMonitorImpl<S>, payment_hash: Option<PaymentHash>) -> Self {
let peer_id = monitor_impl.counterparty_node_id;
let channel_id = Some(monitor_impl.channel_id());
WithChannelMonitor {
Expand All @@ -1221,7 +1221,7 @@ impl<'a, L: Deref> WithChannelMonitor<'a, L> where L::Target: Logger {
}
}

impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
/// For lockorder enforcement purposes, we need to have a single site which constructs the
/// `inner` mutex, otherwise cases where we lock two monitors at the same time (eg in our
/// PartialEq implementation) we may decide a lockorder violation has occurred.
Expand Down Expand Up @@ -1929,7 +1929,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
}
}

impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
/// Helper for get_claimable_balances which does the work for an individual HTLC, generating up
/// to one `Balance` for the HTLC.
fn get_htlc_balance(&self, htlc: &HTLCOutputInCommitment, holder_commitment: bool,
Expand Down Expand Up @@ -2108,7 +2108,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
}

impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
/// Gets the balances in this channel which are either claimable by us if we were to
/// force-close the channel now or which are claimable on-chain (possibly awaiting
/// confirmation).
Expand Down Expand Up @@ -2520,7 +2520,7 @@ pub fn deliberately_bogus_accepted_htlc_witness() -> Vec<Vec<u8>> {
vec![Vec::new(), Vec::new(), Vec::new(), Vec::new(), deliberately_bogus_accepted_htlc_witness_program().into()].into()
}

impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
/// Inserts a revocation secret into this channel monitor. Prunes old preimages if neither
/// needed by holder commitment transactions HTCLs nor by counterparty ones. Unless we haven't already seen
/// counterparty commitment transaction's secret, they are de facto pruned (we can use revocation key).
Expand Down Expand Up @@ -4429,7 +4429,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
}

impl<Signer: WriteableEcdsaChannelSigner, T: Deref, F: Deref, L: Deref> chain::Listen for (ChannelMonitor<Signer>, T, F, L)
impl<Signer: EcdsaChannelSigner, T: Deref, F: Deref, L: Deref> chain::Listen for (ChannelMonitor<Signer>, T, F, L)
where
T::Target: BroadcasterInterface,
F::Target: FeeEstimator,
Expand All @@ -4444,7 +4444,7 @@ where
}
}

impl<Signer: WriteableEcdsaChannelSigner, M, T: Deref, F: Deref, L: Deref> chain::Confirm for (M, T, F, L)
impl<Signer: EcdsaChannelSigner, M, T: Deref, F: Deref, L: Deref> chain::Confirm for (M, T, F, L)
where
M: Deref<Target = ChannelMonitor<Signer>>,
T::Target: BroadcasterInterface,
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/chain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use bitcoin::secp256k1::PublicKey;

use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, MonitorEvent};
use crate::ln::types::ChannelId;
use crate::sign::ecdsa::WriteableEcdsaChannelSigner;
use crate::sign::ecdsa::EcdsaChannelSigner;
use crate::chain::transaction::{OutPoint, TransactionData};
use crate::impl_writeable_tlv_based;

Expand Down Expand Up @@ -257,7 +257,7 @@ pub enum ChannelMonitorUpdateStatus {
/// application crashes.
///
/// See method documentation and [`ChannelMonitorUpdateStatus`] for specific requirements.
pub trait Watch<ChannelSigner: WriteableEcdsaChannelSigner> {
pub trait Watch<ChannelSigner: EcdsaChannelSigner> {
/// Watches a channel identified by `funding_txo` using `monitor`.
///
/// Implementations are responsible for watching the chain for the funding transaction along
Expand Down
21 changes: 9 additions & 12 deletions lightning/src/chain/onchaintx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature};
use bitcoin::secp256k1;

use crate::chain::chaininterface::compute_feerate_sat_per_1000_weight;
use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, ChannelSigner, EntropySource, SignerProvider, ecdsa::WriteableEcdsaChannelSigner};
use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, ChannelSigner, EntropySource, SignerProvider, ecdsa::EcdsaChannelSigner};
use crate::ln::msgs::DecodeError;
use crate::ln::types::PaymentPreimage;
use crate::ln::chan_utils::{self, ChannelTransactionParameters, HTLCOutputInCommitment, HolderCommitmentTransaction};
Expand All @@ -33,7 +33,7 @@ use crate::chain::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER};
use crate::chain::package::{PackageSolvingData, PackageTemplate};
use crate::chain::transaction::MaybeSignedTransaction;
use crate::util::logger::Logger;
use crate::util::ser::{Readable, ReadableArgs, MaybeReadable, UpgradableRequired, Writer, Writeable, VecWriter};
use crate::util::ser::{Readable, ReadableArgs, MaybeReadable, UpgradableRequired, Writer, Writeable};

use crate::io;
use crate::prelude::*;
Expand Down Expand Up @@ -225,7 +225,7 @@ pub(crate) enum FeerateStrategy {
/// OnchainTxHandler receives claiming requests, aggregates them if it's sound, broadcast and
/// do RBF bumping if possible.
#[derive(Clone)]
pub struct OnchainTxHandler<ChannelSigner: WriteableEcdsaChannelSigner> {
pub struct OnchainTxHandler<ChannelSigner: EcdsaChannelSigner> {
channel_value_satoshis: u64,
channel_keys_id: [u8; 32],
destination_script: ScriptBuf,
Expand Down Expand Up @@ -281,7 +281,7 @@ pub struct OnchainTxHandler<ChannelSigner: WriteableEcdsaChannelSigner> {
pub(super) secp_ctx: Secp256k1<secp256k1::All>,
}

impl<ChannelSigner: WriteableEcdsaChannelSigner> PartialEq for OnchainTxHandler<ChannelSigner> {
impl<ChannelSigner: EcdsaChannelSigner> PartialEq for OnchainTxHandler<ChannelSigner> {
fn eq(&self, other: &Self) -> bool {
// `signer`, `secp_ctx`, and `pending_claim_events` are excluded on purpose.
self.channel_value_satoshis == other.channel_value_satoshis &&
Expand All @@ -300,7 +300,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> PartialEq for OnchainTxHandler<
const SERIALIZATION_VERSION: u8 = 1;
const MIN_SERIALIZATION_VERSION: u8 = 1;

impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
pub(crate) fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);

Expand All @@ -312,12 +312,9 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>

self.channel_transaction_parameters.write(writer)?;

let mut key_data = VecWriter(Vec::new());
self.signer.write(&mut key_data)?;
assert!(key_data.0.len() < core::usize::MAX);
assert!(key_data.0.len() < core::u32::MAX as usize);
(key_data.0.len() as u32).write(writer)?;
writer.write_all(&key_data.0[..])?;
// Write a zero-length signer. The data is no longer deserialized as of version 0.0.113 and
// downgrades before version 0.0.113 are no longer supported as of version 0.0.119.
0u32.write(writer)?;

writer.write_all(&(self.pending_claim_requests.len() as u64).to_be_bytes())?;
for (ref ancestor_claim_txid, request) in self.pending_claim_requests.iter() {
Expand Down Expand Up @@ -443,7 +440,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
}
}

impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
pub(crate) fn new(
channel_value_satoshis: u64, channel_keys_id: [u8; 32], destination_script: ScriptBuf,
signer: ChannelSigner, channel_parameters: ChannelTransactionParameters,
Expand Down
Loading

0 comments on commit d3cd080

Please sign in to comment.