NAlgebra replaces custom grid #33
Conversation
Signed-off-by: Sasa Prsic <sasa@ethernal.tech>
Switch sha256 to keccak256
fmiguelgarcia
changed the title
fmiguelgarcia
requested review from
prabal-banerjee,
Leouarz,
kroos47,
0xSasaPrsic,
sh3ll3x3c and
aterentic-ethernal
| .cloned() | ||
| } | ||
|
|
||
| pub fn projected_range_of(&self, app_id: AppId, chunk_size: u32) -> Option<DataLookupRange> { |
There was a problem hiding this comment.
Where are these projected ranges actually used?
|
|
||
| #[derive(Encode, Decode, TypeInfo, Constructor, Debug, Clone)] | ||
| #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] | ||
| pub struct CompactDataLookup { |
There was a problem hiding this comment.
CompactDataLookup here is actually less compact. Every scale-encoded vec starts with a compact integer size, then the items, so the size here is redundant and less compact. Do we really need the range types in here? I never felt like the end range greatly complicated anything.
| use DataProofTryFromError::*; | ||
|
|
||
| let root = <[u8; 32]>::try_from(merkle_proof.root.as_ref()) | ||
| .map_err(|_| InvalidRoot)? | ||
| .into(); | ||
| let leaf = sha2_256(merkle_proof.leaf.as_ref()).into(); | ||
| let leaf = keccak_256(merkle_proof.leaf.as_ref()).into(); |
There was a problem hiding this comment.
This is a completely unrelated change, no? Shouldn't this be in another PR/discussion?
| @@ -269,7 +268,7 @@ impl<N: HeaderBlockNumber, H: HeaderHash> ExtendedHeader for Header<N, H> { | |||
| } | |||
| } | |||
|
|
|||
| #[cfg(test)] | |||
| #[cfg(all(test, feature = "runtime"))] | |||
There was a problem hiding this comment.
Good as long as we're sure runtime gets turned on for CI tests
| use serde::{Deserialize, Serialize}; | ||
|
|
||
| /// Keccak 256 wrapper which supports `beefy-merkle-tree::Hasher`. | ||
| #[derive(PartialEq, Eq, Clone, RuntimeDebug, TypeInfo)] |
There was a problem hiding this comment.
Again, this belongs in another PR right?
| .extend_columns(2) | ||
| .unwrap(); | ||
| let grid = EvaluationGrid::from_extrinsics(exts, 4, 256, 256, seed)? | ||
| .extend_columns(unsafe { NonZeroU16::new_unchecked(2) })?; |
There was a problem hiding this comment.
Please use NonZeroU16::new(2).expect("2>0") instead of using unsafe. This can also be done with a const function like it was done in kate-grid so that the check is actually done at compile time, using a match:
const fn new_nz(a: u16) -> NonZeroU16 {
match a {
0 => [][0],
b => NonZeroU16::new(b).unwrap(),
}
}or something like that.
| codec = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] } | ||
| dusk-bytes = "0.1.6" | ||
| # Internals | ||
| avail-core = { path = "../../core", default-features = false } | ||
| dusk-plonk = { git = "https://github.com/availproject/plonk.git", tag = "v0.12.0-polygon-2" } |
There was a problem hiding this comment.
agh still need to move kate-recovery to use the multiproof apis...
| let (rows, cols) = self.evals.shape(); | ||
| // SAFETY: We cannot construct an `EvaluationGrid` with any dimension `< 1` or `> u16::MAX` | ||
| debug_assert!(rows <= usize::from(u16::MAX) && cols <= usize::from(u16::MAX)); | ||
| unsafe { Dimensions::new_unchecked(rows as u16, cols as u16) } |
There was a problem hiding this comment.
We really don't need an unsafe here... Just new with an expect is the right thing to do here.
| #[allow(clippy::integer_arithmetic)] | ||
| let h_mul: usize = rows / usize::from(NonZeroU16::get(orig_dims.rows())); | ||
| #[allow(clippy::integer_arithmetic)] | ||
| let row_from_lineal_index = |cols, lineal_index| { |
There was a problem hiding this comment.
Misspelling? row_from_linear_index?
| // Round the height up to a power of 2 for ffts | ||
| let height = round_up_power_of_2(current_height); | ||
| let height = current_height | ||
| .checked_next_power_of_two() |
Replaces Grid by NAlgebra
It replaces the grid type by the
NAlgebraMatrix.Avail-Coreis now shared by LC & nodeNo more duplicate code is needed, and
avail-corecan be safely shared betweenLC(or any otherstdsystem) and the avail node, whereWASMandSTDis needed.runtimeonAvail-coreenables the capabilities requires to integrate into our Avail's runtime.Cargo.tomlfiles of unused dependencies.DataLookuprefactorIt has been refactored to simplify the runtime, but the expected serialization format is kept. It improves and simplifies its code, and keeps compatibility of 3rd parties which read from
rpc(using serde serialization) or directly from header (using codec).Code Safety
Stringtype used as error (likeResult<..., String>) by a custom error usingthiserror(non-std)const_assertto ensure constants constraints with no impact on runtime.Dedup Coded
KateRecovery::Indexhas been replaced completely byDataLookup.Dimesionhas been deduped.AppDataIndexsuperseded byDataLookupOther improvements
Clippyto our CI.ensureandfailinavail-coreremoves the needed offrame-support.AppId,BlockLenghtColumns, andBlockLenghtRowsstrong types removeFromimpl to keep it strong.kzgbenchmark has been superseded byreconstruct.test_casecrate.sparce_slice_readerremoves the needed of duplicate padded data before decode it. It will reduces the memory footprint of the node & LC.