StorageDumper: make dump format compatible with StatesDumper
Description
Problem:
https://github.com/neo-project/neo/pull/3281 removes StatesDumper plugin, however, the output format of state dump files generated by StorageDumper is incompatible with StatesDumper format. We have a set of existing tools that rely on output format of StatesDumper.
Note that it was said in https://github.com/neo-project/neo/issues/3271#issuecomment-2126845828 that:
we have some tooling that uses StorageDumper output.
But it's not true, because all our tools use the output of StatesDumper actually.
Solution:
Keep the StatesDumper output format compatible with StorageDumper.
Type of change
- [X] Bug fix (non-breaking change which fixes an issue)
How Has This Been Tested?
- [X] Node synchronisation and resulting dump formats comparison.
Checklist:
- [X] My code follows the style guidelines of this project
- [X] I have performed a self-review of my code
- [X] I have commented my code, particularly in hard-to-understand areas
- [X] I have made corresponding changes to the documentation
- [X] My changes generate no new warnings
- [X] I have added tests that prove my fix is effective or that my feature works
- [X] New and existing unit tests pass locally with my changes
- [X] Any dependent changes have been merged and published in downstream modules
We can adapt our tools, but check with NGD as well.
@NGDAdmin @superboyiii
I strongly dislike this PR. I rarely say sth like this, but This PR is a real BUG. StorageDumper is StorageDumper, it's not StatesDumper. We remove StatesDumper because it has bug and we have a better choice->StorageDumper. Now we are going back... StorageDumper has clear format, each block data in a row, easy to read and compare. And it will not lose data when CLI is interrupted. But now see what we're doing... The bug is back again because some tool is not compatible... I don't understand why A PLUGIN has to be compatible with tools? Shouldn't tools adapt for plugin?
@AnnaShaleva You can try to install this Plugin and resync data. Randomly interrupt CLI for some times and export these dump data. You will see lots of data are lost.
I don't understand why A PLUGIN has to be compatible with tools? Shouldn't tools adapt for plugin?
It's an API/format that is publicly available, so it's not that easy. Take RPC for example, it's a plugin, but this doesn't mean you can change its behavior easily, a lot of other code depends on it. This one of course is not that popular, but still.
Now if there is a real bug with the format (but I'd check if it's a format problem rather than implementation) we can consider switching to StatesDumper one, but IIRC it's not perfect either. This one is not release critical, we can solve it after 3.8 in any direction.
I don't understand why A PLUGIN has to be compatible with tools? Shouldn't tools adapt for plugin?
It's an API/format that is publicly available, so it's not that easy. Take RPC for example, it's a plugin, but this doesn't mean you can change its behavior easily, a lot of other code depends on it. This one of course is not that popular, but still.
Now if there is a real bug with the format (but I'd check if it's a format problem rather than implementation) we can consider switching to
StatesDumperone, but IIRC it's not perfect either. This one is not release critical, we can solve it after 3.8 in any direction.
At least we shouldn't bring bug back. Or I think you need is just making StatesDumper back. Then just revert it but don't change StorageDumper.
Will cose if no further update in this pr.
@AnnaShaleva please move this PR to neo-node repository