Skip to content

Commit 020ce15

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 5de99d1 commit 020ce15

File tree

7 files changed

+21
-13
lines changed

7 files changed

+21
-13
lines changed

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: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3176,7 +3176,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
31763176
///
31773177
/// May return some HTLCs (and their payment_hash) which have timed out and should be failed
31783178
/// back.
3179-
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> {
3179+
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> {
31803180
let mut timed_out_htlcs = Vec::new();
31813181
self.holding_cell_htlc_updates.retain(|htlc_update| {
31823182
match htlc_update {
@@ -3216,6 +3216,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
32163216
channel_id: self.channel_id(),
32173217
data: "funding tx had wrong script/value".to_owned()
32183218
});
3219+
} else if txo_idx > 0xff_ff {
3220+
return Err(msgs::ErrorMessage {
3221+
channel_id: self.channel_id(),
3222+
data: "funding tx output was > 2**16 which makes short_channel_id impossible to calculate".to_owned()
3223+
});
32193224
} else {
32203225
if self.channel_outbound {
32213226
for input in tx.input.iter() {
@@ -3227,6 +3232,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
32273232
}
32283233
}
32293234
}
3235+
if height > 0xff_ff_ff || (*index_in_block) > 0xff_ff_ff {
3236+
panic!("Block was bogus - either height 16 million or had > 16 million transactions");
3237+
}
32303238
self.funding_tx_confirmations = 1;
32313239
self.short_channel_id = Some(((height as u64) << (5*8)) |
32323240
((*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
@@ -2956,7 +2956,7 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
29562956
F::Target: FeeEstimator,
29572957
L::Target: Logger,
29582958
{
2959-
fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) {
2959+
fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]) {
29602960
let header_hash = header.bitcoin_hash();
29612961
log_trace!(self.logger, "Block {} at height {} connected with {} txn matched", header_hash, height, txn_matched.len());
29622962
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
@@ -41,7 +41,7 @@ pub const CHAN_CONFIRM_DEPTH: u32 = 100;
4141
pub fn confirm_transaction<'a, 'b: 'a>(notifier: &'a chaininterface::BlockNotifierRef<'b, &chaininterface::ChainWatchInterfaceUtil>, chain: &chaininterface::ChainWatchInterfaceUtil, tx: &Transaction, chan_id: u32) {
4242
assert!(chain.does_match_tx(tx));
4343
let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
44-
notifier.block_connected_checked(&header, 1, &[tx; 1], &[chan_id; 1]);
44+
notifier.block_connected_checked(&header, 1, &[tx; 1], &[chan_id as usize; 1]);
4545
for i in 2..CHAN_CONFIRM_DEPTH {
4646
header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
4747
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
@@ -410,10 +410,10 @@ fn test_1_conf_open() {
410410
assert!(nodes[1].chain_monitor.does_match_tx(&tx));
411411

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

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

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)