-
Notifications
You must be signed in to change notification settings - Fork 719
fix(sync): record snap progress per backend #12857
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
base: master
Are you sure you want to change the base?
Changes from all commits
036636b
55d024b
efb5294
ac657da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| // SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited | ||
| // SPDX-License-Identifier: LGPL-3.0-only | ||
|
|
||
| using Nethermind.Core.Crypto; | ||
| using Nethermind.Core.Test; | ||
| using Nethermind.Logging; | ||
| using Nethermind.Synchronization.SnapSync; | ||
| using Nethermind.Trie; | ||
| using NUnit.Framework; | ||
|
|
||
| namespace Nethermind.Synchronization.Test.SnapSync; | ||
|
|
||
| [TestFixture] | ||
| public class PatriciaSnapTrieFactoryTests | ||
| { | ||
| // Pinned: databases written by earlier versions are read back with this key and value. | ||
| private static readonly byte[] AccountProgressKey = "AccountProgressKey"u8.ToArray(); | ||
|
|
||
| [Test] | ||
| public void Records_finished_range_phase_in_the_state_db() | ||
| { | ||
| TestMemDb stateDb = new(); | ||
| PatriciaSnapTrieFactory factory = new(new NodeStorage(stateDb), stateDb, LimboLogs.Instance); | ||
| Assert.That(factory.IsRangePhaseFinished(), Is.False); | ||
|
|
||
| factory.MarkRangePhaseFinished(); | ||
|
|
||
| using (Assert.EnterMultipleScope()) | ||
| { | ||
| Assert.That(stateDb[AccountProgressKey], Is.EqualTo(Keccak.MaxValue.BytesToArray())); | ||
| Assert.That(stateDb.WasFlushed, Is.True); | ||
| } | ||
| } | ||
|
|
||
| // The flag shares the state DB with the trie nodes it describes, so neither outlives the other. | ||
| [Test] | ||
| public void Reads_back_a_range_phase_finished_by_an_earlier_run() | ||
| { | ||
| TestMemDb stateDb = new(); | ||
| PatriciaSnapTrieFactory before = new(new NodeStorage(stateDb), stateDb, LimboLogs.Instance); | ||
| before.MarkRangePhaseFinished(); | ||
|
|
||
| PatriciaSnapTrieFactory afterRestart = new(new NodeStorage(stateDb), stateDb, LimboLogs.Instance); | ||
|
|
||
| Assert.That(afterRestart.IsRangePhaseFinished(), Is.True); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,4 +14,9 @@ void FinalizeSync() { } | |
|
|
||
| ISnapTree<PathWithAccount> CreateStateTree(); | ||
| ISnapTree<PathWithStorageSlot> CreateStorageTree(in ValueHash256 accountPath); | ||
|
|
||
| // Marked when the range phase drains, read after EnsureInitialize, so a later run over the same data | ||
| // skips the phase. Only a backend that keeps its store across runs can report true. | ||
| bool IsRangePhaseFinished(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] Required progress members break existing backend implementations ISnapTrieFactory is public, and the two new members have no default bodies even though its existing lifecycle members do. An out-of-tree state backend compiled against the previous contract lacks these methods: rebuilding fails, while an existing binary cannot service the LoadProgress and completion calls when snap sync starts. Default false and no-op semantics would preserve prior behavior for implementations that do not persist progress. |
||
| void MarkRangePhaseFinished(); | ||
|
batrr marked this conversation as resolved.
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MEDIUM] Legacy flat progress markers still select the Patricia backend
Making the flat marker write a no-op only prevents markers created by this version. If a flat node completed the range phase on an older build and stopped during healing while its flat state is still PreGenesis, its state DB already contains AccountProgressKey; FlatStateActivationPolicy runs before this factory and treats any key there as Patricia state. The upgraded node therefore selects Patricia and never reaches this no-op, reproducing the reported restart failure for existing on-disk state. Compatibility handling for the legacy marker would cover this upgrade path.