unsafe warp sync #2366
unsafe warp sync #2366
Conversation
Signed-off-by: turuslan <[email protected]>
Signed-off-by: turuslan <[email protected]>
core/network/warp/sync.cpp
Outdated
| while (true) { | ||
| auto m = scale::encode(vote.message, j.round_number, set).value(); | ||
| auto ok = ed25519.verify(vote.signature, m, vote.id); | ||
| if (ok and ok.value()) { | ||
| break; | ||
| } | ||
| ++set; | ||
| } |
There was a problem hiding this comment.
Malicious peer can do DoS...
- needed reasonable limit of
setvalue. - failure on encoding or verifying should break loop immediately with error (does not matter to continue in this case).
There was a problem hiding this comment.
Yes, new protocols should learn from this mistake by storing some redundant information like set id.
Added limit of block number (new set each block).
Signed-off-by: turuslan <[email protected]> # Conflicts: # core/application/app_configuration.hpp # core/application/impl/app_configuration_impl.cpp # core/application/impl/app_configuration_impl.hpp # core/network/warp/sync.cpp
Signed-off-by: turuslan <[email protected]>
Signed-off-by: turuslan <[email protected]>
Signed-off-by: turuslan <[email protected]>
Signed-off-by: turuslan <[email protected]>
Signed-off-by: turuslan <[email protected]>
There was a problem hiding this comment.
Generally approved, please fix the comments.
Would the node stop after sync or continue with full sync when launched with --unsafe-sync-to?
Should it be launched at the same time with --sync Warp? or just --unsafe-sync-to is enough?
Please address these questions in the pr description.
| const GrandpaJustification &j, | ||
| consensus::grandpa::AuthoritySetId set) { | ||
| HasAuthoritySetChange{header}.scheduled.value(); | ||
| SL_INFO(log_, "unsafe, block {}, set {}", header.number, set); |
There was a problem hiding this comment.
Now it looks like developer's debugging printf. Since it has info log level, please reword it to be more friendly - replace "unsafe" with meaningful description - what this log line should tell.
Was it started, accomplished, in progress or something else?
Referenced issues
Description of the Change
Possible Drawbacks