Skip to content

config: merge RemoveUntraceableHeaders into RemoveUntraceableBlocks #3922

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

Merged
merged 16 commits into from
Jul 3, 2025

Conversation

AnnaShaleva
Copy link
Member

@AnnaShaleva AnnaShaleva commented Jun 2, 2025

Close #3920, close #3795.

TODO:

  • Fix failing tests.
  • Write new tests.
  • Test on real network.
  • Refactor state sync initialisation with trusted header in case if chain's height is too low.
  • Decide if we're OK with extra headers downloading.
  • Fix infinite statesync-bound NeoFSBlockFetcher synchronization (in Blocks mode).
  • Properly handle HeaderHashes initialization on node restart after statesync interruption.
  • Fix block verification in case if header is already added to the chain.
  • Request trusted header prior to synchronisation start.

@AnnaShaleva AnnaShaleva requested a review from roman-khimov as a code owner June 2, 2025 18:15
@AnnaShaleva AnnaShaleva marked this pull request as draft June 2, 2025 18:16
@AnnaShaleva
Copy link
Member Author

Some of statesync module tests are failing due to #3795, will include this issue into this PR, we'll need to implement it anyway soon.

@AnnaShaleva AnnaShaleva changed the title config: merge RemuveUntraceableHeaders into RemoveUntraceableBlocks config: merge RemoveUntraceableHeaders into RemoveUntraceableBlocks Jun 3, 2025
@AnnaShaleva
Copy link
Member Author

@roman-khimov, it's still a draft, but you may review the idea of the last commit, we need to discuss this solution. An alternative (presize) solution messes up the code of statesync module.

Copy link
Member

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

Looks like #3795 part of it is a bit premature.

@AnnaShaleva
Copy link
Member Author

Not yet ready for review, we still need to request trusted block to properly verify next headers.

@AnnaShaleva
Copy link
Member Author

A lot of tests are still failing due to changes related to untraceable blocks removal, so still in the process of fixing them. The main logic is implemented and may be reviewed.

@AnnaShaleva AnnaShaleva force-pushed the ruh branch 11 times, most recently from 10f918a to 802c342 Compare June 26, 2025 13:18
@AnnaShaleva
Copy link
Member Author

@roman-khimov, let's perform some initial review. One problem marked with TODO is left and some new tests are still needed. All existing tests are finally fixed.

Copy link

codecov bot commented Jun 26, 2025

Codecov Report

Attention: Patch coverage is 79.87616% with 65 lines in your changes missing coverage. Please review.

Project coverage is 82.31%. Comparing base (c4d79a2) to head (0a86003).
Report is 21 commits behind head on master.

Files with missing lines Patch % Lines
pkg/core/blockchain.go 77.18% 23 Missing and 11 partials ⚠️
pkg/core/statesync/module.go 30.00% 5 Missing and 2 partials ⚠️
pkg/core/headerhashes.go 92.06% 3 Missing and 2 partials ⚠️
pkg/network/bqueue/operationmode_string.go 0.00% 5 Missing ⚠️
pkg/network/bqueue/queue.go 0.00% 4 Missing ⚠️
pkg/config/ledger_config.go 89.65% 2 Missing and 1 partial ⚠️
pkg/util/uint256.go 75.00% 2 Missing and 1 partial ⚠️
pkg/core/dao/dao.go 75.00% 2 Missing ⚠️
cli/smartcontract/permission.go 50.00% 0 Missing and 1 partial ⚠️
pkg/config/genesis_extensions.go 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3922      +/-   ##
==========================================
- Coverage   82.36%   82.31%   -0.06%     
==========================================
  Files         345      347       +2     
  Lines       48773    48965     +192     
==========================================
+ Hits        40173    40305     +132     
- Misses       6924     6974      +50     
- Partials     1676     1686      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AnnaShaleva AnnaShaleva force-pushed the ruh branch 2 times, most recently from 004e646 to 6cbafb5 Compare June 27, 2025 16:54
Close #3920.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
1. Optimize it a bit by searching from the first suitable batch of
   headers at the DB Search level.
2. Rename it since this method removes the head of header hashes (we'll
   have code that removes tail of it soon).

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Early stop is not hard to support and it will be useful for untraceable
header hashes removal.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Use `cont` instead of `f` for processing function, otherwise it's not
clear what is the return value meaning from the caller's side.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Light nodes synchronized over NeoFSStateExchange won't have these data
anyway, hence it would be nice to have unified behaviour for all light
nodes. Old header hashes are not used anyway.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
@AnnaShaleva AnnaShaleva force-pushed the ruh branch 5 times, most recently from f170b8f to 55a5669 Compare July 2, 2025 12:00
@AnnaShaleva
Copy link
Member Author

AnnaShaleva commented Jul 2, 2025

@roman-khimov just for the record, GH tests are fixed (there was a bug actually, but it revealed only on slow machines), various manual node synchronization scenarios are checked, untraceable blocks removal check is in progress. Ready for review.

@AnnaShaleva
Copy link
Member Author

Untraceable blocks removal on my machine with LevelDB and default GCP takes seconds (4-8 seconds) on syncing mainnet node. I'd say it's acceptable, especially comparing with MPT GC:

2025-07-02T18:02:09.497+0300	INFO	starting transfer data garbage collection	{"index": 1340000}
2025-07-02T18:02:09.497+0300	INFO	starting MPT garbage collection	{"index": 1340000}
2025-07-02T18:03:17.940+0300	INFO	finished MPT garbage collection	{"removed": 757944, "kept": 81143269, "time": "1m8.442434137s"}
2025-07-02T18:03:17.940+0300	INFO	starting untraceable blocks garbage collection	{"from": 1320000, "to": 1340000}
2025-07-02T18:03:26.219+0300	INFO	finished untraceable blocks garbage collection	{"removed": 20000, "time": "8.265136171s"}
2025-07-02T18:03:26.219+0300	INFO	starting header hashes garbage collection	{"index": 1340000}
2025-07-02T18:03:26.238+0300	INFO	finished header hashes garbage collection	{"removed": 20000, "time": "19.103702ms"}
2025-07-02T18:03:31.778+0300	INFO	starting transfer data garbage collection	{"index": 1360000}
2025-07-02T18:03:31.778+0300	INFO	starting MPT garbage collection	{"index": 1360000}
2025-07-02T18:04:26.559+0300	INFO	finished MPT garbage collection	{"removed": 1685056, "kept": 80061977, "time": "54.780080204s"}
2025-07-02T18:04:26.559+0300	INFO	starting untraceable blocks garbage collection	{"from": 1340000, "to": 1360000}
2025-07-02T18:04:33.768+0300	INFO	finished untraceable blocks garbage collection	{"removed": 20000, "time": "7.186264278s"}
2025-07-02T18:04:33.768+0300	INFO	starting header hashes garbage collection	{"index": 1360000}
2025-07-02T18:04:33.785+0300	INFO	finished header hashes garbage collection	{"removed": 20000, "time": "17.466238ms"}
2025-07-02T18:04:38.055+0300	INFO	starting transfer data garbage collection	{"index": 1380000}
2025-07-02T18:04:38.055+0300	INFO	starting MPT garbage collection	{"index": 1380000}
2025-07-02T18:05:53.764+0300	INFO	finished MPT garbage collection	{"removed": 660900, "kept": 79893957, "time": "1m15.708847905s"}
2025-07-02T18:05:53.764+0300	INFO	starting untraceable blocks garbage collection	{"from": 1360000, "to": 1380000}
2025-07-02T18:05:59.504+0300	INFO	finished untraceable blocks garbage collection	{"removed": 20000, "time": "5.729297873s"}
2025-07-02T18:05:59.504+0300	INFO	starting header hashes garbage collection	{"index": 1380000}
2025-07-02T18:05:59.524+0300	INFO	finished header hashes garbage collection	{"removed": 20000, "time": "19.528944ms"}
2025-07-02T18:06:05.581+0300	INFO	starting transfer data garbage collection	{"index": 1400000}
2025-07-02T18:06:05.581+0300	INFO	starting MPT garbage collection	{"index": 1400000}
2025-07-02T18:07:27.426+0300	INFO	finished MPT garbage collection	{"removed": 559072, "kept": 79929146, "time": "1m21.844744804s"}
2025-07-02T18:07:27.426+0300	INFO	starting untraceable blocks garbage collection	{"from": 1380000, "to": 1400000}
2025-07-02T18:07:33.072+0300	INFO	finished untraceable blocks garbage collection	{"removed": 20000, "time": "5.63531087s"}
2025-07-02T18:07:33.072+0300	INFO	starting header hashes garbage collection	{"index": 1400000}
2025-07-02T18:07:33.084+0300	INFO	finished header hashes garbage collection	{"removed": 20000, "time": "12.119894ms"}
2025-07-02T18:07:37.876+0300	INFO	starting transfer data garbage collection	{"index": 1420000}
2025-07-02T18:07:37.876+0300	INFO	starting MPT garbage collection	{"index": 1420000}
2025-07-02T18:08:41.905+0300	INFO	finished MPT garbage collection	{"removed": 578916, "kept": 79920626, "time": "1m4.028915569s"}
2025-07-02T18:08:41.905+0300	INFO	starting untraceable blocks garbage collection	{"from": 1400000, "to": 1420000}
2025-07-02T18:08:50.020+0300	INFO	finished untraceable blocks garbage collection	{"removed": 20000, "time": "8.103554631s"}
2025-07-02T18:08:50.020+0300	INFO	starting header hashes garbage collection	{"index": 1420000}
2025-07-02T18:08:50.036+0300	INFO	finished header hashes garbage collection	{"removed": 20000, "time": "15.226864ms"}
2025-07-02T18:08:54.781+0300	INFO	starting transfer data garbage collection	{"index": 1430000}
2025-07-02T18:08:54.781+0300	INFO	starting MPT garbage collection	{"index": 1430000}
2025-07-02T18:09:46.581+0300	INFO	finished MPT garbage collection	{"removed": 343995, "kept": 80138636, "time": "51.800655758s"}
2025-07-02T18:09:46.582+0300	INFO	starting untraceable blocks garbage collection	{"from": 1420000, "to": 1430000}
2025-07-02T18:09:50.672+0300	INFO	finished untraceable blocks garbage collection	{"removed": 10000, "time": "4.085976939s"}
2025-07-02T18:09:50.672+0300	INFO	starting header hashes garbage collection	{"index": 1430000}
2025-07-02T18:09:50.689+0300	INFO	finished header hashes garbage collection	{"removed": 10000, "time": "16.616925ms"}

@@ -177,6 +177,12 @@ func (s *Module) Init(currChainHeight uint32) error {
return nil
}
if s.bc.BlockHeight() > p-2*s.syncInterval {
trustedH := s.bc.GetConfig().TrustedHeader.Index
if trustedH > s.bc.BlockHeight() {
Copy link
Member

Choose a reason for hiding this comment

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

It's a pretty narrow case (s.bc.BlockHeight() > p-2*s.syncInterval), the node is almost in sync, so to me it should just work. I also doubt we can we end up here without changing TrustedHeight in the config (from the one originally used). Anyway, if the height is good and trusted (and local DB is trusted by definition) we better just keep going.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Should be a part of #2085, discovered during digging into the problem of
inlined structures, ref. go-yaml/yaml#125.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
If batch of headers consists of stale headers only, then don't try to add any
of them. Originally introduced by c3e73c5.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Do not request the whole batch of headers if syncing via
P2PStateExchangeExtensions or via NeoFSStateSyncExtensions. Instead,
request headers starting from some trusted height.

Introducing trusted header allows to enable tests related to
untraceable blocks/headers removal.

Close #3795.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Use LE to log header hash.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
No functional changes.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
No functional changes.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
If corresponding header was stored in the DB we need to verify incoming
block hash against this header. Otherwise, we may end up in a corrupted
DB and malicious block saved to the database.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
When submitting block, we need to distinguish if it already exists on
the chain or if it's a block from future.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
@AnnaShaleva AnnaShaleva requested a review from roman-khimov July 3, 2025 06:40
@roman-khimov roman-khimov merged commit 9713b6f into master Jul 3, 2025
31 of 34 checks passed
@roman-khimov roman-khimov deleted the ruh branch July 3, 2025 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Squash RemoveUntraceableHeaders and RemoveUntraceableBlocks settings Integrate StateSync module with RemoveUntraceableHeaders option
2 participants