validation spawner only recieves one validation binary #2505
Conversation
cla-bot
bot
added
the
s
|
Are we sure we want to go this route? I thought we'd previously talked about potentially running nitro-val instances on mixed architectures. Perhaps we could allow configuring multiple architectures for the validator, and the default would be sending all of them? |
Good idea. |
staker/block_validator.go
Outdated
| validatorValidationWaitToRecordHist = metrics.NewRegisteredHistogram("arb/validator/validations/waitToRecord", nil, metrics.NewBoundedHistogramSample()) | ||
| validatorValidationRecordingHist = metrics.NewRegisteredHistogram("arb/validator/validations/recording", nil, metrics.NewBoundedHistogramSample()) | ||
| validatorValidationWaitToLaunchHist = metrics.NewRegisteredHistogram("arb/validator/validations/waitToRun", nil, metrics.NewBoundedHistogramSample()) | ||
| validatorValidationLaunchHist = metrics.NewRegisteredHistogram("arb/validator/validations/waitToRun", nil, metrics.NewBoundedHistogramSample()) |
There was a problem hiding this comment.
here the metric name needs to be fixed
How about naming it .../validations/launch and previous one .../validations/waitToLaunch to match the variable names?
Alternatively, we could rename the metrics variables to match the names
validator/server_api/json.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| uncompressed, err := arbcompress.Decompress(decoded, 30000000) |
There was a problem hiding this comment.
do we have a guarantee that the wasm won't exceed the 30000000 bytes? currently the uncompressed data size is not checked before compression in ValidationInputToJson -- should we also check the limit there?
nit: we might want to use some named constant for the max size 30000000 (?)
| if err != nil { | ||
| continue | ||
| } | ||
| archWasms[moduleHash] = base64.StdEncoding.EncodeToString(compressed) |
There was a problem hiding this comment.
At some point it might be worth doing something like *jsonapi.PreimagesMapJson to optimize this and avoid extra copying but that probably belongs in a separate PR. As-is it's still more efficient than what we previously had.
| maxSize := 2_000_000 | ||
| var uncompressed []byte | ||
| for { | ||
| uncompressed, err = arbcompress.Decompress(decoded, maxSize) |
There was a problem hiding this comment.
Perhaps we should use a go brotli library for this instead, since it's not in consensus and would have a nicer API? Or alternatively we could add a new decompression stream API to this.
There was a problem hiding this comment.
forgot to address this:
I think we will lose more from having multiple implementations of brotli than we'll gain from a better API in this specific case.
Also, I hope API will improve after wasmer answer and provide a reasonable upper-bound.
This prepares some work towards multi-arch support, but mostly reduces validation entry size