Merge pull request #2043 from valentinewallace/2023-02-initial-send-path-fails
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 27 Feb 2023 18:10:27 +0000 (18:10 +0000)
committerGitHub <noreply@github.com>
Mon, 27 Feb 2023 18:10:27 +0000 (18:10 +0000)
`PaymentPathFailed`: add initial send error details

1  2 
lightning-background-processor/src/lib.rs
lightning/src/chain/channelmonitor.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/outbound_payment.rs
lightning/src/routing/gossip.rs
lightning/src/util/ser_macros.rs

index f2b6814fc2ebd64452a0f75711d078996c9e95f5,51d8a7f866f9046ab24268ec60ce398ef5acfaf0..8711a4aeb5898e393f173886ca8e20de4b9b0d6f
@@@ -33,10 -33,11 +33,10 @@@ use lightning::routing::gossip::{Networ
  use lightning::routing::utxo::UtxoLookup;
  use lightning::routing::router::Router;
  use lightning::routing::scoring::{Score, WriteableScore};
- use lightning::util::events::{Event, EventHandler, EventsProvider};
+ use lightning::util::events::{Event, EventHandler, EventsProvider, PathFailure};
  use lightning::util::logger::Logger;
  use lightning::util::persist::Persister;
  use lightning_rapid_gossip_sync::RapidGossipSync;
 -use lightning::io;
  
  use core::ops::Deref;
  use core::time::Duration;
@@@ -214,10 -215,10 +214,10 @@@ wher
  fn handle_network_graph_update<L: Deref>(
        network_graph: &NetworkGraph<L>, event: &Event
  ) where L::Target: Logger {
-       if let Event::PaymentPathFailed { ref network_update, .. } = event {
-               if let Some(network_update) = network_update {
-                       network_graph.handle_network_update(&network_update);
-               }
+       if let Event::PaymentPathFailed {
+               failure: PathFailure::OnPath { network_update: Some(ref upd) }, .. } = event
+       {
+               network_graph.handle_network_update(upd);
        }
  }
  
@@@ -430,7 -431,7 +430,7 @@@ pub async fn process_events_async
        persister: PS, event_handler: EventHandler, chain_monitor: M, channel_manager: CM,
        gossip_sync: GossipSync<PGS, RGS, G, UL, L>, peer_manager: PM, logger: L, scorer: Option<S>,
        sleeper: Sleeper,
 -) -> Result<(), io::Error>
 +) -> Result<(), lightning::io::Error>
  where
        UL::Target: 'static + UtxoLookup,
        CF::Target: 'static + chain::Filter,
@@@ -672,7 -673,7 +672,7 @@@ mod tests 
        use lightning::routing::router::{DefaultRouter, RouteHop};
        use lightning::routing::scoring::{ChannelUsage, Score};
        use lightning::util::config::UserConfig;
-       use lightning::util::events::{Event, MessageSendEventsProvider, MessageSendEvent};
+       use lightning::util::events::{Event, PathFailure, MessageSendEventsProvider, MessageSendEvent};
        use lightning::util::ser::Writeable;
        use lightning::util::test_utils;
        use lightning::util::persist::KVStorePersister;
                        let logger = Arc::new(test_utils::TestLogger::with_id(format!("node {}", i)));
                        let network = Network::Testnet;
                        let genesis_block = genesis_block(network);
 -                      let network_graph = Arc::new(NetworkGraph::new(genesis_block.header.block_hash(), logger.clone()));
 +                      let network_graph = Arc::new(NetworkGraph::new(network, logger.clone()));
                        let scorer = Arc::new(Mutex::new(TestScorer::new()));
                        let seed = [i as u8; 32];
                        let router = Arc::new(DefaultRouter::new(network_graph.clone(), logger.clone(), seed, scorer.clone()));
                        let now = Duration::from_secs(genesis_block.header.time as u64);
                        let keys_manager = Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos()));
                        let chain_monitor = Arc::new(chainmonitor::ChainMonitor::new(Some(chain_source.clone()), tx_broadcaster.clone(), logger.clone(), fee_estimator.clone(), persister.clone()));
 -                      let best_block = BestBlock::from_genesis(network);
 +                      let best_block = BestBlock::from_network(network);
                        let params = ChainParameters { network, best_block };
                        let manager = Arc::new(ChannelManager::new(fee_estimator.clone(), chain_monitor.clone(), tx_broadcaster.clone(), router.clone(), logger.clone(), keys_manager.clone(), keys_manager.clone(), keys_manager.clone(), UserConfig::default(), params));
                        let p2p_gossip_sync = Arc::new(P2PGossipSync::new(network_graph.clone(), Some(chain_source.clone()), logger.clone()));
                        payment_id: None,
                        payment_hash: PaymentHash([42; 32]),
                        payment_failed_permanently: false,
-                       network_update: None,
-                       all_paths_failed: true,
+                       failure: PathFailure::OnPath { network_update: None },
                        path: path.clone(),
                        short_channel_id: Some(scored_scid),
                        retry: None,
                        payment_id: None,
                        payment_hash: PaymentHash([42; 32]),
                        payment_failed_permanently: true,
-                       network_update: None,
-                       all_paths_failed: true,
+                       failure: PathFailure::OnPath { network_update: None },
                        path: path.clone(),
                        short_channel_id: None,
                        retry: None,
index b2a82330916081163f441996fb0b3cfb84f5b7a1,045ed10b13834000d120bdb4e90a2d391eb7e860..8b281db6030dba2f681720a91eba819918d20d3f
@@@ -49,7 -49,7 +49,7 @@@ use crate::chain::onchaintx::OnchainTxH
  use crate::chain::package::{CounterpartyOfferedHTLCOutput, CounterpartyReceivedHTLCOutput, HolderFundingOutput, HolderHTLCOutput, PackageSolvingData, PackageTemplate, RevokedOutput, RevokedHTLCOutput};
  use crate::chain::Filter;
  use crate::util::logger::Logger;
- use crate::util::ser::{Readable, ReadableArgs, MaybeReadable, Writer, Writeable, U48, OptionDeserWrapper};
+ use crate::util::ser::{Readable, ReadableArgs, RequiredWrapper, MaybeReadable, UpgradableRequired, Writer, Writeable, U48};
  use crate::util::byte_utils;
  use crate::util::events::Event;
  #[cfg(anchors)]
@@@ -314,8 -314,8 +314,8 @@@ impl Readable for CounterpartyCommitmen
                                }
                        }
  
-                       let mut counterparty_delayed_payment_base_key = OptionDeserWrapper(None);
-                       let mut counterparty_htlc_base_key = OptionDeserWrapper(None);
+                       let mut counterparty_delayed_payment_base_key = RequiredWrapper(None);
+                       let mut counterparty_htlc_base_key = RequiredWrapper(None);
                        let mut on_counterparty_tx_csv: u16 = 0;
                        read_tlv_fields!(r, {
                                (0, counterparty_delayed_payment_base_key, required),
@@@ -454,19 -454,15 +454,15 @@@ impl MaybeReadable for OnchainEventEntr
                let mut transaction = None;
                let mut block_hash = None;
                let mut height = 0;
-               let mut event = None;
+               let mut event = UpgradableRequired(None);
                read_tlv_fields!(reader, {
                        (0, txid, required),
                        (1, transaction, option),
                        (2, height, required),
                        (3, block_hash, option),
-                       (4, event, ignorable),
+                       (4, event, upgradable_required),
                });
-               if let Some(ev) = event {
-                       Ok(Some(Self { txid, transaction, height, block_hash, event: ev }))
-               } else {
-                       Ok(None)
-               }
+               Ok(Some(Self { txid, transaction, height, block_hash, event: _init_tlv_based_struct_field!(event, upgradable_required) }))
        }
  }
  
@@@ -4155,7 -4151,7 +4151,7 @@@ mod tests 
                // Prune with one old state and a holder commitment tx holding a few overlaps with the
                // old state.
                let shutdown_pubkey = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 -              let best_block = BestBlock::from_genesis(Network::Testnet);
 +              let best_block = BestBlock::from_network(Network::Testnet);
                let monitor = ChannelMonitor::new(Secp256k1::new(), keys,
                                                  Some(ShutdownScript::new_p2wpkh_from_pubkey(shutdown_pubkey).into_inner()), 0, &Script::new(),
                                                  (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
index 7ef9ba754a9c4722b3302d63f7acf110e3a4bcbb,1fd70ee6fed0e0ca8765fafc19773fc712ac4b61..f07dc86f34d438b184f4eef2ad2aa7d932f8ff59
@@@ -468,7 -468,7 +468,7 @@@ pub(crate) enum MonitorUpdateCompletion
  
  impl_writeable_tlv_based_enum_upgradable!(MonitorUpdateCompletionAction,
        (0, PaymentClaimed) => { (0, payment_hash, required) },
-       (2, EmitEvent) => { (0, event, ignorable) },
+       (2, EmitEvent) => { (0, event, upgradable_required) },
  );
  
  /// State we hold per-peer.
@@@ -2420,10 -2420,10 +2420,10 @@@ wher
                let session_priv = SecretKey::from_slice(&session_priv_bytes[..]).expect("RNG is busted");
  
                let onion_keys = onion_utils::construct_onion_keys(&self.secp_ctx, &path, &session_priv)
-                       .map_err(|_| APIError::InvalidRoute{err: "Pubkey along hop was maliciously selected"})?;
+                       .map_err(|_| APIError::InvalidRoute{err: "Pubkey along hop was maliciously selected".to_owned()})?;
                let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(path, total_value, payment_secret, cur_height, keysend_preimage)?;
                if onion_utils::route_size_insane(&onion_payloads) {
-                       return Err(APIError::InvalidRoute{err: "Route size too large considering onion data"});
+                       return Err(APIError::InvalidRoute{err: "Route size too large considering onion data".to_owned()});
                }
                let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, prng_seed, payment_hash);
  
@@@ -6693,7 -6693,7 +6693,7 @@@ impl Writeable for ClaimableHTLC 
  
  impl Readable for ClaimableHTLC {
        fn read<R: Read>(reader: &mut R) -> Result<Self, DecodeError> {
-               let mut prev_hop = crate::util::ser::OptionDeserWrapper(None);
+               let mut prev_hop = crate::util::ser::RequiredWrapper(None);
                let mut value = 0;
                let mut payment_data: Option<msgs::FinalOnionHopData> = None;
                let mut cltv_expiry = 0;
@@@ -6743,7 -6743,7 +6743,7 @@@ impl Readable for HTLCSource 
                let id: u8 = Readable::read(reader)?;
                match id {
                        0 => {
-                               let mut session_priv: crate::util::ser::OptionDeserWrapper<SecretKey> = crate::util::ser::OptionDeserWrapper(None);
+                               let mut session_priv: crate::util::ser::RequiredWrapper<SecretKey> = crate::util::ser::RequiredWrapper(None);
                                let mut first_hop_htlc_msat: u64 = 0;
                                let mut path = Some(Vec::new());
                                let mut payment_id = None;
@@@ -8673,12 -8673,13 +8673,12 @@@ pub mod bench 
                // Note that this is unrealistic as each payment send will require at least two fsync
                // calls per node.
                let network = bitcoin::Network::Testnet;
 -              let genesis_hash = bitcoin::blockdata::constants::genesis_block(network).header.block_hash();
  
                let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))};
                let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
                let logger_a = test_utils::TestLogger::with_id("node a".to_owned());
                let scorer = Mutex::new(test_utils::TestScorer::new());
 -              let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(genesis_hash, &logger_a)), &scorer);
 +              let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(network, &logger_a)), &scorer);
  
                let mut config: UserConfig = Default::default();
                config.channel_handshake_config.minimum_depth = 1;
                let keys_manager_a = KeysManager::new(&seed_a, 42, 42);
                let node_a = ChannelManager::new(&fee_estimator, &chain_monitor_a, &tx_broadcaster, &router, &logger_a, &keys_manager_a, &keys_manager_a, &keys_manager_a, config.clone(), ChainParameters {
                        network,
 -                      best_block: BestBlock::from_genesis(network),
 +                      best_block: BestBlock::from_network(network),
                });
                let node_a_holder = NodeHolder { node: &node_a };
  
                let keys_manager_b = KeysManager::new(&seed_b, 42, 42);
                let node_b = ChannelManager::new(&fee_estimator, &chain_monitor_b, &tx_broadcaster, &router, &logger_b, &keys_manager_b, &keys_manager_b, &keys_manager_b, config.clone(), ChainParameters {
                        network,
 -                      best_block: BestBlock::from_genesis(network),
 +                      best_block: BestBlock::from_network(network),
                });
                let node_b_holder = NodeHolder { node: &node_b };
  
                assert_eq!(&tx_broadcaster.txn_broadcasted.lock().unwrap()[..], &[tx.clone()]);
  
                let block = Block {
 -                      header: BlockHeader { version: 0x20000000, prev_blockhash: genesis_hash, merkle_root: TxMerkleNode::all_zeros(), time: 42, bits: 42, nonce: 42 },
 +                      header: BlockHeader { version: 0x20000000, prev_blockhash: BestBlock::from_network(network).block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: 42, bits: 42, nonce: 42 },
                        txdata: vec![tx],
                };
                Listen::block_connected(&node_a, &block, 1);
                        _ => panic!("Unexpected event"),
                }
  
 -              let dummy_graph = NetworkGraph::new(genesis_hash, &logger_a);
 +              let dummy_graph = NetworkGraph::new(network, &logger_a);
  
                let mut payment_count: u64 = 0;
                macro_rules! send_payment {
index 2827598aa19424f6d5aaa463c74a445ba284354d,d1203297dec42f8df441b38f73f52eb5bbeddbd4..b424916991e1462a8c216c2711295976c4385411
@@@ -24,7 -24,7 +24,7 @@@ use crate::util::enforcing_trait_impls:
  use crate::util::scid_utils;
  use crate::util::test_utils;
  use crate::util::test_utils::{panicking, TestChainMonitor};
- use crate::util::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose};
+ use crate::util::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose};
  use crate::util::errors::APIError;
  use crate::util::config::UserConfig;
  use crate::util::ser::{ReadableArgs, Writeable};
@@@ -1818,7 -1818,7 +1818,7 @@@ pub fn expect_payment_failed_conditions
  ) {
        if conditions.expected_mpp_parts_remain { assert_eq!(payment_failed_events.len(), 1); } else { assert_eq!(payment_failed_events.len(), 2); }
        let expected_payment_id = match &payment_failed_events[0] {
-               Event::PaymentPathFailed { payment_hash, payment_failed_permanently, path, retry, payment_id, network_update, short_channel_id,
+               Event::PaymentPathFailed { payment_hash, payment_failed_permanently, path, retry, payment_id, failure, short_channel_id,
                        #[cfg(test)]
                        error_code,
                        #[cfg(test)]
                        }
  
                        if let Some(chan_closed) = conditions.expected_blamed_chan_closed {
-                               match network_update {
-                                       Some(NetworkUpdate::ChannelUpdateMessage { ref msg }) if !chan_closed => {
-                                               if let Some(scid) = conditions.expected_blamed_scid {
-                                                       assert_eq!(msg.contents.short_channel_id, scid);
-                                               }
-                                               const CHAN_DISABLED_FLAG: u8 = 2;
-                                               assert_eq!(msg.contents.flags & CHAN_DISABLED_FLAG, 0);
-                                       },
-                                       Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent }) if chan_closed => {
-                                               if let Some(scid) = conditions.expected_blamed_scid {
-                                                       assert_eq!(*short_channel_id, scid);
-                                               }
-                                               assert!(is_permanent);
-                                       },
-                                       Some(_) => panic!("Unexpected update type"),
-                                       None => panic!("Expected update"),
-                               }
+                               if let PathFailure::OnPath { network_update: Some(upd) } = failure {
+                                       match upd {
+                                               NetworkUpdate::ChannelUpdateMessage { ref msg } if !chan_closed => {
+                                                       if let Some(scid) = conditions.expected_blamed_scid {
+                                                               assert_eq!(msg.contents.short_channel_id, scid);
+                                                       }
+                                                       const CHAN_DISABLED_FLAG: u8 = 2;
+                                                       assert_eq!(msg.contents.flags & CHAN_DISABLED_FLAG, 0);
+                                               },
+                                               NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } if chan_closed => {
+                                                       if let Some(scid) = conditions.expected_blamed_scid {
+                                                               assert_eq!(*short_channel_id, scid);
+                                                       }
+                                                       assert!(is_permanent);
+                                               },
+                                               _ => panic!("Unexpected update type"),
+                                       }
+                               } else { panic!("Expected network update"); }
                        }
  
                        payment_id.unwrap()
@@@ -2240,10 -2241,9 +2241,9 @@@ pub fn pass_failed_payment_back<'a, 'b
                        if i == expected_paths.len() - 1 { assert_eq!(events.len(), 2); } else { assert_eq!(events.len(), 1); }
  
                        let expected_payment_id = match events[0] {
-                               Event::PaymentPathFailed { payment_hash, payment_failed_permanently, all_paths_failed, ref path, ref payment_id, .. } => {
+                               Event::PaymentPathFailed { payment_hash, payment_failed_permanently, ref path, ref payment_id, .. } => {
                                        assert_eq!(payment_hash, our_payment_hash);
                                        assert!(payment_failed_permanently);
-                                       assert_eq!(all_paths_failed, i == expected_paths.len() - 1);
                                        for (idx, hop) in expected_route.iter().enumerate() {
                                                assert_eq!(hop.node.get_our_node_id(), path[idx].pubkey);
                                        }
@@@ -2300,7 -2300,7 +2300,7 @@@ pub fn create_node_cfgs<'a>(node_count
  
        for i in 0..node_count {
                let chain_monitor = test_utils::TestChainMonitor::new(Some(&chanmon_cfgs[i].chain_source), &chanmon_cfgs[i].tx_broadcaster, &chanmon_cfgs[i].logger, &chanmon_cfgs[i].fee_estimator, &chanmon_cfgs[i].persister, &chanmon_cfgs[i].keys_manager);
 -              let network_graph = Arc::new(NetworkGraph::new(chanmon_cfgs[i].chain_source.genesis_hash, &chanmon_cfgs[i].logger));
 +              let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &chanmon_cfgs[i].logger));
                let seed = [i as u8; 32];
                nodes.push(NodeCfg {
                        chain_source: &chanmon_cfgs[i].chain_source,
@@@ -2341,7 -2341,7 +2341,7 @@@ pub fn create_node_chanmgrs<'a, 'b>(nod
                let network = Network::Testnet;
                let params = ChainParameters {
                        network,
 -                      best_block: BestBlock::from_genesis(network),
 +                      best_block: BestBlock::from_network(network),
                };
                let node = ChannelManager::new(cfgs[i].fee_estimator, &cfgs[i].chain_monitor, cfgs[i].tx_broadcaster, &cfgs[i].router, cfgs[i].logger, cfgs[i].keys_manager,
                        cfgs[i].keys_manager, cfgs[i].keys_manager, if node_config[i].is_some() { node_config[i].clone().unwrap() } else { test_default_channel_config() }, params);
index 25781f36f237ed26cb495c305797091aba100789,03eef43726280bfcb1ebb2af101802c77f20209f..538f51c3c591558298a8f8eef7871a3bfdd0ae43
@@@ -31,7 -31,7 +31,7 @@@ use crate::ln::msgs
  use crate::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction};
  use crate::util::enforcing_trait_impls::EnforcingSigner;
  use crate::util::test_utils;
- use crate::util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose, ClosureReason, HTLCDestination};
+ use crate::util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination};
  use crate::util::errors::APIError;
  use crate::util::ser::{Writeable, ReadableArgs};
  use crate::util::config::UserConfig;
@@@ -3235,12 -3235,12 +3235,12 @@@ fn do_test_commitment_revoked_fail_back
                        let events = nodes[0].node.get_and_clear_pending_events();
                        assert_eq!(events.len(), 6);
                        match events[0] {
-                               Event::PaymentPathFailed { ref payment_hash, ref network_update, .. } => {
+                               Event::PaymentPathFailed { ref payment_hash, ref failure, .. } => {
                                        assert!(failed_htlcs.insert(payment_hash.0));
                                        // If we delivered B's RAA we got an unknown preimage error, not something
                                        // that we should update our routing table for.
                                        if !deliver_bs_raa {
-                                               assert!(network_update.is_some());
+                                               if let PathFailure::OnPath { network_update: Some(_) } = failure { } else { panic!("Unexpected path failure") }
                                        }
                                },
                                _ => panic!("Unexpected event"),
                                _ => panic!("Unexpected event"),
                        }
                        match events[2] {
-                               Event::PaymentPathFailed { ref payment_hash, ref network_update, .. } => {
+                               Event::PaymentPathFailed { ref payment_hash, failure: PathFailure::OnPath { network_update: Some(_) }, .. } => {
                                        assert!(failed_htlcs.insert(payment_hash.0));
-                                       assert!(network_update.is_some());
                                },
                                _ => panic!("Unexpected event"),
                        }
                                _ => panic!("Unexpected event"),
                        }
                        match events[4] {
-                               Event::PaymentPathFailed { ref payment_hash, ref network_update, .. } => {
+                               Event::PaymentPathFailed { ref payment_hash, failure: PathFailure::OnPath { network_update: Some(_) }, .. } => {
                                        assert!(failed_htlcs.insert(payment_hash.0));
-                                       assert!(network_update.is_some());
                                },
                                _ => panic!("Unexpected event"),
                        }
@@@ -5148,14 -5146,14 +5146,14 @@@ fn do_test_fail_backwards_unrevoked_rem
        let mut as_failds = HashSet::new();
        let mut as_updates = 0;
        for event in as_events.iter() {
-               if let &Event::PaymentPathFailed { ref payment_hash, ref payment_failed_permanently, ref network_update, .. } = event {
+               if let &Event::PaymentPathFailed { ref payment_hash, ref payment_failed_permanently, ref failure, .. } = event {
                        assert!(as_failds.insert(*payment_hash));
                        if *payment_hash != payment_hash_2 {
                                assert_eq!(*payment_failed_permanently, deliver_last_raa);
                        } else {
                                assert!(!payment_failed_permanently);
                        }
-                       if network_update.is_some() {
+                       if let PathFailure::OnPath { network_update: Some(_) } = failure {
                                as_updates += 1;
                        }
                } else if let &Event::PaymentFailed { .. } = event {
        let mut bs_failds = HashSet::new();
        let mut bs_updates = 0;
        for event in bs_events.iter() {
-               if let &Event::PaymentPathFailed { ref payment_hash, ref payment_failed_permanently, ref network_update, .. } = event {
+               if let &Event::PaymentPathFailed { ref payment_hash, ref payment_failed_permanently, ref failure, .. } = event {
                        assert!(bs_failds.insert(*payment_hash));
                        if *payment_hash != payment_hash_1 && *payment_hash != payment_hash_5 {
                                assert_eq!(*payment_failed_permanently, deliver_last_raa);
                        } else {
                                assert!(!payment_failed_permanently);
                        }
-                       if network_update.is_some() {
+                       if let PathFailure::OnPath { network_update: Some(_) } = failure {
                                bs_updates += 1;
                        }
                } else if let &Event::PaymentFailed { .. } = event {
@@@ -5280,7 -5278,7 +5278,7 @@@ fn test_key_derivation_params() 
        let seed = [42; 32];
        let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
        let chain_monitor = test_utils::TestChainMonitor::new(Some(&chanmon_cfgs[0].chain_source), &chanmon_cfgs[0].tx_broadcaster, &chanmon_cfgs[0].logger, &chanmon_cfgs[0].fee_estimator, &chanmon_cfgs[0].persister, &keys_manager);
 -      let network_graph = Arc::new(NetworkGraph::new(chanmon_cfgs[0].chain_source.genesis_hash, &chanmon_cfgs[0].logger));
 +      let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &chanmon_cfgs[0].logger));
        let scorer = Mutex::new(test_utils::TestScorer::new());
        let router = test_utils::TestRouter::new(network_graph.clone(), &scorer);
        let node = NodeCfg { chain_source: &chanmon_cfgs[0].chain_source, logger: &chanmon_cfgs[0].logger, tx_broadcaster: &chanmon_cfgs[0].tx_broadcaster, fee_estimator: &chanmon_cfgs[0].fee_estimator, router, chain_monitor, keys_manager: &keys_manager, network_graph, node_seed: seed, override_init_features: alloc::rc::Rc::new(core::cell::RefCell::new(None)) };
@@@ -5695,12 -5693,10 +5693,10 @@@ fn test_fail_holding_cell_htlc_upon_fre
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 2);
        match &events[0] {
-               &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, .. } => {
+               &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, failure: PathFailure::OnPath { network_update: None }, ref short_channel_id, .. } => {
                        assert_eq!(PaymentId(our_payment_hash.0), *payment_id.as_ref().unwrap());
                        assert_eq!(our_payment_hash.clone(), *payment_hash);
                        assert_eq!(*payment_failed_permanently, false);
-                       assert_eq!(*all_paths_failed, true);
-                       assert_eq!(*network_update, None);
                        assert_eq!(*short_channel_id, Some(route.paths[0][0].short_channel_id));
                },
                _ => panic!("Unexpected event"),
@@@ -5786,12 -5782,10 +5782,10 @@@ fn test_free_and_fail_holding_cell_htlc
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 2);
        match &events[0] {
-               &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, .. } => {
+               &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, failure: PathFailure::OnPath { network_update: None }, ref short_channel_id, .. } => {
                        assert_eq!(payment_id_2, *payment_id.as_ref().unwrap());
                        assert_eq!(payment_hash_2.clone(), *payment_hash);
                        assert_eq!(*payment_failed_permanently, false);
-                       assert_eq!(*all_paths_failed, true);
-                       assert_eq!(*network_update, None);
                        assert_eq!(*short_channel_id, Some(route_2.paths[0][0].short_channel_id));
                },
                _ => panic!("Unexpected event"),
@@@ -6689,8 -6683,7 +6683,7 @@@ fn test_channel_failed_after_message_wi
        // Expect a PaymentPathFailed event with a ChannelFailure network update for the channel between
        // the node originating the error to its next hop.
        match events_5[0] {
-               Event::PaymentPathFailed { network_update:
-                       Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent }), error_code, ..
+               Event::PaymentPathFailed { error_code, failure: PathFailure::OnPath { network_update: Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent }) }, ..
                } => {
                        assert_eq!(short_channel_id, chan_2.0.contents.short_channel_id);
                        assert!(is_permanent);
index 81b95f1ce07230ccd7b46339fd7c28ea6bcaba8e,b83eb41748fdd8a095590c4312b2e8c2baffba33..0f9b5e1f890f76dba579ea9e5b027b9de160a74d
@@@ -17,7 -17,6 +17,6 @@@ use crate::chain::keysinterface::{Entro
  use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
  use crate::ln::channelmanager::{ChannelDetails, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, PaymentId};
  use crate::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA as LDK_DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA;
- use crate::ln::msgs::DecodeError;
  use crate::ln::onion_utils::HTLCFailReason;
  use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters, RoutePath, Router};
  use crate::util::errors::APIError;
@@@ -783,19 -782,18 +782,18 @@@ impl OutboundPayments 
                debug_assert_eq!(paths.len(), path_results.len());
                for (path, path_res) in paths.into_iter().zip(path_results) {
                        if let Err(e) = path_res {
-                               let failed_scid = if let APIError::InvalidRoute { .. } = e {
-                                       None
-                               } else {
+                               if let APIError::MonitorUpdateInProgress = e { continue }
+                               let mut failed_scid = None;
+                               if let APIError::ChannelUnavailable { .. } = e {
                                        let scid = path[0].short_channel_id;
+                                       failed_scid = Some(scid);
                                        route_params.payment_params.previously_failed_channels.push(scid);
-                                       Some(scid)
-                               };
+                               }
                                events.push(events::Event::PaymentPathFailed {
                                        payment_id: Some(payment_id),
                                        payment_hash,
                                        payment_failed_permanently: false,
-                                       network_update: None,
-                                       all_paths_failed: false,
+                                       failure: events::PathFailure::InitialSend { err: e },
                                        path,
                                        short_channel_id: failed_scid,
                                        retry: None,
                   u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
        {
                if route.paths.len() < 1 {
-                       return Err(PaymentSendFailure::ParameterError(APIError::InvalidRoute{err: "There must be at least one path to send over"}));
+                       return Err(PaymentSendFailure::ParameterError(APIError::InvalidRoute{err: "There must be at least one path to send over".to_owned()}));
                }
                if payment_secret.is_none() && route.paths.len() > 1 {
-                       return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError{err: "Payment secret is required for multi-path payments".to_string()}));
+                       return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError{err: "Payment secret is required for multi-path payments".to_owned()}));
                }
                let mut total_value = 0;
                let our_node_id = node_signer.get_node_id(Recipient::Node).unwrap(); // TODO no unwrap
                let mut path_errs = Vec::with_capacity(route.paths.len());
                'path_check: for path in route.paths.iter() {
                        if path.len() < 1 || path.len() > 20 {
-                               path_errs.push(Err(APIError::InvalidRoute{err: "Path didn't go anywhere/had bogus size"}));
+                               path_errs.push(Err(APIError::InvalidRoute{err: "Path didn't go anywhere/had bogus size".to_owned()}));
                                continue 'path_check;
                        }
                        for (idx, hop) in path.iter().enumerate() {
                                if idx != path.len() - 1 && hop.pubkey == our_node_id {
-                                       path_errs.push(Err(APIError::InvalidRoute{err: "Path went through us but wasn't a simple rebalance loop to us"}));
+                                       path_errs.push(Err(APIError::InvalidRoute{err: "Path went through us but wasn't a simple rebalance loop to us".to_owned()}));
                                        continue 'path_check;
                                }
                        }
                        awaiting_retry
                });
  
-               let mut all_paths_failed = false;
                let mut full_failure_ev = None;
                let mut pending_retry_ev = false;
                let mut retry = None;
                                is_retryable_now = false;
                        }
                        if payment.get().remaining_parts() == 0 {
-                               all_paths_failed = true;
                                if payment.get().abandoned() {
                                        if !payment_is_probe {
                                                full_failure_ev = Some(events::Event::PaymentFailed {
                                        payment_id: Some(*payment_id),
                                        payment_hash: payment_hash.clone(),
                                        payment_failed_permanently: !payment_retryable,
-                                       network_update,
-                                       all_paths_failed,
+                                       failure: events::PathFailure::OnPath { network_update },
                                        path: path.clone(),
                                        short_channel_id,
                                        retry,
@@@ -1352,17 -1347,20 +1347,19 @@@ impl_writeable_tlv_based_enum_upgradabl
  
  #[cfg(test)]
  mod tests {
 -      use bitcoin::blockdata::constants::genesis_block;
        use bitcoin::network::constants::Network;
        use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
  
        use crate::ln::PaymentHash;
        use crate::ln::channelmanager::PaymentId;
+       use crate::ln::features::{ChannelFeatures, NodeFeatures};
        use crate::ln::msgs::{ErrorAction, LightningError};
        use crate::ln::outbound_payment::{OutboundPayments, Retry, RetryableSendFailure};
        use crate::routing::gossip::NetworkGraph;
-       use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteParameters};
+       use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters};
        use crate::sync::{Arc, Mutex};
-       use crate::util::events::Event;
+       use crate::util::errors::APIError;
+       use crate::util::events::{Event, PathFailure};
        use crate::util::test_utils;
  
        #[test]
        fn do_fails_paying_after_expiration(on_retry: bool) {
                let outbound_payments = OutboundPayments::new();
                let logger = test_utils::TestLogger::new();
 -              let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
 -              let network_graph = Arc::new(NetworkGraph::new(genesis_hash, &logger));
 +              let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger));
                let scorer = Mutex::new(test_utils::TestScorer::new());
                let router = test_utils::TestRouter::new(network_graph, &scorer);
                let secp_ctx = Secp256k1::new();
        fn do_find_route_error(on_retry: bool) {
                let outbound_payments = OutboundPayments::new();
                let logger = test_utils::TestLogger::new();
 -              let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
 -              let network_graph = Arc::new(NetworkGraph::new(genesis_hash, &logger));
 +              let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger));
                let scorer = Mutex::new(test_utils::TestScorer::new());
                let router = test_utils::TestRouter::new(network_graph, &scorer);
                let secp_ctx = Secp256k1::new();
                        } else { panic!("Unexpected error"); }
                }
        }
+       #[test]
+       fn initial_send_payment_path_failed_evs() {
+               let outbound_payments = OutboundPayments::new();
+               let logger = test_utils::TestLogger::new();
+               let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
+               let network_graph = Arc::new(NetworkGraph::new(genesis_hash, &logger));
+               let scorer = Mutex::new(test_utils::TestScorer::new());
+               let router = test_utils::TestRouter::new(network_graph, &scorer);
+               let secp_ctx = Secp256k1::new();
+               let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet);
+               let sender_pk = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
+               let receiver_pk = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[43; 32]).unwrap());
+               let payment_params = PaymentParameters::from_node_id(sender_pk, 0);
+               let route_params = RouteParameters {
+                       payment_params: payment_params.clone(),
+                       final_value_msat: 0,
+                       final_cltv_expiry_delta: 0,
+               };
+               let failed_scid = 42;
+               let route = Route {
+                       paths: vec![vec![RouteHop {
+                               pubkey: receiver_pk,
+                               node_features: NodeFeatures::empty(),
+                               short_channel_id: failed_scid,
+                               channel_features: ChannelFeatures::empty(),
+                               fee_msat: 0,
+                               cltv_expiry_delta: 0,
+                       }]],
+                       payment_params: Some(payment_params),
+               };
+               router.expect_find_route(route_params.clone(), Ok(route.clone()));
+               let mut route_params_w_failed_scid = route_params.clone();
+               route_params_w_failed_scid.payment_params.previously_failed_channels.push(failed_scid);
+               router.expect_find_route(route_params_w_failed_scid, Ok(route.clone()));
+               router.expect_find_route(route_params.clone(), Ok(route.clone()));
+               router.expect_find_route(route_params.clone(), Ok(route.clone()));
+               // Ensure that a ChannelUnavailable error will result in blaming an scid in the
+               // PaymentPathFailed event.
+               let pending_events = Mutex::new(Vec::new());
+               outbound_payments.send_payment(
+                       PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), route_params.clone(),
+                       &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
+                       &pending_events,
+                       |_, _, _, _, _, _, _, _, _| Err(APIError::ChannelUnavailable { err: "test".to_owned() }))
+                       .unwrap();
+               let mut events = pending_events.lock().unwrap();
+               assert_eq!(events.len(), 2);
+               if let Event::PaymentPathFailed {
+                       short_channel_id,
+                       failure: PathFailure::InitialSend { err: APIError::ChannelUnavailable { .. }}, .. } = events[0]
+               {
+                       assert_eq!(short_channel_id, Some(failed_scid));
+               } else { panic!("Unexpected event"); }
+               if let Event::PaymentFailed { .. } = events[1] { } else { panic!("Unexpected event"); }
+               events.clear();
+               core::mem::drop(events);
+               // Ensure that a MonitorUpdateInProgress "error" will not result in a PaymentPathFailed event.
+               outbound_payments.send_payment(
+                       PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), route_params.clone(),
+                       &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
+                       &pending_events, |_, _, _, _, _, _, _, _, _| Err(APIError::MonitorUpdateInProgress))
+                       .unwrap();
+               {
+                       let events = pending_events.lock().unwrap();
+                       assert_eq!(events.len(), 0);
+               }
+               // Ensure that any other error will result in a PaymentPathFailed event but no blamed scid.
+               outbound_payments.send_payment(
+                       PaymentHash([0; 32]), &None, PaymentId([1; 32]), Retry::Attempts(0), route_params.clone(),
+                       &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
+                       &pending_events,
+                       |_, _, _, _, _, _, _, _, _| Err(APIError::APIMisuseError { err: "test".to_owned() }))
+                       .unwrap();
+               let events = pending_events.lock().unwrap();
+               assert_eq!(events.len(), 2);
+               if let Event::PaymentPathFailed {
+                       short_channel_id,
+                       failure: PathFailure::InitialSend { err: APIError::APIMisuseError { .. }}, .. } = events[0]
+               {
+                       assert_eq!(short_channel_id, None);
+               } else { panic!("Unexpected event"); }
+               if let Event::PaymentFailed { .. } = events[1] { } else { panic!("Unexpected event"); }
+       }
  }
index 3d95368d64ed282f11a378f86814a816e88ed1d5,167f79fb269a4334349337370b4ca6a9dbad6676..3f9313c686aab4bd1454f8b8bda84f762afee36a
@@@ -18,9 -18,6 +18,9 @@@ use bitcoin::hashes::sha256d::Hash as S
  use bitcoin::hashes::Hash;
  use bitcoin::hash_types::BlockHash;
  
 +use bitcoin::network::constants::Network;
 +use bitcoin::blockdata::constants::genesis_block;
 +
  use crate::ln::features::{ChannelFeatures, NodeFeatures, InitFeatures};
  use crate::ln::msgs::{DecodeError, ErrorAction, Init, LightningError, RoutingMessageHandler, NetAddress, MAX_VALUE_MSAT};
  use crate::ln::msgs::{ChannelAnnouncement, ChannelUpdate, NodeAnnouncement, GossipTimestampFilter};
@@@ -886,9 -883,9 +886,9 @@@ impl Readable for ChannelInfo 
                        (0, features, required),
                        (1, announcement_received_time, (default_value, 0)),
                        (2, node_one, required),
-                       (4, one_to_two_wrap, ignorable),
+                       (4, one_to_two_wrap, upgradable_option),
                        (6, node_two, required),
-                       (8, two_to_one_wrap, ignorable),
+                       (8, two_to_one_wrap, upgradable_option),
                        (10, capacity_sats, required),
                        (12, announcement_message, required),
                });
@@@ -1020,7 -1017,7 +1020,7 @@@ impl EffectiveCapacity 
  /// Fees for routing via a given channel or a node
  #[derive(Eq, PartialEq, Copy, Clone, Debug, Hash)]
  pub struct RoutingFees {
 -      /// Flat routing fee in satoshis
 +      /// Flat routing fee in millisatoshis.
        pub base_msat: u32,
        /// Liquidity-based routing fee in millionths of a routed amount.
        /// In other words, 10000 is 1%.
@@@ -1164,7 -1161,7 +1164,7 @@@ impl Readable for NodeInfo 
  
                read_tlv_fields!(reader, {
                        (0, _lowest_inbound_channel_fees, option),
-                       (2, announcement_info_wrap, ignorable),
+                       (2, announcement_info_wrap, upgradable_option),
                        (4, channels, vec_type),
                });
  
@@@ -1268,10 -1265,10 +1268,10 @@@ impl<L: Deref> PartialEq for NetworkGra
  
  impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
        /// Creates a new, empty, network graph.
 -      pub fn new(genesis_hash: BlockHash, logger: L) -> NetworkGraph<L> {
 +      pub fn new(network: Network, logger: L) -> NetworkGraph<L> {
                Self {
                        secp_ctx: Secp256k1::verification_only(),
 -                      genesis_hash,
 +                      genesis_hash: genesis_block(network).header.block_hash(),
                        logger,
                        channels: RwLock::new(IndexedMap::new()),
                        nodes: RwLock::new(IndexedMap::new()),
@@@ -1963,8 -1960,9 +1963,8 @@@ pub(crate) mod tests 
        use crate::sync::Arc;
  
        fn create_network_graph() -> NetworkGraph<Arc<test_utils::TestLogger>> {
 -              let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
                let logger = Arc::new(test_utils::TestLogger::new());
 -              NetworkGraph::new(genesis_hash, logger)
 +              NetworkGraph::new(Network::Testnet, logger)
        }
  
        fn create_gossip_sync(network_graph: &NetworkGraph<Arc<test_utils::TestLogger>>) -> (
                let valid_announcement = get_signed_channel_announcement(|_| {}, node_1_privkey, node_2_privkey, &secp_ctx);
  
                // Test if the UTXO lookups were not supported
 -              let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
 -              let network_graph = NetworkGraph::new(genesis_hash, &logger);
 +              let network_graph = NetworkGraph::new(Network::Testnet, &logger);
                let mut gossip_sync = P2PGossipSync::new(&network_graph, None, &logger);
                match gossip_sync.handle_channel_announcement(&valid_announcement) {
                        Ok(res) => assert!(res),
                // Test if an associated transaction were not on-chain (or not confirmed).
                let chain_source = test_utils::TestChainSource::new(Network::Testnet);
                *chain_source.utxo_ret.lock().unwrap() = UtxoResult::Sync(Err(UtxoLookupError::UnknownTx));
 -              let network_graph = NetworkGraph::new(genesis_hash, &logger);
 +              let network_graph = NetworkGraph::new(Network::Testnet, &logger);
                gossip_sync = P2PGossipSync::new(&network_graph, Some(&chain_source), &logger);
  
                let valid_announcement = get_signed_channel_announcement(|unsigned_announcement| {
                let secp_ctx = Secp256k1::new();
                let logger = test_utils::TestLogger::new();
                let chain_source = test_utils::TestChainSource::new(Network::Testnet);
 -              let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
 -              let network_graph = NetworkGraph::new(genesis_hash, &logger);
 +              let network_graph = NetworkGraph::new(Network::Testnet, &logger);
                let gossip_sync = P2PGossipSync::new(&network_graph, Some(&chain_source), &logger);
  
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
        #[test]
        fn handling_network_update() {
                let logger = test_utils::TestLogger::new();
 -              let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
 -              let network_graph = NetworkGraph::new(genesis_hash, &logger);
 +              let network_graph = NetworkGraph::new(Network::Testnet, &logger);
                let secp_ctx = Secp256k1::new();
  
                let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
  
                {
                        // Get a new network graph since we don't want to track removed nodes in this test with "std"
 -                      let network_graph = NetworkGraph::new(genesis_hash, &logger);
 +                      let network_graph = NetworkGraph::new(Network::Testnet, &logger);
  
                        // Announce a channel to test permanent node failure
                        let valid_channel_announcement = get_signed_channel_announcement(|_| {}, node_1_privkey, node_2_privkey, &secp_ctx);
                // Test the removal of channels with `remove_stale_channels_and_tracking`.
                let logger = test_utils::TestLogger::new();
                let chain_source = test_utils::TestChainSource::new(Network::Testnet);
 -              let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
 -              let network_graph = NetworkGraph::new(genesis_hash, &logger);
 +              let network_graph = NetworkGraph::new(Network::Testnet, &logger);
                let gossip_sync = P2PGossipSync::new(&network_graph, Some(&chain_source), &logger);
                let secp_ctx = Secp256k1::new();
  
index 0a5c0b643dd0c315f2782d94b7caa469dcf4e46d,540856865408e5cf0e28737b0bdfc7315406490a..5e55ab1d3b8182baf80f87e0e1a929d90380057c
@@@ -39,9 -39,12 +39,12 @@@ macro_rules! _encode_tlv 
                        field.write($stream)?;
                }
        };
-       ($stream: expr, $type: expr, $field: expr, ignorable) => {
+       ($stream: expr, $type: expr, $field: expr, upgradable_required) => {
                $crate::_encode_tlv!($stream, $type, $field, required);
        };
+       ($stream: expr, $type: expr, $field: expr, upgradable_option) => {
+               $crate::_encode_tlv!($stream, $type, $field, option);
+       };
        ($stream: expr, $type: expr, $field: expr, (option, encoding: ($fieldty: ty, $encoding: ident))) => {
                $crate::_encode_tlv!($stream, $type, $field.map(|f| $encoding(f)), option);
        };
@@@ -158,9 -161,12 +161,12 @@@ macro_rules! _get_varint_length_prefixe
                        $len.0 += field_len;
                }
        };
-       ($len: expr, $type: expr, $field: expr, ignorable) => {
+       ($len: expr, $type: expr, $field: expr, upgradable_required) => {
                $crate::_get_varint_length_prefixed_tlv_length!($len, $type, $field, required);
        };
+       ($len: expr, $type: expr, $field: expr, upgradable_option) => {
+               $crate::_get_varint_length_prefixed_tlv_length!($len, $type, $field, option);
+       };
  }
  
  /// See the documentation of [`write_tlv_fields`].
@@@ -210,7 -216,10 +216,10 @@@ macro_rules! _check_decoded_tlv_order 
        ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, vec_type) => {{
                // no-op
        }};
-       ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, ignorable) => {{
+       ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, upgradable_required) => {{
+               _check_decoded_tlv_order!($last_seen_type, $typ, $type, $field, required)
+       }};
+       ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, upgradable_option) => {{
                // no-op
        }};
        ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (option: $trait: ident $(, $read_arg: expr)?)) => {{
@@@ -249,7 -258,10 +258,10 @@@ macro_rules! _check_missing_tlv 
        ($last_seen_type: expr, $type: expr, $field: ident, option) => {{
                // no-op
        }};
-       ($last_seen_type: expr, $type: expr, $field: ident, ignorable) => {{
+       ($last_seen_type: expr, $type: expr, $field: ident, upgradable_required) => {{
+               _check_missing_tlv!($last_seen_type, $type, $field, required)
+       }};
+       ($last_seen_type: expr, $type: expr, $field: ident, upgradable_option) => {{
                // no-op
        }};
        ($last_seen_type: expr, $type: expr, $field: ident, (option: $trait: ident $(, $read_arg: expr)?)) => {{
@@@ -280,7 -292,20 +292,20 @@@ macro_rules! _decode_tlv 
        ($reader: expr, $field: ident, option) => {{
                $field = Some($crate::util::ser::Readable::read(&mut $reader)?);
        }};
-       ($reader: expr, $field: ident, ignorable) => {{
+       // `upgradable_required` indicates we're reading a required TLV that may have been upgraded
+       // without backwards compat. We'll error if the field is missing, and return `Ok(None)` if the
+       // field is present but we can no longer understand it.
+       // Note that this variant can only be used within a `MaybeReadable` read.
+       ($reader: expr, $field: ident, upgradable_required) => {{
+               $field = match $crate::util::ser::MaybeReadable::read(&mut $reader)? {
+                       Some(res) => res,
+                       _ => return Ok(None)
+               };
+       }};
+       // `upgradable_option` indicates we're reading an Option-al TLV that may have been upgraded
+       // without backwards compat. $field will be None if the TLV is missing or if the field is present
+       // but we can no longer understand it.
+       ($reader: expr, $field: ident, upgradable_option) => {{
                $field = $crate::util::ser::MaybeReadable::read(&mut $reader)?;
        }};
        ($reader: expr, $field: ident, (option: $trait: ident $(, $read_arg: expr)?)) => {{
@@@ -378,6 -403,7 +403,6 @@@ macro_rules! decode_tlv_stream_with_cus
  macro_rules! _decode_tlv_stream_range {
        ($stream: expr, $range: expr, $rewind: ident, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
         $(, $decode_custom_tlv: expr)?) => { {
 -              use core::ops::RangeBounds;
                use $crate::ln::msgs::DecodeError;
                let mut last_seen_type: Option<u64> = None;
                let mut stream_ref = $stream;
                                                }
                                        },
                                        Err(e) => return Err(e),
 -                                      Ok(t) => if $range.contains(&t.0) { t } else {
 +                                      Ok(t) => if core::ops::RangeBounds::contains(&$range, &t.0) { t } else {
                                                drop(tracking_reader);
  
                                                // Assumes the type id is minimally encoded, which is enforced on read.
@@@ -619,8 -645,11 +644,11 @@@ macro_rules! _init_tlv_based_struct_fie
        ($field: ident, option) => {
                $field
        };
-       ($field: ident, ignorable) => {
-               if $field.is_none() { return Ok(None); } else { $field.unwrap() }
+       ($field: ident, upgradable_required) => {
+               $field.0.unwrap()
+       };
+       ($field: ident, upgradable_option) => {
+               $field
        };
        ($field: ident, required) => {
                $field.0.unwrap()
  #[macro_export]
  macro_rules! _init_tlv_field_var {
        ($field: ident, (default_value, $default: expr)) => {
-               let mut $field = $crate::util::ser::OptionDeserWrapper(None);
+               let mut $field = $crate::util::ser::RequiredWrapper(None);
        };
        ($field: ident, (static_value, $value: expr)) => {
                let $field;
        };
        ($field: ident, required) => {
-               let mut $field = $crate::util::ser::OptionDeserWrapper(None);
+               let mut $field = $crate::util::ser::RequiredWrapper(None);
        };
        ($field: ident, vec_type) => {
                let mut $field = Some(Vec::new());
        ($field: ident, option) => {
                let mut $field = None;
        };
-       ($field: ident, ignorable) => {
+       ($field: ident, upgradable_required) => {
+               let mut $field = $crate::util::ser::UpgradableRequired(None);
+       };
+       ($field: ident, upgradable_option) => {
                let mut $field = None;
        };
  }
@@@ -948,7 -980,7 +979,7 @@@ macro_rules! impl_writeable_tlv_based_e
                                                Ok(Some($st::$tuple_variant_name(Readable::read(reader)?)))
                                        }),*)*
                                        _ if id % 2 == 1 => Ok(None),
-                                       _ => Err(DecodeError::UnknownRequiredFeature),
+                                       _ => Err($crate::ln::msgs::DecodeError::UnknownRequiredFeature),
                                }
                        }
                }
@@@ -1028,6 -1060,47 +1059,47 @@@ mod tests 
                        (0xdeadbeef1badbeef, 0x1bad1dea, Some(0x01020304)));
        }
  
+       #[derive(Debug, PartialEq)]
+       struct TestUpgradable {
+               a: u32,
+               b: u32,
+               c: Option<u32>,
+       }
+       fn upgradable_tlv_reader(s: &[u8]) -> Result<Option<TestUpgradable>, DecodeError> {
+               let mut s = Cursor::new(s);
+               let mut a = 0;
+               let mut b = 0;
+               let mut c: Option<u32> = None;
+               decode_tlv_stream!(&mut s, {(2, a, upgradable_required), (3, b, upgradable_required), (4, c, upgradable_option)});
+               Ok(Some(TestUpgradable { a, b, c, }))
+       }
+       #[test]
+       fn upgradable_tlv_simple_good_cases() {
+               assert_eq!(upgradable_tlv_reader(&::hex::decode(
+                       concat!("0204deadbeef", "03041bad1dea", "0404deadbeef")
+               ).unwrap()[..]).unwrap(),
+               Some(TestUpgradable { a: 0xdeadbeef, b: 0x1bad1dea, c: Some(0xdeadbeef) }));
+               assert_eq!(upgradable_tlv_reader(&::hex::decode(
+                       concat!("0204deadbeef", "03041bad1dea")
+               ).unwrap()[..]).unwrap(),
+               Some(TestUpgradable { a: 0xdeadbeef, b: 0x1bad1dea, c: None}));
+       }
+       #[test]
+       fn missing_required_upgradable() {
+               if let Err(DecodeError::InvalidValue) = upgradable_tlv_reader(&::hex::decode(
+                       concat!("0100", "0204deadbeef")
+                       ).unwrap()[..]) {
+               } else { panic!(); }
+               if let Err(DecodeError::InvalidValue) = upgradable_tlv_reader(&::hex::decode(
+                       concat!("0100", "03041bad1dea")
+               ).unwrap()[..]) {
+               } else { panic!(); }
+       }
        // BOLT TLV test cases
        fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedBigSize<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
                let mut s = Cursor::new(s);