nethermind
nethermind copied to clipboard
Deprecate FastBlock Flag
Fixes: #6295
Changes
- Replace FastBlocks with FastSync
- Marks FastBlocks as deprecated on SyncConfig Interface
- Removes FastBlocks from config files
- Updates ParallelSync tests to accommodate for FastHeaders being synced before testing.
- Removes commented out tests
Types of changes
What types of changes does your code introduce?
- [x] Bugfix (a non-breaking change that fixes an issue)
- [ ] New feature (a non-breaking change that adds functionality)
- [ ] Breaking change (a change that causes existing functionality not to work as expected)
- [ ] Optimization
- [x] Refactoring
- [x] Documentation update
- [ ] Build-related changes
- [ ] Other: Description
Testing
Requires testing
- [x] Yes
- [ ] No
If yes, did you write tests?
- [x] Yes
- [ ] No
Notes on testing
Some tests where updated to remove FastBlocks, and some parallel sync tests where updated to assume FastHeaders have already been synced before testing.
Documentation
Enabling fast sync now automatically enables fast blocks. Since fast blocks depends on fast sync it is no longer needed as a configuration option.
Requires documentation update
- [x] Yes
- [ ] No
Requires explanation in Release Notes
- [x] Yes
- [ ] No
Documentation and release notes should indicate FastBlocks has been deprecated. Although it would be ignored if added, currently adding it to config would not break setups.
Remarks
During a set of tests by @marcindsobczak it was noticed that some Syncing tests faild with the error message Unable to find beacon header at height 1. This is unexpected, forcing a new beacon sync
.
The reason for these errors were FastBlocks being false when FastSync was set to true. Given that that case is hardly ever if ever at all used. It was decided its better to remove FastBlocks completed, deprecating it and replacing its use in logic to the value of FastSync.
This fix is the first phase of fully deprecating FastBlocks
ignore this, I wanted to see all tests that would fail in the case if fastsync was forced. Can't do that locally due to system dependent flaky tests.
I see - so the test needed to change
@obasekiosa Have you synced one node with one-click to confirm that code works correctly? If you don't know how - let me know! :)
I don't know how could you walk me through the overview, left a dm.
Don't know if it is connected, but with 1.26 on MainNet I'm getting
Incorrect config settings found: ConfigType:JsonConfigFile|Category:SyncConfig|Name:FastBlocks
And if I check the mainnet.cfg of the 1.26 I have
"FastBlocks": true,
Don't know if it is connected, but with 1.26 on MainNet I'm getting
Incorrect config settings found: ConfigType:JsonConfigFile|Category:SyncConfig|Name:FastBlocks
And if I check the mainnet.cfg of the 1.26 I have
"FastBlocks": true,
Fast block was deprecated, but it being present in the config shouldn't raise an error. A warning yes but not an error. it's left there to allow for smooth transition, but it's not being used.