Merge pull request #1282 from TheBlueMatt/2022-01-fuzz-overflow
authorvalentinewallace <valentinewallace@users.noreply.github.com>
Thu, 27 Jan 2022 16:42:05 +0000 (11:42 -0500)
committerGitHub <noreply@github.com>
Thu, 27 Jan 2022 16:42:05 +0000 (11:42 -0500)
Avoid overflow in addition when checking counterparty feerates

15 files changed:
fuzz/src/chanmon_consistency.rs
fuzz/src/full_stack.rs
lightning-invoice/src/de.rs
lightning-invoice/src/lib.rs
lightning/src/chain/channelmonitor.rs
lightning/src/chain/keysinterface.rs
lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/peer_handler.rs
lightning/src/routing/router.rs
lightning/src/util/enforcing_trait_impls.rs
lightning/src/util/test_utils.rs

index ef0ae1da82094a9c79812911947a5db9cf428042..8c11ba2c6c712aba66604a62861416dbd3a551e5 100644 (file)
@@ -188,6 +188,7 @@ impl KeysInterface for KeyProvider {
                let id = self.rand_bytes_id.fetch_add(1, atomic::Ordering::Relaxed);
                let keys = InMemorySigner::new(
                        &secp_ctx,
+                       self.get_node_secret(),
                        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, 4, self.node_id]).unwrap(),
                        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, self.node_id]).unwrap(),
                        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, 6, self.node_id]).unwrap(),
@@ -211,7 +212,7 @@ impl KeysInterface for KeyProvider {
        fn read_chan_signer(&self, buffer: &[u8]) -> Result<Self::Signer, DecodeError> {
                let mut reader = std::io::Cursor::new(buffer);
 
-               let inner: InMemorySigner = Readable::read(&mut reader)?;
+               let inner: InMemorySigner = ReadableArgs::read(&mut reader, self.get_node_secret())?;
                let state = self.make_enforcement_state_cell(inner.commitment_seed);
 
                Ok(EnforcingSigner {
index c5d091f2d21ec7f44885ab91b2f40e8ce907880e..7db5aadcdd7a260c7704caf4fb247568cedaae16 100644 (file)
@@ -45,7 +45,7 @@ use lightning::util::errors::APIError;
 use lightning::util::events::Event;
 use lightning::util::enforcing_trait_impls::{EnforcingSigner, EnforcementState};
 use lightning::util::logger::Logger;
-use lightning::util::ser::Readable;
+use lightning::util::ser::ReadableArgs;
 
 use utils::test_logger;
 use utils::test_persister::TestPersister;
@@ -293,6 +293,7 @@ impl KeysInterface for KeyProvider {
                EnforcingSigner::new(if inbound {
                        InMemorySigner::new(
                                &secp_ctx,
+                               self.node_secret.clone(),
                                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, 1, ctr]).unwrap(),
                                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, 2, ctr]).unwrap(),
                                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, 3, ctr]).unwrap(),
@@ -305,6 +306,7 @@ impl KeysInterface for KeyProvider {
                } else {
                        InMemorySigner::new(
                                &secp_ctx,
+                               self.node_secret.clone(),
                                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, 7, ctr]).unwrap(),
                                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, 8, ctr]).unwrap(),
                                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, 9, ctr]).unwrap(),
@@ -324,7 +326,7 @@ impl KeysInterface for KeyProvider {
        }
 
        fn read_chan_signer(&self, mut data: &[u8]) -> Result<EnforcingSigner, DecodeError> {
-               let inner: InMemorySigner = Readable::read(&mut data)?;
+               let inner: InMemorySigner = ReadableArgs::read(&mut data, self.node_secret.clone())?;
                let state = Arc::new(Mutex::new(EnforcementState::new()));
 
                Ok(EnforcingSigner::new_with_revoked(
index 53f6b83da5fdf00fdd33fcf51c4f0502b32d0fa2..9e96849a274c24d9ba1a08845b8d69622c75a4ff 100644 (file)
@@ -24,7 +24,7 @@ use secp256k1::key::PublicKey;
 
 use super::{Invoice, Sha256, TaggedField, ExpiryTime, MinFinalCltvExpiry, Fallback, PayeePubKey, InvoiceSignature, PositiveTimestamp,
        SemanticError, PrivateRoute, Description, RawTaggedField, Currency, RawHrp, SiPrefix, RawInvoice, constants, SignedRawInvoice,
-       RawDataPart, CreationError, InvoiceFeatures};
+       RawDataPart, InvoiceFeatures};
 
 use self::hrp_sm::parse_hrp;
 
@@ -359,7 +359,6 @@ impl FromBase32 for PositiveTimestamp {
                        .expect("7*5bit < 64bit, no overflow possible");
                match PositiveTimestamp::from_unix_timestamp(timestamp) {
                        Ok(t) => Ok(t),
-                       Err(CreationError::TimestampOutOfBounds) => Err(ParseError::TimestampOverflow),
                        Err(_) => unreachable!(),
                }
        }
@@ -516,7 +515,7 @@ impl FromBase32 for ExpiryTime {
 
        fn from_base32(field_data: &[u5]) -> Result<ExpiryTime, ParseError> {
                match parse_int_be::<u64, u5>(field_data, 32)
-                       .and_then(|t| ExpiryTime::from_seconds(t).ok()) // ok, since the only error is out of bounds
+                       .map(|t| ExpiryTime::from_seconds(t))
                {
                        Some(t) => Ok(t),
                        None => Err(ParseError::IntegerOverflowError),
@@ -646,7 +645,6 @@ pub enum ParseError {
        /// Not an error, but used internally to signal that a part of the invoice should be ignored
        /// according to BOLT11
        Skip,
-       TimestampOverflow,
 }
 
 /// Indicates that something went wrong while parsing or validating the invoice. Parsing errors
@@ -709,9 +707,6 @@ impl Display for ParseError {
                        ParseError::Skip => {
                                f.write_str("the tagged field has to be skipped because of an unexpected, but allowed property")
                        },
-                       ParseError::TimestampOverflow => {
-                f.write_str("the invoice's timestamp could not be represented as SystemTime")
-            },
                }
        }
 }
@@ -877,7 +872,7 @@ mod test {
                use bech32::FromBase32;
 
                let input = from_bech32("pu".as_bytes());
-               let expected = Ok(ExpiryTime::from_seconds(60).unwrap());
+               let expected = Ok(ExpiryTime::from_seconds(60));
                assert_eq!(ExpiryTime::from_base32(&input), expected);
 
                let input_too_large = from_bech32("sqqqqqqqqqqqq".as_bytes());
index 681ea3d52ab82cfb2d7302b18091d52459b119fc..c44db77575dec86ebe2e51ff9b432d20ce5f53ed 100644 (file)
@@ -86,18 +86,17 @@ mod sync;
 
 pub use de::{ParseError, ParseOrSemanticError};
 
-// TODO: fix before 2037 (see rust PR #55527)
-/// Defines the maximum UNIX timestamp that can be represented as `SystemTime`. This is checked by
-/// one of the unit tests, please run them.
-const SYSTEM_TIME_MAX_UNIX_TIMESTAMP: u64 = core::i32::MAX as u64;
+/// The number of bits used to represent timestamps as defined in BOLT 11.
+const TIMESTAMP_BITS: usize = 35;
 
-/// Allow the expiry time to be up to one year. Since this reduces the range of possible timestamps
-/// it should be rather low as long as we still have to support 32bit time representations
-const MAX_EXPIRY_TIME: u64 = 60 * 60 * 24 * 356;
+/// The maximum timestamp as [`Duration::as_secs`] since the Unix epoch allowed by [`BOLT 11`].
+///
+/// [BOLT 11]: https://github.com/lightning/bolts/blob/master/11-payment-encoding.md
+pub const MAX_TIMESTAMP: u64 = (1 << TIMESTAMP_BITS) - 1;
 
 /// Default expiry time as defined by [BOLT 11].
 ///
-/// [BOLT 11]: https://github.com/lightningnetwork/lightning-rfc/blob/master/11-payment-encoding.md
+/// [BOLT 11]: https://github.com/lightning/bolts/blob/master/11-payment-encoding.md
 pub const DEFAULT_EXPIRY_TIME: u64 = 3600;
 
 /// Default minimum final CLTV expiry as defined by [BOLT 11].
@@ -105,71 +104,10 @@ pub const DEFAULT_EXPIRY_TIME: u64 = 3600;
 /// Note that this is *not* the same value as rust-lightning's minimum CLTV expiry, which is
 /// provided in [`MIN_FINAL_CLTV_EXPIRY`].
 ///
-/// [BOLT 11]: https://github.com/lightningnetwork/lightning-rfc/blob/master/11-payment-encoding.md
+/// [BOLT 11]: https://github.com/lightning/bolts/blob/master/11-payment-encoding.md
 /// [`MIN_FINAL_CLTV_EXPIRY`]: lightning::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY
 pub const DEFAULT_MIN_FINAL_CLTV_EXPIRY: u64 = 18;
 
-/// This function is used as a static assert for the size of `SystemTime`. If the crate fails to
-/// compile due to it this indicates that your system uses unexpected bounds for `SystemTime`. You
-/// can remove this functions and run the test `test_system_time_bounds_assumptions`. In any case,
-/// please open an issue. If all tests pass you should be able to use this library safely by just
-/// removing this function till we patch it accordingly.
-#[cfg(feature = "std")]
-fn __system_time_size_check() {
-       // Use 2 * sizeof(u64) as expected size since the expected underlying implementation is storing
-       // a `Duration` since `SystemTime::UNIX_EPOCH`.
-       unsafe { let _ = core::mem::transmute_copy::<SystemTime, [u8; 16]>(&SystemTime::UNIX_EPOCH); }
-}
-
-
-/// **Call this function on startup to ensure that all assumptions about the platform are valid.**
-///
-/// Unfortunately we have to make assumptions about the upper bounds of the `SystemTime` type on
-/// your platform which we can't fully verify at compile time and which isn't part of it's contract.
-/// To our best knowledge our assumptions hold for all platforms officially supported by rust, but
-/// since this check is fast we recommend to do it anyway.
-///
-/// If this function fails this is considered a bug. Please open an issue describing your
-/// platform and stating your current system time.
-///
-/// Note that this currently does nothing in `no_std` environments, because they don't have
-/// a `SystemTime` implementation.
-///
-/// # Panics
-/// If the check fails this function panics. By calling this function on startup you ensure that
-/// this wont happen at an arbitrary later point in time.
-pub fn check_platform() {
-       #[cfg(feature = "std")]
-       check_system_time_bounds();
-}
-
-#[cfg(feature = "std")]
-fn check_system_time_bounds() {
-       // The upper and lower bounds of `SystemTime` are not part of its public contract and are
-       // platform specific. That's why we have to test if our assumptions regarding these bounds
-       // hold on the target platform.
-       //
-       // If this test fails on your platform, please don't use the library and open an issue
-       // instead so we can resolve the situation. Currently this library is tested on:
-       //   * Linux (64bit)
-       let fail_date = SystemTime::UNIX_EPOCH + Duration::from_secs(SYSTEM_TIME_MAX_UNIX_TIMESTAMP);
-       let year = Duration::from_secs(60 * 60 * 24 * 365);
-
-       // Make sure that the library will keep working for another year
-       assert!(fail_date.duration_since(SystemTime::now()).unwrap() > year);
-
-       let max_ts = PositiveTimestamp::from_unix_timestamp(
-               SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME
-       ).unwrap();
-       let max_exp = ::ExpiryTime::from_seconds(MAX_EXPIRY_TIME).unwrap();
-
-       assert_eq!(
-               (max_ts.as_time() + *max_exp.as_duration()).duration_since(SystemTime::UNIX_EPOCH).unwrap().as_secs(),
-               SYSTEM_TIME_MAX_UNIX_TIMESTAMP
-       );
-}
-
-
 /// Builder for `Invoice`s. It's the most convenient and advised way to use this library. It ensures
 /// that only a semantically and syntactically correct Invoice can be built using it.
 ///
@@ -330,12 +268,12 @@ pub struct RawDataPart {
        pub tagged_fields: Vec<RawTaggedField>,
 }
 
-/// A timestamp that refers to a date after 1 January 1970 which means its representation as UNIX
-/// timestamp is positive.
+/// A timestamp that refers to a date after 1 January 1970.
 ///
 /// # Invariants
-/// The UNIX timestamp representing the stored time has to be positive and small enough so that
-/// a `ExpiryTime` can be added to it without an overflow.
+///
+/// The Unix timestamp representing the stored time has to be positive and no greater than
+/// [`MAX_TIMESTAMP`].
 #[derive(Eq, PartialEq, Debug, Clone)]
 pub struct PositiveTimestamp(Duration);
 
@@ -445,11 +383,6 @@ pub struct PayeePubKey(pub PublicKey);
 
 /// Positive duration that defines when (relatively to the timestamp) in the future the invoice
 /// expires
-///
-/// # Invariants
-/// The number of seconds this expiry time represents has to be in the range
-/// `0...(SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME)` to avoid overflows when adding it to a
-/// timestamp
 #[derive(Clone, Debug, Hash, Eq, PartialEq)]
 pub struct ExpiryTime(Duration);
 
@@ -557,10 +490,7 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBui
 
        /// Sets the expiry time
        pub fn expiry_time(mut self, expiry_time: Duration) -> Self {
-        match ExpiryTime::from_duration(expiry_time) {
-            Ok(t) => self.tagged_fields.push(TaggedField::ExpiryTime(t)),
-            Err(e) => self.error = Some(e),
-        };
+               self.tagged_fields.push(TaggedField::ExpiryTime(ExpiryTime::from_duration(expiry_time)));
                self
        }
 
@@ -650,7 +580,7 @@ impl<D: tb::Bool, H: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, tb
                self.set_flags()
        }
 
-       /// Sets the timestamp to a duration since the UNIX epoch.
+       /// Sets the timestamp to a duration since the Unix epoch.
        pub fn duration_since_epoch(mut self, time: Duration) -> InvoiceBuilder<D, H, tb::True, C, S> {
                match PositiveTimestamp::from_duration_since_epoch(time) {
                        Ok(t) => self.timestamp = Some(t),
@@ -981,20 +911,17 @@ impl RawInvoice {
 }
 
 impl PositiveTimestamp {
-       /// Create a new `PositiveTimestamp` from a unix timestamp in the Range
-       /// `0...SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME`, otherwise return a
-       /// `CreationError::TimestampOutOfBounds`.
+       /// Creates a `PositiveTimestamp` from a Unix timestamp in the range `0..=MAX_TIMESTAMP`.
+       ///
+       /// Otherwise, returns a [`CreationError::TimestampOutOfBounds`].
        pub fn from_unix_timestamp(unix_seconds: u64) -> Result<Self, CreationError> {
-               if unix_seconds > SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME {
-                       Err(CreationError::TimestampOutOfBounds)
-               } else {
-                       Ok(PositiveTimestamp(Duration::from_secs(unix_seconds)))
-               }
+               Self::from_duration_since_epoch(Duration::from_secs(unix_seconds))
        }
 
-       /// Create a new `PositiveTimestamp` from a `SystemTime` with a corresponding unix timestamp in
-       /// the range `0...SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME`, otherwise return a
-       /// `CreationError::TimestampOutOfBounds`.
+       /// Creates a `PositiveTimestamp` from a [`SystemTime`] with a corresponding Unix timestamp in
+       /// the range `0..=MAX_TIMESTAMP`.
+       ///
+       /// Otherwise, returns a [`CreationError::TimestampOutOfBounds`].
        #[cfg(feature = "std")]
        pub fn from_system_time(time: SystemTime) -> Result<Self, CreationError> {
                time.duration_since(SystemTime::UNIX_EPOCH)
@@ -1002,28 +929,29 @@ impl PositiveTimestamp {
                        .unwrap_or(Err(CreationError::TimestampOutOfBounds))
        }
 
-       /// Create a new `PositiveTimestamp` from a `Duration` since the UNIX epoch in
-       /// the range `0...SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME`, otherwise return a
-       /// `CreationError::TimestampOutOfBounds`.
+       /// Creates a `PositiveTimestamp` from a [`Duration`] since the Unix epoch in the range
+       /// `0..=MAX_TIMESTAMP`.
+       ///
+       /// Otherwise, returns a [`CreationError::TimestampOutOfBounds`].
        pub fn from_duration_since_epoch(duration: Duration) -> Result<Self, CreationError> {
-               if duration.as_secs() <= SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME {
+               if duration.as_secs() <= MAX_TIMESTAMP {
                        Ok(PositiveTimestamp(duration))
                } else {
                        Err(CreationError::TimestampOutOfBounds)
                }
        }
 
-       /// Returns the UNIX timestamp representing the stored time
+       /// Returns the Unix timestamp representing the stored time
        pub fn as_unix_timestamp(&self) -> u64 {
                self.0.as_secs()
        }
 
-       /// Returns the duration of the stored time since the UNIX epoch
+       /// Returns the duration of the stored time since the Unix epoch
        pub fn as_duration_since_epoch(&self) -> Duration {
                self.0
        }
 
-       /// Returns the `SystemTime` representing the stored time
+       /// Returns the [`SystemTime`] representing the stored time
        #[cfg(feature = "std")]
        pub fn as_time(&self) -> SystemTime {
                SystemTime::UNIX_EPOCH + self.0
@@ -1180,7 +1108,7 @@ impl Invoice {
                self.signed_invoice.raw_invoice().data.timestamp.as_time()
        }
 
-       /// Returns the `Invoice`'s timestamp as a duration since the UNIX epoch
+       /// Returns the `Invoice`'s timestamp as a duration since the Unix epoch
        pub fn duration_since_epoch(&self) -> Duration {
                self.signed_invoice.raw_invoice().data.timestamp.0
        }
@@ -1253,9 +1181,11 @@ impl Invoice {
        }
 
        /// Returns whether the expiry time would pass at the given point in time.
-       /// `at_time` is the timestamp as a duration since the UNIX epoch.
+       /// `at_time` is the timestamp as a duration since the Unix epoch.
        pub fn would_expire(&self, at_time: Duration) -> bool {
-               self.duration_since_epoch() + self.expiry_time() < at_time
+               self.duration_since_epoch()
+                       .checked_add(self.expiry_time())
+                       .unwrap_or_else(|| Duration::new(u64::max_value(), 1_000_000_000 - 1)) < at_time
        }
 
        /// Returns the invoice's `min_final_cltv_expiry` time, if present, otherwise
@@ -1376,26 +1306,14 @@ impl Deref for PayeePubKey {
 }
 
 impl ExpiryTime {
-       /// Construct an `ExpiryTime` from seconds. If there exists a `PositiveTimestamp` which would
-       /// overflow on adding the `EpiryTime` to it then this function will return a
-       /// `CreationError::ExpiryTimeOutOfBounds`.
-       pub fn from_seconds(seconds: u64) -> Result<ExpiryTime, CreationError> {
-               if seconds <= MAX_EXPIRY_TIME {
-                       Ok(ExpiryTime(Duration::from_secs(seconds)))
-               } else {
-                       Err(CreationError::ExpiryTimeOutOfBounds)
-               }
+       /// Construct an `ExpiryTime` from seconds.
+       pub fn from_seconds(seconds: u64) -> ExpiryTime {
+               ExpiryTime(Duration::from_secs(seconds))
        }
 
-       /// Construct an `ExpiryTime` from a `Duration`. If there exists a `PositiveTimestamp` which
-       /// would overflow on adding the `EpiryTime` to it then this function will return a
-       /// `CreationError::ExpiryTimeOutOfBounds`.
-       pub fn from_duration(duration: Duration) -> Result<ExpiryTime, CreationError> {
-               if duration.as_secs() <= MAX_EXPIRY_TIME {
-                       Ok(ExpiryTime(duration))
-               } else {
-                       Err(CreationError::ExpiryTimeOutOfBounds)
-               }
+       /// Construct an `ExpiryTime` from a `Duration`.
+       pub fn from_duration(duration: Duration) -> ExpiryTime {
+               ExpiryTime(duration)
        }
 
        /// Returns the expiry time in seconds
@@ -1464,12 +1382,9 @@ pub enum CreationError {
        /// The specified route has too many hops and can't be encoded
        RouteTooLong,
 
-       /// The unix timestamp of the supplied date is <0 or can't be represented as `SystemTime`
+       /// The Unix timestamp of the supplied date is less than zero or greater than 35-bits
        TimestampOutOfBounds,
 
-       /// The supplied expiry time could cause an overflow if added to a `PositiveTimestamp`
-       ExpiryTimeOutOfBounds,
-
        /// The supplied millisatoshi amount was greater than the total bitcoin supply.
        InvalidAmount,
 }
@@ -1479,8 +1394,7 @@ impl Display for CreationError {
                match self {
                        CreationError::DescriptionTooLong => f.write_str("The supplied description string was longer than 639 bytes"),
                        CreationError::RouteTooLong => f.write_str("The specified route has too many hops and can't be encoded"),
-                       CreationError::TimestampOutOfBounds => f.write_str("The unix timestamp of the supplied date is <0 or can't be represented as `SystemTime`"),
-                       CreationError::ExpiryTimeOutOfBounds => f.write_str("The supplied expiry time could cause an overflow if added to a `PositiveTimestamp`"),
+                       CreationError::TimestampOutOfBounds => f.write_str("The Unix timestamp of the supplied date is less than zero or greater than 35-bits"),
                        CreationError::InvalidAmount => f.write_str("The supplied millisatoshi amount was greater than the total bitcoin supply"),
                }
        }
@@ -1572,17 +1486,10 @@ mod test {
 
        #[test]
        fn test_system_time_bounds_assumptions() {
-               ::check_platform();
-
                assert_eq!(
-                       ::PositiveTimestamp::from_unix_timestamp(::SYSTEM_TIME_MAX_UNIX_TIMESTAMP + 1),
+                       ::PositiveTimestamp::from_unix_timestamp(::MAX_TIMESTAMP + 1),
                        Err(::CreationError::TimestampOutOfBounds)
                );
-
-               assert_eq!(
-                       ::ExpiryTime::from_seconds(::MAX_EXPIRY_TIME + 1),
-                       Err(::CreationError::ExpiryTimeOutOfBounds)
-               );
        }
 
        #[test]
index 8786f47db0d91b101483cd4d7136f69aed615342..e6f6ceda2b0b16f712963843a23fe4a478490f29 100644 (file)
@@ -3479,6 +3479,7 @@ mod tests {
                        SecretKey::from_slice(&[41; 32]).unwrap(),
                        SecretKey::from_slice(&[41; 32]).unwrap(),
                        SecretKey::from_slice(&[41; 32]).unwrap(),
+                       SecretKey::from_slice(&[41; 32]).unwrap(),
                        [41; 32],
                        0,
                        [0; 32]
index 7538d0a83033c03f2f92f17a0dab8ff5c9be1ec5..dffe060d91bb71a5f33e8c023de6cac94fb9826d 100644 (file)
@@ -31,10 +31,10 @@ use bitcoin::secp256k1::recovery::RecoverableSignature;
 use bitcoin::secp256k1;
 
 use util::{byte_utils, transaction_utils};
-use util::ser::{Writeable, Writer, Readable};
+use util::ser::{Writeable, Writer, Readable, ReadableArgs};
 
 use chain::transaction::OutPoint;
-use ln::chan_utils;
+use ln::{chan_utils, PaymentPreimage};
 use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, HolderCommitmentTransaction, ChannelTransactionParameters, CommitmentTransaction, ClosingTransaction};
 use ln::msgs::UnsignedChannelAnnouncement;
 use ln::script::ShutdownScript;
@@ -226,7 +226,14 @@ pub trait BaseSign {
        /// secret won't leave us without a broadcastable holder transaction.
        /// Policy checks should be implemented in this function, including checking the amount
        /// sent to us and checking the HTLCs.
-       fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction) -> Result<(), ()>;
+       ///
+       /// The preimages of outgoing HTLCs that were fulfilled since the last commitment are provided.
+       /// A validating signer should ensure that an HTLC output is removed only when the matching
+       /// preimage is provided, or when the value to holder is restored.
+       ///
+       /// NOTE: all the relevant preimages will be provided, but there may also be additional
+       /// irrelevant or duplicate preimages.
+       fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction, preimages: Vec<PaymentPreimage>) -> Result<(), ()>;
        /// Gets the holder's channel public keys and basepoints
        fn pubkeys(&self) -> &ChannelPublicKeys;
        /// Gets an arbitrary identifier describing the set of keys which are provided back to you in
@@ -240,9 +247,16 @@ pub trait BaseSign {
        ///
        /// Policy checks should be implemented in this function, including checking the amount
        /// sent to us and checking the HTLCs.
+       ///
+       /// The preimages of outgoing HTLCs that were fulfilled since the last commitment are provided.
+       /// A validating signer should ensure that an HTLC output is removed only when the matching
+       /// preimage is provided, or when the value to holder is restored.
+       ///
+       /// NOTE: all the relevant preimages will be provided, but there may also be additional
+       /// irrelevant or duplicate preimages.
        //
        // TODO: Document the things someone using this interface should enforce before signing.
-       fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()>;
+       fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()>;
        /// Validate the counterparty's revocation.
        ///
        /// This is required in order for the signer to make sure that the state has moved
@@ -332,13 +346,17 @@ pub trait BaseSign {
        /// chosen to forgo their output as dust.
        fn sign_closing_transaction(&self, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;
 
-       /// Signs a channel announcement message with our funding key, proving it comes from one
-       /// of the channel participants.
+       /// Signs a channel announcement message with our funding key and our node secret key (aka
+       /// node_id or network_key), proving it comes from one of the channel participants.
+       ///
+       /// The first returned signature should be from our node secret key, the second from our
+       /// funding key.
        ///
        /// Note that if this fails or is rejected, the channel will not be publicly announced and
        /// our counterparty may (though likely will not) close the channel on us for violating the
        /// protocol.
-       fn sign_channel_announcement(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;
+       fn sign_channel_announcement(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>)
+               -> Result<(Signature, Signature), ()>;
 
        /// Set the counterparty static channel data, including basepoints,
        /// counterparty_selected/holder_selected_contest_delay and funding outpoint.
@@ -433,6 +451,8 @@ pub struct InMemorySigner {
        pub commitment_seed: [u8; 32],
        /// Holder public keys and basepoints
        pub(crate) holder_channel_pubkeys: ChannelPublicKeys,
+       /// Private key of our node secret, used for signing channel announcements
+       node_secret: SecretKey,
        /// Counterparty public keys and counterparty/holder selected_contest_delay, populated on channel acceptance
        channel_parameters: Option<ChannelTransactionParameters>,
        /// The total value of this channel
@@ -445,6 +465,7 @@ impl InMemorySigner {
        /// Create a new InMemorySigner
        pub fn new<C: Signing>(
                secp_ctx: &Secp256k1<C>,
+               node_secret: SecretKey,
                funding_key: SecretKey,
                revocation_base_key: SecretKey,
                payment_key: SecretKey,
@@ -464,6 +485,7 @@ impl InMemorySigner {
                        delayed_payment_base_key,
                        htlc_base_key,
                        commitment_seed,
+                       node_secret,
                        channel_value_satoshis,
                        holder_channel_pubkeys,
                        channel_parameters: None,
@@ -601,14 +623,14 @@ impl BaseSign for InMemorySigner {
                chan_utils::build_commitment_secret(&self.commitment_seed, idx)
        }
 
-       fn validate_holder_commitment(&self, _holder_tx: &HolderCommitmentTransaction) -> Result<(), ()> {
+       fn validate_holder_commitment(&self, _holder_tx: &HolderCommitmentTransaction, _preimages: Vec<PaymentPreimage>) -> Result<(), ()> {
                Ok(())
        }
 
        fn pubkeys(&self) -> &ChannelPublicKeys { &self.holder_channel_pubkeys }
        fn channel_keys_id(&self) -> [u8; 32] { self.channel_keys_id }
 
-       fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
+       fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, _preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
                let trusted_tx = commitment_tx.trust();
                let keys = trusted_tx.keys();
 
@@ -706,9 +728,10 @@ impl BaseSign for InMemorySigner {
                Ok(closing_tx.trust().sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx))
        }
 
-       fn sign_channel_announcement(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
+       fn sign_channel_announcement(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>)
+       -> Result<(Signature, Signature), ()> {
                let msghash = hash_to_message!(&Sha256dHash::hash(&msg.encode()[..])[..]);
-               Ok(secp_ctx.sign(&msghash, &self.funding_key))
+               Ok((secp_ctx.sign(&msghash, &self.node_secret), secp_ctx.sign(&msghash, &self.funding_key)))
        }
 
        fn ready_channel(&mut self, channel_parameters: &ChannelTransactionParameters) {
@@ -743,8 +766,8 @@ impl Writeable for InMemorySigner {
        }
 }
 
-impl Readable for InMemorySigner {
-       fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+impl ReadableArgs<SecretKey> for InMemorySigner {
+       fn read<R: io::Read>(reader: &mut R, node_secret: SecretKey) -> Result<Self, DecodeError> {
                let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
                let funding_key = Readable::read(reader)?;
@@ -770,6 +793,7 @@ impl Readable for InMemorySigner {
                        payment_key,
                        delayed_payment_base_key,
                        htlc_base_key,
+                       node_secret,
                        commitment_seed,
                        channel_value_satoshis,
                        holder_channel_pubkeys,
@@ -923,6 +947,7 @@ impl KeysManager {
 
                InMemorySigner::new(
                        &self.secp_ctx,
+                       self.node_secret,
                        funding_key,
                        revocation_base_key,
                        payment_key,
@@ -1105,7 +1130,7 @@ impl KeysInterface for KeysManager {
        }
 
        fn read_chan_signer(&self, reader: &[u8]) -> Result<Self::Signer, DecodeError> {
-               InMemorySigner::read(&mut io::Cursor::new(reader))
+               InMemorySigner::read(&mut io::Cursor::new(reader), self.get_node_secret())
        }
 
        fn sign_invoice(&self, hrp_bytes: &[u8], invoice_data: &[u5]) -> Result<RecoverableSignature, ()> {
index 236b1e498f45ef26b86b1b5b5d46d870028e70ea..a6c5649c1d954e5b76f7efe8e28dd675d9d76c31 100644 (file)
@@ -20,6 +20,7 @@ use chain::channelmonitor::ChannelMonitor;
 use chain::transaction::OutPoint;
 use chain::{ChannelMonitorUpdateErr, Listen, Watch};
 use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure};
+use ln::channel::AnnouncementSigsState;
 use ln::features::InitFeatures;
 use ln::msgs;
 use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler};
@@ -1402,6 +1403,11 @@ fn monitor_failed_no_reestablish_response() {
        let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
        let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;
+       {
+               let mut lock;
+               get_channel_ref!(nodes[0], lock, channel_id).announcement_sigs_state = AnnouncementSigsState::PeerReceived;
+               get_channel_ref!(nodes[1], lock, channel_id).announcement_sigs_state = AnnouncementSigsState::PeerReceived;
+       }
 
        // Route the payment and deliver the initial commitment_signed (with a monitor update failure
        // on receipt).
@@ -1789,9 +1795,9 @@ fn monitor_update_claim_fail_no_response() {
        claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
 }
 
-// confirm_a_first and restore_b_before_conf are wholly unrelated to earlier bools and
 // restore_b_before_conf has no meaning if !confirm_a_first
-fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: bool) {
+// restore_b_before_lock has no meaning if confirm_a_first
+fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: bool, restore_b_before_lock: bool) {
        // Test that if the monitor update generated by funding_transaction_generated fails we continue
        // the channel setup happily after the update is restored.
        let chanmon_cfgs = create_chanmon_cfgs(2);
@@ -1833,6 +1839,8 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf:
        if confirm_a_first {
                confirm_transaction(&nodes[0], &funding_tx);
                nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendFundingLocked, nodes[1].node.get_our_node_id()));
+               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+               assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
        } else {
                assert!(!restore_b_before_conf);
                confirm_transaction(&nodes[1], &funding_tx);
@@ -1851,6 +1859,12 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf:
                assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
                assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
        }
+       if !confirm_a_first && !restore_b_before_lock {
+               confirm_transaction(&nodes[0], &funding_tx);
+               nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendFundingLocked, nodes[1].node.get_our_node_id()));
+               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+               assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
+       }
 
        chanmon_cfgs[1].persister.set_update_ret(Ok(()));
        let (outpoint, latest_update, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
@@ -1858,13 +1872,19 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf:
        check_added_monitors!(nodes[1], 0);
 
        let (channel_id, (announcement, as_update, bs_update)) = if !confirm_a_first {
-               nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingLocked, nodes[0].node.get_our_node_id()));
-
-               confirm_transaction(&nodes[0], &funding_tx);
-               let (funding_locked, channel_id) = create_chan_between_nodes_with_value_confirm_second(&nodes[1], &nodes[0]);
-               (channel_id, create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked))
+               if !restore_b_before_lock {
+                       let (funding_locked, channel_id) = create_chan_between_nodes_with_value_confirm_second(&nodes[0], &nodes[1]);
+                       (channel_id, create_chan_between_nodes_with_value_b(&nodes[1], &nodes[0], &funding_locked))
+               } else {
+                       nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingLocked, nodes[0].node.get_our_node_id()));
+                       confirm_transaction(&nodes[0], &funding_tx);
+                       let (funding_locked, channel_id) = create_chan_between_nodes_with_value_confirm_second(&nodes[1], &nodes[0]);
+                       (channel_id, create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked))
+               }
        } else {
                if restore_b_before_conf {
+                       assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+                       assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
                        confirm_transaction(&nodes[1], &funding_tx);
                }
                let (funding_locked, channel_id) = create_chan_between_nodes_with_value_confirm_second(&nodes[0], &nodes[1]);
@@ -1884,9 +1904,10 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf:
 
 #[test]
 fn during_funding_monitor_fail() {
-       do_during_funding_monitor_fail(true, true);
-       do_during_funding_monitor_fail(true, false);
-       do_during_funding_monitor_fail(false, false);
+       do_during_funding_monitor_fail(true, true, false);
+       do_during_funding_monitor_fail(true, false, false);
+       do_during_funding_monitor_fail(false, false, false);
+       do_during_funding_monitor_fail(false, false, true);
 }
 
 #[test]
index 8daea4d7548fda4d1f72d50b4e3f46774d404f8a..fc48a6b9a1ceb777eea964f19b89bd729072b60f 100644 (file)
@@ -162,19 +162,43 @@ enum OutboundHTLCState {
        Committed,
        /// Remote removed this (outbound) HTLC. We're waiting on their commitment_signed to finalize
        /// the change (though they'll need to revoke before we fail the payment).
-       RemoteRemoved(Option<HTLCFailReason>),
+       RemoteRemoved(OutboundHTLCOutcome),
        /// Remote removed this and sent a commitment_signed (implying we've revoke_and_ack'ed it), but
        /// the remote side hasn't yet revoked their previous state, which we need them to do before we
        /// can do any backwards failing. Implies AwaitingRemoteRevoke.
        /// We also have not yet removed this HTLC in a commitment_signed message, and are waiting on a
        /// remote revoke_and_ack on a previous state before we can do so.
-       AwaitingRemoteRevokeToRemove(Option<HTLCFailReason>),
+       AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome),
        /// Remote removed this and sent a commitment_signed (implying we've revoke_and_ack'ed it), but
        /// the remote side hasn't yet revoked their previous state, which we need them to do before we
        /// can do any backwards failing. Implies AwaitingRemoteRevoke.
        /// We have removed this HTLC in our latest commitment_signed and are now just waiting on a
        /// revoke_and_ack to drop completely.
-       AwaitingRemovedRemoteRevoke(Option<HTLCFailReason>),
+       AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome),
+}
+
+#[derive(Clone)]
+enum OutboundHTLCOutcome {
+       Success(Option<PaymentPreimage>),
+       Failure(HTLCFailReason),
+}
+
+impl From<Option<HTLCFailReason>> for OutboundHTLCOutcome {
+       fn from(o: Option<HTLCFailReason>) -> Self {
+               match o {
+                       None => OutboundHTLCOutcome::Success(None),
+                       Some(r) => OutboundHTLCOutcome::Failure(r)
+               }
+       }
+}
+
+impl<'a> Into<Option<&'a HTLCFailReason>> for &'a OutboundHTLCOutcome {
+       fn into(self) -> Option<&'a HTLCFailReason> {
+               match self {
+                       OutboundHTLCOutcome::Success(_) => None,
+                       OutboundHTLCOutcome::Failure(ref r) => Some(r)
+               }
+       }
 }
 
 struct OutboundHTLCOutput {
@@ -281,6 +305,26 @@ pub(super) enum ChannelUpdateStatus {
        Disabled,
 }
 
+/// We track when we sent an `AnnouncementSignatures` to our peer in a few states, described here.
+#[derive(PartialEq)]
+pub enum AnnouncementSigsState {
+       /// We have not sent our peer an `AnnouncementSignatures` yet, or our peer disconnected since
+       /// we sent the last `AnnouncementSignatures`.
+       NotSent,
+       /// We sent an `AnnouncementSignatures` to our peer since the last time our peer disconnected.
+       /// This state never appears on disk - instead we write `NotSent`.
+       MessageSent,
+       /// We sent a `CommitmentSigned` after the last `AnnouncementSignatures` we sent. Because we
+       /// only ever have a single `CommitmentSigned` pending at once, if we sent one after sending
+       /// `AnnouncementSignatures` then we know the peer received our `AnnouncementSignatures` if
+       /// they send back a `RevokeAndACK`.
+       /// This state never appears on disk - instead we write `NotSent`.
+       Committed,
+       /// We received a `RevokeAndACK`, effectively ack-ing our `AnnouncementSignatures`, at this
+       /// point we no longer need to re-send our `AnnouncementSignatures` again on reconnect.
+       PeerReceived,
+}
+
 /// An enum indicating whether the local or remote side offered a given HTLC.
 enum HTLCInitiator {
        LocalOffered,
@@ -306,6 +350,7 @@ struct CommitmentStats<'a> {
        htlcs_included: Vec<(HTLCOutputInCommitment, Option<&'a HTLCSource>)>, // the list of HTLCs (dust HTLCs *included*) which were not ignored when building the transaction
        local_balance_msat: u64, // local balance before fees but considering dust limits
        remote_balance_msat: u64, // remote balance before fees but considering dust limits
+       preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment
 }
 
 /// Used when calculating whether we or the remote can afford an additional HTLC.
@@ -374,6 +419,19 @@ pub(super) struct MonitorRestoreUpdates {
        pub finalized_claimed_htlcs: Vec<HTLCSource>,
        pub funding_broadcastable: Option<Transaction>,
        pub funding_locked: Option<msgs::FundingLocked>,
+       pub announcement_sigs: Option<msgs::AnnouncementSignatures>,
+}
+
+/// The return value of `channel_reestablish`
+pub(super) struct ReestablishResponses {
+       pub funding_locked: Option<msgs::FundingLocked>,
+       pub raa: Option<msgs::RevokeAndACK>,
+       pub commitment_update: Option<msgs::CommitmentUpdate>,
+       pub order: RAACommitmentOrder,
+       pub mon_update: Option<ChannelMonitorUpdate>,
+       pub holding_cell_failed_htlcs: Vec<(HTLCSource, PaymentHash)>,
+       pub announcement_sigs: Option<msgs::AnnouncementSignatures>,
+       pub shutdown_msg: Option<msgs::Shutdown>,
 }
 
 /// If the majority of the channels funds are to the fundee and the initiator holds only just
@@ -430,6 +488,19 @@ pub(super) struct Channel<Signer: Sign> {
 
        channel_id: [u8; 32],
        channel_state: u32,
+
+       // When we reach max(6 blocks, minimum_depth), we need to send an AnnouncementSigs message to
+       // our peer. However, we want to make sure they received it, or else rebroadcast it when we
+       // next connect.
+       // We do so here, see `AnnouncementSigsSent` for more details on the state(s).
+       // Note that a number of our tests were written prior to the behavior here which retransmits
+       // AnnouncementSignatures until after an RAA completes, so the behavior is short-circuited in
+       // many tests.
+       #[cfg(any(test, feature = "_test_utils"))]
+       pub(crate) announcement_sigs_state: AnnouncementSigsState,
+       #[cfg(not(any(test, feature = "_test_utils")))]
+       announcement_sigs_state: AnnouncementSigsState,
+
        secp_ctx: Secp256k1<secp256k1::All>,
        channel_value_satoshis: u64,
 
@@ -767,6 +838,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                        channel_id: keys_provider.get_secure_random_bytes(),
                        channel_state: ChannelState::OurInitSent as u32,
+                       announcement_sigs_state: AnnouncementSigsState::NotSent,
                        secp_ctx,
                        channel_value_satoshis,
 
@@ -1065,6 +1137,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                        channel_id: msg.temporary_channel_id,
                        channel_state: (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32),
+                       announcement_sigs_state: AnnouncementSigsState::NotSent,
                        secp_ctx,
 
                        latest_monitor_update_id: 0,
@@ -1274,6 +1347,8 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                }
 
+               let mut preimages: Vec<PaymentPreimage> = Vec::new();
+
                for ref htlc in self.pending_outbound_htlcs.iter() {
                        let (include, state_name) = match htlc.state {
                                OutboundHTLCState::LocalAnnounced(_) => (generated_by_local, "LocalAnnounced"),
@@ -1283,16 +1358,27 @@ impl<Signer: Sign> Channel<Signer> {
                                OutboundHTLCState::AwaitingRemovedRemoteRevoke(_) => (false, "AwaitingRemovedRemoteRevoke"),
                        };
 
+                       let preimage_opt = match htlc.state {
+                               OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(p)) => p,
+                               OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(p)) => p,
+                               OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(p)) => p,
+                               _ => None,
+                       };
+
+                       if let Some(preimage) = preimage_opt {
+                               preimages.push(preimage);
+                       }
+
                        if include {
                                add_htlc_output!(htlc, true, Some(&htlc.source), state_name);
                                local_htlc_total_msat += htlc.amount_msat;
                        } else {
                                log_trace!(logger, "   ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, log_bytes!(htlc.payment_hash.0), htlc.amount_msat, state_name);
                                match htlc.state {
-                                       OutboundHTLCState::AwaitingRemoteRevokeToRemove(None)|OutboundHTLCState::AwaitingRemovedRemoteRevoke(None) => {
+                                       OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_))|OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) => {
                                                value_to_self_msat_offset -= htlc.amount_msat as i64;
                                        },
-                                       OutboundHTLCState::RemoteRemoved(None) => {
+                                       OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(_)) => {
                                                if !generated_by_local {
                                                        value_to_self_msat_offset -= htlc.amount_msat as i64;
                                                }
@@ -1387,6 +1473,7 @@ impl<Signer: Sign> Channel<Signer> {
                        htlcs_included,
                        local_balance_msat: value_to_self_msat as u64,
                        remote_balance_msat: value_to_remote_msat as u64,
+                       preimages
                }
        }
 
@@ -1858,7 +1945,7 @@ impl<Signer: Sign> Channel<Signer> {
                log_trace!(logger, "Initial counterparty tx for channel {} is: txid {} tx {}",
                        log_bytes!(self.channel_id()), counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
 
-               let counterparty_signature = self.holder_signer.sign_counterparty_commitment(&counterparty_initial_commitment_tx, &self.secp_ctx)
+               let counterparty_signature = self.holder_signer.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.secp_ctx)
                                .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0;
 
                // We sign "counterparty" commitment transaction, allowing them to broadcast the tx if they wish.
@@ -1912,7 +1999,7 @@ impl<Signer: Sign> Channel<Signer> {
                        self.counterparty_funding_pubkey()
                );
 
-               self.holder_signer.validate_holder_commitment(&holder_commitment_tx)
+               self.holder_signer.validate_holder_commitment(&holder_commitment_tx, Vec::new())
                        .map_err(|_| ChannelError::Close("Failed to validate our commitment".to_owned()))?;
 
                // Now that we're past error-generating stuff, update our local state:
@@ -1989,7 +2076,7 @@ impl<Signer: Sign> Channel<Signer> {
                        self.counterparty_funding_pubkey()
                );
 
-               self.holder_signer.validate_holder_commitment(&holder_commitment_tx)
+               self.holder_signer.validate_holder_commitment(&holder_commitment_tx, Vec::new())
                        .map_err(|_| ChannelError::Close("Failed to validate our commitment".to_owned()))?;
 
 
@@ -2018,7 +2105,10 @@ impl<Signer: Sign> Channel<Signer> {
                Ok((channel_monitor, self.funding_transaction.as_ref().cloned().unwrap()))
        }
 
-       pub fn funding_locked<L: Deref>(&mut self, msg: &msgs::FundingLocked, logger: &L) -> Result<(), ChannelError> where L::Target: Logger {
+       /// Handles a funding_locked message from our peer. If we've already sent our funding_locked
+       /// and the channel is now usable (and public), this may generate an announcement_signatures to
+       /// reply with.
+       pub fn funding_locked<L: Deref>(&mut self, msg: &msgs::FundingLocked, node_pk: PublicKey, genesis_block_hash: BlockHash, best_block: &BestBlock, logger: &L) -> Result<Option<msgs::AnnouncementSignatures>, ChannelError> where L::Target: Logger {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
                        self.workaround_lnd_bug_4006 = Some(msg.clone());
                        return Err(ChannelError::Ignore("Peer sent funding_locked when we needed a channel_reestablish. The peer is likely lnd, see https://github.com/lightningnetwork/lnd/issues/4006".to_owned()));
@@ -2042,7 +2132,7 @@ impl<Signer: Sign> Channel<Signer> {
                                return Err(ChannelError::Close("Peer sent a reconnect funding_locked with a different point".to_owned()));
                        }
                        // They probably disconnected/reconnected and re-sent the funding_locked, which is required
-                       return Ok(());
+                       return Ok(None);
                } else {
                        return Err(ChannelError::Close("Peer sent a funding_locked at a strange time".to_owned()));
                }
@@ -2052,7 +2142,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                log_info!(logger, "Received funding_locked from peer for channel {}", log_bytes!(self.channel_id()));
 
-               Ok(())
+               Ok(self.get_announcement_sigs(node_pk, genesis_block_hash, best_block.height(), logger))
        }
 
        /// Returns transaction if there is pending funding transaction that is yet to broadcast
@@ -2393,9 +2483,9 @@ impl<Signer: Sign> Channel<Signer> {
                // transaction).
                let mut removed_outbound_total_msat = 0;
                for ref htlc in self.pending_outbound_htlcs.iter() {
-                       if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(None) = htlc.state {
+                       if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) = htlc.state {
                                removed_outbound_total_msat += htlc.amount_msat;
-                       } else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(None) = htlc.state {
+                       } else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) = htlc.state {
                                removed_outbound_total_msat += htlc.amount_msat;
                        }
                }
@@ -2494,21 +2584,25 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// Marks an outbound HTLC which we have received update_fail/fulfill/malformed
        #[inline]
-       fn mark_outbound_htlc_removed(&mut self, htlc_id: u64, check_preimage: Option<PaymentHash>, fail_reason: Option<HTLCFailReason>) -> Result<&OutboundHTLCOutput, ChannelError> {
+       fn mark_outbound_htlc_removed(&mut self, htlc_id: u64, check_preimage: Option<PaymentPreimage>, fail_reason: Option<HTLCFailReason>) -> Result<&OutboundHTLCOutput, ChannelError> {
+               assert!(!(check_preimage.is_some() && fail_reason.is_some()), "cannot fail while we have a preimage");
                for htlc in self.pending_outbound_htlcs.iter_mut() {
                        if htlc.htlc_id == htlc_id {
-                               match check_preimage {
-                                       None => {},
-                                       Some(payment_hash) =>
+                               let outcome = match check_preimage {
+                                       None => fail_reason.into(),
+                                       Some(payment_preimage) => {
+                                               let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
                                                if payment_hash != htlc.payment_hash {
                                                        return Err(ChannelError::Close(format!("Remote tried to fulfill HTLC ({}) with an incorrect preimage", htlc_id)));
                                                }
+                                               OutboundHTLCOutcome::Success(Some(payment_preimage))
+                                       }
                                };
                                match htlc.state {
                                        OutboundHTLCState::LocalAnnounced(_) =>
                                                return Err(ChannelError::Close(format!("Remote tried to fulfill/fail HTLC ({}) before it had been committed", htlc_id))),
                                        OutboundHTLCState::Committed => {
-                                               htlc.state = OutboundHTLCState::RemoteRemoved(fail_reason);
+                                               htlc.state = OutboundHTLCState::RemoteRemoved(outcome);
                                        },
                                        OutboundHTLCState::AwaitingRemoteRevokeToRemove(_) | OutboundHTLCState::AwaitingRemovedRemoteRevoke(_) | OutboundHTLCState::RemoteRemoved(_) =>
                                                return Err(ChannelError::Close(format!("Remote tried to fulfill/fail HTLC ({}) that they'd already fulfilled/failed", htlc_id))),
@@ -2527,8 +2621,7 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Close("Peer sent update_fulfill_htlc when we needed a channel_reestablish".to_owned()));
                }
 
-               let payment_hash = PaymentHash(Sha256::hash(&msg.payment_preimage.0[..]).into_inner());
-               self.mark_outbound_htlc_removed(msg.htlc_id, Some(payment_hash), None).map(|htlc| (htlc.source.clone(), htlc.amount_msat))
+               self.mark_outbound_htlc_removed(msg.htlc_id, Some(msg.payment_preimage), None).map(|htlc| (htlc.source.clone(), htlc.amount_msat))
        }
 
        pub fn update_fail_htlc(&mut self, msg: &msgs::UpdateFailHTLC, fail_reason: HTLCFailReason) -> Result<(), ChannelError> {
@@ -2655,7 +2748,7 @@ impl<Signer: Sign> Channel<Signer> {
                );
 
                let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number - 1, &self.secp_ctx);
-               self.holder_signer.validate_holder_commitment(&holder_commitment_tx)
+               self.holder_signer.validate_holder_commitment(&holder_commitment_tx, commitment_stats.preimages)
                        .map_err(|_| (None, ChannelError::Close("Failed to validate our commitment".to_owned())))?;
                let per_commitment_secret = self.holder_signer.release_commitment_secret(self.cur_holder_commitment_transaction_number + 1);
 
@@ -2689,12 +2782,13 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                }
                for htlc in self.pending_outbound_htlcs.iter_mut() {
-                       if let Some(fail_reason) = if let &mut OutboundHTLCState::RemoteRemoved(ref mut fail_reason) = &mut htlc.state {
-                               Some(fail_reason.take())
-                       } else { None } {
+                       if let &mut OutboundHTLCState::RemoteRemoved(ref mut outcome) = &mut htlc.state {
                                log_trace!(logger, "Updating HTLC {} to AwaitingRemoteRevokeToRemove due to commitment_signed in channel {}.",
                                        log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id));
-                               htlc.state = OutboundHTLCState::AwaitingRemoteRevokeToRemove(fail_reason);
+                               // Grab the preimage, if it exists, instead of cloning
+                               let mut reason = OutboundHTLCOutcome::Success(None);
+                               mem::swap(outcome, &mut reason);
+                               htlc.state = OutboundHTLCState::AwaitingRemoteRevokeToRemove(reason);
                                need_commitment = true;
                        }
                }
@@ -2938,6 +3032,10 @@ impl<Signer: Sign> Channel<Signer> {
                self.counterparty_cur_commitment_point = Some(msg.next_per_commitment_point);
                self.cur_counterparty_commitment_transaction_number -= 1;
 
+               if self.announcement_sigs_state == AnnouncementSigsState::Committed {
+                       self.announcement_sigs_state = AnnouncementSigsState::PeerReceived;
+               }
+
                log_trace!(logger, "Updating HTLCs on receipt of RAA in channel {}...", log_bytes!(self.channel_id()));
                let mut to_forward_infos = Vec::new();
                let mut revoked_htlcs = Vec::new();
@@ -2963,9 +3061,9 @@ impl<Signer: Sign> Channel<Signer> {
                                } else { true }
                        });
                        pending_outbound_htlcs.retain(|htlc| {
-                               if let &OutboundHTLCState::AwaitingRemovedRemoteRevoke(ref fail_reason) = &htlc.state {
+                               if let &OutboundHTLCState::AwaitingRemovedRemoteRevoke(ref outcome) = &htlc.state {
                                        log_trace!(logger, " ...removing outbound AwaitingRemovedRemoteRevoke {}", log_bytes!(htlc.payment_hash.0));
-                                       if let Some(reason) = fail_reason.clone() { // We really want take() here, but, again, non-mut ref :(
+                                       if let OutboundHTLCOutcome::Failure(reason) = outcome.clone() { // We really want take() here, but, again, non-mut ref :(
                                                revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason));
                                        } else {
                                                finalized_claimed_htlcs.push(htlc.source.clone());
@@ -3019,11 +3117,12 @@ impl<Signer: Sign> Channel<Signer> {
                                        log_trace!(logger, " ...promoting outbound LocalAnnounced {} to Committed", log_bytes!(htlc.payment_hash.0));
                                        htlc.state = OutboundHTLCState::Committed;
                                }
-                               if let Some(fail_reason) = if let &mut OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref mut fail_reason) = &mut htlc.state {
-                                       Some(fail_reason.take())
-                               } else { None } {
+                               if let &mut OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref mut outcome) = &mut htlc.state {
                                        log_trace!(logger, " ...promoting outbound AwaitingRemoteRevokeToRemove {} to AwaitingRemovedRemoteRevoke", log_bytes!(htlc.payment_hash.0));
-                                       htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke(fail_reason);
+                                       // Grab the preimage, if it exists, instead of cloning
+                                       let mut reason = OutboundHTLCOutcome::Success(None);
+                                       mem::swap(outcome, &mut reason);
+                                       htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke(reason);
                                        require_commitment = true;
                                }
                        }
@@ -3213,6 +3312,11 @@ impl<Signer: Sign> Channel<Signer> {
                        self.channel_state = ChannelState::ShutdownComplete as u32;
                        return;
                }
+
+               if self.announcement_sigs_state == AnnouncementSigsState::MessageSent || self.announcement_sigs_state == AnnouncementSigsState::Committed {
+                       self.announcement_sigs_state = AnnouncementSigsState::NotSent;
+               }
+
                // Upon reconnect we have to start the closing_signed dance over, but shutdown messages
                // will be retransmitted.
                self.last_sent_closing_fee = None;
@@ -3289,7 +3393,7 @@ impl<Signer: Sign> Channel<Signer> {
        /// Indicates that the latest ChannelMonitor update has been committed by the client
        /// successfully and we should restore normal operation. Returns messages which should be sent
        /// to the remote side.
-       pub fn monitor_updating_restored<L: Deref>(&mut self, logger: &L) -> MonitorRestoreUpdates where L::Target: Logger {
+       pub fn monitor_updating_restored<L: Deref>(&mut self, logger: &L, node_pk: PublicKey, genesis_block_hash: BlockHash, best_block_height: u32) -> MonitorRestoreUpdates where L::Target: Logger {
                assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, ChannelState::MonitorUpdateFailed as u32);
                self.channel_state &= !(ChannelState::MonitorUpdateFailed as u32);
 
@@ -3312,6 +3416,8 @@ impl<Signer: Sign> Channel<Signer> {
                        })
                } else { None };
 
+               let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, best_block_height, logger);
+
                let mut accepted_htlcs = Vec::new();
                mem::swap(&mut accepted_htlcs, &mut self.monitor_pending_forwards);
                let mut failed_htlcs = Vec::new();
@@ -3324,7 +3430,7 @@ impl<Signer: Sign> Channel<Signer> {
                        self.monitor_pending_commitment_signed = false;
                        return MonitorRestoreUpdates {
                                raa: None, commitment_update: None, order: RAACommitmentOrder::RevokeAndACKFirst,
-                               accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, funding_broadcastable, funding_locked
+                               accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, funding_broadcastable, funding_locked, announcement_sigs
                        };
                }
 
@@ -3343,7 +3449,7 @@ impl<Signer: Sign> Channel<Signer> {
                        if commitment_update.is_some() { "a" } else { "no" }, if raa.is_some() { "an" } else { "no" },
                        match order { RAACommitmentOrder::CommitmentFirst => "commitment", RAACommitmentOrder::RevokeAndACKFirst => "RAA"});
                MonitorRestoreUpdates {
-                       raa, commitment_update, order, accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, funding_broadcastable, funding_locked
+                       raa, commitment_update, order, accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, funding_broadcastable, funding_locked, announcement_sigs
                }
        }
 
@@ -3457,7 +3563,9 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// May panic if some calls other than message-handling calls (which will all Err immediately)
        /// have been called between remove_uncommitted_htlcs_and_mark_paused and this call.
-       pub fn channel_reestablish<L: Deref>(&mut self, msg: &msgs::ChannelReestablish, logger: &L) -> Result<(Option<msgs::FundingLocked>, Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, Option<ChannelMonitorUpdate>, RAACommitmentOrder, Vec<(HTLCSource, PaymentHash)>, Option<msgs::Shutdown>), ChannelError> where L::Target: Logger {
+       pub fn channel_reestablish<L: Deref>(&mut self, msg: &msgs::ChannelReestablish, logger: &L,
+               node_pk: PublicKey, genesis_block_hash: BlockHash, best_block: &BestBlock)
+       -> Result<ReestablishResponses, ChannelError> where L::Target: Logger {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == 0 {
                        // While BOLT 2 doesn't indicate explicitly we should error this channel here, it
                        // almost certainly indicates we are going to end up out-of-sync in some way, so we
@@ -3501,6 +3609,8 @@ impl<Signer: Sign> Channel<Signer> {
                        })
                } else { None };
 
+               let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, best_block.height(), logger);
+
                if self.channel_state & (ChannelState::FundingSent as u32) == ChannelState::FundingSent as u32 {
                        // If we're waiting on a monitor update, we shouldn't re-send any funding_locked's.
                        if self.channel_state & (ChannelState::OurFundingLocked as u32) == 0 ||
@@ -3509,15 +3619,27 @@ impl<Signer: Sign> Channel<Signer> {
                                        return Err(ChannelError::Close("Peer claimed they saw a revoke_and_ack but we haven't sent funding_locked yet".to_owned()));
                                }
                                // Short circuit the whole handler as there is nothing we can resend them
-                               return Ok((None, None, None, None, RAACommitmentOrder::CommitmentFirst, Vec::new(), shutdown_msg));
+                               return Ok(ReestablishResponses {
+                                       funding_locked: None,
+                                       raa: None, commitment_update: None, mon_update: None,
+                                       order: RAACommitmentOrder::CommitmentFirst,
+                                       holding_cell_failed_htlcs: Vec::new(),
+                                       shutdown_msg, announcement_sigs,
+                               });
                        }
 
                        // We have OurFundingLocked set!
                        let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
-                       return Ok((Some(msgs::FundingLocked {
-                               channel_id: self.channel_id(),
-                               next_per_commitment_point,
-                       }), None, None, None, RAACommitmentOrder::CommitmentFirst, Vec::new(), shutdown_msg));
+                       return Ok(ReestablishResponses {
+                               funding_locked: Some(msgs::FundingLocked {
+                                       channel_id: self.channel_id(),
+                                       next_per_commitment_point,
+                               }),
+                               raa: None, commitment_update: None, mon_update: None,
+                               order: RAACommitmentOrder::CommitmentFirst,
+                               holding_cell_failed_htlcs: Vec::new(),
+                               shutdown_msg, announcement_sigs,
+                       });
                }
 
                let required_revoke = if msg.next_remote_commitment_number + 1 == INITIAL_COMMITMENT_NUMBER - self.cur_holder_commitment_transaction_number {
@@ -3541,7 +3663,7 @@ impl<Signer: Sign> Channel<Signer> {
                // the corresponding revoke_and_ack back yet.
                let next_counterparty_commitment_number = INITIAL_COMMITMENT_NUMBER - self.cur_counterparty_commitment_transaction_number + if (self.channel_state & ChannelState::AwaitingRemoteRevoke as u32) != 0 { 1 } else { 0 };
 
-               let resend_funding_locked = if msg.next_local_commitment_number == 1 && INITIAL_COMMITMENT_NUMBER - self.cur_holder_commitment_transaction_number == 1 {
+               let funding_locked = if msg.next_local_commitment_number == 1 && INITIAL_COMMITMENT_NUMBER - self.cur_holder_commitment_transaction_number == 1 {
                        // We should never have to worry about MonitorUpdateFailed resending FundingLocked
                        let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
                        Some(msgs::FundingLocked {
@@ -3563,18 +3685,39 @@ impl<Signer: Sign> Channel<Signer> {
                                // have received some updates while we were disconnected. Free the holding cell
                                // now!
                                match self.free_holding_cell_htlcs(logger) {
-                                       Err(ChannelError::Close(msg)) => return Err(ChannelError::Close(msg)),
+                                       Err(ChannelError::Close(msg)) => Err(ChannelError::Close(msg)),
                                        Err(ChannelError::Warn(_)) | Err(ChannelError::Ignore(_)) | Err(ChannelError::CloseDelayBroadcast(_)) =>
                                                panic!("Got non-channel-failing result from free_holding_cell_htlcs"),
-                                       Ok((Some((commitment_update, monitor_update)), htlcs_to_fail)) => {
-                                               return Ok((resend_funding_locked, required_revoke, Some(commitment_update), Some(monitor_update), self.resend_order.clone(), htlcs_to_fail, shutdown_msg));
+                                       Ok((Some((commitment_update, monitor_update)), holding_cell_failed_htlcs)) => {
+                                               Ok(ReestablishResponses {
+                                                       funding_locked, shutdown_msg, announcement_sigs,
+                                                       raa: required_revoke,
+                                                       commitment_update: Some(commitment_update),
+                                                       order: self.resend_order.clone(),
+                                                       mon_update: Some(monitor_update),
+                                                       holding_cell_failed_htlcs,
+                                               })
                                        },
-                                       Ok((None, htlcs_to_fail)) => {
-                                               return Ok((resend_funding_locked, required_revoke, None, None, self.resend_order.clone(), htlcs_to_fail, shutdown_msg));
+                                       Ok((None, holding_cell_failed_htlcs)) => {
+                                               Ok(ReestablishResponses {
+                                                       funding_locked, shutdown_msg, announcement_sigs,
+                                                       raa: required_revoke,
+                                                       commitment_update: None,
+                                                       order: self.resend_order.clone(),
+                                                       mon_update: None,
+                                                       holding_cell_failed_htlcs,
+                                               })
                                        },
                                }
                        } else {
-                               return Ok((resend_funding_locked, required_revoke, None, None, self.resend_order.clone(), Vec::new(), shutdown_msg));
+                               Ok(ReestablishResponses {
+                                       funding_locked, shutdown_msg, announcement_sigs,
+                                       raa: required_revoke,
+                                       commitment_update: None,
+                                       order: self.resend_order.clone(),
+                                       mon_update: None,
+                                       holding_cell_failed_htlcs: Vec::new(),
+                               })
                        }
                } else if msg.next_local_commitment_number == next_counterparty_commitment_number - 1 {
                        if required_revoke.is_some() {
@@ -3585,12 +3728,24 @@ impl<Signer: Sign> Channel<Signer> {
 
                        if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) != 0 {
                                self.monitor_pending_commitment_signed = true;
-                               return Ok((resend_funding_locked, None, None, None, self.resend_order.clone(), Vec::new(), shutdown_msg));
+                               Ok(ReestablishResponses {
+                                       funding_locked, shutdown_msg, announcement_sigs,
+                                       commitment_update: None, raa: None, mon_update: None,
+                                       order: self.resend_order.clone(),
+                                       holding_cell_failed_htlcs: Vec::new(),
+                               })
+                       } else {
+                               Ok(ReestablishResponses {
+                                       funding_locked, shutdown_msg, announcement_sigs,
+                                       raa: required_revoke,
+                                       commitment_update: Some(self.get_last_commitment_update(logger)),
+                                       order: self.resend_order.clone(),
+                                       mon_update: None,
+                                       holding_cell_failed_htlcs: Vec::new(),
+                               })
                        }
-
-                       return Ok((resend_funding_locked, required_revoke, Some(self.get_last_commitment_update(logger)), None, self.resend_order.clone(), Vec::new(), shutdown_msg));
                } else {
-                       return Err(ChannelError::Close("Peer attempted to reestablish channel with a very old remote commitment transaction".to_owned()));
+                       Err(ChannelError::Close("Peer attempted to reestablish channel with a very old remote commitment transaction".to_owned()))
                }
        }
 
@@ -4159,7 +4314,7 @@ impl<Signer: Sign> Channel<Signer> {
        /// Allowed in any state (including after shutdown)
        pub fn is_usable(&self) -> bool {
                let mask = ChannelState::ChannelFunded as u32 | BOTH_SIDES_SHUTDOWN_MASK;
-               (self.channel_state & mask) == (ChannelState::ChannelFunded as u32)
+               (self.channel_state & mask) == (ChannelState::ChannelFunded as u32) && !self.monitor_pending_funding_locked
        }
 
        /// Returns true if this channel is currently available for use. This is a superset of
@@ -4243,11 +4398,13 @@ impl<Signer: Sign> Channel<Signer> {
 
                if need_commitment_update {
                        if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
-                               let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
-                               return Some(msgs::FundingLocked {
-                                       channel_id: self.channel_id,
-                                       next_per_commitment_point,
-                               });
+                               if self.channel_state & (ChannelState::PeerDisconnected as u32) == 0 {
+                                       let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
+                                       return Some(msgs::FundingLocked {
+                                               channel_id: self.channel_id,
+                                               next_per_commitment_point,
+                                       });
+                               }
                        } else {
                                self.monitor_pending_funding_locked = true;
                        }
@@ -4258,11 +4415,12 @@ impl<Signer: Sign> Channel<Signer> {
        /// When a transaction is confirmed, we check whether it is or spends the funding transaction
        /// In the first case, we store the confirmation height and calculating the short channel id.
        /// In the second, we simply return an Err indicating we need to be force-closed now.
-       pub fn transactions_confirmed<L: Deref>(&mut self, block_hash: &BlockHash, height: u32, txdata: &TransactionData, logger: &L)
-       -> Result<Option<msgs::FundingLocked>, ClosureReason> where L::Target: Logger {
+       pub fn transactions_confirmed<L: Deref>(&mut self, block_hash: &BlockHash, height: u32,
+               txdata: &TransactionData, genesis_block_hash: BlockHash, node_pk: PublicKey, logger: &L)
+       -> Result<(Option<msgs::FundingLocked>, Option<msgs::AnnouncementSignatures>), ClosureReason> where L::Target: Logger {
                let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
-               for &(index_in_block, tx) in txdata.iter() {
-                       if let Some(funding_txo) = self.get_funding_txo() {
+               if let Some(funding_txo) = self.get_funding_txo() {
+                       for &(index_in_block, tx) in txdata.iter() {
                                // If we haven't yet sent a funding_locked, but are in FundingSent (ignoring
                                // whether they've sent a funding_locked or not), check if we should send one.
                                if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 {
@@ -4306,7 +4464,8 @@ impl<Signer: Sign> Channel<Signer> {
                                        // may have already happened for this block).
                                        if let Some(funding_locked) = self.check_get_funding_locked(height) {
                                                log_info!(logger, "Sending a funding_locked to our peer for channel {}", log_bytes!(self.channel_id));
-                                               return Ok(Some(funding_locked));
+                                               let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, height, logger);
+                                               return Ok((Some(funding_locked), announcement_sigs));
                                        }
                                }
                                for inp in tx.input.iter() {
@@ -4317,7 +4476,7 @@ impl<Signer: Sign> Channel<Signer> {
                                }
                        }
                }
-               Ok(None)
+               Ok((None, None))
        }
 
        /// When a new block is connected, we check the height of the block against outbound holding
@@ -4331,8 +4490,13 @@ impl<Signer: Sign> Channel<Signer> {
        ///
        /// May return some HTLCs (and their payment_hash) which have timed out and should be failed
        /// back.
-       pub fn best_block_updated<L: Deref>(&mut self, height: u32, highest_header_time: u32, logger: &L)
-       -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), ClosureReason> where L::Target: Logger {
+       pub fn best_block_updated<L: Deref>(&mut self, height: u32, highest_header_time: u32, genesis_block_hash: BlockHash, node_pk: PublicKey, logger: &L)
+       -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>, Option<msgs::AnnouncementSignatures>), ClosureReason> where L::Target: Logger {
+               self.do_best_block_updated(height, highest_header_time, Some((genesis_block_hash, node_pk)), logger)
+       }
+
+       fn do_best_block_updated<L: Deref>(&mut self, height: u32, highest_header_time: u32, genesis_node_pk: Option<(BlockHash, PublicKey)>, logger: &L)
+       -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>, Option<msgs::AnnouncementSignatures>), ClosureReason> where L::Target: Logger {
                let mut timed_out_htlcs = Vec::new();
                // This mirrors the check in ChannelManager::decode_update_add_htlc_onion, refusing to
                // forward an HTLC when our counterparty should almost certainly just fail it for expiring
@@ -4353,8 +4517,11 @@ impl<Signer: Sign> Channel<Signer> {
                self.update_time_counter = cmp::max(self.update_time_counter, highest_header_time);
 
                if let Some(funding_locked) = self.check_get_funding_locked(height) {
+                       let announcement_sigs = if let Some((genesis_block_hash, node_pk)) = genesis_node_pk {
+                               self.get_announcement_sigs(node_pk, genesis_block_hash, height, logger)
+                       } else { None };
                        log_info!(logger, "Sending a funding_locked to our peer for channel {}", log_bytes!(self.channel_id));
-                       return Ok((Some(funding_locked), timed_out_htlcs));
+                       return Ok((Some(funding_locked), timed_out_htlcs, announcement_sigs));
                }
 
                let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
@@ -4386,7 +4553,10 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ClosureReason::FundingTimedOut);
                }
 
-               Ok((None, timed_out_htlcs))
+               let announcement_sigs = if let Some((genesis_block_hash, node_pk)) = genesis_node_pk {
+                       self.get_announcement_sigs(node_pk, genesis_block_hash, height, logger)
+               } else { None };
+               Ok((None, timed_out_htlcs, announcement_sigs))
        }
 
        /// Indicates the funding transaction is no longer confirmed in the main chain. This may
@@ -4401,10 +4571,11 @@ impl<Signer: Sign> Channel<Signer> {
                        // larger. If we don't know that time has moved forward, we can just set it to the last
                        // time we saw and it will be ignored.
                        let best_time = self.update_time_counter;
-                       match self.best_block_updated(reorg_height, best_time, logger) {
-                               Ok((funding_locked, timed_out_htlcs)) => {
+                       match self.do_best_block_updated(reorg_height, best_time, None, logger) {
+                               Ok((funding_locked, timed_out_htlcs, announcement_sigs)) => {
                                        assert!(funding_locked.is_none(), "We can't generate a funding with 0 confirmations?");
                                        assert!(timed_out_htlcs.is_empty(), "We can't have accepted HTLCs with a timeout before our funding confirmation?");
+                                       assert!(announcement_sigs.is_none(), "We can't generate an announcement_sigs with 0 confirmations?");
                                        Ok(())
                                },
                                Err(e) => Err(e)
@@ -4500,7 +4671,7 @@ impl<Signer: Sign> Channel<Signer> {
        fn get_outbound_funding_created_signature<L: Deref>(&mut self, logger: &L) -> Result<Signature, ChannelError> where L::Target: Logger {
                let counterparty_keys = self.build_remote_transaction_keys()?;
                let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
-               Ok(self.holder_signer.sign_counterparty_commitment(&counterparty_initial_commitment_tx, &self.secp_ctx)
+               Ok(self.holder_signer.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.secp_ctx)
                                .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0)
        }
 
@@ -4552,25 +4723,24 @@ impl<Signer: Sign> Channel<Signer> {
                })
        }
 
-       /// Gets an UnsignedChannelAnnouncement, as well as a signature covering it using our
-       /// bitcoin_key, if available, for this channel. The channel must be publicly announceable and
-       /// available for use (have exchanged FundingLocked messages in both directions). Should be used
-       /// for both loose and in response to an AnnouncementSignatures message from the remote peer.
+       /// Gets an UnsignedChannelAnnouncement for this channel. The channel must be publicly
+       /// announceable and available for use (have exchanged FundingLocked messages in both
+       /// directions). Should be used for both broadcasted announcements and in response to an
+       /// AnnouncementSignatures message from the remote peer.
+       ///
        /// Will only fail if we're not in a state where channel_announcement may be sent (including
        /// closing).
+       ///
        /// Note that the "channel must be funded" requirement is stricter than BOLT 7 requires - see
        /// https://github.com/lightningnetwork/lightning-rfc/issues/468
        ///
        /// This will only return ChannelError::Ignore upon failure.
-       pub fn get_channel_announcement(&self, node_id: PublicKey, chain_hash: BlockHash) -> Result<(msgs::UnsignedChannelAnnouncement, Signature), ChannelError> {
+       fn get_channel_announcement(&self, node_id: PublicKey, chain_hash: BlockHash) -> Result<msgs::UnsignedChannelAnnouncement, ChannelError> {
                if !self.config.announced_channel {
                        return Err(ChannelError::Ignore("Channel is not available for public announcements".to_owned()));
                }
-               if self.channel_state & (ChannelState::ChannelFunded as u32) == 0 {
-                       return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement until the channel funding has been locked".to_owned()));
-               }
-               if (self.channel_state & (ChannelState::LocalShutdownSent as u32 | ChannelState::ShutdownComplete as u32)) != 0 {
-                       return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement once the channel is closing".to_owned()));
+               if !self.is_usable() {
+                       return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel is not currently usable".to_owned()));
                }
 
                let were_node_one = node_id.serialize()[..] < self.counterparty_node_id.serialize()[..];
@@ -4586,19 +4756,61 @@ impl<Signer: Sign> Channel<Signer> {
                        excess_data: Vec::new(),
                };
 
-               let sig = self.holder_signer.sign_channel_announcement(&msg, &self.secp_ctx)
-                       .map_err(|_| ChannelError::Ignore("Signer rejected channel_announcement".to_owned()))?;
+               Ok(msg)
+       }
+
+       fn get_announcement_sigs<L: Deref>(&mut self, node_pk: PublicKey, genesis_block_hash: BlockHash, best_block_height: u32, logger: &L)
+       -> Option<msgs::AnnouncementSignatures> where L::Target: Logger {
+               if self.funding_tx_confirmation_height == 0 || self.funding_tx_confirmation_height + 5 > best_block_height {
+                       return None;
+               }
+
+               if !self.is_usable() {
+                       return None;
+               }
+
+               if self.channel_state & ChannelState::PeerDisconnected as u32 != 0 {
+                       log_trace!(logger, "Cannot create an announcement_signatures as our peer is disconnected");
+                       return None;
+               }
+
+               if self.announcement_sigs_state != AnnouncementSigsState::NotSent {
+                       return None;
+               }
+
+               log_trace!(logger, "Creating an announcement_signatures message for channel {}", log_bytes!(self.channel_id()));
+               let announcement = match self.get_channel_announcement(node_pk, genesis_block_hash) {
+                       Ok(a) => a,
+                       Err(_) => {
+                               log_trace!(logger, "Cannot create an announcement_signatures as channel is not public.");
+                               return None;
+                       }
+               };
+               let (our_node_sig, our_bitcoin_sig) = match self.holder_signer.sign_channel_announcement(&announcement, &self.secp_ctx) {
+                       Err(_) => {
+                               log_error!(logger, "Signer rejected channel_announcement signing. Channel will not be announced!");
+                               return None;
+                       },
+                       Ok(v) => v
+               };
+               self.announcement_sigs_state = AnnouncementSigsState::MessageSent;
 
-               Ok((msg, sig))
+               Some(msgs::AnnouncementSignatures {
+                       channel_id: self.channel_id(),
+                       short_channel_id: self.get_short_channel_id().unwrap(),
+                       node_signature: our_node_sig,
+                       bitcoin_signature: our_bitcoin_sig,
+               })
        }
 
        /// Signs the given channel announcement, returning a ChannelError::Ignore if no keys are
        /// available.
-       fn sign_channel_announcement(&self, our_node_secret: &SecretKey, our_node_id: PublicKey, msghash: secp256k1::Message, announcement: msgs::UnsignedChannelAnnouncement, our_bitcoin_sig: Signature) -> Result<msgs::ChannelAnnouncement, ChannelError> {
+       fn sign_channel_announcement(&self, our_node_id: PublicKey, announcement: msgs::UnsignedChannelAnnouncement) -> Result<msgs::ChannelAnnouncement, ChannelError> {
                if let Some((their_node_sig, their_bitcoin_sig)) = self.announcement_sigs {
                        let were_node_one = announcement.node_id_1 == our_node_id;
 
-                       let our_node_sig = self.secp_ctx.sign(&msghash, our_node_secret);
+                       let (our_node_sig, our_bitcoin_sig) = self.holder_signer.sign_channel_announcement(&announcement, &self.secp_ctx)
+                               .map_err(|_| ChannelError::Ignore("Signer rejected channel_announcement".to_owned()))?;
                        Ok(msgs::ChannelAnnouncement {
                                node_signature_1: if were_node_one { our_node_sig } else { their_node_sig },
                                node_signature_2: if were_node_one { their_node_sig } else { our_node_sig },
@@ -4614,8 +4826,8 @@ impl<Signer: Sign> Channel<Signer> {
        /// Processes an incoming announcement_signatures message, providing a fully-signed
        /// channel_announcement message which we can broadcast and storing our counterparty's
        /// signatures for later reconstruction/rebroadcast of the channel_announcement.
-       pub fn announcement_signatures(&mut self, our_node_secret: &SecretKey, our_node_id: PublicKey, chain_hash: BlockHash, msg: &msgs::AnnouncementSignatures) -> Result<msgs::ChannelAnnouncement, ChannelError> {
-               let (announcement, our_bitcoin_sig) = self.get_channel_announcement(our_node_id.clone(), chain_hash)?;
+       pub fn announcement_signatures(&mut self, our_node_id: PublicKey, chain_hash: BlockHash, best_block_height: u32, msg: &msgs::AnnouncementSignatures) -> Result<msgs::ChannelAnnouncement, ChannelError> {
+               let announcement = self.get_channel_announcement(our_node_id.clone(), chain_hash)?;
 
                let msghash = hash_to_message!(&Sha256d::hash(&announcement.encode()[..])[..]);
 
@@ -4631,19 +4843,25 @@ impl<Signer: Sign> Channel<Signer> {
                }
 
                self.announcement_sigs = Some((msg.node_signature, msg.bitcoin_signature));
+               if self.funding_tx_confirmation_height == 0 || self.funding_tx_confirmation_height + 5 > best_block_height {
+                       return Err(ChannelError::Ignore(
+                               "Got announcement_signatures prior to the required six confirmations - we may not have received a block yet that our peer has".to_owned()));
+               }
 
-               self.sign_channel_announcement(our_node_secret, our_node_id, msghash, announcement, our_bitcoin_sig)
+               self.sign_channel_announcement(our_node_id, announcement)
        }
 
        /// Gets a signed channel_announcement for this channel, if we previously received an
        /// announcement_signatures from our counterparty.
-       pub fn get_signed_channel_announcement(&self, our_node_secret: &SecretKey, our_node_id: PublicKey, chain_hash: BlockHash) -> Option<msgs::ChannelAnnouncement> {
-               let (announcement, our_bitcoin_sig) = match self.get_channel_announcement(our_node_id.clone(), chain_hash) {
+       pub fn get_signed_channel_announcement(&self, our_node_id: PublicKey, chain_hash: BlockHash, best_block_height: u32) -> Option<msgs::ChannelAnnouncement> {
+               if self.funding_tx_confirmation_height == 0 || self.funding_tx_confirmation_height + 5 > best_block_height {
+                       return None;
+               }
+               let announcement = match self.get_channel_announcement(our_node_id.clone(), chain_hash) {
                        Ok(res) => res,
                        Err(_) => return None,
                };
-               let msghash = hash_to_message!(&Sha256d::hash(&announcement.encode()[..])[..]);
-               match self.sign_channel_announcement(our_node_secret, our_node_id, msghash, announcement, our_bitcoin_sig) {
+               match self.sign_channel_announcement(our_node_id, announcement) {
                        Ok(res) => Some(res),
                        Err(_) => None,
                }
@@ -4891,11 +5109,12 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                }
                for htlc in self.pending_outbound_htlcs.iter_mut() {
-                       if let Some(fail_reason) = if let &mut OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref mut fail_reason) = &mut htlc.state {
-                               Some(fail_reason.take())
-                       } else { None } {
+                       if let &mut OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref mut outcome) = &mut htlc.state {
                                log_trace!(logger, " ...promoting outbound AwaitingRemoteRevokeToRemove {} to AwaitingRemovedRemoteRevoke", log_bytes!(htlc.payment_hash.0));
-                               htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke(fail_reason);
+                               // Grab the preimage, if it exists, instead of cloning
+                               let mut reason = OutboundHTLCOutcome::Success(None);
+                               mem::swap(outcome, &mut reason);
+                               htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke(reason);
                        }
                }
                if let Some((feerate, update_state)) = self.pending_update_fee {
@@ -4918,6 +5137,10 @@ impl<Signer: Sign> Channel<Signer> {
                        Err(e) => return Err(e),
                };
 
+               if self.announcement_sigs_state == AnnouncementSigsState::MessageSent {
+                       self.announcement_sigs_state = AnnouncementSigsState::Committed;
+               }
+
                self.latest_monitor_update_id += 1;
                let monitor_update = ChannelMonitorUpdate {
                        update_id: self.latest_monitor_update_id,
@@ -4964,7 +5187,7 @@ impl<Signer: Sign> Channel<Signer> {
                                htlcs.push(htlc);
                        }
 
-                       let res = self.holder_signer.sign_counterparty_commitment(&commitment_stats.tx, &self.secp_ctx)
+                       let res = self.holder_signer.sign_counterparty_commitment(&commitment_stats.tx, commitment_stats.preimages, &self.secp_ctx)
                                .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?;
                        signature = res.0;
                        htlc_signatures = res.1;
@@ -5177,6 +5400,29 @@ impl Readable for ChannelUpdateStatus {
        }
 }
 
+impl Writeable for AnnouncementSigsState {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+               // We only care about writing out the current state as if we had just disconnected, at
+               // which point we always set anything but AnnouncementSigsReceived to NotSent.
+               match self {
+                       AnnouncementSigsState::NotSent => 0u8.write(writer),
+                       AnnouncementSigsState::MessageSent => 0u8.write(writer),
+                       AnnouncementSigsState::Committed => 0u8.write(writer),
+                       AnnouncementSigsState::PeerReceived => 1u8.write(writer),
+               }
+       }
+}
+
+impl Readable for AnnouncementSigsState {
+       fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+               Ok(match <u8 as Readable>::read(reader)? {
+                       0 => AnnouncementSigsState::NotSent,
+                       1 => AnnouncementSigsState::PeerReceived,
+                       _ => return Err(DecodeError::InvalidValue),
+               })
+       }
+}
+
 impl<Signer: Sign> Writeable for Channel<Signer> {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
                // Note that we write out as if remove_uncommitted_htlcs_and_mark_paused had just been
@@ -5253,6 +5499,8 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                        }
                }
 
+               let mut preimages: Vec<&Option<PaymentPreimage>> = vec![];
+
                (self.pending_outbound_htlcs.len() as u64).write(writer)?;
                for htlc in self.pending_outbound_htlcs.iter() {
                        htlc.htlc_id.write(writer)?;
@@ -5273,14 +5521,22 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                                        // resend the claim/fail on reconnect as we all (hopefully) the missing CS.
                                        1u8.write(writer)?;
                                },
-                               &OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref fail_reason) => {
+                               &OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref outcome) => {
                                        3u8.write(writer)?;
-                                       fail_reason.write(writer)?;
-                               },
-                               &OutboundHTLCState::AwaitingRemovedRemoteRevoke(ref fail_reason) => {
+                                       if let OutboundHTLCOutcome::Success(preimage) = outcome {
+                                               preimages.push(preimage);
+                                       }
+                                       let reason: Option<&HTLCFailReason> = outcome.into();
+                                       reason.write(writer)?;
+                               }
+                               &OutboundHTLCState::AwaitingRemovedRemoteRevoke(ref outcome) => {
                                        4u8.write(writer)?;
-                                       fail_reason.write(writer)?;
-                               },
+                                       if let OutboundHTLCOutcome::Success(preimage) = outcome {
+                                               preimages.push(preimage);
+                                       }
+                                       let reason: Option<&HTLCFailReason> = outcome.into();
+                                       reason.write(writer)?;
+                               }
                        }
                }
 
@@ -5434,6 +5690,8 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                        (9, self.target_closing_feerate_sats_per_kw, option),
                        (11, self.monitor_pending_finalized_fulfills, vec_type),
                        (13, self.channel_creation_height, required),
+                       (15, preimages, vec_type),
+                       (17, self.announcement_sigs_state, required),
                });
 
                Ok(())
@@ -5519,9 +5777,18 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
                                state: match <u8 as Readable>::read(reader)? {
                                        0 => OutboundHTLCState::LocalAnnounced(Box::new(Readable::read(reader)?)),
                                        1 => OutboundHTLCState::Committed,
-                                       2 => OutboundHTLCState::RemoteRemoved(Readable::read(reader)?),
-                                       3 => OutboundHTLCState::AwaitingRemoteRevokeToRemove(Readable::read(reader)?),
-                                       4 => OutboundHTLCState::AwaitingRemovedRemoteRevoke(Readable::read(reader)?),
+                                       2 => {
+                                               let option: Option<HTLCFailReason> = Readable::read(reader)?;
+                                               OutboundHTLCState::RemoteRemoved(option.into())
+                                       },
+                                       3 => {
+                                               let option: Option<HTLCFailReason> = Readable::read(reader)?;
+                                               OutboundHTLCState::AwaitingRemoteRevokeToRemove(option.into())
+                                       },
+                                       4 => {
+                                               let option: Option<HTLCFailReason> = Readable::read(reader)?;
+                                               OutboundHTLCState::AwaitingRemovedRemoteRevoke(option.into())
+                                       },
                                        _ => return Err(DecodeError::InvalidValue),
                                },
                        });
@@ -5675,6 +5942,12 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
                // only, so we default to that if none was written.
                let mut channel_type = Some(ChannelTypeFeatures::only_static_remote_key());
                let mut channel_creation_height = Some(serialized_height);
+               let mut preimages_opt: Option<Vec<Option<PaymentPreimage>>> = None;
+
+               // If we read an old Channel, for simplicity we just treat it as "we never sent an
+               // AnnouncementSignatures" which implies we'll re-send it on reconnect, but that's fine.
+               let mut announcement_sigs_state = Some(AnnouncementSigsState::NotSent);
+
                read_tlv_fields!(reader, {
                        (0, announcement_sigs, option),
                        (1, minimum_depth, option),
@@ -5687,8 +5960,29 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
                        (9, target_closing_feerate_sats_per_kw, option),
                        (11, monitor_pending_finalized_fulfills, vec_type),
                        (13, channel_creation_height, option),
+                       (15, preimages_opt, vec_type),
+                       (17, announcement_sigs_state, option),
                });
 
+               if let Some(preimages) = preimages_opt {
+                       let mut iter = preimages.into_iter();
+                       for htlc in pending_outbound_htlcs.iter_mut() {
+                               match &htlc.state {
+                                       OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(None)) => {
+                                               htlc.state = OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(iter.next().ok_or(DecodeError::InvalidValue)?));
+                                       }
+                                       OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(None)) => {
+                                               htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(iter.next().ok_or(DecodeError::InvalidValue)?));
+                                       }
+                                       _ => {}
+                               }
+                       }
+                       // We expect all preimages to be consumed above
+                       if iter.next().is_some() {
+                               return Err(DecodeError::InvalidValue);
+                       }
+               }
+
                let chan_features = channel_type.as_ref().unwrap();
                if chan_features.supports_unknown_bits() || chan_features.requires_unknown_bits() {
                        // If the channel was written by a new version and negotiated with features we don't
@@ -5710,6 +6004,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
                        config: config.unwrap(),
                        channel_id,
                        channel_state,
+                       announcement_sigs_state: announcement_sigs_state.unwrap(),
                        secp_ctx,
                        channel_value_satoshis,
 
@@ -6190,6 +6485,7 @@ mod tests {
 
                let mut signer = InMemorySigner::new(
                        &secp_ctx,
+                       SecretKey::from_slice(&hex::decode("4242424242424242424242424242424242424242424242424242424242424242").unwrap()[..]).unwrap(),
                        SecretKey::from_slice(&hex::decode("30ff4956bbdd3222d44cc5e8a1261dab1e07957bdac5ae88fe3261ef321f3749").unwrap()[..]).unwrap(),
                        SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(),
                        SecretKey::from_slice(&hex::decode("1111111111111111111111111111111111111111111111111111111111111111").unwrap()[..]).unwrap(),
index 0ca9dbd465be9985cb6363780c1168be63aff362..2097dfda9f187a56eb652e1c8122eab1185e6a12 100644 (file)
@@ -1533,7 +1533,7 @@ macro_rules! maybe_break_monitor_err {
 macro_rules! handle_chan_restoration_locked {
        ($self: ident, $channel_lock: expr, $channel_state: expr, $channel_entry: expr,
         $raa: expr, $commitment_update: expr, $order: expr, $chanmon_update: expr,
-        $pending_forwards: expr, $funding_broadcastable: expr, $funding_locked: expr) => { {
+        $pending_forwards: expr, $funding_broadcastable: expr, $funding_locked: expr, $announcement_sigs: expr) => { {
                let mut htlc_forwards = None;
                let counterparty_node_id = $channel_entry.get().get_counterparty_node_id();
 
@@ -1568,14 +1568,14 @@ macro_rules! handle_chan_restoration_locked {
                                        node_id: counterparty_node_id,
                                        msg,
                                });
-                               if let Some(announcement_sigs) = $self.get_announcement_sigs($channel_entry.get()) {
-                                       $channel_state.pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
-                                               node_id: counterparty_node_id,
-                                               msg: announcement_sigs,
-                                       });
-                               }
                                $channel_state.short_to_id.insert($channel_entry.get().get_short_channel_id().unwrap(), $channel_entry.get().channel_id());
                        }
+                       if let Some(msg) = $announcement_sigs {
+                               $channel_state.pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
+                                       node_id: counterparty_node_id,
+                                       msg,
+                               });
+                       }
 
                        let funding_broadcastable: Option<Transaction> = $funding_broadcastable; // Force type-checking to resolve
                        if let Some(monitor_update) = chanmon_update {
@@ -2891,27 +2891,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                })
        }
 
-       fn get_announcement_sigs(&self, chan: &Channel<Signer>) -> Option<msgs::AnnouncementSignatures> {
-               if !chan.should_announce() {
-                       log_trace!(self.logger, "Can't send announcement_signatures for private channel {}", log_bytes!(chan.channel_id()));
-                       return None
-               }
-
-               let (announcement, our_bitcoin_sig) = match chan.get_channel_announcement(self.get_our_node_id(), self.genesis_hash.clone()) {
-                       Ok(res) => res,
-                       Err(_) => return None, // Only in case of state precondition violations eg channel is closing
-               };
-               let msghash = hash_to_message!(&Sha256dHash::hash(&announcement.encode()[..])[..]);
-               let our_node_sig = self.secp_ctx.sign(&msghash, &self.our_network_key);
-
-               Some(msgs::AnnouncementSignatures {
-                       channel_id: chan.channel_id(),
-                       short_channel_id: chan.get_short_channel_id().unwrap(),
-                       node_signature: our_node_sig,
-                       bitcoin_signature: our_bitcoin_sig,
-               })
-       }
-
        #[allow(dead_code)]
        // Messages of up to 64KB should never end up more than half full with addresses, as that would
        // be absurd. We ensure this by checking that at least 500 (our stated public contract on when
@@ -2969,7 +2948,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
                let mut announced_chans = false;
                for (_, chan) in channel_state.by_id.iter() {
-                       if let Some(msg) = chan.get_signed_channel_announcement(&self.our_network_key, self.get_our_node_id(), self.genesis_hash.clone()) {
+                       if let Some(msg) = chan.get_signed_channel_announcement(self.get_our_node_id(), self.genesis_hash.clone(), self.best_block.read().unwrap().height()) {
                                channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelAnnouncement {
                                        msg,
                                        update_msg: match self.get_channel_update_for_broadcast(chan) {
@@ -4077,18 +4056,19 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                return;
                        }
 
-                       let updates = channel.get_mut().monitor_updating_restored(&self.logger);
-                       let channel_update = if updates.funding_locked.is_some() && channel.get().is_usable() && !channel.get().should_announce() {
+                       let updates = channel.get_mut().monitor_updating_restored(&self.logger, self.get_our_node_id(), self.genesis_hash, self.best_block.read().unwrap().height());
+                       let channel_update = if updates.funding_locked.is_some() && channel.get().is_usable() {
                                // We only send a channel_update in the case where we are just now sending a
-                               // funding_locked and the channel is in a usable state. Further, we rely on the
-                               // normal announcement_signatures process to send a channel_update for public
-                               // channels, only generating a unicast channel_update if this is a private channel.
+                               // funding_locked and the channel is in a usable state. We may re-send a
+                               // channel_update later through the announcement_signatures process for public
+                               // channels, but there's no reason not to just inform our counterparty of our fees
+                               // now.
                                Some(events::MessageSendEvent::SendChannelUpdate {
                                        node_id: channel.get().get_counterparty_node_id(),
                                        msg: self.get_channel_update_for_unicast(channel.get()).unwrap(),
                                })
                        } else { None };
-                       chan_restoration_res = handle_chan_restoration_locked!(self, channel_lock, channel_state, channel, updates.raa, updates.commitment_update, updates.order, None, updates.accepted_htlcs, updates.funding_broadcastable, updates.funding_locked);
+                       chan_restoration_res = handle_chan_restoration_locked!(self, channel_lock, channel_state, channel, updates.raa, updates.commitment_update, updates.order, None, updates.accepted_htlcs, updates.funding_broadcastable, updates.funding_locked, updates.announcement_sigs);
                        if let Some(upd) = channel_update {
                                channel_state.pending_msg_events.push(upd);
                        }
@@ -4254,23 +4234,21 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                if chan.get().get_counterparty_node_id() != *counterparty_node_id {
                                        return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id));
                                }
-                               try_chan_entry!(self, chan.get_mut().funding_locked(&msg, &self.logger), channel_state, chan);
-                               if let Some(announcement_sigs) = self.get_announcement_sigs(chan.get()) {
-                                       log_trace!(self.logger, "Sending announcement_signatures for {} in response to funding_locked", log_bytes!(chan.get().channel_id()));
-                                       // If we see locking block before receiving remote funding_locked, we broadcast our
-                                       // announcement_sigs at remote funding_locked reception. If we receive remote
-                                       // funding_locked before seeing locking block, we broadcast our announcement_sigs at locking
-                                       // block connection. We should guanrantee to broadcast announcement_sigs to our peer whatever
-                                       // the order of the events but our peer may not receive it due to disconnection. The specs
-                                       // lacking an acknowledgement for announcement_sigs we may have to re-send them at peer
-                                       // connection in the future if simultaneous misses by both peers due to network/hardware
-                                       // failures is an issue. Note, to achieve its goal, only one of the announcement_sigs needs
-                                       // to be received, from then sigs are going to be flood to the whole network.
+                               let announcement_sigs_opt = try_chan_entry!(self, chan.get_mut().funding_locked(&msg, self.get_our_node_id(),
+                                       self.genesis_hash.clone(), &self.best_block.read().unwrap(), &self.logger), channel_state, chan);
+                               if let Some(announcement_sigs) = announcement_sigs_opt {
+                                       log_trace!(self.logger, "Sending announcement_signatures for channel {}", log_bytes!(chan.get().channel_id()));
                                        channel_state.pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
                                                node_id: counterparty_node_id.clone(),
                                                msg: announcement_sigs,
                                        });
                                } else if chan.get().is_usable() {
+                                       // If we're sending an announcement_signatures, we'll send the (public)
+                                       // channel_update after sending a channel_announcement when we receive our
+                                       // counterparty's announcement_signatures. Thus, we only bother to send a
+                                       // channel_update here if the channel is not public, i.e. we're not sending an
+                                       // announcement_signatures.
+                                       log_trace!(self.logger, "Sending private initial channel_update for our counterparty on channel {}", log_bytes!(chan.get().channel_id()));
                                        channel_state.pending_msg_events.push(events::MessageSendEvent::SendChannelUpdate {
                                                node_id: counterparty_node_id.clone(),
                                                msg: self.get_channel_update_for_unicast(chan.get()).unwrap(),
@@ -4674,7 +4652,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                }
 
                                channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelAnnouncement {
-                                       msg: try_chan_entry!(self, chan.get_mut().announcement_signatures(&self.our_network_key, self.get_our_node_id(), self.genesis_hash.clone(), msg), channel_state, chan),
+                                       msg: try_chan_entry!(self, chan.get_mut().announcement_signatures(
+                                               self.get_our_node_id(), self.genesis_hash.clone(), self.best_block.read().unwrap().height(), msg), channel_state, chan),
                                        // Note that announcement_signatures fails if the channel cannot be announced,
                                        // so get_channel_update_for_broadcast will never fail by the time we get here.
                                        update_msg: self.get_channel_update_for_broadcast(chan.get()).unwrap(),
@@ -4735,10 +4714,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        // disconnect, so Channel's reestablish will never hand us any holding cell
                                        // freed HTLCs to fail backwards. If in the future we no longer drop pending
                                        // add-HTLCs on disconnect, we may be handed HTLCs to fail backwards here.
-                                       let (funding_locked, revoke_and_ack, commitment_update, monitor_update_opt, order, htlcs_failed_forward, shutdown) =
-                                               try_chan_entry!(self, chan.get_mut().channel_reestablish(msg, &self.logger), channel_state, chan);
+                                       let responses = try_chan_entry!(self, chan.get_mut().channel_reestablish(
+                                               msg, &self.logger, self.our_network_pubkey.clone(), self.genesis_hash,
+                                               &*self.best_block.read().unwrap()), channel_state, chan);
                                        let mut channel_update = None;
-                                       if let Some(msg) = shutdown {
+                                       if let Some(msg) = responses.shutdown_msg {
                                                channel_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown {
                                                        node_id: counterparty_node_id.clone(),
                                                        msg,
@@ -4753,11 +4733,13 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                });
                                        }
                                        let need_lnd_workaround = chan.get_mut().workaround_lnd_bug_4006.take();
-                                       chan_restoration_res = handle_chan_restoration_locked!(self, channel_state_lock, channel_state, chan, revoke_and_ack, commitment_update, order, monitor_update_opt, Vec::new(), None, funding_locked);
+                                       chan_restoration_res = handle_chan_restoration_locked!(
+                                               self, channel_state_lock, channel_state, chan, responses.raa, responses.commitment_update, responses.order,
+                                               responses.mon_update, Vec::new(), None, responses.funding_locked, responses.announcement_sigs);
                                        if let Some(upd) = channel_update {
                                                channel_state.pending_msg_events.push(upd);
                                        }
-                                       (htlcs_failed_forward, need_lnd_workaround)
+                                       (responses.holding_cell_failed_htlcs, need_lnd_workaround)
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
                        }
@@ -5260,7 +5242,7 @@ where
                        *best_block = BestBlock::new(header.prev_blockhash, new_height)
                }
 
-               self.do_chain_event(Some(new_height), |channel| channel.best_block_updated(new_height, header.time, &self.logger));
+               self.do_chain_event(Some(new_height), |channel| channel.best_block_updated(new_height, header.time, self.genesis_hash.clone(), self.get_our_node_id(), &self.logger));
        }
 }
 
@@ -5281,7 +5263,8 @@ where
                log_trace!(self.logger, "{} transactions included in block {} at height {} provided", txdata.len(), block_hash, height);
 
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
-               self.do_chain_event(Some(height), |channel| channel.transactions_confirmed(&block_hash, height, txdata, &self.logger).map(|a| (a, Vec::new())));
+               self.do_chain_event(Some(height), |channel| channel.transactions_confirmed(&block_hash, height, txdata, self.genesis_hash.clone(), self.get_our_node_id(), &self.logger)
+                       .map(|(a, b)| (a, Vec::new(), b)));
        }
 
        fn best_block_updated(&self, header: &BlockHeader, height: u32) {
@@ -5296,7 +5279,7 @@ where
 
                *self.best_block.write().unwrap() = BestBlock::new(block_hash, height);
 
-               self.do_chain_event(Some(height), |channel| channel.best_block_updated(height, header.time, &self.logger));
+               self.do_chain_event(Some(height), |channel| channel.best_block_updated(height, header.time, self.genesis_hash.clone(), self.get_our_node_id(), &self.logger));
 
                macro_rules! max_time {
                        ($timestamp: expr) => {
@@ -5353,9 +5336,9 @@ where
                self.do_chain_event(None, |channel| {
                        if let Some(funding_txo) = channel.get_funding_txo() {
                                if funding_txo.txid == *txid {
-                                       channel.funding_transaction_unconfirmed(&self.logger).map(|_| (None, Vec::new()))
-                               } else { Ok((None, Vec::new())) }
-                       } else { Ok((None, Vec::new())) }
+                                       channel.funding_transaction_unconfirmed(&self.logger).map(|()| (None, Vec::new(), None))
+                               } else { Ok((None, Vec::new(), None)) }
+                       } else { Ok((None, Vec::new(), None)) }
                });
        }
 }
@@ -5371,7 +5354,7 @@ where
        /// Calls a function which handles an on-chain event (blocks dis/connected, transactions
        /// un/confirmed, etc) on each channel, handling any resulting errors or messages generated by
        /// the function.
-       fn do_chain_event<FN: Fn(&mut Channel<Signer>) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), ClosureReason>>
+       fn do_chain_event<FN: Fn(&mut Channel<Signer>) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>, Option<msgs::AnnouncementSignatures>), ClosureReason>>
                        (&self, height_opt: Option<u32>, f: FN) {
                // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called
                // during initialization prior to the chain_monitor being fully configured in some cases.
@@ -5386,7 +5369,7 @@ where
                        let pending_msg_events = &mut channel_state.pending_msg_events;
                        channel_state.by_id.retain(|_, channel| {
                                let res = f(channel);
-                               if let Ok((chan_res, mut timed_out_pending_htlcs)) = res {
+                               if let Ok((funding_locked_opt, mut timed_out_pending_htlcs, announcement_sigs)) = res {
                                        for (source, payment_hash) in timed_out_pending_htlcs.drain(..) {
                                                let chan_update = self.get_channel_update_for_unicast(&channel).map(|u| u.encode_with_len()).unwrap(); // Cannot add/recv HTLCs before we have a short_id so unwrap is safe
                                                timed_out_htlcs.push((source, payment_hash,  HTLCFailReason::Reason {
@@ -5394,28 +5377,39 @@ where
                                                        data: chan_update,
                                                }));
                                        }
-                                       if let Some(funding_locked) = chan_res {
+                                       if let Some(funding_locked) = funding_locked_opt {
                                                pending_msg_events.push(events::MessageSendEvent::SendFundingLocked {
                                                        node_id: channel.get_counterparty_node_id(),
                                                        msg: funding_locked,
                                                });
-                                               if let Some(announcement_sigs) = self.get_announcement_sigs(channel) {
-                                                       log_trace!(self.logger, "Sending funding_locked and announcement_signatures for {}", log_bytes!(channel.channel_id()));
-                                                       pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
-                                                               node_id: channel.get_counterparty_node_id(),
-                                                               msg: announcement_sigs,
-                                                       });
-                                               } else if channel.is_usable() {
-                                                       log_trace!(self.logger, "Sending funding_locked WITHOUT announcement_signatures but with private channel_update for our counterparty on channel {}", log_bytes!(channel.channel_id()));
+                                               if channel.is_usable() {
+                                                       log_trace!(self.logger, "Sending funding_locked with private initial channel_update for our counterparty on channel {}", log_bytes!(channel.channel_id()));
                                                        pending_msg_events.push(events::MessageSendEvent::SendChannelUpdate {
                                                                node_id: channel.get_counterparty_node_id(),
                                                                msg: self.get_channel_update_for_unicast(channel).unwrap(),
                                                        });
                                                } else {
-                                                       log_trace!(self.logger, "Sending funding_locked WITHOUT announcement_signatures for {}", log_bytes!(channel.channel_id()));
+                                                       log_trace!(self.logger, "Sending funding_locked WITHOUT channel_update for {}", log_bytes!(channel.channel_id()));
                                                }
                                                short_to_id.insert(channel.get_short_channel_id().unwrap(), channel.channel_id());
                                        }
+                                       if let Some(announcement_sigs) = announcement_sigs {
+                                               log_trace!(self.logger, "Sending announcement_signatures for channel {}", log_bytes!(channel.channel_id()));
+                                               pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
+                                                       node_id: channel.get_counterparty_node_id(),
+                                                       msg: announcement_sigs,
+                                               });
+                                               if let Some(height) = height_opt {
+                                                       if let Some(announcement) = channel.get_signed_channel_announcement(self.get_our_node_id(), self.genesis_hash, height) {
+                                                               pending_msg_events.push(events::MessageSendEvent::BroadcastChannelAnnouncement {
+                                                                       msg: announcement,
+                                                                       // Note that announcement_signatures fails if the channel cannot be announced,
+                                                                       // so get_channel_update_for_broadcast will never fail by the time we get here.
+                                                                       update_msg: self.get_channel_update_for_broadcast(channel).unwrap(),
+                                                               });
+                                                       }
+                                               }
+                                       }
                                } else if let Err(reason) = res {
                                        if let Some(short_id) = channel.get_short_channel_id() {
                                                short_to_id.remove(&short_id);
index 5ec16f90fc9bbba19bebb59e25f6621cbab2a84f..b7d7011c93341e9b779fdbf4c4734db90e300caf 100644 (file)
@@ -600,7 +600,14 @@ pub fn create_chan_between_nodes_with_value_confirm_first<'a, 'b, 'c, 'd>(node_r
 pub fn create_chan_between_nodes_with_value_confirm_second<'a, 'b, 'c>(node_recv: &Node<'a, 'b, 'c>, node_conf: &Node<'a, 'b, 'c>) -> ((msgs::FundingLocked, msgs::AnnouncementSignatures), [u8; 32]) {
        let channel_id;
        let events_6 = node_conf.node.get_and_clear_pending_msg_events();
-       assert_eq!(events_6.len(), 2);
+       assert_eq!(events_6.len(), 3);
+       let announcement_sigs_idx = if let MessageSendEvent::SendChannelUpdate { ref node_id, msg: _ } = events_6[1] {
+               assert_eq!(*node_id, node_recv.node.get_our_node_id());
+               2
+       } else if let MessageSendEvent::SendChannelUpdate { ref node_id, msg: _ } = events_6[2] {
+               assert_eq!(*node_id, node_recv.node.get_our_node_id());
+               1
+       } else { panic!("Unexpected event: {:?}", events_6[1]); };
        ((match events_6[0] {
                MessageSendEvent::SendFundingLocked { ref node_id, ref msg } => {
                        channel_id = msg.channel_id.clone();
@@ -608,7 +615,7 @@ pub fn create_chan_between_nodes_with_value_confirm_second<'a, 'b, 'c>(node_recv
                        msg.clone()
                },
                _ => panic!("Unexpected event"),
-       }, match events_6[1] {
+       }, match events_6[announcement_sigs_idx] {
                MessageSendEvent::SendAnnouncementSignatures { ref node_id, ref msg } => {
                        assert_eq!(*node_id, node_recv.node.get_our_node_id());
                        msg.clone()
@@ -2002,8 +2009,7 @@ macro_rules! handle_chan_reestablish_msgs {
                                                idx += 1;
                                                RAACommitmentOrder::CommitmentFirst
                                        },
-                                       &MessageSendEvent::SendChannelUpdate { .. } => RAACommitmentOrder::CommitmentFirst,
-                                       _ => panic!("Unexpected event"),
+                                       _ => RAACommitmentOrder::CommitmentFirst,
                                }
                        } else {
                                RAACommitmentOrder::CommitmentFirst
@@ -2023,16 +2029,18 @@ macro_rules! handle_chan_reestablish_msgs {
                                                commitment_update = Some(updates.clone());
                                                idx += 1;
                                        },
-                                       &MessageSendEvent::SendChannelUpdate { .. } => {},
-                                       _ => panic!("Unexpected event"),
+                                       _ => {},
                                }
                        }
 
                        if let Some(&MessageSendEvent::SendChannelUpdate { ref node_id, ref msg }) = msg_events.get(idx) {
                                assert_eq!(*node_id, $dst_node.node.get_our_node_id());
+                               idx += 1;
                                assert_eq!(msg.contents.flags & 2, 0); // "disabled" flag must not be set as we just reconnected.
                        }
 
+                       assert_eq!(msg_events.len(), idx);
+
                        (funding_locked, revoke_and_ack, commitment_update, order)
                }
        }
@@ -2105,9 +2113,9 @@ pub fn reconnect_nodes<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a,
                        let announcement_event = node_a.node.get_and_clear_pending_msg_events();
                        if !announcement_event.is_empty() {
                                assert_eq!(announcement_event.len(), 1);
-                               if let MessageSendEvent::SendAnnouncementSignatures { .. } = announcement_event[0] {
+                               if let MessageSendEvent::SendChannelUpdate { .. } = announcement_event[0] {
                                        //TODO: Test announcement_sigs re-sending
-                               } else { panic!("Unexpected event!"); }
+                               } else { panic!("Unexpected event! {:?}", announcement_event[0]); }
                        }
                } else {
                        assert!(chan_msgs.0.is_none());
@@ -2162,9 +2170,11 @@ pub fn reconnect_nodes<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a,
                        let announcement_event = node_b.node.get_and_clear_pending_msg_events();
                        if !announcement_event.is_empty() {
                                assert_eq!(announcement_event.len(), 1);
-                               if let MessageSendEvent::SendAnnouncementSignatures { .. } = announcement_event[0] {
-                                       //TODO: Test announcement_sigs re-sending
-                               } else { panic!("Unexpected event!"); }
+                               match announcement_event[0] {
+                                       MessageSendEvent::SendChannelUpdate { .. } => {},
+                                       MessageSendEvent::SendAnnouncementSignatures { .. } => {},
+                                       _ => panic!("Unexpected event {:?}!", announcement_event[0]),
+                               }
                        }
                } else {
                        assert!(chan_msgs.0.is_none());
index dd9d3a3c4762e1aa1ef9f735800d23360679249b..6f6e51a6f98313039f1b7e3e723d2c9602144864 100644 (file)
@@ -483,10 +483,54 @@ fn do_test_1_conf_open(connect_style: ConnectStyle) {
        let tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 100000, 10001, InitFeatures::known(), InitFeatures::known());
        mine_transaction(&nodes[1], &tx);
        nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingLocked, nodes[0].node.get_our_node_id()));
+       assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
 
        mine_transaction(&nodes[0], &tx);
-       let (funding_locked, _) = create_chan_between_nodes_with_value_confirm_second(&nodes[1], &nodes[0]);
-       let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked);
+       let as_msg_events = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(as_msg_events.len(), 2);
+       let as_funding_locked = if let MessageSendEvent::SendFundingLocked { ref node_id, ref msg } = as_msg_events[0] {
+               assert_eq!(*node_id, nodes[1].node.get_our_node_id());
+               msg.clone()
+       } else { panic!("Unexpected event"); };
+       if let MessageSendEvent::SendChannelUpdate { ref node_id, msg: _ } = as_msg_events[1] {
+               assert_eq!(*node_id, nodes[1].node.get_our_node_id());
+       } else { panic!("Unexpected event"); }
+
+       nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &as_funding_locked);
+       let bs_msg_events = nodes[1].node.get_and_clear_pending_msg_events();
+       assert_eq!(bs_msg_events.len(), 1);
+       if let MessageSendEvent::SendChannelUpdate { ref node_id, msg: _ } = bs_msg_events[0] {
+               assert_eq!(*node_id, nodes[0].node.get_our_node_id());
+       } else { panic!("Unexpected event"); }
+
+       send_payment(&nodes[0], &[&nodes[1]], 100_000);
+
+       // After 6 confirmations, as required by the spec, we'll send announcement_signatures and
+       // broadcast the channel_announcement (but not before exactly 6 confirmations).
+       connect_blocks(&nodes[0], 4);
+       assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+       connect_blocks(&nodes[0], 1);
+       nodes[1].node.handle_announcement_signatures(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendAnnouncementSignatures, nodes[1].node.get_our_node_id()));
+       assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+
+       connect_blocks(&nodes[1], 5);
+       let bs_announce_events = nodes[1].node.get_and_clear_pending_msg_events();
+       assert_eq!(bs_announce_events.len(), 2);
+       let bs_announcement_sigs = if let MessageSendEvent::SendAnnouncementSignatures { ref node_id, ref msg } = bs_announce_events[0] {
+               assert_eq!(*node_id, nodes[0].node.get_our_node_id());
+               msg.clone()
+       } else { panic!("Unexpected event"); };
+       let (bs_announcement, bs_update) = if let MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } = bs_announce_events[1] {
+               (msg.clone(), update_msg.clone())
+       } else { panic!("Unexpected event"); };
+
+       nodes[0].node.handle_announcement_signatures(&nodes[1].node.get_our_node_id(), &bs_announcement_sigs);
+       let as_announce_events = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(as_announce_events.len(), 1);
+       let (announcement, as_update) = if let MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } = as_announce_events[0] {
+               (msg.clone(), update_msg.clone())
+       } else { panic!("Unexpected event"); };
+       assert_eq!(announcement, bs_announcement);
 
        for node in nodes {
                assert!(node.net_graph_msg_handler.handle_channel_announcement(&announcement).unwrap());
@@ -730,7 +774,7 @@ fn test_update_fee_that_funder_cannot_afford() {
                        &mut htlcs,
                        &local_chan.channel_transaction_parameters.as_counterparty_broadcastable()
                );
-               local_chan_signer.sign_counterparty_commitment(&commitment_tx, &secp_ctx).unwrap()
+               local_chan_signer.sign_counterparty_commitment(&commitment_tx, Vec::new(), &secp_ctx).unwrap()
        };
 
        let commit_signed_msg = msgs::CommitmentSigned {
@@ -1466,7 +1510,7 @@ fn test_fee_spike_violation_fails_htlc() {
                        &mut vec![(accepted_htlc_info, ())],
                        &local_chan.channel_transaction_parameters.as_counterparty_broadcastable()
                );
-               local_chan_signer.sign_counterparty_commitment(&commitment_tx, &secp_ctx).unwrap()
+               local_chan_signer.sign_counterparty_commitment(&commitment_tx, Vec::new(), &secp_ctx).unwrap()
        };
 
        let commit_signed_msg = msgs::CommitmentSigned {
@@ -3807,15 +3851,7 @@ fn test_funding_peer_disconnect() {
 
        confirm_transaction(&nodes[0], &tx);
        let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
-       let chan_id;
-       assert_eq!(events_1.len(), 1);
-       match events_1[0] {
-               MessageSendEvent::SendFundingLocked { ref node_id, ref msg } => {
-                       assert_eq!(*node_id, nodes[1].node.get_our_node_id());
-                       chan_id = msg.channel_id;
-               },
-               _ => panic!("Unexpected event"),
-       }
+       assert!(events_1.is_empty());
 
        reconnect_nodes(&nodes[0], &nodes[1], (false, true), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
 
@@ -3824,53 +3860,106 @@ fn test_funding_peer_disconnect() {
 
        confirm_transaction(&nodes[1], &tx);
        let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
-       assert_eq!(events_2.len(), 2);
-       let funding_locked = match events_2[0] {
+       assert!(events_2.is_empty());
+
+       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
+       let as_reestablish = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
+       nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
+       let bs_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+
+       // nodes[0] hasn't yet received a funding_locked, so it only sends that on reconnect.
+       nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish);
+       let events_3 = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events_3.len(), 1);
+       let as_funding_locked = match events_3[0] {
+               MessageSendEvent::SendFundingLocked { ref node_id, ref msg } => {
+                       assert_eq!(*node_id, nodes[1].node.get_our_node_id());
+                       msg.clone()
+               },
+               _ => panic!("Unexpected event {:?}", events_3[0]),
+       };
+
+       // nodes[1] received nodes[0]'s funding_locked on the first reconnect above, so it should send
+       // announcement_signatures as well as channel_update.
+       nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_reestablish);
+       let events_4 = nodes[1].node.get_and_clear_pending_msg_events();
+       assert_eq!(events_4.len(), 3);
+       let chan_id;
+       let bs_funding_locked = match events_4[0] {
                MessageSendEvent::SendFundingLocked { ref node_id, ref msg } => {
                        assert_eq!(*node_id, nodes[0].node.get_our_node_id());
+                       chan_id = msg.channel_id;
                        msg.clone()
                },
-               _ => panic!("Unexpected event"),
+               _ => panic!("Unexpected event {:?}", events_4[0]),
        };
-       let bs_announcement_sigs = match events_2[1] {
+       let bs_announcement_sigs = match events_4[1] {
                MessageSendEvent::SendAnnouncementSignatures { ref node_id, ref msg } => {
                        assert_eq!(*node_id, nodes[0].node.get_our_node_id());
                        msg.clone()
                },
-               _ => panic!("Unexpected event"),
+               _ => panic!("Unexpected event {:?}", events_4[1]),
        };
+       match events_4[2] {
+               MessageSendEvent::SendChannelUpdate { ref node_id, msg: _ } => {
+                       assert_eq!(*node_id, nodes[0].node.get_our_node_id());
+               },
+               _ => panic!("Unexpected event {:?}", events_4[2]),
+       }
 
-       reconnect_nodes(&nodes[0], &nodes[1], (true, true), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+       // Re-deliver nodes[0]'s funding_locked, which nodes[1] can safely ignore. It currently
+       // generates a duplicative private channel_update
+       nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &as_funding_locked);
+       let events_5 = nodes[1].node.get_and_clear_pending_msg_events();
+       assert_eq!(events_5.len(), 1);
+       match events_5[0] {
+               MessageSendEvent::SendChannelUpdate { ref node_id, msg: _ } => {
+                       assert_eq!(*node_id, nodes[0].node.get_our_node_id());
+               },
+               _ => panic!("Unexpected event {:?}", events_5[0]),
+       };
 
-       nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &funding_locked);
-       nodes[0].node.handle_announcement_signatures(&nodes[1].node.get_our_node_id(), &bs_announcement_sigs);
-       let events_3 = nodes[0].node.get_and_clear_pending_msg_events();
-       assert_eq!(events_3.len(), 2);
-       let as_announcement_sigs = match events_3[0] {
+       // When we deliver nodes[1]'s funding_locked, however, nodes[0] will generate its
+       // announcement_signatures.
+       nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &bs_funding_locked);
+       let events_6 = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events_6.len(), 1);
+       let as_announcement_sigs = match events_6[0] {
                MessageSendEvent::SendAnnouncementSignatures { ref node_id, ref msg } => {
                        assert_eq!(*node_id, nodes[1].node.get_our_node_id());
                        msg.clone()
                },
-               _ => panic!("Unexpected event"),
+               _ => panic!("Unexpected event {:?}", events_6[0]),
        };
-       let (as_announcement, as_update) = match events_3[1] {
+
+       // When we deliver nodes[1]'s announcement_signatures to nodes[0], nodes[0] should immediately
+       // broadcast the channel announcement globally, as well as re-send its (now-public)
+       // channel_update.
+       nodes[0].node.handle_announcement_signatures(&nodes[1].node.get_our_node_id(), &bs_announcement_sigs);
+       let events_7 = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(events_7.len(), 1);
+       let (chan_announcement, as_update) = match events_7[0] {
                MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } => {
                        (msg.clone(), update_msg.clone())
                },
-               _ => panic!("Unexpected event"),
+               _ => panic!("Unexpected event {:?}", events_7[0]),
        };
 
+       // Finally, deliver nodes[0]'s announcement_signatures to nodes[1] and make sure it creates the
+       // same channel_announcement.
        nodes[1].node.handle_announcement_signatures(&nodes[0].node.get_our_node_id(), &as_announcement_sigs);
-       let events_4 = nodes[1].node.get_and_clear_pending_msg_events();
-       assert_eq!(events_4.len(), 1);
-       let (_, bs_update) = match events_4[0] {
+       let events_8 = nodes[1].node.get_and_clear_pending_msg_events();
+       assert_eq!(events_8.len(), 1);
+       let bs_update = match events_8[0] {
                MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } => {
-                       (msg.clone(), update_msg.clone())
+                       assert_eq!(*msg, chan_announcement);
+                       update_msg.clone()
                },
-               _ => panic!("Unexpected event"),
+               _ => panic!("Unexpected event {:?}", events_8[0]),
        };
 
-       nodes[0].net_graph_msg_handler.handle_channel_announcement(&as_announcement).unwrap();
+       // Provide the channel announcement and public updates to the network graph
+       nodes[0].net_graph_msg_handler.handle_channel_announcement(&chan_announcement).unwrap();
        nodes[0].net_graph_msg_handler.handle_channel_update(&bs_update).unwrap();
        nodes[0].net_graph_msg_handler.handle_channel_update(&as_update).unwrap();
 
@@ -3918,14 +4007,14 @@ fn test_funding_peer_disconnect() {
 
        reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
 
-       // as_announcement should be re-generated exactly by broadcast_node_announcement.
+       // The channel announcement should be re-generated exactly by broadcast_node_announcement.
        nodes[0].node.broadcast_node_announcement([0, 0, 0], [0; 32], Vec::new());
        let msgs = nodes[0].node.get_and_clear_pending_msg_events();
        let mut found_announcement = false;
        for event in msgs.iter() {
                match event {
                        MessageSendEvent::BroadcastChannelAnnouncement { ref msg, .. } => {
-                               if *msg == as_announcement { found_announcement = true; }
+                               if *msg == chan_announcement { found_announcement = true; }
                        },
                        MessageSendEvent::BroadcastNodeAnnouncement { .. } => {},
                        _ => panic!("Unexpected event"),
index 86cd344ce352dea1544f27d894a94813a6ca3930..b910d4bf7798135ec08d8e84fc550fb907e318fb 100644 (file)
@@ -902,7 +902,11 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
                                                                                        Ok(x) => x,
                                                                                        Err(e) => {
                                                                                                match e {
-                                                                                                       (msgs::DecodeError::UnknownRequiredFeature, _) => {
+                                                                                                       // Note that to avoid recursion we never call
+                                                                                                       // `do_attempt_write_data` from here, causing
+                                                                                                       // the messages enqueued here to not actually
+                                                                                                       // be sent before the peer is disconnected.
+                                                                                                       (msgs::DecodeError::UnknownRequiredFeature, Some(ty)) if is_gossip_msg(ty) => {
                                                                                                                log_gossip!(self.logger, "Got a channel/node announcement with an unknown required feature flag, you may want to update!");
                                                                                                                continue;
                                                                                                        }
@@ -916,6 +920,11 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
                                                                                                                self.enqueue_message(peer, &msgs::WarningMessage { channel_id: [0; 32], data: "Unreadable/bogus gossip message".to_owned() });
                                                                                                                continue;
                                                                                                        }
+                                                                                                       (msgs::DecodeError::UnknownRequiredFeature, ty) => {
+                                                                                                               log_gossip!(self.logger, "Received a message with an unknown required feature flag or TLV, you may want to update!");
+                                                                                                               self.enqueue_message(peer, &msgs::WarningMessage { channel_id: [0; 32], data: format!("Received an unknown required feature/TLV in message type {:?}", ty) });
+                                                                                                               return Err(PeerHandleError { no_connection_possible: false });
+                                                                                                       }
                                                                                                        (msgs::DecodeError::UnknownVersion, _) => return Err(PeerHandleError { no_connection_possible: false }),
                                                                                                        (msgs::DecodeError::InvalidValue, _) => {
                                                                                                                log_debug!(self.logger, "Got an invalid value while deserializing message");
index 420bbaed0ca626af10c33efaba0fed01aee77790..cf2afbbbadb7e0baabba60d1f4f17e24799a8e03 100644 (file)
@@ -4963,6 +4963,11 @@ pub(crate) mod test_utils {
 #[cfg(all(test, feature = "unstable", not(feature = "no-std")))]
 mod benches {
        use super::*;
+       use bitcoin::hashes::Hash;
+       use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+       use chain::transaction::OutPoint;
+       use ln::channelmanager::{ChannelCounterparty, ChannelDetails};
+       use ln::features::{InitFeatures, InvoiceFeatures};
        use routing::scoring::Scorer;
        use util::logger::{Logger, Record};
 
@@ -4973,50 +4978,91 @@ mod benches {
                fn log(&self, _record: &Record) {}
        }
 
-       #[bench]
-       fn generate_routes(bench: &mut Bencher) {
+       struct ZeroPenaltyScorer;
+       impl Score for ZeroPenaltyScorer {
+               fn channel_penalty_msat(
+                       &self, _short_channel_id: u64, _send_amt: u64, _capacity_msat: Option<u64>, _source: &NodeId, _target: &NodeId
+               ) -> u64 { 0 }
+               fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
+               fn payment_path_successful(&mut self, _path: &[&RouteHop]) {}
+       }
+
+       fn read_network_graph() -> NetworkGraph {
                let mut d = test_utils::get_route_file().unwrap();
-               let graph = NetworkGraph::read(&mut d).unwrap();
-               let nodes = graph.read_only().nodes().clone();
-               let scorer = Scorer::with_fixed_penalty(0);
+               NetworkGraph::read(&mut d).unwrap()
+       }
 
-               // First, get 100 (source, destination) pairs for which route-getting actually succeeds...
-               let mut path_endpoints = Vec::new();
-               let mut seed: usize = 0xdeadbeef;
-               'load_endpoints: for _ in 0..100 {
-                       loop {
-                               seed *= 0xdeadbeef;
-                               let src = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
-                               seed *= 0xdeadbeef;
-                               let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
-                               let payment_params = PaymentParameters::from_node_id(dst);
-                               let amt = seed as u64 % 1_000_000;
-                               if get_route(&src, &payment_params, &graph, None, amt, 42, &DummyLogger{}, &scorer).is_ok() {
-                                       path_endpoints.push((src, dst, amt));
-                                       continue 'load_endpoints;
-                               }
-                       }
+       fn payer_pubkey() -> PublicKey {
+               let secp_ctx = Secp256k1::new();
+               PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap())
+       }
+
+       #[inline]
+       fn first_hop(node_id: PublicKey) -> ChannelDetails {
+               ChannelDetails {
+                       channel_id: [0; 32],
+                       counterparty: ChannelCounterparty {
+                               features: InitFeatures::known(),
+                               node_id,
+                               unspendable_punishment_reserve: 0,
+                               forwarding_info: None,
+                       },
+                       funding_txo: Some(OutPoint {
+                               txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0
+                       }),
+                       short_channel_id: Some(1),
+                       channel_value_satoshis: 10_000_000,
+                       user_channel_id: 0,
+                       balance_msat: 10_000_000,
+                       outbound_capacity_msat: 10_000_000,
+                       inbound_capacity_msat: 0,
+                       unspendable_punishment_reserve: None,
+                       confirmations_required: None,
+                       force_close_spend_delay: None,
+                       is_outbound: true,
+                       is_funding_locked: true,
+                       is_usable: true,
+                       is_public: true,
                }
+       }
 
-               // ...then benchmark finding paths between the nodes we learned.
-               let mut idx = 0;
-               bench.iter(|| {
-                       let (src, dst, amt) = path_endpoints[idx % path_endpoints.len()];
-                       let payment_params = PaymentParameters::from_node_id(dst);
-                       assert!(get_route(&src, &payment_params, &graph, None, amt, 42, &DummyLogger{}, &scorer).is_ok());
-                       idx += 1;
-               });
+       #[bench]
+       fn generate_routes_with_zero_penalty_scorer(bench: &mut Bencher) {
+               let network_graph = read_network_graph();
+               let scorer = ZeroPenaltyScorer;
+               generate_routes(bench, &network_graph, scorer, InvoiceFeatures::empty());
        }
 
        #[bench]
-       fn generate_mpp_routes(bench: &mut Bencher) {
-               let mut d = test_utils::get_route_file().unwrap();
-               let graph = NetworkGraph::read(&mut d).unwrap();
+       fn generate_mpp_routes_with_zero_penalty_scorer(bench: &mut Bencher) {
+               let network_graph = read_network_graph();
+               let scorer = ZeroPenaltyScorer;
+               generate_routes(bench, &network_graph, scorer, InvoiceFeatures::known());
+       }
+
+       #[bench]
+       fn generate_routes_with_default_scorer(bench: &mut Bencher) {
+               let network_graph = read_network_graph();
+               let scorer = Scorer::default();
+               generate_routes(bench, &network_graph, scorer, InvoiceFeatures::empty());
+       }
+
+       #[bench]
+       fn generate_mpp_routes_with_default_scorer(bench: &mut Bencher) {
+               let network_graph = read_network_graph();
+               let scorer = Scorer::default();
+               generate_routes(bench, &network_graph, scorer, InvoiceFeatures::known());
+       }
+
+       fn generate_routes<S: Score>(
+               bench: &mut Bencher, graph: &NetworkGraph, mut scorer: S, features: InvoiceFeatures
+       ) {
                let nodes = graph.read_only().nodes().clone();
-               let scorer = Scorer::with_fixed_penalty(0);
+               let payer = payer_pubkey();
 
                // First, get 100 (source, destination) pairs for which route-getting actually succeeds...
-               let mut path_endpoints = Vec::new();
+               let mut routes = Vec::new();
+               let mut route_endpoints = Vec::new();
                let mut seed: usize = 0xdeadbeef;
                'load_endpoints: for _ in 0..100 {
                        loop {
@@ -5024,21 +5070,37 @@ mod benches {
                                let src = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                seed *= 0xdeadbeef;
                                let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
-                               let payment_params = PaymentParameters::from_node_id(dst).with_features(InvoiceFeatures::known());
+                               let params = PaymentParameters::from_node_id(dst).with_features(features.clone());
+                               let first_hop = first_hop(src);
                                let amt = seed as u64 % 1_000_000;
-                               if get_route(&src, &payment_params, &graph, None, amt, 42, &DummyLogger{}, &scorer).is_ok() {
-                                       path_endpoints.push((src, dst, amt));
+                               if let Ok(route) = get_route(&payer, &params, &graph, Some(&[&first_hop]), amt, 42, &DummyLogger{}, &scorer) {
+                                       routes.push(route);
+                                       route_endpoints.push((first_hop, params, amt));
                                        continue 'load_endpoints;
                                }
                        }
                }
 
+               // ...and seed the scorer with success and failure data...
+               for route in routes {
+                       let amount = route.get_total_amount();
+                       if amount < 250_000 {
+                               for path in route.paths {
+                                       scorer.payment_path_successful(&path.iter().collect::<Vec<_>>());
+                               }
+                       } else if amount > 750_000 {
+                               for path in route.paths {
+                                       let short_channel_id = path[path.len() / 2].short_channel_id;
+                                       scorer.payment_path_failed(&path.iter().collect::<Vec<_>>(), short_channel_id);
+                               }
+                       }
+               }
+
                // ...then benchmark finding paths between the nodes we learned.
                let mut idx = 0;
                bench.iter(|| {
-                       let (src, dst, amt) = path_endpoints[idx % path_endpoints.len()];
-                       let payment_params = PaymentParameters::from_node_id(dst).with_features(InvoiceFeatures::known());
-                       assert!(get_route(&src, &payment_params, &graph, None, amt, 42, &DummyLogger{}, &scorer).is_ok());
+                       let (first_hop, params, amt) = &route_endpoints[idx % route_endpoints.len()];
+                       assert!(get_route(&payer, params, &graph, Some(&[first_hop]), *amt, 42, &DummyLogger{}, &scorer).is_ok());
                        idx += 1;
                });
        }
index 58137e584d478a09d492005affd240ca93b963cf..2e22df7c5af10c1c4e68dc9ec6baf40990ab7840 100644 (file)
@@ -8,7 +8,7 @@
 // licenses.
 
 use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, HolderCommitmentTransaction, CommitmentTransaction, ChannelTransactionParameters, TrustedCommitmentTransaction, ClosingTransaction};
-use ln::{chan_utils, msgs};
+use ln::{chan_utils, msgs, PaymentPreimage};
 use chain::keysinterface::{Sign, InMemorySigner, BaseSign};
 
 use prelude::*;
@@ -102,7 +102,7 @@ impl BaseSign for EnforcingSigner {
                self.inner.release_commitment_secret(idx)
        }
 
-       fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction) -> Result<(), ()> {
+       fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction, _preimages: Vec<PaymentPreimage>) -> Result<(), ()> {
                let mut state = self.state.lock().unwrap();
                let idx = holder_tx.commitment_number();
                assert!(idx == state.last_holder_commitment || idx == state.last_holder_commitment - 1, "expecting to validate the current or next holder commitment - trying {}, current {}", idx, state.last_holder_commitment);
@@ -113,7 +113,7 @@ impl BaseSign for EnforcingSigner {
        fn pubkeys(&self) -> &ChannelPublicKeys { self.inner.pubkeys() }
        fn channel_keys_id(&self) -> [u8; 32] { self.inner.channel_keys_id() }
 
-       fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
+       fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
                self.verify_counterparty_commitment_tx(commitment_tx, secp_ctx);
 
                {
@@ -129,7 +129,7 @@ impl BaseSign for EnforcingSigner {
                        state.last_counterparty_commitment = cmp::min(last_commitment_number, actual_commitment_number)
                }
 
-               Ok(self.inner.sign_counterparty_commitment(commitment_tx, secp_ctx).unwrap())
+               Ok(self.inner.sign_counterparty_commitment(commitment_tx, preimages, secp_ctx).unwrap())
        }
 
        fn validate_counterparty_revocation(&self, idx: u64, _secret: &SecretKey) -> Result<(), ()> {
@@ -190,7 +190,8 @@ impl BaseSign for EnforcingSigner {
                Ok(self.inner.sign_closing_transaction(closing_tx, secp_ctx).unwrap())
        }
 
-       fn sign_channel_announcement(&self, msg: &msgs::UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
+       fn sign_channel_announcement(&self, msg: &msgs::UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>)
+       -> Result<(Signature, Signature), ()> {
                self.inner.sign_channel_announcement(msg, secp_ctx)
        }
 
index 4169514412ca49fbab1fa2252b4d9f0f57a87f3e..dee6907832ea1c974a8209571dfd186b8fa2e557 100644 (file)
@@ -79,7 +79,8 @@ impl keysinterface::KeysInterface for OnlyReadsKeysInterface {
        fn get_secure_random_bytes(&self) -> [u8; 32] { [0; 32] }
 
        fn read_chan_signer(&self, mut reader: &[u8]) -> Result<Self::Signer, msgs::DecodeError> {
-               let inner: InMemorySigner = Readable::read(&mut reader)?;
+               let dummy_sk = SecretKey::from_slice(&[42; 32]).unwrap();
+               let inner: InMemorySigner = ReadableArgs::read(&mut reader, dummy_sk)?;
                let state = Arc::new(Mutex::new(EnforcementState::new()));
 
                Ok(EnforcingSigner::new_with_revoked(
@@ -519,7 +520,7 @@ impl keysinterface::KeysInterface for TestKeysInterface {
        fn read_chan_signer(&self, buffer: &[u8]) -> Result<Self::Signer, msgs::DecodeError> {
                let mut reader = io::Cursor::new(buffer);
 
-               let inner: InMemorySigner = Readable::read(&mut reader)?;
+               let inner: InMemorySigner = ReadableArgs::read(&mut reader, self.get_node_secret())?;
                let state = self.make_enforcement_state_cell(inner.commitment_seed);
 
                Ok(EnforcingSigner::new_with_revoked(