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

add empty / error header constructor, put u32::MAX as size for compac… #84

Merged
merged 6 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
9 changes: 8 additions & 1 deletion core/src/data_lookup/compact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ impl CompactDataLookup {

impl From<DataLookup> for CompactDataLookup {
fn from(lookup: DataLookup) -> Self {
CompactDataLookup::from_expanded(&lookup)
if lookup.is_error {
CompactDataLookup {
size: u32::MAX,
index: Vec::default(),
}
} else {
CompactDataLookup::from_expanded(&lookup)
}
}
}
23 changes: 20 additions & 3 deletions core/src/data_lookup/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub enum Error {
)]
pub struct DataLookup {
pub(crate) index: Vec<(AppId, DataLookupRange)>,
pub(crate) is_error: bool,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about making DataLookup optional in the header instead of adding is_error?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or even Result?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not in favour of doing drastic changes again, any thoughts @markopoloparadox ?

Copy link
Contributor

@markopoloparadox markopoloparadox Apr 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in the previous call, we cannot touch CompactDataLookup for the time being. Making DataLookup an Option or Result will not yield the desired outcome since we need that struct to be valid in order to decode it into CompactDataLookup.

IE it easier to add a flag and create a corresponding CompactDataLookup out of it then to write special functions to handle Option<DataLookup> into CompactDataLookup conversion and vice versa

}

impl DataLookup {
Expand Down Expand Up @@ -112,12 +113,25 @@ impl DataLookup {
})
.collect::<Result<_, _>>()?;

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

/// This function is only used when something has gone wrong during header extension building
pub fn new_empty() -> Self {
Self { index: Vec::new() }
Self {
index: Vec::new(),
is_error: false,
}
}

pub fn new_error() -> Self {
Self {
index: Vec::new(),
is_error: true,
}
}
}

Expand Down Expand Up @@ -146,7 +160,10 @@ impl TryFrom<CompactDataLookup> for DataLookup {
index.push((prev_id, offset..compacted.size));
}

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

Ok(lookup)
Expand Down
8 changes: 8 additions & 0 deletions core/src/header/extension/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#[cfg_attr(feature = "runtime", derive(PassByCodec))]
#[repr(u8)]
pub enum HeaderExtension {
V3(v3::HeaderExtension) = 2,

Check warning on line 19 in core/src/header/extension/mod.rs

View workflow job for this annotation

GitHub Actions / build_and_test

casting integer literal to `u8` is unnecessary
}

/// It forwards the call to the inner version of the header. Any invalid version will return the
Expand Down Expand Up @@ -52,6 +52,14 @@
forward_to_version!(self, cols)
}

pub fn get_empty_header(&self, data_root: H256) -> HeaderExtension {
forward_to_version!(self, get_empty_header, data_root).into()
}

pub fn get_faulty_header(&self, data_root: H256) -> HeaderExtension {
forward_to_version!(self, get_faulty_header, data_root).into()
}

pub fn get_header_version(&self) -> HeaderVersion {
match self {
HeaderExtension::V3(_) => HeaderVersion::V3,
Expand Down
20 changes: 20 additions & 0 deletions core/src/header/extension/v3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,24 @@ impl HeaderExtension {
pub fn cols(&self) -> u16 {
self.commitment.cols
}

pub fn get_empty_header(&self, data_root: H256) -> Self {
let empty_commitment: Vec<u8> = vec![];
let empty_app_lookup = DataLookup::new_empty();
let commitment = KateCommitment::new(0, 0, data_root, empty_commitment);
HeaderExtension {
app_lookup: empty_app_lookup,
commitment,
}
}

pub fn get_faulty_header(&self, data_root: H256) -> Self {
let empty_commitment: Vec<u8> = vec![];
let error_app_lookup = DataLookup::new_error();
let commitment = KateCommitment::new(0, 0, data_root, empty_commitment);
HeaderExtension {
app_lookup: error_app_lookup,
commitment,
}
}
}
Loading