Introduces `SliceRoot` #9130

akhi3030 · 2025-11-18T17:16:05Z

Problem

We are in the process of introducing double merkle for alpenglow. In particular, we will be supporting chain and double merkle tree.

Summary of Changes

Introducing a new type to represent the root hash will help in improving the readability of the code and make it easier to understand.

Note to reviewers

I am not happy with how many new crates depend upon votor-messages in this PR. Happy to receive feedback on alternative locations to define SliceRoot.

codecov-commenter · 2025-11-19T16:33:03Z

Codecov Report

❌ Patch coverage is 97.86477% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.6%. Comparing base (b23518a) to head (51370f5).
⚠️ Report is 35 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #9130   +/-   ##
=======================================
  Coverage    82.6%    82.6%           
=======================================
  Files         890      891    +1     
  Lines      320729   320758   +29     
=======================================
+ Hits       265011   265062   +51     
+ Misses      55718    55696   -22

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ledger-tool/src/output.rs

ledger/src/shred/merkle.rs

kobi-lizard · 2025-11-19T22:01:12Z

votor-messages/src/slice_root.rs

@@ -0,0 +1,33 @@
+//! Module to define [`SliceRoot`].


AlpenglowBlockId will also go in here so maybe name this differently or just put them in consensus_message.rs IDK

Even though I ended up not making the fields of Sliceroot private, in general, when introducing new types, it is a good idea to try to make them private. In which case, it also helps if the types are in their own module otherwise other code can access the internal fields. So I think I would prefer to introduce AlpenglowBlockId in yet another module.

alexpyattaev · 2025-11-25T07:30:36Z

Can you link a design doc to the PR so we keep a better record of why this is happening?

akhi3030 · 2025-11-25T07:33:09Z

Can you link a design doc to the PR so we keep a better record of why this is happening?

@kobi-lizard: have you written something down somewhere? The only "design" that I am aware of is anza-xyz/alpenglow#518.

alexpyattaev · 2025-11-25T07:35:22Z

votor-messages/src/slice_root.rs

+    Copy, Clone, Default, Debug, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize,
+)]
+#[repr(transparent)]
+pub struct SliceRoot(pub Hash);


Why are we placing it in Votor if this deals with shreds? Maybe time for rotor crate?

Also not sure why do we need a wrapper around Hash at all. Could we define an alias instead?

Why are we placing it in Votor if this deals with shreds? Maybe time for rotor crate?

I am definitely not happy with the current placement. I like the idea of introducing a rotor crate.

Also not sure why do we need a wrapper around Hash at all. Could we define an alias instead?

With type aliases, the compiler will not hold my hand to ensure that I am using the right type. I can use any old Hash. Using a wrapper struct helps in providing documentation and the additional feedback at the call sites to help the developer check that they are using the right type and not just some random Hash.

I see. Ether way, making turbine depend on Votor feels wrong :)

Agreed with Alex (and Ashwin from https://github.com/anza-xyz/agave/pull/9130/files#r2560958890) - I don't think votor is the right place for this. Rather, I agree with Ashwin's comment from linked comment:

Then the hash relevant to shreds could be named -> FecSetRoot and could live in ledger or turbine crates.

Since the erasure coding and FEC sets are created in solana-ledger, my vote would be solana-ledger

kobi-lizard · 2025-11-25T16:26:35Z

Why this is happening and why alias is wrong:
Right now Hash is used to mean many things. You can see in this PR sometimes we put comments next to Hash "this is the Merkle root" or such. Right now if an argument/variable has a variation of "block_id" in the name the chained Merkle root is used. Some of these need to change to double Merkle, some of it needs to stay for the time being. When I started drafting the Double Merkle stuff I was overwhelmed with keeping track of which Hash is what: chained Merkle root, double Merkle root, different Hash? You could swap one for the other and Rust wouldn't tell you what's wrong. With alias it's the same.
Point is, types are for slow people like me so that Rust tells them "change this thing here" and I don't have to have the meaning of different Hashes in 100 places uploaded to my brain.

AshwinSekar · 2025-11-25T18:06:12Z

votor-messages/src/slice_root.rs

+    Copy, Clone, Default, Debug, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize,
+)]
+#[repr(transparent)]
+pub struct SliceRoot(pub Hash);


I find this name pretty confusing as slice is already pretty overloaded, could we rename this to FECSetRoot or SetRoot?

Agreed with Ashwin, I'm not a fan of SliceRoot. We already have terminology to refer to this concept (FEC Set) so I see no need to introduce more terminology especially when slice is already a Rust keyword.

I think something like FecSetRoot or FecSetMerkleRoot makes sense; given that elements of a merkle tree are hashes, we probably don't need to include Hash in the name but I'd probably be fine with it too

AshwinSekar · 2025-11-25T18:09:49Z

runtime/src/bank.rs

    }

-    pub fn parent_block_id(&self) -> Option<Hash> {
+    pub fn parent_block_id(&self) -> Option<SliceRoot> {


I think if we're going to rename the types we could do something like:
Block id hash -> ChainedMerkleBlockId
new block id hash -> DoubleMerkleBlockId

Both of these can live in votor-messages since they are primarily used for consensus and then we can remove the runtime dependency

Then the hash relevant to shreds could be named -> FecSetRoot and could live in ledger or turbine crates.

I haven't walked all the dependencies to see if this is feasible but if we split into a higher level of granularity then we might be able to avoid setting up a new crate or having everything depend on votor-messages

@AshwinSekar: not sure I completely understand the proposal.

When we do introduce double merkle, won't we need to update the ledger and turnbine crates accordingly?

votor-messages/src/slice_root.rs

alexpyattaev · 2025-11-28T21:55:35Z

A conceptual question: why are we renaming FEC sets into slices in this PR?

akhi3030 · 2025-11-29T19:15:10Z

A conceptual question: why are we renaming FEC sets into slices in this PR?

I am sure Quentin (who somehow I cannot tag here) and @kobi-lizard can explain this better. The general idea is that FEC set refers to the specific technology being used instead of what the actual data structure is. The idea is that a block is made up of some data structure that can be called Slices and not the specific technology that we happen to be using to implement them.

I agree with you though that sneaking in new terminology like this is probably not a good idea and we should get wider buy-in before we do that. I plan on renaming the structs based on #9130 (comment).

alexpyattaev · 2025-11-29T20:10:22Z

I agree with you though that sneaking in new terminology like this is probably not a good idea and we should get wider buy-in before we do that. I plan on renaming the structs based on #9130 (comment).

What benefit does renaming bring? I can see a bunch of problems it creates (code churn, mess in discussions etc), which problems does it solve? Once there is a meaningful change in the logic of the protocol we should consider name changes.

akhi3030 · 2025-12-01T07:17:09Z

I agree with you though that sneaking in new terminology like this is probably not a good idea and we should get wider buy-in before we do that. I plan on renaming the structs based on #9130 (comment).

What benefit does renaming bring? I can see a bunch of problems it creates (code churn, mess in discussions etc), which problems does it solve? Once there is a meaningful change in the logic of the protocol we should consider name changes.

Indeed... these are precisely the reasons to not shoehorn in a renaming and get general consensus before attempting it.

akhi3030 · 2025-12-11T13:24:14Z

Ended up creating #9504 instead of modifying this one as updating this seemed to be more work. Closing this.

akhi3030 force-pushed the introduce-slice-root branch 2 times, most recently from 0421c44 to 7ffe975 Compare November 18, 2025 18:45

wip

28d4e7b

akhi3030 force-pushed the introduce-slice-root branch from 7ffe975 to 28d4e7b Compare November 18, 2025 19:16

akhi3030 added 3 commits November 18, 2025 20:07

add missing file

b9a5944

cargo lock updates

bbfbae6

fixes in ledger-tool

eb8b4da

akhi3030 force-pushed the introduce-slice-root branch 2 times, most recently from 9823850 to 030c2b6 Compare November 19, 2025 15:04

fix abi

a35ce2f

akhi3030 force-pushed the introduce-slice-root branch from 030c2b6 to a35ce2f Compare November 19, 2025 15:16

misc. fixes

10cee86

kobi-lizard reviewed Nov 19, 2025

View reviewed changes

akhi3030 added 8 commits November 20, 2025 10:39

some fixes in ledger-tool based on PR feedback

4ba9eee

Merge branch 'master' into introduce-slice-root

1796149

introduce new_random

d305d71

update additional cargo.lock files

fe8a763

compiler fixes

3647e27

Merge branch 'master' into introduce-slice-root

6547f06

Merge branch 'master' into introduce-slice-root

189675c

update cargo.lock files

77e6e32

akhi3030 changed the title ~~wip: intro SliceRoot~~ Introduces SliceRoot Nov 25, 2025

akhi3030 marked this pull request as ready for review November 25, 2025 06:46

akhi3030 requested a review from a team as a code owner November 25, 2025 06:46

akhi3030 requested review from AshwinSekar, alexpyattaev, bw-solana and kobi-lizard November 25, 2025 06:46

alexpyattaev reviewed Nov 25, 2025

View reviewed changes

AshwinSekar reviewed Nov 25, 2025

View reviewed changes

AshwinSekar requested a review from steviez November 25, 2025 18:10

AshwinSekar reviewed Nov 25, 2025

View reviewed changes

votor-messages/src/slice_root.rs Outdated Show resolved Hide resolved

SliceRoot stores the byte directly instead of Hash

4a5260b

akhi3030 force-pushed the introduce-slice-root branch from 9ff23f9 to 266510c Compare November 28, 2025 16:52

fix abi

0545153

akhi3030 force-pushed the introduce-slice-root branch from 266510c to 0545153 Compare November 29, 2025 18:57

derive default with Hash

51370f5

akhi3030 mentioned this pull request Dec 11, 2025

Introduces FecSetRoot #9504

Open

akhi3030 closed this Dec 11, 2025

akhi3030 deleted the introduce-slice-root branch December 11, 2025 13:24

Introduces SliceRoot #9130

Introduces SliceRoot #9130

Uh oh!

Conversation

akhi3030 commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Summary of Changes

Note to reviewers

Uh oh!

codecov-commenter commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexpyattaev commented Nov 25, 2025

Uh oh!

akhi3030 commented Nov 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kobi-lizard commented Nov 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AshwinSekar Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alexpyattaev commented Nov 28, 2025

Uh oh!

akhi3030 commented Nov 29, 2025

Uh oh!

alexpyattaev commented Nov 29, 2025

Uh oh!

akhi3030 commented Dec 1, 2025

Uh oh!

akhi3030 commented Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Introduces `SliceRoot` #9130

Introduces `SliceRoot` #9130

akhi3030 commented Nov 18, 2025 •

edited

Loading

codecov-commenter commented Nov 19, 2025 •

edited

Loading

AshwinSekar Nov 25, 2025 •

edited

Loading