Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update translate to use defensive! #2985

Merged
merged 15 commits into from
Jan 19, 2024
Merged
88 changes: 77 additions & 11 deletions substrate/frame/support/src/storage/generator/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,10 @@ where
let value = match unhashed::get::<O>(&current_key) {
Some(value) => value,
None => {
log::error!("Invalid translate: fail to decode old value");
crate::defensive!(
"Invalid translation: failed to decode old value for key",
array_bytes::bytes2hex("0x", &current_key)
);
return Some(current_key)
},
};
Expand All @@ -205,7 +208,10 @@ where
let key = match K::decode(&mut key_material) {
Ok(key) => key,
Err(_) => {
log::error!("Invalid translate: fail to decode key");
crate::defensive!(
"Invalid translation: failed to decode key",
array_bytes::bytes2hex("0x", &current_key)
);
return Some(current_key)
},
};
Expand Down Expand Up @@ -389,6 +395,75 @@ mod test_iterators {
});
}

#[cfg(debug_assertions)]
#[test]
#[should_panic]
fn map_translate_with_bad_key_in_debug_mode() {
sp_io::TestExternalities::default().execute_with(|| {
type Map = self::frame_system::Map<Runtime>;
let prefix = Map::prefix_hash().to_vec();

// Wrong key
unhashed::put(&[prefix.clone(), vec![1, 2, 3]].concat(), &3u64.encode());

// debug_assert should cause a
Map::translate(|_k1, v: u64| Some(v * 2));
assert_eq!(Map::iter().collect::<Vec<_>>(), vec![(3, 6), (0, 0), (2, 4), (1, 2)]);
})
}

#[cfg(debug_assertions)]
#[test]
#[should_panic]
fn map_translate_with_bad_value_in_debug_mode() {
sp_io::TestExternalities::default().execute_with(|| {
type Map = self::frame_system::Map<Runtime>;
let prefix = Map::prefix_hash().to_vec();

// Wrong value
unhashed::put(
&[prefix.clone(), crate::Blake2_128Concat::hash(&6u16.encode())].concat(),
&vec![1],
);

Map::translate(|_k1, v: u64| Some(v * 2));
assert_eq!(Map::iter().collect::<Vec<_>>(), vec![(3, 6), (0, 0), (2, 4), (1, 2)]);
})
}

#[cfg(not(debug_assertions))]
#[test]
fn map_translate_with_bad_key_in_production_mode() {
sp_io::TestExternalities::default().execute_with(|| {
type Map = self::frame_system::Map<Runtime>;
let prefix = Map::prefix_hash().to_vec();

// Wrong key
unhashed::put(&[prefix.clone(), vec![1, 2, 3]].concat(), &3u64.encode());

Map::translate(|_k1, v: u64| Some(v * 2));
assert_eq!(Map::iter().collect::<Vec<_>>(), vec![]);
})
}

#[cfg(not(debug_assertions))]
#[test]
fn map_translate_with_bad_value_in_production_mode() {
sp_io::TestExternalities::default().execute_with(|| {
type Map = self::frame_system::Map<Runtime>;
let prefix = Map::prefix_hash().to_vec();

// Wrong value
unhashed::put(
&[prefix.clone(), crate::Blake2_128Concat::hash(&6u16.encode())].concat(),
&vec![1],
);

Map::translate(|_k1, v: u64| Some(v * 2));
assert_eq!(Map::iter().collect::<Vec<_>>(), vec![]);
})
}

#[test]
fn map_reversible_reversible_iteration() {
sp_io::TestExternalities::default().execute_with(|| {
Expand Down Expand Up @@ -425,15 +500,6 @@ mod test_iterators {
Map::insert(i as u16, i as u64);
}

// Wrong key
unhashed::put(&[prefix.clone(), vec![1, 2, 3]].concat(), &3u64.encode());
muharem marked this conversation as resolved.
Show resolved Hide resolved

// Wrong value
unhashed::put(
&[prefix.clone(), crate::Blake2_128Concat::hash(&6u16.encode())].concat(),
&vec![1],
);

Map::translate(|_k1, v: u64| Some(v * 2));
assert_eq!(Map::iter().collect::<Vec<_>>(), vec![(3, 6), (0, 0), (2, 4), (1, 2)]);
})
Expand Down
3 changes: 2 additions & 1 deletion substrate/frame/support/src/storage/types/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,8 @@ where
///
/// By returning `None` from `f` for an element, you'll remove it from the map.
///
/// NOTE: If a value fail to decode because storage is corrupted then it is skipped.
/// NOTE: If a value fails to decode because storage is corrupted, then it will log an error and
/// be skipped in production, or panic in development.
pub fn translate<O: Decode, F: FnMut(Key, O) -> Option<Value>>(f: F) {
<Self as crate::storage::IterableStorageMap<Key, Value>>::translate(f)
}
Expand Down
Loading