Move UTXO-lookup into pub utility function from RoutingMsgHandler

This makes the public utility methods in `NetworkGraph` able to do
UTXO lookups ala `NetworkMsgHandler`'s `RoutingMessageHandler`
implementation, slightly simplifying the public interface.

We also take this opportunity to verify signatures before calling
out to UTXO lookups, under the "do actions in order of
cheapest-to-most-expensive to reduce DoS surface" principle.

diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs
index ef7669f1eafd7f477182452fb9006610bd95f2e0..95abf9718c54bc1a863dd888167ca803f19162d9 100644 (file)
--- a/fuzz/src/router.rs
+++ b/fuzz/src/router.rs
@@ -175,14 +175,13 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                                let msg = decode_msg_with_len16!(msgs::ChannelAnnouncement, 64*4, 32+8+33*4);
                                node_pks.insert(msg.contents.node_id_1);
                                node_pks.insert(msg.contents.node_id_2);
-                               let _ = net_graph.update_channel_from_announcement::<secp256k1::VerifyOnly>(&msg, None, None);
+                               let _ = net_graph.update_channel_from_announcement::<secp256k1::VerifyOnly, &FuzzChainSource>(&msg, &None, None);
                        },
                        2 => {
                                let msg = decode_msg_with_len16!(msgs::ChannelAnnouncement, 64*4, 32+8+33*4);
                                node_pks.insert(msg.contents.node_id_1);
                                node_pks.insert(msg.contents.node_id_2);
-                               let val = slice_to_be64(get_slice!(8));
-                               let _ = net_graph.update_channel_from_announcement::<secp256k1::VerifyOnly>(&msg, Some(val), None);
+                               let _ = net_graph.update_channel_from_announcement::<secp256k1::VerifyOnly, &FuzzChainSource>(&msg, &Some(&FuzzChainSource { input: Arc::clone(&input) }), None);
                        },
                        3 => {
                                let _ = net_graph.update_channel(&decode_msg!(msgs::ChannelUpdate, 136), None);

diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs
index 932f3779deff99d4456a4d81f3ec6c43310810da..5003ca512f520df073c1b94e612ced49bb6b5ca1 100644 (file)
--- a/lightning/src/routing/network_graph.rs
+++ b/lightning/src/routing/network_graph.rs
@@ -125,40 +125,7 @@ impl<C: Deref + Sync + Send, L: Deref + Sync + Send> RoutingMessageHandler for N
        }
 
        fn handle_channel_announcement(&self, msg: &msgs::ChannelAnnouncement) -> Result<bool, LightningError> {
-               if msg.contents.node_id_1 == msg.contents.node_id_2 || msg.contents.bitcoin_key_1 == msg.contents.bitcoin_key_2 {
-                       return Err(LightningError{err: "Channel announcement node had a channel with itself".to_owned(), action: ErrorAction::IgnoreError});
-               }
-
-               let utxo_value = match &self.chain_access {
-                       &None => {
-                               // Tentatively accept, potentially exposing us to DoS attacks
-                               None
-                       },
-                       &Some(ref chain_access) => {
-                               match chain_access.get_utxo(&msg.contents.chain_hash, msg.contents.short_channel_id) {
-                                       Ok(TxOut { value, script_pubkey }) => {
-                                               let expected_script = Builder::new().push_opcode(opcodes::all::OP_PUSHNUM_2)
-                                                                                   .push_slice(&msg.contents.bitcoin_key_1.serialize())
-                                                                                   .push_slice(&msg.contents.bitcoin_key_2.serialize())
-                                                                                   .push_opcode(opcodes::all::OP_PUSHNUM_2)
-                                                                                   .push_opcode(opcodes::all::OP_CHECKMULTISIG).into_script().to_v0_p2wsh();
-                                               if script_pubkey != expected_script {
-                                                       return Err(LightningError{err: format!("Channel announcement key ({}) didn't match on-chain script ({})", script_pubkey.to_hex(), expected_script.to_hex()), action: ErrorAction::IgnoreError});
-                                               }
-                                               //TODO: Check if value is worth storing, use it to inform routing, and compare it
-                                               //to the new HTLC max field in channel_update
-                                               Some(value)
-                                       },
-                                       Err(chain::AccessError::UnknownChain) => {
-                                               return Err(LightningError{err: format!("Channel announced on an unknown chain ({})", msg.contents.chain_hash.encode().to_hex()), action: ErrorAction::IgnoreError});
-                                       },
-                                       Err(chain::AccessError::UnknownTx) => {
-                                               return Err(LightningError{err: "Channel announced without corresponding UTXO entry".to_owned(), action: ErrorAction::IgnoreError});
-                                       },
-                               }
-                       },
-               };
-               let result = self.network_graph.write().unwrap().update_channel_from_announcement(msg, utxo_value, Some(&self.secp_ctx));
+               let result = self.network_graph.write().unwrap().update_channel_from_announcement(msg, &self.chain_access, Some(&self.secp_ctx));
                log_trace!(self.logger, "Added channel_announcement for {}{}", msg.contents.short_channel_id, if !msg.contents.excess_data.is_empty() { " with excess uninterpreted data!" } else { "" });
                result
        }
@@ -605,17 +572,14 @@ impl NetworkGraph {
        /// RoutingMessageHandler implementation to call it indirectly. This may be useful to accept
        /// routing messages without checking their signatures.
        ///
-       /// If the channel has been confirmed to exist on chain (with correctly-formatted scripts on
-       /// chain), set utxo_value to the value of the output on chain, otherwise leave it as None.
-       /// The UTXO value is then used in routing calculation if we have no better information on the
-       /// maximum HTLC value that can be sent over the channel.
-       ///
-       /// Further, setting utxo_value to Some indicates that the announcement message is genuine,
-       /// allowing us to update existing channel data in the case of reorgs or to replace bogus
-       /// channel data generated by a DoS attacker.
+       /// If a `chain::Access` object is provided via `chain_access`, it will be called to verify
+       /// the corresponding UTXO exists on chain and is correctly-formatted.
        ///
        /// Announcement signatures are checked here only if Secp256k1 object is provided.
-       pub fn update_channel_from_announcement<T: secp256k1::Verification>(&mut self, msg: &msgs::ChannelAnnouncement, utxo_value: Option<u64>, secp_ctx: Option<&Secp256k1<T>>) -> Result<bool, LightningError> {
+       pub fn update_channel_from_announcement<T: secp256k1::Verification, C: Deref>
+                       (&mut self, msg: &msgs::ChannelAnnouncement, chain_access: &Option<C>, secp_ctx: Option<&Secp256k1<T>>)
+                       -> Result<bool, LightningError>
+                       where C::Target: chain::Access {
                if msg.contents.node_id_1 == msg.contents.node_id_2 || msg.contents.bitcoin_key_1 == msg.contents.bitcoin_key_2 {
                        return Err(LightningError{err: "Channel announcement node had a channel with itself".to_owned(), action: ErrorAction::IgnoreError});
                }
@@ -628,8 +592,37 @@ impl NetworkGraph {
                        secp_verify_sig!(sig_verifier, &msg_hash, &msg.bitcoin_signature_2, &msg.contents.bitcoin_key_2);
                }
 
-               let should_relay = msg.contents.excess_data.is_empty();
+               let utxo_value = match &chain_access {
+                       &None => {
+                               // Tentatively accept, potentially exposing us to DoS attacks
+                               None
+                       },
+                       &Some(ref chain_access) => {
+                               match chain_access.get_utxo(&msg.contents.chain_hash, msg.contents.short_channel_id) {
+                                       Ok(TxOut { value, script_pubkey }) => {
+                                               let expected_script = Builder::new().push_opcode(opcodes::all::OP_PUSHNUM_2)
+                                                                                   .push_slice(&msg.contents.bitcoin_key_1.serialize())
+                                                                                   .push_slice(&msg.contents.bitcoin_key_2.serialize())
+                                                                                   .push_opcode(opcodes::all::OP_PUSHNUM_2)
+                                                                                   .push_opcode(opcodes::all::OP_CHECKMULTISIG).into_script().to_v0_p2wsh();
+                                               if script_pubkey != expected_script {
+                                                       return Err(LightningError{err: format!("Channel announcement key ({}) didn't match on-chain script ({})", script_pubkey.to_hex(), expected_script.to_hex()), action: ErrorAction::IgnoreError});
+                                               }
+                                               //TODO: Check if value is worth storing, use it to inform routing, and compare it
+                                               //to the new HTLC max field in channel_update
+                                               Some(value)
+                                       },
+                                       Err(chain::AccessError::UnknownChain) => {
+                                               return Err(LightningError{err: format!("Channel announced on an unknown chain ({})", msg.contents.chain_hash.encode().to_hex()), action: ErrorAction::IgnoreError});
+                                       },
+                                       Err(chain::AccessError::UnknownTx) => {
+                                               return Err(LightningError{err: "Channel announced without corresponding UTXO entry".to_owned(), action: ErrorAction::IgnoreError});
+                                       },
+                               }
+                       },
+               };
 
+               let should_relay = msg.contents.excess_data.is_empty();
                let chan_info = ChannelInfo {
                                features: msg.contents.features.clone(),
                                node_one: msg.contents.node_id_1.clone(),