Harden block handler (Versioning) #4178
Conversation
sydhds
commented
Jun 30, 2023
sydhds
commented
- document all added functions
- try in sandbox /simulation/labnet
- unit tests on the added/changed features
- make tests compile
- make tests pass
- add logs allowing easy debugging in case the changes caused problems
- if the API has changed, update the API specification
sydhds
force-pushed
the
feature/harden_block_handler_versioning_1
branch
from
f02d2d2 to
4cc60f5
Compare
sydhds
force-pushed
the
feature/harden_block_handler_versioning_1
branch
from
08382e3 to
97a683f
Compare
…m:massalabs/massa into feature/harden_block_handler_versioning_1
| if *entry_value == 0 { | ||
| self.stats.network_version_counters.remove(&removed_version); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if announced_network_version != 0 { |
There was a problem hiding this comment.
didn't we remove the "0" special case ?
There was a problem hiding this comment.
We did in block header but we use it in mip store stats, when we received None we process it like it is nerwork version 0, this simplifies the code
There was a problem hiding this comment.
what if someone announces Some(0) ? I feel like it might simplify things now but in a year or so we will forget that 0 is a special case and make mistakes when modifying the code
There was a problem hiding this comment.
Some(0) should be semantically the same as announcing None, because we can never roll back versions. (We can always introduce a new version that rolls back previous changes, but we can't go from active version of 2 to 0.
There was a problem hiding this comment.
I'm still not a big fan of the "future devs will read the docs and always have in mind that Some(0) is equivalent to 0 when developing" approach, but if you all think it will never cause problems, I'm OK with it, otherwise we should probably change it. Could also be a followup
