Fix #3267 #3367
Fix #3267 #3367
Conversation
| writer.endRecord() | ||
|
|
||
| ## RestPublishedBeaconBlockBody | ||
| proc readValue*(reader: var JsonReader[RestJson], | ||
| value: var RestPublishedBeaconBlockBody) {. |
There was a problem hiding this comment.
why this new approach based on field members? the approach where we first read the header and use the slot number to enforce a "correct" body seems more accurate (and it matches the algorithm we use for loading SSZ)
There was a problem hiding this comment.
publishBlock call do not accept any other encoding except application/json. So we can't reuse slot number approach here like we made for SSZ. Also it will decode json using single pass. Before this PR we have used N passes (depends on number of forks).
There was a problem hiding this comment.
alright - we'll see for how long we can sustain this strategy - a body is not guaranteed to change between forks, so at some point it might lead to ambiguity - the slot is the only sure way of telling what type is expected
There was a problem hiding this comment.
I know that i have removed slot check and i can actually restore it, but after deserialization. The specification is not very clear here.
There was a problem hiding this comment.
it's probably a good idea to have it here, yes - the block would sooner or later be thrown out anyway (because it will not match the fork of the state), but it'll be easier to diagnose errors if they're caught here
Enable bellatrix blocks support.
Refactor publishBlock algorithm for json block deserialization.