Skip to content

Commit 5c37023

Browse files
committed
Use usize for transaction-position-in-block values
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.
1 parent 7fc07b3 commit 5c37023

10 files changed

+20
-16
lines changed

fuzz/src/chanmon_consistency.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
311311
let mut posn = Vec::with_capacity(channel_txn.len());
312312
for i in 0..channel_txn.len() {
313313
txn.push(&channel_txn[i]);
314-
posn.push(i as u32 + 1);
314+
posn.push(i + 1);
315315
}
316316
$node.block_connected(&header, 1, &txn, &posn);
317317
for i in 2..100 {

fuzz/src/full_stack.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ impl<'a> MoneyLossDetector<'a> {
183183
hash_map::Entry::Vacant(e) => {
184184
e.insert(self.height);
185185
txn.push(tx);
186-
txn_idxs.push(idx as u32 + 1);
186+
txn_idxs.push(idx + 1);
187187
},
188188
_ => {},
189189
}

fuzz/src/router.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ impl ChainWatchInterface for DummyChainWatcher {
7575
fn install_watch_tx(&self, _txid: &Txid, _script_pub_key: &Script) { }
7676
fn install_watch_outpoint(&self, _outpoint: (Txid, u32), _out_script: &Script) { }
7777
fn watch_all_txn(&self) { }
78-
fn filter_block(&self, _block: &Block) -> Vec<u32> {
78+
fn filter_block(&self, _block: &Block) -> Vec<usize> {
7979
Vec::new()
8080
}
8181
fn reentered(&self) -> usize { 0 }

lightning/src/chain/chaininterface.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ pub trait ChainWatchInterface: Sync + Send {
5555

5656
/// Gets the list of transaction indices within a given block that the ChainWatchInterface is
5757
/// watching for.
58-
fn filter_block(&self, block: &Block) -> Vec<u32>;
58+
fn filter_block(&self, block: &Block) -> Vec<usize>;
5959

6060
/// Returns a usize that changes when the ChainWatchInterface's watched data is modified.
6161
/// 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 {
8686
///
8787
/// This also means those counting confirmations using block_connected callbacks should watch
8888
/// for duplicate headers and not count them towards confirmations!
89-
fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]);
89+
fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]);
9090
/// Notifies a listener that a block was disconnected.
9191
/// Unlike block_connected, this *must* never be called twice for the same disconnect event.
9292
/// 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
280280
let matched_indexes = self.chain_monitor.filter_block(block);
281281
let mut matched_txn = Vec::new();
282282
for index in matched_indexes.iter() {
283-
matched_txn.push(&block.txdata[*index as usize]);
283+
matched_txn.push(&block.txdata[*index]);
284284
}
285285
reentered = self.block_connected_checked(&block.header, height, matched_txn.as_slice(), matched_indexes.as_slice());
286286
}
@@ -292,7 +292,7 @@ impl<'a, CL: Deref<Target = ChainListener + 'a> + 'a, C: Deref> BlockNotifier<'a
292292
/// Returns true if notified listeners registered additional watch data (implying that the
293293
/// block must be re-scanned and this function called again prior to further block_connected
294294
/// calls, see ChainListener::block_connected for more info).
295-
pub fn block_connected_checked(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) -> bool {
295+
pub fn block_connected_checked(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]) -> bool {
296296
let last_seen = self.chain_monitor.reentered();
297297

298298
let listeners = self.listeners.lock().unwrap();
@@ -361,13 +361,13 @@ impl ChainWatchInterface for ChainWatchInterfaceUtil {
361361
Err(ChainError::NotSupported)
362362
}
363363

364-
fn filter_block(&self, block: &Block) -> Vec<u32> {
364+
fn filter_block(&self, block: &Block) -> Vec<usize> {
365365
let mut matched_index = Vec::new();
366366
{
367367
let watched = self.watched.lock().unwrap();
368368
for (index, transaction) in block.txdata.iter().enumerate() {
369369
if self.does_match_tx_unguarded(transaction, &watched) {
370-
matched_index.push(index as u32);
370+
matched_index.push(index);
371371
}
372372
}
373373
}

lightning/src/ln/channel.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3299,7 +3299,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
32993299
///
33003300
/// May return some HTLCs (and their payment_hash) which have timed out and should be failed
33013301
/// back.
3302-
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> {
3302+
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> {
33033303
let mut timed_out_htlcs = Vec::new();
33043304
self.holding_cell_htlc_updates.retain(|htlc_update| {
33053305
match htlc_update {
@@ -3350,6 +3350,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
33503350
}
33513351
}
33523352
}
3353+
if height > 0xff_ff_ff || (*index_in_block) > 0xff_ff_ff {
3354+
panic!("Block was bogus - either height 16 million or had > 16 million transactions");
3355+
}
3356+
assert!(txo_idx <= 0xffff); // txo_idx is a (u16 as usize), so this is just listed here for completeness
33533357
self.funding_tx_confirmations = 1;
33543358
self.short_channel_id = Some(((height as u64) << (5*8)) |
33553359
((*index_in_block as u64) << (2*8)) |

lightning/src/ln/channelmanager.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2963,7 +2963,7 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
29632963
F::Target: FeeEstimator,
29642964
L::Target: Logger,
29652965
{
2966-
fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) {
2966+
fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]) {
29672967
let header_hash = header.bitcoin_hash();
29682968
log_trace!(self.logger, "Block {} at height {} connected with {} txn matched", header_hash, height, txn_matched.len());
29692969
let _ = self.total_consistency_lock.read().unwrap();

lightning/src/ln/channelmonitor.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ impl<Key : Send + cmp::Eq + hash::Hash, ChanSigner: ChannelKeys, T: Deref + Sync
183183
L::Target: Logger,
184184
C::Target: ChainWatchInterface,
185185
{
186-
fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], _indexes_of_txn_matched: &[u32]) {
186+
fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], _indexes_of_txn_matched: &[usize]) {
187187
let block_hash = header.bitcoin_hash();
188188
{
189189
let mut monitors = self.monitors.lock().unwrap();

lightning/src/ln/functional_test_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ pub const CHAN_CONFIRM_DEPTH: u32 = 100;
3939
pub fn confirm_transaction<'a, 'b: 'a>(notifier: &'a chaininterface::BlockNotifierRef<'b, &chaininterface::ChainWatchInterfaceUtil>, chain: &chaininterface::ChainWatchInterfaceUtil, tx: &Transaction, chan_id: u32) {
4040
assert!(chain.does_match_tx(tx));
4141
let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
42-
notifier.block_connected_checked(&header, 1, &[tx; 1], &[chan_id; 1]);
42+
notifier.block_connected_checked(&header, 1, &[tx; 1], &[chan_id as usize; 1]);
4343
for i in 2..CHAN_CONFIRM_DEPTH {
4444
header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
4545
notifier.block_connected_checked(&header, i, &vec![], &[0; 0]);

lightning/src/ln/functional_tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,10 +408,10 @@ fn test_1_conf_open() {
408408
assert!(nodes[1].chain_monitor.does_match_tx(&tx));
409409

410410
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
411-
nodes[1].block_notifier.block_connected_checked(&header, 1, &[&tx; 1], &[tx.version; 1]);
411+
nodes[1].block_notifier.block_connected_checked(&header, 1, &[&tx; 1], &[tx.version as usize; 1]);
412412
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()));
413413

414-
nodes[0].block_notifier.block_connected_checked(&header, 1, &[&tx; 1], &[tx.version; 1]);
414+
nodes[0].block_notifier.block_connected_checked(&header, 1, &[&tx; 1], &[tx.version as usize; 1]);
415415
let (funding_locked, _) = create_chan_between_nodes_with_value_confirm_second(&nodes[1], &nodes[0]);
416416
let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked);
417417

lightning/src/util/test_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ impl ChainWatchInterface for TestChainWatcher {
371371
fn install_watch_tx(&self, _txid: &Txid, _script_pub_key: &Script) { }
372372
fn install_watch_outpoint(&self, _outpoint: (Txid, u32), _out_script: &Script) { }
373373
fn watch_all_txn(&self) { }
374-
fn filter_block<'a>(&self, _block: &'a Block) -> Vec<u32> {
374+
fn filter_block<'a>(&self, _block: &'a Block) -> Vec<usize> {
375375
Vec::new()
376376
}
377377
fn reentered(&self) -> usize { 0 }

0 commit comments

Comments
 (0)