go/storage/mkvs: Use cbor.UnmarshalTrusted for internal metadata

[kostko]

No description provided.

[codecov]

Codecov Report

Merging #2800 into master will increase coverage by 0.20%.
The diff coverage is 55.55%.

@@            Coverage Diff             @@
##           master    #2800      +/-   ##
==========================================
+ Coverage   61.78%   61.98%   +0.20%     
==========================================
  Files         386      386              
  Lines       37198    37204       +6     
==========================================
+ Hits        22981    23060      +79     
+ Misses      11184    11093      -91     
- Partials     3033     3051      +18     

Impacted Files	Coverage Δ
go/common/cbor/cbor.go	56.25% <33.33%> (-5.29%)	⬇️
go/storage/mkvs/db/badger/badger.go	52.91% <100.00%> (+0.44%)	⬆️
go/worker/common/host/interface.go	38.46% <0.00%> (-15.39%)	⬇️
go/consensus/tendermint/api/api.go	73.58% <0.00%> (-15.10%)	⬇️
...ompute/txnscheduler/algorithm/batching/batching.go	78.66% <0.00%> (-6.67%)	⬇️
go/runtime/tagindexer/tagindexer.go	68.47% <0.00%> (-4.35%)	⬇️
go/worker/storage/service_external.go	47.31% <0.00%> (-4.31%)	⬇️
go/runtime/transaction/transaction.go	75.90% <0.00%> (-2.41%)	⬇️
go/worker/storage/committee/checkpointer.go	66.37% <0.00%> (-1.77%)	⬇️
...o/consensus/tendermint/apps/scheduler/scheduler.go	71.70% <0.00%> (-1.38%)	⬇️
... and 20 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0396c4c...ffa4115. Read the comment docs.

[ptrus]

Would it make sense adding a test case to cbor_test.go ? hm, i guess it's already tested in the upstream lib, maybe a test case for the storage code that would fail with old code.

[kostko]

hm, i guess it's already tested in the upstream lib, maybe a test case for the storage code that would fail with old code.

Yeah we don't currently have CBOR tests as those are well tested upstream. We could add some basic API tests for CBOR. But for the database case the problem is that we would need to generate huge data sets (e.g., trees with more than 132k updated nodes) to trigger this which would probably slow tests down. I'll try and see how that would look like.

[kostko]

Ok, it's not that bad, 132k elements takes 2 seconds to process, commit and finalize (locally, probably more on CI?) so we can leave this test in.

[pro-wh]

what are the defaults for these on the untrusted decOptions?

[kostko]

The default is 131072. Maybe we should consider making this explicit and/or even changing it (e.g., lowering it) for untrusted settings?

[pro-wh]

eh, maybe a comment if anything

[ridicoulous]

@kostko hello. I have an issue with calling StateToGenesis beacon api: rpc error: code = Internal desc = grpc: failed to unmarshal the received message cbor: exceeded max number of key-value pairs 131072 for CBOR map at block 7188323. How I can explicitly call UnmarshalTrusted method at this point?