Use usize for transaction-position-in-block values 2020-06-c-bindings-cleanups-2
authorMatt Corallo <git@bluematt.me>
Sat, 13 Jun 2020 01:01:32 +0000 (21:01 -0400)
committerMatt Corallo <git@bluematt.me>
Tue, 23 Jun 2020 20:12:55 +0000 (16:12 -0400)
We use them largely as indexes into a Vec<Transaction> so there's
little reason for them to be u32s. Instead, use them as usize
everywhere.

We also take this opportunity to add range checks before
short_channel_id calculation, as we could otherwise end up with a
bogus short_channel_id due to an output index out of range.

fuzz/src/chanmon_consistency.rs
fuzz/src/full_stack.rs
fuzz/src/router.rs
lightning/src/chain/chaininterface.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/channelmonitor.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/util/test_utils.rs

index 8be994bd7f2c14dd971705aa4e811b55f081ca2d..eba0cb9e4f91adcb17699623ca991aa3a6c13b7c 100644 (file)
@@ -311,7 +311,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                        let mut posn = Vec::with_capacity(channel_txn.len());
                        for i in 0..channel_txn.len() {
                                txn.push(&channel_txn[i]);
-                               posn.push(i as u32 + 1);
+                               posn.push(i + 1);
                        }
                        $node.block_connected(&header, 1, &txn, &posn);
                        for i in 2..100 {
index 61699136f3522a0d61bfdec770866fb0c7b41ffd..18e31fc9499c2b73879c4cd3fefa5a9f418909f2 100644 (file)
@@ -183,7 +183,7 @@ impl<'a> MoneyLossDetector<'a> {
                                hash_map::Entry::Vacant(e) => {
                                        e.insert(self.height);
                                        txn.push(tx);
-                                       txn_idxs.push(idx as u32 + 1);
+                                       txn_idxs.push(idx + 1);
                                },
                                _ => {},
                        }
index c7abd1524ca904ea61f43f5f9bb77b7d89375e50..3a6b5bdf40cd9af51eb0335de6fbdfd521abe9f9 100644 (file)
@@ -75,7 +75,7 @@ impl ChainWatchInterface for DummyChainWatcher {
        fn install_watch_tx(&self, _txid: &Txid, _script_pub_key: &Script) { }
        fn install_watch_outpoint(&self, _outpoint: (Txid, u32), _out_script: &Script) { }
        fn watch_all_txn(&self) { }
-       fn filter_block(&self, _block: &Block) -> Vec<u32> {
+       fn filter_block(&self, _block: &Block) -> Vec<usize> {
                Vec::new()
        }
        fn reentered(&self) -> usize { 0 }
index 7c215f3635c88c17045d2c61aa71f585f3aab2f7..2e874296f5ab94d363c9244db192fa72a3bfb76d 100644 (file)
@@ -55,7 +55,7 @@ pub trait ChainWatchInterface: Sync + Send {
 
        /// Gets the list of transaction indices within a given block that the ChainWatchInterface is
        /// watching for.
-       fn filter_block(&self, block: &Block) -> Vec<u32>;
+       fn filter_block(&self, block: &Block) -> Vec<usize>;
 
        /// Returns a usize that changes when the ChainWatchInterface's watched data is modified.
        /// Users of `filter_block` should pre-save a copy of `reentered`'s return value and use it to
@@ -86,7 +86,7 @@ pub trait ChainListener: Sync + Send {
        ///
        /// This also means those counting confirmations using block_connected callbacks should watch
        /// for duplicate headers and not count them towards confirmations!
-       fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]);
+       fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]);
        /// Notifies a listener that a block was disconnected.
        /// Unlike block_connected, this *must* never be called twice for the same disconnect event.
        /// Height must be the one of the block which was disconnected (not new height of the best chain)
@@ -280,7 +280,7 @@ impl<'a, CL: Deref<Target = ChainListener + 'a> + 'a, C: Deref> BlockNotifier<'a
                        let matched_indexes = self.chain_monitor.filter_block(block);
                        let mut matched_txn = Vec::new();
                        for index in matched_indexes.iter() {
-                               matched_txn.push(&block.txdata[*index as usize]);
+                               matched_txn.push(&block.txdata[*index]);
                        }
                        reentered = self.block_connected_checked(&block.header, height, matched_txn.as_slice(), matched_indexes.as_slice());
                }
@@ -292,7 +292,7 @@ impl<'a, CL: Deref<Target = ChainListener + 'a> + 'a, C: Deref> BlockNotifier<'a
        /// Returns true if notified listeners registered additional watch data (implying that the
        /// block must be re-scanned and this function called again prior to further block_connected
        /// calls, see ChainListener::block_connected for more info).
-       pub fn block_connected_checked(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) -> bool {
+       pub fn block_connected_checked(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]) -> bool {
                let last_seen = self.chain_monitor.reentered();
 
                let listeners = self.listeners.lock().unwrap();
@@ -361,13 +361,13 @@ impl ChainWatchInterface for ChainWatchInterfaceUtil {
                Err(ChainError::NotSupported)
        }
 
-       fn filter_block(&self, block: &Block) -> Vec<u32> {
+       fn filter_block(&self, block: &Block) -> Vec<usize> {
                let mut matched_index = Vec::new();
                {
                        let watched = self.watched.lock().unwrap();
                        for (index, transaction) in block.txdata.iter().enumerate() {
                                if self.does_match_tx_unguarded(transaction, &watched) {
-                                       matched_index.push(index as u32);
+                                       matched_index.push(index);
                                }
                        }
                }
index b4a24833919b96fa26b9037ecea1844d253d0e0e..706acfa2dc674d3fd74863c1d5227e21f567a1b2 100644 (file)
@@ -3299,7 +3299,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        ///
        /// May return some HTLCs (and their payment_hash) which have timed out and should be failed
        /// back.
-       pub fn block_connected(&mut self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> {
+       pub fn block_connected(&mut self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> {
                let mut timed_out_htlcs = Vec::new();
                self.holding_cell_htlc_updates.retain(|htlc_update| {
                        match htlc_update {
@@ -3350,6 +3350,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                                }
                                                        }
                                                }
+                                               if height > 0xff_ff_ff || (*index_in_block) > 0xff_ff_ff {
+                                                       panic!("Block was bogus - either height 16 million or had > 16 million transactions");
+                                               }
+                                               assert!(txo_idx <= 0xffff); // txo_idx is a (u16 as usize), so this is just listed here for completeness
                                                self.funding_tx_confirmations = 1;
                                                self.short_channel_id = Some(((height as u64)          << (5*8)) |
                                                                             ((*index_in_block as u64) << (2*8)) |
index f453ffc061b540cd970f60afc8e5f75e9ceec201..a8a0529899594b28d0c3970e08744b29622fb314 100644 (file)
@@ -2963,7 +2963,7 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
         F::Target: FeeEstimator,
                                L::Target: Logger,
 {
-       fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) {
+       fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]) {
                let header_hash = header.bitcoin_hash();
                log_trace!(self.logger, "Block {} at height {} connected with {} txn matched", header_hash, height, txn_matched.len());
                let _ = self.total_consistency_lock.read().unwrap();
index 330427118778f332599d1de979bd33a3122ad5ab..ce37944ca2f7bfd5224b562d3c1de64471b5c59c 100644 (file)
@@ -183,7 +183,7 @@ impl<Key : Send + cmp::Eq + hash::Hash, ChanSigner: ChannelKeys, T: Deref + Sync
              L::Target: Logger,
         C::Target: ChainWatchInterface,
 {
-       fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], _indexes_of_txn_matched: &[u32]) {
+       fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], _indexes_of_txn_matched: &[usize]) {
                let block_hash = header.bitcoin_hash();
                {
                        let mut monitors = self.monitors.lock().unwrap();
index e408bf13f6875c7dabc2be550232efda169fdde7..24fe127d70e771462ffdf1062a51d0f3483b8340 100644 (file)
@@ -39,7 +39,7 @@ pub const CHAN_CONFIRM_DEPTH: u32 = 100;
 pub fn confirm_transaction<'a, 'b: 'a>(notifier: &'a chaininterface::BlockNotifierRef<'b, &chaininterface::ChainWatchInterfaceUtil>, chain: &chaininterface::ChainWatchInterfaceUtil, tx: &Transaction, chan_id: u32) {
        assert!(chain.does_match_tx(tx));
        let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       notifier.block_connected_checked(&header, 1, &[tx; 1], &[chan_id; 1]);
+       notifier.block_connected_checked(&header, 1, &[tx; 1], &[chan_id as usize; 1]);
        for i in 2..CHAN_CONFIRM_DEPTH {
                header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
                notifier.block_connected_checked(&header, i, &vec![], &[0; 0]);
index 0a058152e1a8e415837f451a583a3743d42b9207..c0851e1d11e5e015046252df5c2afdc90df4ddf0 100644 (file)
@@ -408,10 +408,10 @@ fn test_1_conf_open() {
        assert!(nodes[1].chain_monitor.does_match_tx(&tx));
 
        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       nodes[1].block_notifier.block_connected_checked(&header, 1, &[&tx; 1], &[tx.version; 1]);
+       nodes[1].block_notifier.block_connected_checked(&header, 1, &[&tx; 1], &[tx.version as usize; 1]);
        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()));
 
-       nodes[0].block_notifier.block_connected_checked(&header, 1, &[&tx; 1], &[tx.version; 1]);
+       nodes[0].block_notifier.block_connected_checked(&header, 1, &[&tx; 1], &[tx.version as usize; 1]);
        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);
 
index 145465ef40b80cb710915011f8b5ff54128cd4f8..2134c26f9f605e81126bab0632e25eac4227c523 100644 (file)
@@ -371,7 +371,7 @@ impl ChainWatchInterface for TestChainWatcher {
        fn install_watch_tx(&self, _txid: &Txid, _script_pub_key: &Script) { }
        fn install_watch_outpoint(&self, _outpoint: (Txid, u32), _out_script: &Script) { }
        fn watch_all_txn(&self) { }
-       fn filter_block<'a>(&self, _block: &'a Block) -> Vec<u32> {
+       fn filter_block<'a>(&self, _block: &'a Block) -> Vec<usize> {
                Vec::new()
        }
        fn reentered(&self) -> usize { 0 }