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

fix: remove unwrap in storage #100

Merged
merged 3 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ git # Deoxys Changelog

## Next release

- fix: sync, remove `unwrap` in storage
- fix(classes): Fixed classes on the RPC level by adding ordering and complete deserialisation
- fix: class update
- feat: store key/value in `--disble-root` mode
Expand Down
8 changes: 4 additions & 4 deletions crates/client/db/src/storage_handler/block_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@ impl BlockHashView {
pub fn insert(&mut self, block_number: u64, block_hash: &Felt252Wrapper) -> Result<(), DeoxysStorageError> {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::BlockNumberToHash);
db.put_cf(&column, bincode::serialize(&block_number).unwrap(), bincode::serialize(&block_hash).unwrap())
db.put_cf(&column, bincode::serialize(&block_number)?, bincode::serialize(&block_hash)?)
.map_err(|_| DeoxysStorageError::StorageInsertionError(StorageType::BlockHash))
}

pub fn get(&self, block_number: u64) -> Result<Option<Felt252Wrapper>, DeoxysStorageError> {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::BlockNumberToHash);
let block_hash = db
.get_cf(&column, bincode::serialize(&block_number).unwrap())
.get_cf(&column, bincode::serialize(&block_number)?)
.map_err(|_| DeoxysStorageError::StorageRetrievalError(StorageType::BlockHash))?
.map(|bytes| bincode::deserialize::<Felt252Wrapper>(&bytes[..]));

match block_hash {
Some(Ok(block_hash)) => Ok(Some(block_hash)),
Some(Err(_)) => Err(DeoxysStorageError::StorageDecodeError(StorageType::BlockHash)),
Some(Err(_)) => Err(DeoxysStorageError::StorageSerdeError),
None => Ok(None),
}
}
Expand All @@ -32,7 +32,7 @@ impl BlockHashView {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::BlockNumberToHash);

match db.key_may_exist_cf(&column, bincode::serialize(&block_number).unwrap()) {
match db.key_may_exist_cf(&column, bincode::serialize(&block_number)?) {
true => Ok(self.get(block_number)?.is_some()),
false => Ok(false),
}
Expand Down
6 changes: 3 additions & 3 deletions crates/client/db/src/storage_handler/block_number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ impl BlockNumberView {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::BlockHashToNumber);

db.put_cf(&column, bincode::serialize(&block_hash).unwrap(), bincode::serialize(&block_number).unwrap())
db.put_cf(&column, bincode::serialize(&block_hash)?, bincode::serialize(&block_number)?)
.map_err(|_| DeoxysStorageError::StorageInsertionError(StorageType::BlockNumber))
}
pub fn get(&self, block_hash: &Felt252Wrapper) -> Result<Option<u64>, DeoxysStorageError> {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::BlockHashToNumber);
let block_number = db
.get_cf(&column, bincode::serialize(&block_hash).unwrap())
.get_cf(&column, bincode::serialize(&block_hash)?)
.map_err(|_| DeoxysStorageError::StorageRetrievalError(StorageType::BlockNumber))?
.map(|bytes| bincode::deserialize::<u64>(&bytes[..]));

Expand All @@ -32,7 +32,7 @@ impl BlockNumberView {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::BlockHashToNumber);

match db.key_may_exist_cf(&column, bincode::serialize(&block_hash).unwrap()) {
match db.key_may_exist_cf(&column, bincode::serialize(&block_hash)?) {
true => Ok(self.get(block_hash)?.is_some()),
false => Ok(false),
}
Expand Down
6 changes: 3 additions & 3 deletions crates/client/db/src/storage_handler/block_state_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ impl BlockStateDiffView {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::BlockStateDiff);

db.put_cf(&column, bincode::serialize(&block_number).unwrap(), bincode::serialize(&state_diff).unwrap())
db.put_cf(&column, bincode::serialize(&block_number)?, bincode::serialize(&state_diff)?)
.map_err(|_| DeoxysStorageError::StorageInsertionError(StorageType::BlockStateDiff))
}

Expand All @@ -19,7 +19,7 @@ impl BlockStateDiffView {
let column = db.get_column(Column::BlockStateDiff);

let state_diff = db
.get_cf(&column, bincode::serialize(&block_number).unwrap())
.get_cf(&column, bincode::serialize(&block_number)?)
.map_err(|_| DeoxysStorageError::StorageRetrievalError(StorageType::BlockStateDiff))?
.map(|bytes| bincode::deserialize::<StateDiff>(&bytes[..]));

Expand All @@ -34,7 +34,7 @@ impl BlockStateDiffView {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::BlockStateDiff);

match db.key_may_exist_cf(&column, bincode::serialize(&block_number).unwrap()) {
match db.key_may_exist_cf(&column, bincode::serialize(&block_number)?) {
true => Ok(self.get(block_number)?.is_some()),
false => Ok(false),
}
Expand Down
8 changes: 4 additions & 4 deletions crates/client/db/src/storage_handler/contract_class_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ impl StorageView for ContractClassDataView {
let column = db.get_column(Column::ContractClassData);

let contract_class_data = db
.get_cf(&column, bincode::serialize(&class_hash).unwrap())
.get_cf(&column, bincode::serialize(&class_hash)?)
.map_err(|_| DeoxysStorageError::StorageRetrievalError(StorageType::ContractClassData))?
.map(|bytes| StorageContractClassData::decode(&mut &bytes[..]));

match contract_class_data {
Some(Ok(contract_class_data)) => Ok(Some(contract_class_data)),
Some(Err(_)) => Err(DeoxysStorageError::StorageDecodeError(StorageType::Class)),
Some(Err(_)) => Err(DeoxysStorageError::StorageSerdeError),
None => Ok(None),
}
}
Expand All @@ -35,7 +35,7 @@ impl StorageView for ContractClassDataView {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::ContractClassData);

match db.key_may_exist_cf(&column, bincode::serialize(&class_hash).unwrap()) {
match db.key_may_exist_cf(&column, bincode::serialize(&class_hash)?) {
true => Ok(self.get(class_hash)?.is_some()),
false => Ok(false),
}
Expand All @@ -57,7 +57,7 @@ impl StorageViewMut for ContractClassDataViewMut {

let mut batch = WriteBatchWithTransaction::<true>::default();
for (key, value) in self.0.into_iter() {
batch.put_cf(&column, bincode::serialize(&key).unwrap(), value.encode());
batch.put_cf(&column, bincode::serialize(&key)?, value.encode());
}
db.write(batch).map_err(|_| DeoxysStorageError::StorageCommitError(StorageType::ContractClassData))
}
Expand Down
6 changes: 3 additions & 3 deletions crates/client/db/src/storage_handler/contract_class_hashes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ impl StorageView for ContractClassHashesView {
let column = db.get_column(Column::ContractClassHashes);

let compiled_class_hash = db
.get_cf(&column, bincode::serialize(&class_hash).unwrap())
.get_cf(&column, bincode::serialize(&class_hash)?)
.map_err(|_| DeoxysStorageError::StorageRetrievalError(StorageType::ContractClassHashes))?
.map(|bytes| bincode::deserialize::<CompiledClassHash>(&bytes[..]));

Expand All @@ -33,7 +33,7 @@ impl StorageView for ContractClassHashesView {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::ContractClassHashes);

match db.key_may_exist_cf(&column, bincode::serialize(&class_hash).unwrap()) {
match db.key_may_exist_cf(&column, bincode::serialize(&class_hash)?) {
true => Ok(self.get(class_hash)?.is_some()),
false => Ok(false),
}
Expand All @@ -55,7 +55,7 @@ impl StorageViewMut for ContractClassHashesViewMut {

let mut batch = WriteBatchWithTransaction::<true>::default();
for (key, value) in self.0.into_iter() {
batch.put_cf(&column, bincode::serialize(&key).unwrap(), bincode::serialize(&value).unwrap());
batch.put_cf(&column, bincode::serialize(&key)?, bincode::serialize(&value)?);
}
db.write(batch).map_err(|_| DeoxysStorageError::StorageCommitError(StorageType::ContractClassHashes))
}
Expand Down
23 changes: 11 additions & 12 deletions crates/client/db/src/storage_handler/contract_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ impl StorageView for ContractDataView {
let column = db.get_column(Column::ContractData);

match db
.get_cf(&column, bincode::serialize(&contract_address).unwrap())
.get_cf(&column, bincode::serialize(&contract_address)?)
.map_err(|_| DeoxysStorageError::StorageRetrievalError(StorageType::ContractData))?
{
Some(bytes) => Ok(Some(
Expand All @@ -36,7 +36,7 @@ impl StorageView for ContractDataView {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::ContractData);

match db.key_may_exist_cf(&column, bincode::serialize(&contract_address).unwrap()) {
match db.key_may_exist_cf(&column, bincode::serialize(&contract_address)?) {
true => Ok(self.get(contract_address)?.is_some()),
false => Ok(false),
}
Expand All @@ -63,11 +63,10 @@ impl ContractDataView {
let column = db.get_column(Column::ContractData);

let contract_data = match db
.get_cf(&column, bincode::serialize(&contract_address).unwrap())
.get_cf(&column, bincode::serialize(&contract_address)?)
.map_err(|_| DeoxysStorageError::StorageRetrievalError(StorageType::ContractData))?
{
Some(bytes) => bincode::deserialize::<StorageContractData>(&bytes[..])
.map_err(|_| DeoxysStorageError::StorageDecodeError(StorageType::ContractData))?,
Some(bytes) => bincode::deserialize::<StorageContractData>(&bytes[..])?,
None => StorageContractData::default(),
};

Expand All @@ -83,7 +82,7 @@ impl ContractDataView {
let column = db.get_column(Column::ContractData);

let contract_data = match db
.get_cf(&column, bincode::serialize(&contract_address).unwrap())
.get_cf(&column, bincode::serialize(&contract_address)?)
.map_err(|_| DeoxysStorageError::StorageRetrievalError(StorageType::ContractData))?
{
Some(bytes) => bincode::deserialize::<StorageContractData>(&bytes[..])
Expand Down Expand Up @@ -137,14 +136,14 @@ impl StorageViewMut for ContractDataViewMut {
let mut batch = WriteBatchWithTransaction::<true>::default();
for (key, mut contract_data, (class_hash, nonce)) in izip!(keys, histories, values) {
if let Some(class_hash) = class_hash {
contract_data.class_hash.push(block_number, class_hash).unwrap();
contract_data.class_hash.push(block_number, class_hash)?;
}

if let Some(nonce) = nonce {
contract_data.nonce.push(block_number, nonce).unwrap();
contract_data.nonce.push(block_number, nonce)?;
}

batch.put_cf(&column, bincode::serialize(&key).unwrap(), bincode::serialize(&contract_data).unwrap());
batch.put_cf(&column, bincode::serialize(&key)?, bincode::serialize(&contract_data)?);
}
db.write(batch).map_err(|_| DeoxysStorageError::StorageCommitError(StorageType::ContractData))
}
Expand All @@ -159,7 +158,7 @@ impl StorageView for ContractDataViewMut {
let column = db.get_column(Column::ContractData);

match db
.get_cf(&column, bincode::serialize(&contract_address).unwrap())
.get_cf(&column, bincode::serialize(&contract_address)?)
.map_err(|_| DeoxysStorageError::StorageRetrievalError(StorageType::ContractData))?
{
Some(bytes) => Ok(Some(
Expand All @@ -174,7 +173,7 @@ impl StorageView for ContractDataViewMut {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::ContractData);

match db.key_may_exist_cf(&column, bincode::serialize(&contract_address).unwrap()) {
match db.key_may_exist_cf(&column, bincode::serialize(&contract_address)?) {
true => Ok(self.get(contract_address)?.is_some()),
false => Ok(false),
}
Expand Down Expand Up @@ -203,7 +202,7 @@ impl StorageViewRevetible for ContractDataViewMut {
.delete(key)
.map_err(|_| DeoxysStorageError::StorageRevertError(StorageType::ContractData, block_number))?,
_ => db
.put_cf(&column, key, bincode::serialize(&contract_data).unwrap())
.put_cf(&column, key, bincode::serialize(&contract_data)?)
.map_err(|_| DeoxysStorageError::StorageRevertError(StorageType::ContractData, block_number))?,
}
}
Expand Down
20 changes: 10 additions & 10 deletions crates/client/db/src/storage_handler/contract_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ impl StorageViewMut for ContractStorageViewMut {

let mut batch = WriteBatchWithTransaction::<true>::default();
for (key, mut history, value) in izip!(keys, histories, values) {
history.push(block_number, value).unwrap();
batch.put_cf(&column, bincode::serialize(&key).unwrap(), bincode::serialize(&history).unwrap());
history.push(block_number, value)?;
batch.put_cf(&column, bincode::serialize(&key)?, bincode::serialize(&history)?);
}
db.write(batch).map_err(|_| DeoxysStorageError::StorageCommitError(StorageType::ContractStorage))
}
Expand All @@ -79,7 +79,7 @@ impl StorageView for ContractStorageViewMut {
let column = db.get_column(Column::ContractStorage);

let history: History<StarkFelt> = match db
.get_cf(&column, bincode::serialize(key).unwrap())
.get_cf(&column, bincode::serialize(key)?)
.map_err(|_| DeoxysStorageError::StorageRetrievalError(StorageType::ContractStorage))?
.map(|bytes| bincode::deserialize(&bytes))
{
Expand All @@ -95,7 +95,7 @@ impl StorageView for ContractStorageViewMut {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::ContractStorage);

match db.key_may_exist_cf(&column, bincode::serialize(&key).unwrap()) {
match db.key_may_exist_cf(&column, bincode::serialize(&key)?) {
true => Ok(self.get(key)?.is_some()),
false => Ok(false),
}
Expand All @@ -108,7 +108,7 @@ impl StorageViewRevetible for ContractStorageViewMut {
let db = Arc::new(DeoxysBackend::expose_db());

let mut read_options = ReadOptions::default();
read_options.set_iterate_lower_bound(bincode::serialize(&block_number).unwrap());
read_options.set_iterate_lower_bound(bincode::serialize(&block_number)?);

// Currently we only use this iterator to retrieve the latest block number. A better way to
// do this would be to decouple the highest block lazy_static in `l2.rs`, but in the
Expand Down Expand Up @@ -152,7 +152,7 @@ impl StorageViewRevetible for ContractStorageViewMut {
let change_set = Arc::try_unwrap(change_set).expect("Arc should not be aliased");
for key in change_set.into_iter() {
let db = Arc::clone(&db);
let key = bincode::serialize(&key).unwrap();
let key = bincode::serialize(&key)?;

set.spawn(async move {
let column = db.get_column(Column::ContractStorage);
Expand All @@ -178,7 +178,7 @@ impl StorageViewRevetible for ContractStorageViewMut {
}
}
false => {
if db.put_cf(&column, key.clone(), bincode::serialize(&history).unwrap()).is_err() {
if db.put_cf(&column, key.clone(), bincode::serialize(&history)?).is_err() {
return Err(DeoxysStorageError::StorageRevertError(
StorageType::ContractStorage,
block_number,
Expand Down Expand Up @@ -209,7 +209,7 @@ impl ContractStorageView {
let column = db.get_column(Column::ContractStorage);

let history: History<StarkFelt> = match db
.get_cf(&column, bincode::serialize(key).unwrap())
.get_cf(&column, bincode::serialize(key)?)
.map_err(|_| DeoxysStorageError::StorageRetrievalError(StorageType::ContractStorage))?
.map(|bytes| bincode::deserialize(&bytes))
{
Expand All @@ -231,7 +231,7 @@ impl StorageView for ContractStorageView {
let column = db.get_column(Column::ContractStorage);

let history: History<StarkFelt> = match db
.get_cf(&column, bincode::serialize(key).unwrap())
.get_cf(&column, bincode::serialize(key)?)
.map_err(|_| DeoxysStorageError::StorageRetrievalError(StorageType::ContractStorage))?
.map(|bytes| bincode::deserialize(&bytes))
{
Expand All @@ -247,7 +247,7 @@ impl StorageView for ContractStorageView {
let db = DeoxysBackend::expose_db();
let column = db.get_column(Column::ContractStorage);

match db.key_may_exist_cf(&column, bincode::serialize(&key).unwrap()) {
match db.key_may_exist_cf(&column, bincode::serialize(&key)?) {
true => Ok(self.get(key)?.is_some()),
false => Ok(false),
}
Expand Down
9 changes: 7 additions & 2 deletions crates/client/db/src/storage_handler/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ use serde::{Deserialize, Serialize};
#[serde(bound = "T: Serialize + DeserializeOwned")]
pub struct History<T>(pub Vec<(u64, T)>);

#[derive(Debug)]
pub enum HistoryError {
ValueNotOrdered,
}

/// A simple history implementation that stores values at a given index.
/// It allows to get the value at a given index, push a new value with an index,
/// and revert the history to a given index.
Expand All @@ -17,9 +22,9 @@ where
{
/// Push a new value with an index.
/// If the index is smaller or equal to the last index, it will return an error.
pub fn push(&mut self, index: u64, value: T) -> Result<(), ()> {
pub fn push(&mut self, index: u64, value: T) -> Result<(), HistoryError> {
match self.0.last() {
Some((last_index, _)) if index <= *last_index => Err(()),
Some((last_index, _)) if index <= *last_index => Err(HistoryError::ValueNotOrdered),
_ => {
self.0.push((index, value));
Ok(())
Expand Down
16 changes: 16 additions & 0 deletions crates/client/db/src/storage_handler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,24 @@ pub enum DeoxysStorageError {
StorageEncodeError(StorageType),
#[error("failed to decode {0}")]
StorageDecodeError(StorageType),
#[error("failed to serialize/deserialize")]
StorageSerdeError,
#[error("failed to revert {0} to block {1}")]
StorageRevertError(StorageType, u64),
#[error("failed to push new value in history")]
StorageHistoryError,
}

impl From<bincode::Error> for DeoxysStorageError {
fn from(_: bincode::Error) -> Self {
DeoxysStorageError::StorageSerdeError
}
}

impl From<history::HistoryError> for DeoxysStorageError {
fn from(_: history::HistoryError) -> Self {
DeoxysStorageError::StorageHistoryError
}
}

#[derive(Debug)]
Expand Down