Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

commitment: clarify update kind based on afterMap #13348

Merged
merged 4 commits into from
Jan 13, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 45 additions & 8 deletions erigon-lib/commitment/hex_patricia_hashed.go
Original file line number Diff line number Diff line change
Expand Up @@ -1566,6 +1566,42 @@ func (hph *HexPatriciaHashed) createCellGetter(b []byte, updateKey []byte, row,
}
}

// updateKind is a type of update that is being applied to the trie structure.
type updateKind uint8

const (
// updateKindDelete means after we processed longest common prefix, row ended up empty.
updateKindDelete updateKind = 0b0

// updateKindPropagate is an update operation ended up with a single nibble which is leaf or extension node.
// We do not store keys with only one cell as a value in db, instead we copy them upwards to the parent branch.
//
// In case current prefix existed before and node is fused to upper level, this causes deletion for current prefix
// and update of branch value on upper level.
// e.g.: leaf was at prefix 0xbeef, but we fuse it in level above, so
// - delete 0xbeef
// - update 0xbee
updateKindPropagate updateKind = 0b01

// updateKindBranch is an update operation ended up as a branch of 2+ cells.
// That does not necessarily means that branch is NEW, it could be an existing branch that was updated.
antonis19 marked this conversation as resolved.
Show resolved Hide resolved
updateKindBranch updateKind = 0b10
)

// Kind defines how exactly given update should be folded upwards to the parent branch or root.
// It also returns number of nibbles that left in branch after the operation.
antonis19 marked this conversation as resolved.
Show resolved Hide resolved
func afterMapUpdateKind(afterMap uint16) (kind updateKind, nibblesAfterUpdate int) {
nibblesAfterUpdate = bits.OnesCount16(afterMap)
switch nibblesAfterUpdate {
case 0:
return updateKindDelete, nibblesAfterUpdate
case 1:
return updateKindPropagate, nibblesAfterUpdate
default:
return updateKindBranch, nibblesAfterUpdate
}
}

// The purpose of fold is to reduce hph.currentKey[:hph.currentKeyLen]. It should be invoked
// until that current key becomes a prefix of hashedKey that we will process next
// (in other words until the needFolding function returns 0)
Expand Down Expand Up @@ -1597,16 +1633,16 @@ func (hph *HexPatriciaHashed) fold() (err error) {

depth := hph.depths[row]
updateKey := hexToCompact(hph.currentKey[:updateKeyLen])
partsCount := bits.OnesCount16(hph.afterMap[row])
defer func() { hph.depthsToTxNum[depth] = 0 }()

if hph.trace {
fmt.Printf("fold: (row=%d, {%s}, depth=%d) prefix [%x] touchMap: %016b afterMap: %016b \n",
row, updatedNibs(hph.touchMap[row]&hph.afterMap[row]), depth, hph.currentKey[:hph.currentKeyLen], hph.touchMap[row], hph.afterMap[row])
}

switch partsCount {
case 0: // Everything deleted
updateKind, nibblesLeftAfterUpdate := afterMapUpdateKind(hph.afterMap[row])
switch updateKind {
case updateKindDelete: // Everything deleted
if hph.touchMap[row] != 0 {
if row == 0 {
// Root is deleted because the tree is empty
Expand Down Expand Up @@ -1636,7 +1672,7 @@ func (hph *HexPatriciaHashed) fold() (err error) {
} else {
hph.currentKeyLen = 0
}
case 1: // Leaf or extension node
case updateKindPropagate: // Leaf or extension node
if hph.touchMap[row] != 0 {
// any modifications
if row == 0 {
Expand All @@ -1650,9 +1686,10 @@ func (hph *HexPatriciaHashed) fold() (err error) {
cell := &hph.grid[row][nibble]
upCell.extLen = 0
upCell.stateHashLen = 0
// propagate cell into parent row
upCell.fillFromLowerCell(cell, depth, hph.currentKey[upDepth:hph.currentKeyLen], nibble)
// Delete if it existed
if hph.branchBefore[row] {

if hph.branchBefore[row] { // encode Delete if prefix existed before
//fmt.Printf("delete existed row %d prefix %x\n", row, updateKey)
_, err := hph.branchEncoder.CollectUpdate(hph.ctx, updateKey, 0, hph.touchMap[row], 0, RetrieveCellNoop)
if err != nil {
Expand All @@ -1664,7 +1701,7 @@ func (hph *HexPatriciaHashed) fold() (err error) {
if hph.trace {
fmt.Printf("formed leaf (%d %x, depth=%d) [%x] %s\n", row, nibble, depth, updateKey, cell.FullString())
}
default: // Branch node
case updateKindBranch:
antonis19 marked this conversation as resolved.
Show resolved Hide resolved
if hph.touchMap[row] != 0 { // any modifications
if row == 0 {
hph.rootTouched = true
Expand All @@ -1682,7 +1719,7 @@ func (hph *HexPatriciaHashed) fold() (err error) {
}

// Calculate total length of all hashes
totalBranchLen := 17 - partsCount // For every empty cell, one byte
totalBranchLen := 17 - nibblesLeftAfterUpdate // For every empty cell, one byte
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't totalBranchLen be renamed to emptyCellsInBranchLen ?

Copy link
Member Author

Choose a reason for hiding this comment

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

this is branch len, counting not only amount of empty cells, but we initialize it with that value. Later we add to this len 33 bytes for each existing cell (so 0xa0[roothash]).

Copy link
Member Author

Choose a reason for hiding this comment

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

so rename is incorrect and not needed

Copy link
Member

@antonis19 antonis19 Jan 10, 2025

Choose a reason for hiding this comment

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

How is the rename incorrect when it is counting the number of 1's in the afterMap bitmap ? 17 - number of 1's == number of 0s , right?

Copy link
Member Author

Choose a reason for hiding this comment

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

just read code block till the end and you can see that this variable is not just count of empty cells.

for bitset, j := hph.afterMap[row], 0; bitset != 0; j++ {
bit := bitset & -bitset
nibble := bits.TrailingZeros16(bit)
Expand Down
Loading