Skip to content

trie/triedb/pathdb: fix initialization flaws #28715

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.

Sign up for GitHub

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

Closed
holiman wants to merge 1 commit into from
Closed

trie/triedb/pathdb: fix initialization flaws #28715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions trie/triedb/pathdb/database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,9 @@ func (db *Database) Initialized(genesisRoot common.Hash) bool {
inited = true
}
})
if !inited {
inited = rawdb.ReadSnapSyncStatusFlag(db.diskdb) != rawdb.StateSyncUnknown
}
return inited
}

Expand Down
6 changes: 4 additions & 2 deletions trie/triedb/pathdb/history.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,8 +576,10 @@ func truncateFromHead(db ethdb.Batcher, freezer *rawdb.ResettableFreezer, nhead
return 0, err
}
// Ensure that the truncation target falls within the specified range.
if ohead < nhead || nhead < otail {
return 0, fmt.Errorf("out of range, tail: %d, head: %d, target: %d", otail, ohead, nhead)
if nhead != 0 { // truncating to zero is always possible
Copy link
Contributor Author

@holiman holiman Dec 21, 2023

Choose a reason for hiding this comment

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

I think this is not a sufficient fix. The code further down:

	// Load the meta objects in range [nhead+1, ohead]
	blobs, err := rawdb.ReadStateHistoryMetaList(freezer, nhead+1, ohead-nhead)
	if err != nil {
		return 0, err
	}
	batch := db.NewBatch()
	for _, blob := range blobs {
		var m meta
		if err := m.decode(blob); err != nil {
			return 0, err
		}
		rawdb.DeleteStateID(batch, m.root)
	}

The call to ReadStateHistoryMetaList will read the entire chunk of data into memory, since it internally does

	return db.AncientRange(stateHistoryMeta, start-1, count, 0)

using 0 as the maxbytes ( if maxBytes is not specified, 'count' items will be returned if they are present).

Also, I'm not sure how this would behave if we try to load 5M, but 0-4M are already gone.

A better fix might be to something like this:

	if nhead != 0 { // truncating to zero is always possible
		ohead, err = freezer.TruncateHead(0)
		if err != nil {
			return 0, err
		}
		return int(ohead - nhead), nil
	}

But if resetting to 0, we should also set back the tail to zero. Hm... @rjl493456442 thoughts?

Copy link
Member

@rjl493456442 rjl493456442 Dec 21, 2023

Choose a reason for hiding this comment

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

This fix is not correct.

The leftover state history can be in range of [tail, head].
The reset to 0 won't work because the state segment before tail is not longer accessible, but we try to load them here blobs, err := rawdb.ReadStateHistoryMetaList(freezer, nhead+1, ohead-nhead)

if ohead < nhead || nhead < otail {
return 0, fmt.Errorf("out of range, tail: %d, head: %d, target: %d", otail, ohead, nhead)
}
}
// Short circuit if nothing to truncate.
if ohead == nhead {
Expand Down