diff --git a/substrate/core/client/db/src/lib.rs b/substrate/core/client/db/src/lib.rs
index eebd69d88dd2abdbd50f61ff10bcd352584f9b23..e73d3df62c9c0b87a8ff51022d827b52753f2c64 100644
--- a/substrate/core/client/db/src/lib.rs
+++ b/substrate/core/client/db/src/lib.rs
@@ -883,7 +883,7 @@ impl<Block: BlockT<Hash=H256>> Backend<Block> {
transaction,
columns::KEY_LOOKUP,
r.number
- );
+ )?;
}
// canonicalize: set the number lookup to map to this block's hash.
@@ -894,18 +894,18 @@ impl<Block: BlockT<Hash=H256>> Backend<Block> {
columns::KEY_LOOKUP,
e.number,
e.hash
- );
+ )?;
}
}
- let lookup_key = utils::number_and_hash_to_lookup_key(best_to.0, &best_to.1);
+ let lookup_key = utils::number_and_hash_to_lookup_key(best_to.0, &best_to.1)?;
transaction.put(columns::META, meta_keys::BEST_BLOCK, &lookup_key);
utils::insert_number_to_key_mapping(
transaction,
columns::KEY_LOOKUP,
best_to.0,
best_to.1,
- );
+ )?;
Ok((enacted, retracted))
}
@@ -946,7 +946,7 @@ impl<Block: BlockT<Hash=H256>> Backend<Block> {
if let Some(justification) = justification {
transaction.put(
columns::JUSTIFICATION,
- &utils::number_and_hash_to_lookup_key(number, hash),
+ &utils::number_and_hash_to_lookup_key(number, hash)?,
&justification.encode(),
);
}
@@ -1021,7 +1021,7 @@ impl<Block: BlockT<Hash=H256>> Backend<Block> {
let number = pending_block.header.number().clone();
// blocks are keyed by number + hash.
- let lookup_key = utils::number_and_hash_to_lookup_key(number, hash);
+ let lookup_key = utils::number_and_hash_to_lookup_key(number, hash)?;
let (enacted, retracted) = if pending_block.leaf_state.is_best() {
self.set_head_with_transaction(&mut transaction, parent_hash, (number, hash))?
@@ -1034,7 +1034,7 @@ impl<Block: BlockT<Hash=H256>> Backend<Block> {
columns::KEY_LOOKUP,
number,
hash,
- );
+ )?;
transaction.put(columns::HEADER, &lookup_key, &pending_block.header.encode());
if let Some(body) = pending_block.body {
@@ -1177,7 +1177,7 @@ impl<Block: BlockT<Hash=H256>> Backend<Block> {
if self.storage.state_db.best_canonical().map(|c| f_num.saturated_into::<u64>() > c).unwrap_or(true) {
let parent_hash = f_header.parent_hash().clone();
- let lookup_key = utils::number_and_hash_to_lookup_key(f_num, f_hash.clone());
+ let lookup_key = utils::number_and_hash_to_lookup_key(f_num, f_hash.clone())?;
transaction.put(columns::META, meta_keys::FINALIZED_BLOCK, &lookup_key);
let commit = self.storage.state_db.canonicalize_block(&f_hash)
@@ -1344,7 +1344,7 @@ impl<Block> client::backend::Backend<Block, Blake2Hasher> for Backend<Block> whe
let hash = self.blockchain.hash(best)?.ok_or_else(
|| client::error::Error::UnknownBlock(
format!("Error reverting to {}. Block hash not found.", best)))?;
- let key = utils::number_and_hash_to_lookup_key(best.clone(), &hash);
+ let key = utils::number_and_hash_to_lookup_key(best.clone(), &hash)?;
transaction.put(columns::META, meta_keys::BEST_BLOCK, &key);
transaction.delete(columns::KEY_LOOKUP, removed.hash().as_ref());
children::remove_children(&mut transaction, columns::META, meta_keys::CHILDREN_PREFIX, hash);
diff --git a/substrate/core/client/db/src/light.rs b/substrate/core/client/db/src/light.rs
index 7d2f1e62d311f9ad89b942b3ff6e311b2c7f4208..95f50ab6f63171230303318bdc9642681a3aae26 100644
--- a/substrate/core/client/db/src/light.rs
+++ b/substrate/core/client/db/src/light.rs
@@ -211,7 +211,7 @@ impl<Block: BlockT> LightStorage<Block> {
/// should be the parent of `best_to`. In the case where we set an existing block
/// to be best, `route_to` should equal to `best_to`.
fn set_head_with_transaction(&self, transaction: &mut DBTransaction, route_to: Block::Hash, best_to: (NumberFor<Block>, Block::Hash)) -> Result<(), client::error::Error> {
- let lookup_key = utils::number_and_hash_to_lookup_key(best_to.0, &best_to.1);
+ let lookup_key = utils::number_and_hash_to_lookup_key(best_to.0, &best_to.1)?;
// handle reorg.
let meta = self.meta.read();
@@ -234,7 +234,7 @@ impl<Block: BlockT> LightStorage<Block> {
transaction,
columns::KEY_LOOKUP,
retracted.number
- );
+ )?;
}
for enacted in tree_route.enacted() {
@@ -243,7 +243,7 @@ impl<Block: BlockT> LightStorage<Block> {
columns::KEY_LOOKUP,
enacted.number,
enacted.hash
- );
+ )?;
}
}
@@ -253,7 +253,7 @@ impl<Block: BlockT> LightStorage<Block> {
columns::KEY_LOOKUP,
best_to.0,
best_to.1,
- );
+ )?;
Ok(())
}
@@ -274,7 +274,7 @@ impl<Block: BlockT> LightStorage<Block> {
).into())
}
- let lookup_key = utils::number_and_hash_to_lookup_key(header.number().clone(), hash);
+ let lookup_key = utils::number_and_hash_to_lookup_key(header.number().clone(), hash)?;
transaction.put(columns::META, meta_keys::FINALIZED_BLOCK, &lookup_key);
// build new CHT(s) if required
@@ -293,7 +293,7 @@ impl<Block: BlockT> LightStorage<Block> {
)?;
transaction.put(
columns::CHT,
- &cht_key(HEADER_CHT_PREFIX, new_cht_start),
+ &cht_key(HEADER_CHT_PREFIX, new_cht_start)?,
new_header_cht_root.as_ref()
);
@@ -311,7 +311,7 @@ impl<Block: BlockT> LightStorage<Block> {
)?;
transaction.put(
columns::CHT,
- &cht_key(CHANGES_TRIE_CHT_PREFIX, new_cht_start),
+ &cht_key(CHANGES_TRIE_CHT_PREFIX, new_cht_start)?,
new_changes_trie_cht_root.as_ref()
);
}
@@ -331,7 +331,7 @@ impl<Block: BlockT> LightStorage<Block> {
columns::KEY_LOOKUP,
prune_block,
hash
- );
+ )?;
transaction.delete(columns::HEADER, &lookup_key);
}
prune_block += One::one();
@@ -358,7 +358,7 @@ impl<Block: BlockT> LightStorage<Block> {
let cht_number = cht::block_to_cht_number(cht_size, block).ok_or_else(no_cht_for_block)?;
let cht_start = cht::start_number(cht_size, cht_number);
- self.db.get(columns::CHT, &cht_key(cht_type, cht_start)).map_err(db_err)?
+ self.db.get(columns::CHT, &cht_key(cht_type, cht_start)?).map_err(db_err)?
.ok_or_else(no_cht_for_block)
.and_then(|hash| Block::Hash::decode(&mut &*hash).ok_or_else(no_cht_for_block))
}
@@ -414,7 +414,7 @@ impl<Block> LightBlockchainStorage<Block> for LightStorage<Block>
}
// blocks are keyed by number + hash.
- let lookup_key = utils::number_and_hash_to_lookup_key(number, &hash);
+ let lookup_key = utils::number_and_hash_to_lookup_key(number, &hash)?;
if leaf_state.is_best() {
self.set_head_with_transaction(&mut transaction, parent_hash, (number, hash))?;
@@ -425,7 +425,7 @@ impl<Block> LightBlockchainStorage<Block> for LightStorage<Block>
columns::KEY_LOOKUP,
number,
hash,
- );
+ )?;
transaction.put(columns::HEADER, &lookup_key, &header.encode());
let is_genesis = number.is_zero();
@@ -562,10 +562,10 @@ impl<Block> LightBlockchainStorage<Block> for LightStorage<Block>
}
/// Build the key for inserting header-CHT at given block.
-fn cht_key<N: TryInto<u32>>(cht_type: u8, block: N) -> [u8; 5] {
+fn cht_key<N: TryInto<u32>>(cht_type: u8, block: N) -> ClientResult<[u8; 5]> {
let mut key = [cht_type; 5];
- key[1..].copy_from_slice(&utils::number_index_key(block));
- key
+ key[1..].copy_from_slice(&utils::number_index_key(block)?);
+ Ok(key)
}
#[cfg(test)]
diff --git a/substrate/core/client/db/src/utils.rs b/substrate/core/client/db/src/utils.rs
index 39862dba8575090a8d03bd2ac71ca8b8da004bcd..e0e36c6dbce19ec6c1d5b4a21ec5badd2eaa0e6d 100644
--- a/substrate/core/client/db/src/utils.rs
+++ b/substrate/core/client/db/src/utils.rs
@@ -31,8 +31,8 @@ use parity_codec::Decode;
use trie::DBValue;
use runtime_primitives::generic::BlockId;
use runtime_primitives::traits::{
- Block as BlockT, Header as HeaderT, Zero, UniqueSaturatedFrom,
- UniqueSaturatedInto, CheckedConversion
+ Block as BlockT, Header as HeaderT, Zero,
+ UniqueSaturatedFrom, UniqueSaturatedInto,
};
#[cfg(feature = "kvdb-rocksdb")]
use crate::DatabaseSettings;
@@ -84,25 +84,31 @@ pub type NumberIndexKey = [u8; 4];
///
/// In the current database schema, this kind of key is only used for
/// lookups into an index, NOT for storing header data or others.
-pub fn number_index_key<N: TryInto<u32>>(n: N) -> NumberIndexKey {
- let n = n.checked_into::<u32>().unwrap();
- [
+pub fn number_index_key<N: TryInto<u32>>(n: N) -> client::error::Result<NumberIndexKey> {
+ let n = n.try_into().map_err(|_|
+ client::error::Error::Backend("Block number cannot be converted to u32".into())
+ )?;
+
+ Ok([
(n >> 24) as u8,
((n >> 16) & 0xff) as u8,
((n >> 8) & 0xff) as u8,
(n & 0xff) as u8
- ]
+ ])
}
/// Convert number and hash into long lookup key for blocks that are
/// not in the canonical chain.
-pub fn number_and_hash_to_lookup_key<N, H>(number: N, hash: H) -> Vec<u8> where
+pub fn number_and_hash_to_lookup_key<N, H>(
+ number: N,
+ hash: H,
+) -> client::error::Result<Vec<u8>> where
N: TryInto<u32>,
- H: AsRef<[u8]>
+ H: AsRef<[u8]>,
{
- let mut lookup_key = number_index_key(number).to_vec();
+ let mut lookup_key = number_index_key(number)?.to_vec();
lookup_key.extend_from_slice(hash.as_ref());
- lookup_key
+ Ok(lookup_key)
}
/// Convert block lookup key into block number.
@@ -124,8 +130,9 @@ pub fn remove_number_to_key_mapping<N: TryInto<u32>>(
transaction: &mut DBTransaction,
key_lookup_col: Option<u32>,
number: N,
-) {
- transaction.delete(key_lookup_col, number_index_key(number).as_ref())
+) -> client::error::Result<()> {
+ transaction.delete(key_lookup_col, number_index_key(number)?.as_ref());
+ Ok(())
}
/// Remove key mappings.
@@ -134,9 +141,10 @@ pub fn remove_key_mappings<N: TryInto<u32>, H: AsRef<[u8]>>(
key_lookup_col: Option<u32>,
number: N,
hash: H,
-) {
- remove_number_to_key_mapping(transaction, key_lookup_col, number);
+) -> client::error::Result<()> {
+ remove_number_to_key_mapping(transaction, key_lookup_col, number)?;
transaction.delete(key_lookup_col, hash.as_ref());
+ Ok(())
}
/// Place a number mapping into the database. This maps number to current perceived
@@ -146,12 +154,13 @@ pub fn insert_number_to_key_mapping<N: TryInto<u32> + Clone, H: AsRef<[u8]>>(
key_lookup_col: Option<u32>,
number: N,
hash: H,
-) {
+) -> client::error::Result<()> {
transaction.put_vec(
key_lookup_col,
- number_index_key(number.clone()).as_ref(),
- number_and_hash_to_lookup_key(number, hash),
- )
+ number_index_key(number.clone())?.as_ref(),
+ number_and_hash_to_lookup_key(number, hash)?,
+ );
+ Ok(())
}
/// Insert a hash to key mapping in the database.
@@ -160,12 +169,13 @@ pub fn insert_hash_to_key_mapping<N: TryInto<u32>, H: AsRef<[u8]> + Clone>(
key_lookup_col: Option<u32>,
number: N,
hash: H,
-) {
+) -> client::error::Result<()> {
transaction.put_vec(
key_lookup_col,
hash.clone().as_ref(),
- number_and_hash_to_lookup_key(number, hash),
- )
+ number_and_hash_to_lookup_key(number, hash)?,
+ );
+ Ok(())
}
/// Convert block id to block lookup key.
@@ -182,7 +192,7 @@ pub fn block_id_to_lookup_key<Block>(
let res = match id {
BlockId::Number(n) => db.get(
key_lookup_col,
- number_index_key(n).as_ref(),
+ number_index_key(n)?.as_ref(),
),
BlockId::Hash(h) => db.get(key_lookup_col, h.as_ref()),
};
@@ -318,3 +328,19 @@ pub fn read_meta<Block>(db: &dyn KeyValueDB, col_meta: Option<u32>, col_header:
genesis_hash,
})
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use runtime_primitives::testing::{Block as RawBlock, ExtrinsicWrapper};
+ type Block = RawBlock<ExtrinsicWrapper<u32>>;
+
+ #[test]
+ fn number_index_key_doesnt_panic() {
+ let id = BlockId::<Block>::Number(72340207214430721);
+ match id {
+ BlockId::Number(n) => number_index_key(n).expect_err("number should overflow u32"),
+ _ => unreachable!(),
+ };
+ }
+}
diff --git a/substrate/core/client/src/client.rs b/substrate/core/client/src/client.rs
index 5122cbac032624b7ada891e8a9f975130afee77f..9241d8fb0a418c1f8b95300990eb0161c4222b41 100644
--- a/substrate/core/client/src/client.rs
+++ b/substrate/core/client/src/client.rs
@@ -2633,4 +2633,14 @@ pub(crate) mod tests {
b3.hash(),
);
}
+
+ #[test]
+ fn get_header_by_block_number_doesnt_panic() {
+ let client = test_client::new();
+
+ // backend uses u32 for block numbers, make sure we don't panic when
+ // trying to convert
+ let id = BlockId::<Block>::Number(72340207214430721);
+ client.header(&id).expect_err("invalid block number overflows u32");
+ }
}