Handroll validation input preimages JSON marshal/unmarshal #1734
Conversation
cla-bot
bot
added
the
s
validator/server_api/json.go
Outdated
|
|
||
| "github.com/ethereum/go-ethereum/common" | ||
|
|
||
| "github.com/offchainlabs/nitro/validator" | ||
| ) | ||
|
|
||
| type PreimagesMapJson struct { |
There was a problem hiding this comment.
the same will also be used in execution-api. Let's put it somewhere else, like under /util/, to be used by both.
There was a problem hiding this comment.
I've moved validator/server_api/json.go to util/jsonapi/validation.go. Let me know if I should do the split differently.
There was a problem hiding this comment.
I think only PreimagesMapJson should be there. This will be used by multiple APIs, but the rest of this file seems validation-api specific.
validator/server_api/json.go
Outdated
| encoding := base64.StdEncoding | ||
| for key, value := range m.Map { | ||
| size += 5 // "":"" | ||
| size += encoding.EncodedLen(len(key)) |
There was a problem hiding this comment.
can have a var/const that has the encodelen of a common.Hash and use it instead of calling EncodeLen over and over.
Also - since len of comas is calculated outside this loop - adding 5 + encLen(32) per item outside this loop makes sense
There was a problem hiding this comment.
I've moved the calculation out of the loop, but I don't think we need to pull encoding.EncodedLen(32) out into a global because it's just (n + 2) / 3 * 4
validator/server_api/json.go
Outdated
| if maxKeyLen > len(buf) { | ||
| return fmt.Errorf("preimage key base64 possible length %v is greater than buffer size of %v", maxKeyLen, len(buf)) | ||
| } | ||
| keyLen, err := encoding.Decode(buf, data[:strLen]) |
There was a problem hiding this comment.
no need to decode into buf.
You can error-out if maxKeyLen > len(key) and decode directly into key[:] avoiding the copy.
There was a problem hiding this comment.
Unfortunately maxKeyLen will necessarily be greater than len(key) because of base64 padding. encoding.DecodedLen will always return a multiple of 3, for keys that'll be 33 bytes which is unfortunately too long to safely decode directly into key with.
I'm not sure this is a good idea but it definitely reduces allocations.