From b474628f03e447fa851034962282035241e46149 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Fri, 28 Mar 2025 11:27:37 +0800 Subject: [PATCH] core/filtermaps, ethdb: workaround for prevent unexpected node deletion This pull request introduces a workaround in the database range deleter: if key == hash(value), the entry will be excluded from deletion. In hash mode, trie nodes are stored using their hash as the database key without any prefix. As a result, they may unintentionally collide with entries marked for deletion. This workaround will remain in place until hash mode is fully deprecated. --- core/filtermaps/filtermaps.go | 3 +-- ethdb/errors.go | 23 +++++++++++++++++++++ ethdb/leveldb/leveldb.go | 14 +++++++++---- ethdb/pebble/pebble.go | 38 ++++++++++++++++++++++++++++++++--- 4 files changed, 69 insertions(+), 9 deletions(-) create mode 100644 ethdb/errors.go diff --git a/core/filtermaps/filtermaps.go b/core/filtermaps/filtermaps.go index db7ab0a426..7d0d5f44bd 100644 --- a/core/filtermaps/filtermaps.go +++ b/core/filtermaps/filtermaps.go @@ -29,7 +29,6 @@ import ( "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/ethdb" - "github.com/ethereum/go-ethereum/ethdb/leveldb" "github.com/ethereum/go-ethereum/log" ) @@ -377,7 +376,7 @@ func (f *FilterMaps) safeDeleteRange(removeFn func(ethdb.KeyValueRangeDeleter) e } return true } - if err != leveldb.ErrTooManyKeys { + if errors.Is(err, ethdb.ErrTooManyKeys) { log.Error(action+" failed", "error", err) return false } diff --git a/ethdb/errors.go b/ethdb/errors.go new file mode 100644 index 0000000000..dc2376d6e5 --- /dev/null +++ b/ethdb/errors.go @@ -0,0 +1,23 @@ +// Copyright 2025 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package ethdb + +import "errors" + +// ErrTooManyKeys is returned when the number of keys pending range +// deletion exceeds the allowed limit. +var ErrTooManyKeys = errors.New("exceeded maximum keys for range deletion") diff --git a/ethdb/leveldb/leveldb.go b/ethdb/leveldb/leveldb.go index 7f47523b82..5c4f659400 100644 --- a/ethdb/leveldb/leveldb.go +++ b/ethdb/leveldb/leveldb.go @@ -23,6 +23,7 @@ package leveldb import ( "bytes" "fmt" + "github.com/ethereum/go-ethereum/crypto" "sync" "time" @@ -207,8 +208,6 @@ func (db *Database) Delete(key []byte) error { return db.db.Delete(key, nil) } -var ErrTooManyKeys = errors.New("too many keys in deleted range") - // DeleteRange deletes all of the keys (and values) in the range [start,end) // (inclusive on start, exclusive on end). // Note that this is a fallback implementation as leveldb does not natively @@ -221,14 +220,21 @@ func (db *Database) DeleteRange(start, end []byte) error { it := db.NewIterator(nil, start) defer it.Release() - var count int + var ( + count int + buff = crypto.NewKeccakState() + ) for it.Next() && bytes.Compare(end, it.Key()) > 0 { + // Prevent deletion for trie nodes in hash mode + if h := crypto.HashData(buff, it.Value()); h == common.BytesToHash(it.Key()) { + continue + } count++ if count > 10000 { // should not block for more than a second if err := batch.Write(); err != nil { return err } - return ErrTooManyKeys + return ethdb.ErrTooManyKeys } if err := batch.Delete(it.Key()); err != nil { return err diff --git a/ethdb/pebble/pebble.go b/ethdb/pebble/pebble.go index b87ecb2595..c44758ce6e 100644 --- a/ethdb/pebble/pebble.go +++ b/ethdb/pebble/pebble.go @@ -28,6 +28,7 @@ import ( "github.com/cockroachdb/pebble" "github.com/cockroachdb/pebble/bloom" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/metrics" @@ -338,15 +339,46 @@ func (d *Database) Delete(key []byte) error { return d.db.Delete(key, d.writeOptions) } -// DeleteRange deletes all of the keys (and values) in the range [start,end) -// (inclusive on start, exclusive on end). +// DeleteRange removes all keys and values within the range [start, end) +// (inclusive on start, exclusive on end). Although Pebble provides native +// range deletion support, it is not used here because it may accidentally +// remove hash scheme trie nodes due to improper prefixing. This workaround +// should remain in place until the hash mode is fully deprecated. +// +// If only a partial deletion occurs, ErrTooManyKeys is returned. +// The caller should retry until the deletion is fully completed. func (d *Database) DeleteRange(start, end []byte) error { d.quitLock.RLock() defer d.quitLock.RUnlock() + if d.closed { return pebble.ErrClosed } - return d.db.DeleteRange(start, end, d.writeOptions) + batch := d.NewBatch() + it := d.NewIterator(nil, start) + defer it.Release() + + var ( + count int + buff = crypto.NewKeccakState() + ) + for it.Next() && bytes.Compare(end, it.Key()) > 0 { + // Prevent deletion for trie nodes in hash mode + if h := crypto.HashData(buff, it.Value()); h == common.BytesToHash(it.Key()) { + continue + } + count++ + if count > 10000 { // should not block for more than a second + if err := batch.Write(); err != nil { + return err + } + return ethdb.ErrTooManyKeys + } + if err := batch.Delete(it.Key()); err != nil { + return err + } + } + return batch.Write() } // NewBatch creates a write-only key-value store that buffers changes to its host