Skip to content

Feature/init bintrie#88

Open
thomas-quadratic wants to merge 22 commits intohyperledger:mainfrom
Quadratic-Labs:feature/init-bintrie
Open

Feature/init bintrie#88
thomas-quadratic wants to merge 22 commits intohyperledger:mainfrom
Quadratic-Labs:feature/init-bintrie

Conversation

@thomas-quadratic
Copy link
Contributor

PR description

In this branch, we continue initiating the Binary Trie for state. We include Nodes and Get and Put Visitors. We also abstracted BitSequence used for location.

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
@github-actions
Copy link

  • I thought about the changelog.

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
byte[] curr = current.encode();
assertThat(prev[0] < (curr[0] & 0xFF))
.as(String.format("Test Encoded LexOrder %s < %s", prev[0], curr[0]))
.isTrue();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think

previous = current;
is missing. WHen I add it , the test is failing

assertThat(trie.getRoot()).as("Stem root").isInstanceOf(StemNode.class);
trie.remove(key);
trie.remove(key);
trie.flatten();
Copy link
Contributor

Choose a reason for hiding this comment

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

why we need to call flatten ? I think we should do it during deletion 🤔

keyFactory.fromHexString(
"0x00112233445566778899aabbccddeeff00112233445566778899aabbccddee00");
Bytes32 value2 =
Bytes32.fromHexString("0x0100000000000000000000000000000000000000000000000000000000000000");
Copy link
Contributor

Choose a reason for hiding this comment

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

I will prefer to have two different values for value 1 and value 2

* Class representing a sequence of bits, used as prefixes in a Binary Trie. Implementation using
* packed bits into byte array.
*/
public class BytesPackedBitSequence extends BitSequence<BytesPackedBitSequence> {
Copy link
Contributor

Choose a reason for hiding this comment

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

it will be nice to explain how it's working , for a new de it could be hard to follow

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 agree. Meanwhile, I have implemented BytesBitSequence with a Byte per Bit, and so the representation is much simplified. However, the way we map to keys in the database should indeed be documented and explained.
Now if we come to batching nodes in the DB to reduce the number of iops, we might not need an encoding anymore.

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
@thomas-quadratic
Copy link
Contributor Author

The StoredBinTrie has been implemented, together with a much simpler BitSequence implementation BytesBitSequence, using a Byte per Bit to represent the sequence. I think it is better as a basis for debugging and optimisations.

@thomas-quadratic thomas-quadratic requested a review from matkt July 18, 2025 04:23
@thomas-quadratic thomas-quadratic marked this pull request as ready for review July 18, 2025 04:24
@matkt
Copy link
Contributor

matkt commented Jul 22, 2025

I think BASIC_DATA_LEAF_KEY and others keys should not be in verkle, maybe a common package ?

thomas-quadratic and others added 11 commits July 23, 2025 14:49
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
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