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

Represent failure with index instead of is_error in data lookup. #88

Merged
merged 1 commit into from
Apr 22, 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
27 changes: 16 additions & 11 deletions core/src/data_lookup/compact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ where
}
}

// If .size is equal to u32::MAX then the no commitment was generated
// If .size is 0, and index contains items then no commitment was generated
// because of an error that occurred.
//
// This is just a temporary solution that will be replaced by a more
Expand All @@ -56,17 +56,22 @@ impl CompactDataLookup {
Self { size, index }
}

pub fn is_error(&self) -> bool {
// For backward compatibility, case when size is u32::MAX is also supported
self.size == u32::MAX || (self.size == 0 && !self.index.is_empty())
}

// Data lookup is not valid if size is 0 and lookup index is not empty
fn new_error() -> Self {
Self {
size: 0,
index: [DataLookupItem::new(AppId(0), 0)].to_vec(),
}
}

pub fn from_data_lookup(lookup: &DataLookup) -> Self {
if lookup.is_error {
// Data lookup is not valid if size is 0 and lookup index is not empty
return CompactDataLookup {
size: 0,
index: [DataLookupItem {
app_id: AppId(0),
start: 0,
}]
.to_vec(),
};
if lookup.is_error() {
return Self::new_error();
}

let index = lookup
Expand Down
25 changes: 9 additions & 16 deletions core/src/data_lookup/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
)]
pub struct DataLookup {
pub(crate) index: Vec<(AppId, DataLookupRange)>,
pub(crate) is_error: bool,
}

impl DataLookup {
Expand All @@ -45,6 +44,10 @@
self.len() == 0
}

pub fn is_error(&self) -> bool {
self.is_empty() && !self.index.is_empty()
}

pub fn range_of(&self, app_id: AppId) -> Option<DataLookupRange> {
self.index
.iter()
Expand Down Expand Up @@ -113,25 +116,18 @@
})
.collect::<Result<_, _>>()?;

Ok(Self {
index,
is_error: false,
})
Ok(Self { index })
}

/// This function is used a block contains no data submissions.
pub fn new_empty() -> Self {
Self {
index: Vec::new(),
is_error: false,
}
Self { index: Vec::new() }
}

/// This function is only used when something has gone wrong during header extension building
pub fn new_error() -> Self {
Self {
index: Vec::new(),
is_error: true,
index: vec![(AppId(0), 0..0)],
}
}
}
Expand All @@ -140,7 +136,7 @@
type Error = Error;

fn try_from(compacted: CompactDataLookup) -> Result<Self, Self::Error> {
if compacted.size == u32::MAX {
if compacted.is_error() {
return Ok(DataLookup::new_error());
}

Expand All @@ -165,10 +161,7 @@
index.push((prev_id, offset..compacted.size));
}

let lookup = DataLookup {
index,
is_error: false,
};
let lookup = DataLookup { index };
ensure!(lookup.len() == compacted.size, Error::DataNotSorted);

Ok(lookup)
Expand All @@ -181,7 +174,7 @@
impl Encode for DataLookup {
/// Encodes as a `compact::DataLookup`.
fn encode(&self) -> Vec<u8> {
let compacted: CompactDataLookup = CompactDataLookup::from_data_lookup(&self);

Check warning on line 177 in core/src/data_lookup/mod.rs

View workflow job for this annotation

GitHub Actions / build_and_test

this expression creates a reference which is immediately dereferenced by the compiler
compacted.encode()
}
}
Expand Down
Loading