forks/go-ethereum
1
0
Fork 1
mirror of https://github.com/ethereum/go-ethereum.git synced 2026-02-26 07:37:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

triedb/pathdb: fix journal resolution in pathdb (#32097)

Browse source 
This pull request fixes a flaw in the PBSS state iterator, which
could return empty account or storage data.

In PBSS, multiple in-memory diff layers and a write buffer are
maintained. These layers are persisted to the database and reloaded after
node restarts. However, since the state data is encoded using RLP, the
distinction between nil and an empty byte slice is lost during the encode/decode
process. As a result, invalid state values such as `[]byte{}` can appear in PBSS
and ultimately be returned by the state iterator.


Checkout
https://github.com/ethereum/go-ethereum/blob/master/triedb/pathdb/iterator_fast.go#L270
for more iterator details.

It's a long-term existent issue and now be activated since the snapshot
integration.
The error `err="range contains deletion"` will occur when Geth tries to
serve other
peers with SNAP protocol request.

---------

Co-authored-by: Felix Lange <fjl@twurst.com>
This commit is contained in:
rjl493456442 2025-06-26 23:19:02 +08:00 committed by GitHub
parent a92f2b86e3
commit 36bcc24fbe
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 91 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

93
triedb/pathdb/iterator_test.go
View file

@ -816,7 +816,8 @@ func testAccountIteratorDeletions(t *testing.T, newIterator func(db *Database, r
config := &Config{
NoAsyncGeneration: true,
}
db := New(rawdb.NewMemoryDatabase(), config, false)
memoryDB := rawdb.NewMemoryDatabase()
db := New(memoryDB, config, false)
// Stack three diff layers on top with various overlaps
db.Update(common.HexToHash("0x02"), types.EmptyRootHash, 1, trienode.NewMergedNodeSet(),
@ -831,31 +832,55 @@ func testAccountIteratorDeletions(t *testing.T, newIterator func(db *Database, r
db.Update(common.HexToHash("0x04"), common.HexToHash("0x03"), 3, trienode.NewMergedNodeSet(),
NewStateSetWithOrigin(randomAccountSet("0x33", "0x44", "0x55"), nil, nil, nil, false))
// The output should be 11,33,44,55
it := newIterator(db, common.HexToHash("0x04"), common.Hash{})
// Do a quick check
verifyIterator(t, 4, it, verifyAccount)
it.Release()
verify := func() {
// The output should be 11,33,44,55
it := newIterator(db, common.HexToHash("0x04"), common.Hash{})
// Do a quick check
verifyIterator(t, 4, it, verifyAccount)
it.Release()
// And a more detailed verification that we indeed do not see '0x22'
it = newIterator(db, common.HexToHash("0x04"), common.Hash{})
defer it.Release()
for it.Next() {
hash := it.Hash()
if it.Account() == nil {
t.Errorf("iterator returned nil-value for hash %x", hash)
}
if hash == deleted {
t.Errorf("expected deleted elem %x to not be returned by iterator", deleted)
// And a more detailed verification that we indeed do not see '0x22'
it = newIterator(db, common.HexToHash("0x04"), common.Hash{})
defer it.Release()
for it.Next() {
hash := it.Hash()
if it.Account() == nil {
t.Errorf("iterator returned nil-value for hash %x", hash)
}
if hash == deleted {
t.Errorf("expected deleted elem %x to not be returned by iterator", deleted)
}
}
}
verify()
if err := db.Journal(common.HexToHash("0x04")); err != nil {
t.Fatalf("Failed to journal the database, %v", err)
}
if err := db.Close(); err != nil {
t.Fatalf("Failed to close the database, %v", err)
}
db = New(memoryDB, config, false)
verify()
}
func TestStorageIteratorDeletions(t *testing.T) {
config := &Config{
NoAsyncGeneration: true,
}
db := New(rawdb.NewMemoryDatabase(), config, false)
memoryDB := rawdb.NewMemoryDatabase()
db := New(memoryDB, config, false)
restart := func(head common.Hash) {
if err := db.Journal(head); err != nil {
t.Fatalf("Failed to journal the database, %v", err)
}
if err := db.Close(); err != nil {
t.Fatalf("Failed to close the database, %v", err)
}
db = New(memoryDB, config, false)
}
// Stack three diff layers on top with various overlaps
db.Update(common.HexToHash("0x02"), types.EmptyRootHash, 1, trienode.NewMergedNodeSet(),
@ -874,6 +899,19 @@ func TestStorageIteratorDeletions(t *testing.T) {
verifyIterator(t, 3, it, verifyStorage)
it.Release()
// Ensure the iteration result aligns after the database restart
restart(common.HexToHash("0x03"))
// The output should be 02,04,05,06
it, _ = db.StorageIterator(common.HexToHash("0x03"), common.HexToHash("0xaa"), common.Hash{})
verifyIterator(t, 4, it, verifyStorage)
it.Release()
// The output should be 04,05,06
it, _ = db.StorageIterator(common.HexToHash("0x03"), common.HexToHash("0xaa"), common.HexToHash("0x03"))
verifyIterator(t, 3, it, verifyStorage)
it.Release()
// Destruct the whole storage
accounts := map[common.Hash][]byte{
common.HexToHash("0xaa"): nil,
@ -885,6 +923,12 @@ func TestStorageIteratorDeletions(t *testing.T) {
verifyIterator(t, 0, it, verifyStorage)
it.Release()
// Ensure the iteration result aligns after the database restart
restart(common.HexToHash("0x04"))
it, _ = db.StorageIterator(common.HexToHash("0x04"), common.HexToHash("0xaa"), common.Hash{})
verifyIterator(t, 0, it, verifyStorage)
it.Release()
// Re-insert the slots of the same account
db.Update(common.HexToHash("0x05"), common.HexToHash("0x04"), 4, trienode.NewMergedNodeSet(),
NewStateSetWithOrigin(randomAccountSet("0xaa"), randomStorageSet([]string{"0xaa"}, [][]string{{"0x07", "0x08", "0x09"}}, nil), nil, nil, false))
@ -894,6 +938,14 @@ func TestStorageIteratorDeletions(t *testing.T) {
verifyIterator(t, 3, it, verifyStorage)
it.Release()
// Ensure the iteration result aligns after the database restart
restart(common.HexToHash("0x05"))
// The output should be 07,08,09
it, _ = db.StorageIterator(common.HexToHash("0x05"), common.HexToHash("0xaa"), common.Hash{})
verifyIterator(t, 3, it, verifyStorage)
it.Release()
// Destruct the whole storage but re-create the account in the same layer
db.Update(common.HexToHash("0x06"), common.HexToHash("0x05"), 5, trienode.NewMergedNodeSet(),
NewStateSetWithOrigin(randomAccountSet("0xaa"), randomStorageSet([]string{"0xaa"}, [][]string{{"0x11", "0x12"}}, [][]string{{"0x07", "0x08", "0x09"}}), nil, nil, false))
@ -903,6 +955,13 @@ func TestStorageIteratorDeletions(t *testing.T) {
it.Release()
verifyIterator(t, 2, db.tree.get(common.HexToHash("0x06")).(*diffLayer).newBinaryStorageIterator(common.HexToHash("0xaa"), common.Hash{}), verifyStorage)
// Ensure the iteration result aligns after the database restart
restart(common.HexToHash("0x06"))
it, _ = db.StorageIterator(common.HexToHash("0x06"), common.HexToHash("0xaa"), common.Hash{})
verifyIterator(t, 2, it, verifyStorage) // The output should be 11,12
it.Release()
verifyIterator(t, 2, db.tree.get(common.HexToHash("0x06")).(*diffLayer).newBinaryStorageIterator(common.HexToHash("0xaa"), common.Hash{}), verifyStorage)
}
// TestStaleIterator tests if the iterator could correctly terminate the iteration

23
triedb/pathdb/states.go
View file

@ -387,8 +387,8 @@ func (s *stateSet) decode(r *rlp.Stream) error {
if err := r.Decode(&dec); err != nil {
return fmt.Errorf("load diff accounts: %v", err)
}
for i := 0; i < len(dec.AddrHashes); i++ {
accountSet[dec.AddrHashes[i]] = dec.Accounts[i]
for i := range dec.AddrHashes {
accountSet[dec.AddrHashes[i]] = empty2nil(dec.Accounts[i])
}
s.accountData = accountSet
@ -407,8 +407,8 @@ func (s *stateSet) decode(r *rlp.Stream) error {
}
for _, entry := range storages {
storageSet[entry.AddrHash] = make(map[common.Hash][]byte, len(entry.Keys))
for i := 0; i < len(entry.Keys); i++ {
storageSet[entry.AddrHash][entry.Keys[i]] = entry.Vals[i]
for i := range entry.Keys {
storageSet[entry.AddrHash][entry.Keys[i]] = empty2nil(entry.Vals[i])
}
}
s.storageData = storageSet
@ -550,8 +550,8 @@ func (s *StateSetWithOrigin) decode(r *rlp.Stream) error {
if err := r.Decode(&accounts); err != nil {
return fmt.Errorf("load diff account origin set: %v", err)
}
for i := 0; i < len(accounts.Accounts); i++ {
accountSet[accounts.Addresses[i]] = accounts.Accounts[i]
for i := range accounts.Accounts {
accountSet[accounts.Addresses[i]] = empty2nil(accounts.Accounts[i])
}
s.accountOrigin = accountSet
@ -570,10 +570,17 @@ func (s *StateSetWithOrigin) decode(r *rlp.Stream) error {
}
for _, storage := range storages {
storageSet[storage.Address] = make(map[common.Hash][]byte)
for i := 0; i < len(storage.Keys); i++ {
storageSet[storage.Address][storage.Keys[i]] = storage.Vals[i]
for i := range storage.Keys {
storageSet[storage.Address][storage.Keys[i]] = empty2nil(storage.Vals[i])
}
}
s.storageOrigin = storageSet
return nil
}
func empty2nil(b []byte) []byte {
if len(b) == 0 {
return nil
}
return b
}