[1.0] Add max_reversible_blocks_allowed(), use in net_plugin to limit sync-fetch-span #609
Conversation
…et_plugin for limiting sync_fetch_span instead of calculating the value in net_plugin
libraries/chain/controller.cpp
Outdated
| int32_t max_reversible_blocks_allowed() const { | ||
| if (conf.max_reversible_blocks == 0) | ||
| return std::numeric_limits<int32_t>::max(); | ||
| return conf.max_reversible_blocks - fork_db_savanna_size(); |
There was a problem hiding this comment.
shouldn't it also return std::numeric_limits<int32_t>::max() when we have a legacy fork_db instead of max_reversible_blocks?
Otherwise in net_plugin you could have (reversible_remaining < sync_fetch_span). I guess maybe it is not a big deal, it just may limit sync_fetch_span which could seem strange.
There was a problem hiding this comment.
Maybe. As is, if max-reversible-blocks is default 3600 it limits sync-fetch-span to 3600. I kind of thought that a good thing. But would be ok with changing it to return MAX_INT32 if legacy fork db. @arhag what do you think?
There was a problem hiding this comment.
Personally, I think std::numeric_limits<int32_t>::max() if more bulletproof, as you don't have to check how the return value is used in net_plugin to know it won't add a limitation.
There was a problem hiding this comment.
I do think it should return the max value from this function if only dealing with the legacy fork db. It is also consistent with your existing comment describing max_reversible_blocks_allowed in controller.hpp.
… legacy fork db
|
Note:start |
Instead of
net_plugincalculating how many reversible blocks are allowed to limitsync-fetch-span, call a method oncontroller. Now logic about which fork database to check is encapsulated to the controller.Resolves #608