How do we ensure new consensus field are well tested

[terencechain]

One takeaway from #12263 is when adding a new consensus (especially List) field to a consensus object, it gets inherently riskier in the Prysm code. This is due to Go initializing nil as 0 or empty. Too many places where we cast to the underlying types and copy everything field-by-field. This means the author has to be careful to initialize the new field correctly or the reviewer must review carefully and ensure the unit test has satisfactory coverage. Both are hard to enforce. I'm open to ideas on how to enforce such behavior better. Some ideas I thought about

• Static analysis. This behavior primarily exhibits from protobuf object conversion like converting blind to non blind or the other way around
• Better test setup. Have util.NewBlock function to return a non-empty block

[rauljordan]

One approach is we could encapsulate this information in how a beacon block is initialized and built. For example, in Rust we use a pattern called the builder pattern which works as follows:

let block = BeaconBlock::new()
  .deposit(deposit_items)
  .voluntary_exits(exits)
  .attestations(atts)
  ...
  .build()?

The idea is that we have a function called BeaconBlock::new() which gives you something called a BeaconBlockHandle, not an actual beacon block. This handle has methods to update its fields, such as .SetBLSChanges(changes), or SetAttestations(atts). To turn that handle into a beacon block, one has to call a .Build() method. One proposal is this build method can use the reflect package to inspect all necessary fields have been set based on the block type and that they have the correct defaults. This could be the canonical way of initializing a block and has an ergonomic, functional API.

With this approach, one must call .Build() method to get a block as that is the only way to initialize one. If we forget to add an important field even in a single place, the code will fail to run. This reduces the surface area of where we need to audit the codebase to a single method, the .Build() method.

The #12240 PR is an example of the current status, where we will need to remember to update that function in a future hard fork once fields are added and could lead to another silent failure affecting mainnet if we are not careful.