blockchain: rename to blocksync service#6755
Conversation
|
I honestly feel like changing fast-sync nomenclature will be too much. It's already so engrained in so many things. Can we stick with just renaming the reactor/service for now? |
|
Tend to agree with renaming of blockchain to blocksync. Ideally, we just rename what is "fast sync" to "block sync". This may only be docs |
|
When I was making the change I also asked myself whether renaming fast sync would be too much. I do agree that it is engrained in the minds of us as developers and many members of the wider community and it will continue to be called fast sync long after this gets merged. I do however think that having consistent terminology throughout the repo slightly outweighs this and I've tried to be very thorough about it (with the exception of the |
I am onboard with the rename, but not with renaming blockchain to blocksync. Blockchain is a generlized pkg for serving blocks, we just happen to have fast/block sync in it. If we pull out fast sync from blockchain this would be simpler. Also this PR doesnt solve #4594? |
But I don't think it is a generalized pkg for serving blocks. Its purely for "block sync". It's not linked with the RPC, which serves blocks by directly connecting to the blockstore (and the blockstore is its own package). You can also (if you really wanted to) use consensus to fetch blocks. Note: when I say "Its purely for "block sync"." - this is an opinion not a fact |
Codecov Report
@@ Coverage Diff @@
## master #6755 +/- ##
==========================================
+ Coverage 62.43% 62.45% +0.01%
==========================================
Files 306 306
Lines 40384 40384
==========================================
+ Hits 25214 25220 +6
+ Misses 13386 13385 -1
+ Partials 1784 1779 -5
|
This is largely a cherry pick of #6755 with some additional fixups added where detected. This change moves the blockchain package to a package called blocksync. Additionally, it renames the relevant uses of the term `fastsync` to `blocksync`. closes: #9227 #### PR checklist - [ ] Tests written/updated, or no tests needed - [x] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [x] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed
This is largely a cherry pick of #6755 with some additional fixups added where detected. This change moves the blockchain package to a package called blocksync. Additionally, it renames the relevant uses of the term `fastsync` to `blocksync`. closes: #9227 #### PR checklist - [ ] Tests written/updated, or no tests needed - [x] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [x] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed
Closes: #6751, #4594
I have tried to rename both the blockchain package and service as well as the term fast sync to the single term block sync. There are some exceptions. In all ADR's, the term of fast sync is left as is. In the config, in order to avoid a breaking change,
FastSyncConfigremains the same. Do we want to change this and thus break the config file (we might have already broken it with the privVal changes)?