Skip to content

Commit

Permalink
eth/protocols/snap: fix snap sync failure on empty storage range (#28…
Browse files Browse the repository at this point in the history
…306)

This change addresses an issue in snap sync, specifically when the entire sync process can be halted due to an encountered empty storage range.

Currently, on the snap sync client side, the response to an empty (partial) storage range is discarded as a non-delivery. However, this response can be a valid response, when the particular range requested does not contain any slots.

For instance, consider a large contract where the entire key space is divided into 16 chunks, and there are no available slots in the last chunk [0xf] -> [end]. When the node receives a request for this particular range, the response includes:

    The proof with origin [0xf]
    A nil storage slot set

If we simply discard this response, the finalization of the last range will be skipped, halting the entire sync process indefinitely. The test case TestSyncWithUnevenStorage can reproduce the scenario described above.

In addition, this change also defines the common variables MaxAddress and MaxHash.
  • Loading branch information
rjl493456442 authored Oct 13, 2023
1 parent 2f66d7c commit 1cb3b6a
Show file tree
Hide file tree
Showing 10 changed files with 128 additions and 40 deletions.
2 changes: 1 addition & 1 deletion cmd/clef/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -1206,7 +1206,7 @@ func GenDoc(ctx *cli.Context) error {
URL: accounts.URL{Path: ".. ignored .."},
},
{
Address: common.HexToAddress("0xffffffffffffffffffffffffffffffffffffffff"),
Address: common.MaxAddress,
},
}})
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/devp2p/internal/ethtest/snap.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ type accRangeTest struct {
func (s *Suite) TestSnapGetAccountRange(t *utesting.T) {
var (
root = s.chain.RootAt(999)
ffHash = common.HexToHash("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
ffHash = common.MaxHash
zero = common.Hash{}
firstKeyMinus1 = common.HexToHash("0x00bf49f440a1cd0527e4d06e2765654c0f56452257516d793a9b8d604dcfdf29")
firstKey = common.HexToHash("0x00bf49f440a1cd0527e4d06e2765654c0f56452257516d793a9b8d604dcfdf2a")
Expand Down Expand Up @@ -125,7 +125,7 @@ type stRangesTest struct {
// TestSnapGetStorageRanges various forms of GetStorageRanges requests.
func (s *Suite) TestSnapGetStorageRanges(t *utesting.T) {
var (
ffHash = common.HexToHash("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
ffHash = common.MaxHash
zero = common.Hash{}
firstKey = common.HexToHash("0x00bf49f440a1cd0527e4d06e2765654c0f56452257516d793a9b8d604dcfdf2a")
secondKey = common.HexToHash("0x09e47cd5056a689e708f22fe1f932709a320518e444f5f7d8d46a3da523d6606")
Expand Down
6 changes: 6 additions & 0 deletions common/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ const (
var (
hashT = reflect.TypeOf(Hash{})
addressT = reflect.TypeOf(Address{})

// MaxAddress represents the maximum possible address value.
MaxAddress = HexToAddress("0xffffffffffffffffffffffffffffffffffffffff")

// MaxHash represents the maximum possible hash value.
MaxHash = HexToHash("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
)

// Hash represents the 32 byte Keccak256 hash of arbitrary data.
Expand Down
2 changes: 1 addition & 1 deletion core/state_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func TestStateProcessorErrors(t *testing.T) {
)

defer blockchain.Stop()
bigNumber := new(big.Int).SetBytes(common.FromHex("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"))
bigNumber := new(big.Int).SetBytes(common.MaxHash.Bytes())
tooBigNumber := new(big.Int).Set(bigNumber)
tooBigNumber.Add(tooBigNumber, common.Big1)
for i, tt := range []struct {
Expand Down
2 changes: 1 addition & 1 deletion eth/protocols/snap/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ func ServiceGetStorageRangesQuery(chain *core.BlockChain, req *GetStorageRangesP
if len(req.Origin) > 0 {
origin, req.Origin = common.BytesToHash(req.Origin), nil
}
var limit = common.HexToHash("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
var limit = common.MaxHash
if len(req.Limit) > 0 {
limit, req.Limit = common.BytesToHash(req.Limit), nil
}
Expand Down
2 changes: 1 addition & 1 deletion eth/protocols/snap/range.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (r *hashRange) End() common.Hash {
// If the end overflows (non divisible range), return a shorter interval
next, overflow := new(uint256.Int).AddOverflow(r.current, r.step)
if overflow {
return common.HexToHash("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
return common.MaxHash
}
return next.SubUint64(next, 1).Bytes32()
}
Expand Down
10 changes: 5 additions & 5 deletions eth/protocols/snap/range_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestHashRanges(t *testing.T) {
common.HexToHash("0x3fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"),
common.HexToHash("0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"),
common.HexToHash("0xbfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"),
common.HexToHash("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"),
common.MaxHash,
},
},
// Split a divisible part of the hash range up into 2 chunks
Expand All @@ -58,7 +58,7 @@ func TestHashRanges(t *testing.T) {
},
ends: []common.Hash{
common.HexToHash("0x8fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"),
common.HexToHash("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"),
common.MaxHash,
},
},
// Split the entire hash range into a non divisible 3 chunks
Expand All @@ -73,7 +73,7 @@ func TestHashRanges(t *testing.T) {
ends: []common.Hash{
common.HexToHash("0x5555555555555555555555555555555555555555555555555555555555555555"),
common.HexToHash("0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab"),
common.HexToHash("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"),
common.MaxHash,
},
},
// Split a part of hash range into a non divisible 3 chunks
Expand All @@ -88,7 +88,7 @@ func TestHashRanges(t *testing.T) {
ends: []common.Hash{
common.HexToHash("0x6aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
common.HexToHash("0xb555555555555555555555555555555555555555555555555555555555555555"),
common.HexToHash("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"),
common.MaxHash,
},
},
// Split a part of hash range into a non divisible 3 chunks, but with a
Expand All @@ -108,7 +108,7 @@ func TestHashRanges(t *testing.T) {
ends: []common.Hash{
common.HexToHash("0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff5"),
common.HexToHash("0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffb"),
common.HexToHash("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"),
common.MaxHash,
},
},
}
Expand Down
13 changes: 10 additions & 3 deletions eth/protocols/snap/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ func (s *Syncer) loadSyncStatus() {
last := common.BigToHash(new(big.Int).Add(next.Big(), step))
if i == accountConcurrency-1 {
// Make sure we don't overflow if the step is not a proper divisor
last = common.HexToHash("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
last = common.MaxHash
}
batch := ethdb.HookedBatch{
Batch: s.db.NewBatch(),
Expand Down Expand Up @@ -1874,7 +1874,7 @@ func (s *Syncer) processAccountResponse(res *accountResponse) {
return
}
// Some accounts are incomplete, leave as is for the storage and contract
// task assigners to pick up and fill.
// task assigners to pick up and fill
}

// processBytecodeResponse integrates an already validated bytecode response
Expand Down Expand Up @@ -2624,7 +2624,7 @@ func (s *Syncer) OnStorage(peer SyncPeer, id uint64, hashes [][]common.Hash, slo
// the requested data. For storage range queries that means the state being
// retrieved was either already pruned remotely, or the peer is not yet
// synced to our head.
if len(hashes) == 0 {
if len(hashes) == 0 && len(proof) == 0 {
logger.Debug("Peer rejected storage request")
s.statelessPeers[peer.ID()] = struct{}{}
s.lock.Unlock()
Expand All @@ -2636,6 +2636,13 @@ func (s *Syncer) OnStorage(peer SyncPeer, id uint64, hashes [][]common.Hash, slo
// Reconstruct the partial tries from the response and verify them
var cont bool

// If a proof was attached while the response is empty, it indicates that the
// requested range specified with 'origin' is empty. Construct an empty state
// response locally to finalize the range.
if len(hashes) == 0 && len(proof) > 0 {
hashes = append(hashes, []common.Hash{})
slots = append(slots, [][]byte{})
}
for i := 0; i < len(hashes); i++ {
// Convert the keys and proofs into an internal format
keys := make([][]byte, len(hashes[i]))
Expand Down
Loading

0 comments on commit 1cb3b6a

Please sign in to comment.