Add optional length limit to ProgressiveVariableList

[eserilev]

Add an optional length limit to ProgressiveVariableList

This lets Gloas fields easily enforce their spec count limits

[codecov]

Codecov Report

❌ Patch coverage is 87.34177% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.25%. Comparing base (3110fa0) to head (f865786).

Files with missing lines	Patch %	Lines
src/progressive_variable_list.rs	86.66%	8 Missing ⚠️
src/serde_utils/prog_list_of_hex_fixed_vec.rs	66.66%	1 Missing ⚠️
src/serde_utils/prog_list_of_hex_prog_var_list.rs	91.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #86      +/-   ##
==========================================
+ Coverage   53.37%   59.25%   +5.88%     
==========================================
  Files          18       18              
  Lines         622      675      +53     
==========================================
+ Hits          332      400      +68     
+ Misses        290      275      -15     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[michaelsproul]

huh, I thought progressive lists didn't have any limits?

where are the limits defined?

[eserilev]

they dont have limits. This adds a guard to decoding so that we dont end up decoding a massive object and OOM ourselves

for example MAX_ATTESTATIONS_ELECTRA in blocks, we should fail early decoding a block whose list of attestations exceeds the max

we dont have to necessarily include the guard here, but I thought it might be the cleanest option

this issue was flagged by one of the security researchers here:
https://discord.com/channels/595666850260713488/892088344438255616/1547691356615479377

[michaelsproul]

my understanding was that we'd try to enforce bounds one level up at the network later, e.g. don't decode a block larger than X kB

I think we have those limits in LH but they are set absurdly high at the moment due to the payload size. We could probably greatly reduce the cap for blocks

[michaelsproul]

I'm coming around to this change, I think we should try to put the length limit checks as "low" and early as possible.

I tried putting them into the decode for BeaconBlock, but even that feels a bit too high because we sometimes construct BeaconBlocks without SSZ decoding (e.g. during proposal, or receiving a JSON BeaconBlock over HTTP).

[michaelsproul]

Impl looks solid, I think this is the way to go.

I'll try cooking up a Lighthouse branch to use these changes.

[michaelsproul]

rebased on main

[michaelsproul]

@eserilev Why did you revert the early failures in Deserialize/ContextDeserialize? (this commit: 31f9025)

They seemed OK to me, did you just want to save on complexity?

[michaelsproul]

I checked all your commits and they look good, so I'm happy to merge this.