Skip to content

Conversation

@veljkovranic
Copy link
Contributor

@veljkovranic veljkovranic commented Oct 24, 2025

Description

This PR introduces an option to select if your leaves are accessed by regular or a bit-reverse order of the index.
This feature is needed in certain cases, eg. committing to the results of NTT and the reordering of the leaves is too costly to perform before the commitment.


  • Targeted PR against correct branch (main)
  • Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

@veljkovranic veljkovranic requested a review from a team as a code owner October 24, 2025 10:28
@veljkovranic veljkovranic requested review from Pratyush, mmagician and z-tech and removed request for a team October 24, 2025 10:28
@z-tech
Copy link

z-tech commented Oct 24, 2025

Hi, if you never reorder the vector v then what prevents you from leaving the MT library as lexicographic and handling the mapping just outside?

t.commit(v);
for i in SOME_ORDER {
   t.get_leaf(to_reverse_bit_index(i));
   or
   t.get_leaf(from_reverse_bit(i));
}

What is the difference between this pseudocode and the PR?

@veljkovranic
Copy link
Contributor Author

So the data that we are commiting to is a polynomial.
This leaves us with two options:

  1. Either reorded the NTT output data, so the rest of the methods stay the same (evaluation at a point, conversion to other coefficient/eval form, folding, etc). But this is costly if the polynomial is huge.
  2. Or you treat the data as-is and now your whole codebase needs to support the bit-reverse ordering in all of the methods listed above.

Btw, nice to meet you, @WizardOfMenlo suggested for me to have a chat with you, multiple times, just didn't expect it to happen here 😄

// 1 (001) -> 4 (100)
// 2 (010) -> 2 (010)
// 3 (011) -> 6 (110)
// 4 (100) -> 1 (001)
Copy link

Choose a reason for hiding this comment

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

So you mean that using ordering BIT_REVERSED the leaves in physical memory locations corresponding to 0 and 4 would get hashed into an internal node (rather than 0 and 1) ?

If so, can you write tests that cover this?

@z-tech
Copy link

z-tech commented Oct 24, 2025

I think I understand the intent now. Personally, I think it would be more adaptable to open a new constructor for MerkleTree that accepts an Iterator over the vector you don't want to reorder. Something along those lines has been done in the EfficientSumcheck repo: https://github.com/compsec-epfl/efficient-sumcheck/blob/main/src/streams/stream_iterator.rs#L12

That way people can create their own orderings without modifying the actual MT Library code. It would also mean no breaking changes for the existing constructor.

What are your thoughts?

&leaf_crh_params,
&two_to_one_params,
&leaves,
LeafOrderingMode::NATURAL,
Copy link

Choose a reason for hiding this comment

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

Are theselexicographic and reverse_lexicographic ?

@veljkovranic
Copy link
Contributor Author

I can close it in this case, since it will be an Iterator either way. The only thing that is important is the next() operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants