Extend fork choice tests for testing is_data_available logic

[hwwhww]

Address #3194

A proposed test format for testing is_data_available in Deneb fork choice.

New on_block step fields:

{
    block: string           -- the name of the `block_<32-byte-root>.ssz_snappy` file.
                              To execute `on_block(store, block)` with the given attestation.
    blobs: string           -- optional, the name of the `blobs_<32-byte-root>.ssz_snappy` file.
                               The blobs file content is a `List[Blob, MAX_REQUEST_BLOB_SIDECARS]` SSZ object.
    proofs: array of byte48 hex string(s) -- optional, the proofs of blob commitments.
    is_data_available: bool -- optional, default to `true`.
    valid: bool             -- optional, default to `true`.
                               If it's `false`, this execution step is expected to be invalid.
}  

Rules:

• blobs and proofs are new fields from Deneb EIP-4844. These are the expected values from retrieve_blobs_and_proofs() helper inside is_data_available() helper.
• blobs indicates the List[Blob, MAX_REQUEST_BLOB_SIDECARS] SSZ object file name.
• is_data_available is the expected result of is_data_available() helper. It's mainly for debugging.
• If none of these parameters are set, the test runner should assume that is_data_available() helper returns true.

Example 1 the valid case:

- {tick: 0}
- checks:
    time: 0
    head: {slot: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    justified_checkpoint: {epoch: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    finalized_checkpoint: {epoch: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    proposer_boost_root: '0x0000000000000000000000000000000000000000000000000000000000000000'
- {tick: 12}
- checks:
    time: 12
    head: {slot: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    justified_checkpoint: {epoch: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    finalized_checkpoint: {epoch: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    proposer_boost_root: '0x0000000000000000000000000000000000000000000000000000000000000000'
- block: block_0x35cc92e64d4355cabe5d6cccae81fbcc892df49fb197544143c20e1ad7478b3f
  blobs: blobs_0x35c0675608d8816943ad750344d7ee588eb831e64abb5ad9327f5b5656413205
  proofs: ['0x8e61e79030994035e6d9ffb2eaa473d2916fa5be78bd9e54251f32739e6e6fc46ae25c0aff74db62ba1e2aff9f576ad1']
  is_data_available: true
  valid: true
- checks:
    time: 12
    head: {slot: 1, root: '0x47a6367ff77988c1cf043b1caac9c2af164c458f8eca1db4a28d2a111a35e577'}
    justified_checkpoint: {epoch: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    finalized_checkpoint: {epoch: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    proposer_boost_root: '0x47a6367ff77988c1cf043b1caac9c2af164c458f8eca1db4a28d2a111a35e577'

Example 2, with incorrect proof:

- {tick: 0}
- checks:
    time: 0
    head: {slot: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    justified_checkpoint: {epoch: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    finalized_checkpoint: {epoch: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    proposer_boost_root: '0x0000000000000000000000000000000000000000000000000000000000000000'
- {tick: 12}
- checks:
    time: 12
    head: {slot: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    justified_checkpoint: {epoch: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    finalized_checkpoint: {epoch: 0, root: '0x0c2762bbfa9e3ae0434153569a7821dd56ccdfb1b530176288b53ba833bd1df4'}
    proposer_boost_root: '0x0000000000000000000000000000000000000000000000000000000000000000'
- block: block_0x35cc92e64d4355cabe5d6cccae81fbcc892df49fb197544143c20e1ad7478b3f
  blobs: blobs_0x35c0675608d8816943ad750344d7ee588eb831e64abb5ad9327f5b5656413205
  proofs: ['0xc00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000']
  is_data_available: false
  valid: false

/cc @mkalinin @potuz, I hope to hear your feedback on this new format 🙏

[mkalinin]

head: {slot: 1, root: '0x47a6367ff77988c1cf043b1caac9c2af164c458f8eca1db4a28d2a111a35e577'}

In the first example I would expect the root of the head to be 0x35cc92e64d4355cabe5d6cccae81fbcc892df49fb197544143c20e1ad7478b3f in the end

List[Blob, MAX_REQUEST_BLOB_SIDECARS]

Why would we use MAX_REQUEST_BLOB_SIDECARS instead of MAX_BLOBS_PER_BLOCK?

For is_data_available I can see the following set of values:

• true -- default value, client doesn't run the blobs validation, is_data_available() returns True
• false -- client doesn't run the blobs validation, is_data_available() returns False; i don't know if this is even useful
• undefined -- client runs the validation and obtains is_data_available() return value from the validation

[hwwhww]

@mkalinin

Why would we use MAX_REQUEST_BLOB_SIDECARS instead of MAX_BLOBS_PER_BLOCK?

You're right, MAX_BLOBS_PER_BLOCK makes more sense. 👍
is_data_available is false

For is_data_available I can see the following set of values:

• true -- default value, client doesn't run the blobs validation, is_data_available() returns True
• false -- client doesn't run the blobs validation, is_data_available() returns False; i don't know if this is even useful
• undefined -- client runs the validation and obtains is_data_available() return value from the validation

Sorry, I was not clear. I was thinking about the opposite way:

• true -- client runs the blobs validation, is_data_available() should return True from validation
• false -- client runs the blobs validation, is_data_available() should return False from validation
• undefined -- client doesn't run validation. mocking is_data_available() returns True

[mkalinin]

Sorry, I was not clear. I was thinking about the opposite way:

• true -- client runs the blobs validation, is_data_available() should return True from validation
• false -- client runs the blobs validation, is_data_available() should return False from validation
• undefined -- client doesn't run validation. mocking is_data_available() returns True

IMO, this approach suites better for unit tests for is_data_available() function where a client should check the output of a function itself. Where if we want to test forkchoice we should rather specify the output that this function should return if we're mocking it, or specify if we want a client to run this function with given data.

[potuz]

I am not really following, I think clients should always run the validation so I don't see the need for the field is_data_available. If the blobs are present in the tick, then is_data_available should be equivalent to the validity of the proofs. And if the blobs are not present, then it should be false. I do not think there's need for discerning the scenario in which is_data_available() is true but the proofs don't pass validation or the more bizzare cases where is_data_available can be set to true without blobs for example. I think simply removing ths field and implicitly assuming false if there are no blobs and if there are blobs, assuming true only if they pass validation, that should be more than enough for us.

[mkalinin]

I am not really following, I think clients should always run the validation so I don't see the need for the field is_data_available.

One potential use case that I kept in mind is writing FC tests with no consideration for data availability, but this can be achieved by generating blocks with no blobs in them I guess

[hwwhww]

Thank you for your feedback!

@mkalinin

IMO, this approach suites better for unit tests for is_data_available() function where a client should check the output of a function itself.

Given that we do have verify_blob_kzg_proof_batch unit tests which cover most is_data_available functionality, I think it is more useful to verify if clients implemented it correctly rather than pyspec-only unit tests.

@potuz

If the blobs are present in the tick, then is_data_available should be equivalent to the validity of the proofs. And if the blobs are not present, then it should be false.

I just figured out that we didn’t specify the len(blob_kzg_commitments) == 0 in is_data_available , which we should have. 😱 I will add a patch.

One potential use case that I kept in mind is writing FC tests with no consideration for data availability, but this can be achieved by generating blocks with no blobs in them I guess

Yeah, now I’m inclined to this way, too, for the simplicity.

[djrtwo]

I think clients should always run the validation so I don't see the need for the field is_data_available

Yeah, I think I agree. I will say that in the event we extend the data using DAS, we might consider adding the boolean to dictate the availability instead of including full payloads with the test. But I don't think we need the premature optimization

[hwwhww]

replaced by #3463