shred: expose chained merkle root #435
Conversation
ledger/src/shred.rs
Outdated
| @@ -334,6 +334,16 @@ macro_rules! dispatch { | |||
| Self::ShredData(shred) => shred.$name($($arg, )?), | |||
| } | |||
| } | |||
| }; | |||
| (allow_dead $vis:vis fn $name:ident(&self $(, $arg:ident : $ty:ty)?) $(-> $out:ty)?) => { | |||
There was a problem hiding this comment.
temporarily adding this for clippy checks, will remove in #102
There was a problem hiding this comment.
Instead of adding a new variant for the macro, can you just add #[allow(dead_code)] in line 313?
ledger/src/shred.rs
Outdated
| @@ -334,6 +334,16 @@ macro_rules! dispatch { | |||
| Self::ShredData(shred) => shred.$name($($arg, )?), | |||
| } | |||
| } | |||
| }; | |||
| (allow_dead $vis:vis fn $name:ident(&self $(, $arg:ident : $ty:ty)?) $(-> $out:ty)?) => { | |||
There was a problem hiding this comment.
Instead of adding a new variant for the macro, can you just add #[allow(dead_code)] in line 313?
ledger/src/shred.rs
Outdated
| chained: true, | ||
| resigned, | ||
| } => merkle::ShredData::get_chained_merkle_root_offset(proof_size, resigned).ok(), | ||
| _ => None, |
There was a problem hiding this comment.
_ => will silently ignore new variants added to the enum.
Can you please use the explicit:
ShredVariant::MerkleCode { proof_size:_, chained: false, resigned:_ } => None,
ShredVariant::MerkleData { proof_size:_, chained: false, resigned:_ } => None,so that we get compile errors when new variants are added?
ledger/src/shred/merkle.rs
Outdated
| Self::get_chained_merkle_root_offset(proof_size, resigned) | ||
| } | ||
|
|
||
| pub(crate) fn get_chained_merkle_root_offset( |
There was a problem hiding this comment.
shouldn't these be pub(super) as well?
similarly the other one below?
There was a problem hiding this comment.
ah ya good call, still get confused on how module hierarchy works sometimes.
shred.rs declares mod merkle, and the mod layout is able to access ShredData/ShredCode::get_merkle_root as a parent module? 😕 . i would've assumed they are siblings.
/- shred_code
/- shred_data
ledger - shred - layout
AshwinSekar
force-pushed
the
shred-chained-merkle-root
branch
from
2578849
to
8fe3ce6
Compare
|
Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis. |
* shred: expose chained merkle root * pr feedback: macro, pub(super), _=> none (cherry picked from commit cfd5b71)
…nza-xyz#459) shred: expose chained merkle root (anza-xyz#435) * shred: expose chained merkle root * pr feedback: macro, pub(super), _=> none (cherry picked from commit cfd5b71) Co-authored-by: Ashwin Sekar <ashwin@anza.xyz>
Split from #102
Expose the chained merkle root from shred layout and impl for use in blockstore.
Contributes to solana-labs#34897