Add optimistic sync tests by hwwhww · Pull Request #2982 · ethereum/consensus-specs

hwwhww · 2022-08-23T18:00:19Z

Issue

Address ethereum/consensus-spec-tests#28

Description

Test format: #2965

Example

test_from_syncing_to_invalid is the basic test from ethereum/consensus-spec-tests#28 issue description:

B0 <- B1 <- B2 <- B3 
  \
    <- B1' <- B2' <- B3' <- B4'

1. Client imports [B0 ... B3], payload_status: {status: "VALID", ...} for each block in this branch
2. Client reorgs to and optimistically imports [B1' ... B3'], payload_status: {status: "SYNCING", ...} for each of this blocks
3. Client imports B4' with payload_status: {status: "INVALID", latestValidHash: B0.body.execution_payload.block_hash, ...}
4. head == B3

`minimal/bellatrix/sync/optimistic/pyspec_tests/from_syncing_to_invalid/steps.yaml` (20220908 fixed)

- {tick: 7728}
- block_hash: '0xb9a07089b65517ade3874fc891cc6206dfe5e753ac3bfb8dbf2cf0fe514b2a98'
  payload_status: {status: VALID, latest_valid_hash: '0xb9a07089b65517ade3874fc891cc6206dfe5e753ac3bfb8dbf2cf0fe514b2a98',
    validation_error: null}
- {block: block_0x3d1241fb9fa6d442d45e1f2153abdb1158cfe90f273978b6212b80d15053d1d8}
- checks:
    head: {slot: 9, root: '0x30b495d00fb542d2c513a464bba858295b884ab2451953e5876804a049afc2ef'}
- block_hash: '0xcfa86982c4ca3a0c429d223ee3000f64305e7bbb6ed57f12748a3ca1bb73e2ff'
  payload_status: {status: VALID, latest_valid_hash: '0xcfa86982c4ca3a0c429d223ee3000f64305e7bbb6ed57f12748a3ca1bb73e2ff',
    validation_error: null}
- {block: block_0x2a06b3082961d053d0b59c1fd232b298acc0353cdd58480c0d2dab16403f85fb}
- checks:
    head: {slot: 10, root: '0x1486942d4953830f9bbf43141838db41686edc43ca870d5b22c500e5e0c0898b'}
- block_hash: '0xb051302363f1bdb6ef94a54cee5958539aedda44229baaf1dae3e8db08baa5de'
  payload_status: {status: VALID, latest_valid_hash: '0xb051302363f1bdb6ef94a54cee5958539aedda44229baaf1dae3e8db08baa5de',
    validation_error: null}
- {block: block_0xeb2300a9028f0dc91cd026d74010f7a0122a48984d2b207071e7015698cf7887}
- checks:
    head: {slot: 11, root: '0xfadf15c33151200546709d8bdeed7f00de88e15f93d9ffed48d8c1673f257c52'}
- block_hash: '0x6b5e520e48f6736070911079162157dd87cc473da78b328a5a77d18069d223b2'
  payload_status: {status: VALID, latest_valid_hash: '0x6b5e520e48f6736070911079162157dd87cc473da78b328a5a77d18069d223b2',
    validation_error: null}
- {block: block_0x28cfd6113e88cfdd3903c576bada1f79aa7f3955e2ef30604e63091680a8b625}
- checks:
    head: {slot: 12, root: '0x44155297210f07a144d61d57e239dc1908d08ae42edcee79a9e3d8b803ea0877'}
- block_hash: '0x13d0f34b2c411f286db9f8164333a97e20793ac64313a5f68fb20a7decc1487d'
  payload_status: {status: SYNCING, latest_valid_hash: null, validation_error: null}
- {block: block_0xa1e8cd339e0ac129a0892197507f7d49d731d1a3db08d32036f926ac1dbaf335}
- checks:
    head: {slot: 10, root: '0x3f6acbc5be90c353052aad42117cac2c9577c103adea76d1775fcec45b4c0c72'}
- block_hash: '0xfa8e3d85ee065a9350cd480ac76de90f6d8238d1c93f1f5fb7339d05e280d832'
  payload_status: {status: SYNCING, latest_valid_hash: null, validation_error: null}
- {block: block_0x3c131a495076f7af107cb3a02f84f7c8fff22ccd5d9204e50e95f4c1ed640269}
- checks:
    head: {slot: 11, root: '0x55dd6957eea9d4af18165761974233976808c88b64cbcd9f53328e514f1c5cca'}
- block_hash: '0x3b99e4259065cc75f1c8d658d3c37a4d63cb712a263fb410b93771b109aeb47f'
  payload_status: {status: SYNCING, latest_valid_hash: null, validation_error: null}
- {block: block_0x23aa3b0a1ca807c7de4724944aa46f19bb0530198bf5ac77196f2c877337c65e}
- checks:
    head: {slot: 12, root: '0xe43498e53d60a08019406571637ae1b63122e3b9e2a7e97a2c7378dcacef6c55'}
- block_hash: '0xe874f491ac490606d848657cc80c48f92975820c35d81e9ece632fc3702ac378'
  payload_status: {status: INVALID, latest_valid_hash: '0xb9a07089b65517ade3874fc891cc6206dfe5e753ac3bfb8dbf2cf0fe514b2a98',
    validation_error: invalid}
- {block: block_0x0fd4c324301e908366d2e53a0b9f601ffb4c33f5726939a975a9d4d5fa73a293,
  valid: false}
- checks:
    head: {slot: 12, root: '0x44155297210f07a144d61d57e239dc1908d08ae42edcee79a9e3d8b803ea0877'}

Testing tool implementation

MegaStore: combine fork choice's Store and optimistic sync's OptimisticStore during the test scripts.
It doesn't change fork choice specs and it doesn't mock ExecutionEngine.
In these tests, all the beacon blocks are valid except for the payload which is determined by the payload_status value. i.e., no other state transition exceptions except for notify_new_payload.

Sync test vectors:

20220831 updated
- minimal-sync-20220831.tar.gz
- mainnet-sync-20220831.tar.gz
20220908 updated
- mainnet-sync-20220908.tar.gz
- minimal-sync-20220908.tar.gz

TODOs:

Add more test cases
Check if the results are correct as the optimistic sync spec described
Refactoring
Check with Extend fork_choice test format with on_payload_info #2965

mkalinin · 2022-08-31T10:15:58Z

tests/core/pyspec/eth2spec/test/helpers/optimistic_sync.py

+            el_block_hash = current_block.body.execution_payload.block_hash
+
+    if payload_status.status != PayloadStatusV1Status.VALID:
+        valid = False


Should the block be invalid (valid = False) only when the status does belong to INVALIDATED subset? I believe current logic will produce valid: False output in case when the status is e.g. SYNCING.

Good point! So we may have to set it manually since SYNCING block's validity that we output here will be determined later in optimistic nodes.

I updated it in ac717b1, now it is set by parameter valid from the test script.

mkalinin · 2022-08-31T10:24:22Z

Awesome work! Looks good to me except validity status of optimistic blocks, and the following duplication in the output:

- {block: block_0xa1e8cd339e0ac129a0892197507f7d49d731d1a3db08d32036f926ac1dbaf335,
  valid: false}
- {block: block_0xa1e8cd339e0ac129a0892197507f7d49d731d1a3db08d32036f926ac1dbaf335}

It looks like we have to add an is_payload_valid flag that will specify the case when the block is invalid due to its payload being invalid. CL clients that doesn't support optimistic tests format should ignore this flag and follow the valid one, others will be ignoring valid: false when is_payload_valid: false. This is a thought off the top of my head and doesn't look pretty, it worth to think more on it

hwwhww · 2022-08-31T14:38:21Z

@mkalinin

duplication in the output

Fixed in ac717b1. Thanks!

- {tick: 7728}
- payload_status: {status: PayloadStatusV1Status.VALID, latest_valid_hash: '0xb9a07089b65517ade3874fc891cc6206dfe5e753ac3bfb8dbf2cf0fe514b2a98',
    validation_error: 'null'}
- {block: block_0x3d1241fb9fa6d442d45e1f2153abdb1158cfe90f273978b6212b80d15053d1d8}
- checks:
    head: {slot: 9, root: '0x30b495d00fb542d2c513a464bba858295b884ab2451953e5876804a049afc2ef'}
- payload_status: {status: PayloadStatusV1Status.VALID, latest_valid_hash: '0xcfa86982c4ca3a0c429d223ee3000f64305e7bbb6ed57f12748a3ca1bb73e2ff',
    validation_error: 'null'}
- {block: block_0x2a06b3082961d053d0b59c1fd232b298acc0353cdd58480c0d2dab16403f85fb}
- checks:
    head: {slot: 10, root: '0x1486942d4953830f9bbf43141838db41686edc43ca870d5b22c500e5e0c0898b'}
- payload_status: {status: PayloadStatusV1Status.VALID, latest_valid_hash: '0xb051302363f1bdb6ef94a54cee5958539aedda44229baaf1dae3e8db08baa5de',
    validation_error: 'null'}
- {block: block_0xeb2300a9028f0dc91cd026d74010f7a0122a48984d2b207071e7015698cf7887}
- checks:
    head: {slot: 11, root: '0xfadf15c33151200546709d8bdeed7f00de88e15f93d9ffed48d8c1673f257c52'}
- payload_status: {status: PayloadStatusV1Status.VALID, latest_valid_hash: '0x6b5e520e48f6736070911079162157dd87cc473da78b328a5a77d18069d223b2',
    validation_error: 'null'}
- {block: block_0x28cfd6113e88cfdd3903c576bada1f79aa7f3955e2ef30604e63091680a8b625}
- checks:
    head: {slot: 12, root: '0x44155297210f07a144d61d57e239dc1908d08ae42edcee79a9e3d8b803ea0877'}
- payload_status: {status: PayloadStatusV1Status.SYNCING, latest_valid_hash: 'null',
    validation_error: 'null'}
- {block: block_0xa1e8cd339e0ac129a0892197507f7d49d731d1a3db08d32036f926ac1dbaf335,
  valid: false}
- checks:
    head: {slot: 10, root: '0x3f6acbc5be90c353052aad42117cac2c9577c103adea76d1775fcec45b4c0c72'}
- payload_status: {status: PayloadStatusV1Status.SYNCING, latest_valid_hash: 'null',
    validation_error: 'null'}
- {block: block_0x3c131a495076f7af107cb3a02f84f7c8fff22ccd5d9204e50e95f4c1ed640269,
  valid: false}
- checks:
    head: {slot: 11, root: '0x55dd6957eea9d4af18165761974233976808c88b64cbcd9f53328e514f1c5cca'}
- payload_status: {status: PayloadStatusV1Status.SYNCING, latest_valid_hash: 'null',
    validation_error: 'null'}
- {block: block_0x23aa3b0a1ca807c7de4724944aa46f19bb0530198bf5ac77196f2c877337c65e,
  valid: false}
- checks:
    head: {slot: 12, root: '0xe43498e53d60a08019406571637ae1b63122e3b9e2a7e97a2c7378dcacef6c55'}
- payload_status: {status: PayloadStatusV1Status.INVALID, latest_valid_hash: '0xb9a07089b65517ade3874fc891cc6206dfe5e753ac3bfb8dbf2cf0fe514b2a98',
    validation_error: invalid}
- {block: block_0x0fd4c324301e908366d2e53a0b9f601ffb4c33f5726939a975a9d4d5fa73a293,
  valid: false}
- checks:
    head: {slot: 12, root: '0x44155297210f07a144d61d57e239dc1908d08ae42edcee79a9e3d8b803ea0877'}

is_payload_valid flag

Right, that was why I said "In these tests, all the beacon blocks are valid except for the payload which is determined by the payload_status value". With this assumption, the valid flag means is_payload_valid.

I'm fine with either way.

hwwhww · 2022-08-31T17:12:57Z

Sync test vectors updates

20220831 updated
- minimal-sync-20220831.tar.gz
- mainnet-sync-20220831.tar.gz

mkalinin · 2022-09-01T06:56:11Z

Right, that was why I said "In these tests, all the beacon blocks are valid except for the payload which is determined by the payload_status value". With this assumption, the valid flag means is_payload_valid.

Got it! Let's keep it simple 👍 So, CL clients who do support this format should ignore valid flag values

potuz · 2022-09-01T21:21:56Z

Since this is a draft I will have a request to change the semanthics of valid here to instead of is_payload_valid to mean "the payload is not invalid". It would be way less intrusive for Prysm, I do not know about other clients though.

michaelsproul · 2022-09-02T07:34:14Z

Some preliminary feedback on the test format:

I would prefer PayloadStatusV1Status.SYNCING to be "SYNCING" without the type prefix. I had to implement custom logic to strip it which no other test in the suite requires.
Quoted 'null' values are strange and don't seem to be consistent with the rest of the suite either. I think null is cleaner.
There's no block_hash in the step as described in Extend fork_choice test format with on_payload_info #2965. Is the payload status meant to apply to the most recently listed block?

(haven't actually got them running yet, will post again once I have)

hwwhww · 2022-09-02T12:50:36Z

@potuz good call. we can address it in the test format.

@michaelsproul

I would prefer PayloadStatusV1Status.SYNCING to be "SYNCING" without the type prefix. I had to implement custom logic to strip it which no other test in the suite requires.
There's no block_hash in the step as described in #2965.

You're right. These bugs are fixed in 0f8b5ae 🙏

Quoted 'null' values are strange and don't seem to be consistent with the rest of the suite either. I think null is cleaner.

Agreed 👍 It will be as same as the engine API response. I updated it in 0f8b5ae

Updated example:

- {tick: 7728}
- block_hash: '0xb9a07089b65517ade3874fc891cc6206dfe5e753ac3bfb8dbf2cf0fe514b2a98'
  payload_status: {status: VALID, latest_valid_hash: '0xb9a07089b65517ade3874fc891cc6206dfe5e753ac3bfb8dbf2cf0fe514b2a98',
    validation_error: null}
- {block: block_0x3d1241fb9fa6d442d45e1f2153abdb1158cfe90f273978b6212b80d15053d1d8}
- checks:
    head: {slot: 9, root: '0x30b495d00fb542d2c513a464bba858295b884ab2451953e5876804a049afc2ef'}
- block_hash: '0xcfa86982c4ca3a0c429d223ee3000f64305e7bbb6ed57f12748a3ca1bb73e2ff'
  payload_status: {status: VALID, latest_valid_hash: '0xcfa86982c4ca3a0c429d223ee3000f64305e7bbb6ed57f12748a3ca1bb73e2ff',
    validation_error: null}
- {block: block_0x2a06b3082961d053d0b59c1fd232b298acc0353cdd58480c0d2dab16403f85fb}
- checks:
    head: {slot: 10, root: '0x1486942d4953830f9bbf43141838db41686edc43ca870d5b22c500e5e0c0898b'}
- block_hash: '0xb051302363f1bdb6ef94a54cee5958539aedda44229baaf1dae3e8db08baa5de'
  payload_status: {status: VALID, latest_valid_hash: '0xb051302363f1bdb6ef94a54cee5958539aedda44229baaf1dae3e8db08baa5de',
    validation_error: null}
- {block: block_0xeb2300a9028f0dc91cd026d74010f7a0122a48984d2b207071e7015698cf7887}
- checks:
    head: {slot: 11, root: '0xfadf15c33151200546709d8bdeed7f00de88e15f93d9ffed48d8c1673f257c52'}
- block_hash: '0x6b5e520e48f6736070911079162157dd87cc473da78b328a5a77d18069d223b2'
  payload_status: {status: VALID, latest_valid_hash: '0x6b5e520e48f6736070911079162157dd87cc473da78b328a5a77d18069d223b2',
    validation_error: null}
- {block: block_0x28cfd6113e88cfdd3903c576bada1f79aa7f3955e2ef30604e63091680a8b625}
- checks:
    head: {slot: 12, root: '0x44155297210f07a144d61d57e239dc1908d08ae42edcee79a9e3d8b803ea0877'}
- block_hash: '0x13d0f34b2c411f286db9f8164333a97e20793ac64313a5f68fb20a7decc1487d'
  payload_status: {status: SYNCING, latest_valid_hash: null, validation_error: null}
- {block: block_0xa1e8cd339e0ac129a0892197507f7d49d731d1a3db08d32036f926ac1dbaf335,
  valid: false}
- checks:
    head: {slot: 10, root: '0x3f6acbc5be90c353052aad42117cac2c9577c103adea76d1775fcec45b4c0c72'}
- block_hash: '0xfa8e3d85ee065a9350cd480ac76de90f6d8238d1c93f1f5fb7339d05e280d832'
  payload_status: {status: SYNCING, latest_valid_hash: null, validation_error: null}
- {block: block_0x3c131a495076f7af107cb3a02f84f7c8fff22ccd5d9204e50e95f4c1ed640269,
  valid: false}
- checks:
    head: {slot: 11, root: '0x55dd6957eea9d4af18165761974233976808c88b64cbcd9f53328e514f1c5cca'}
- block_hash: '0x3b99e4259065cc75f1c8d658d3c37a4d63cb712a263fb410b93771b109aeb47f'
  payload_status: {status: SYNCING, latest_valid_hash: null, validation_error: null}
- {block: block_0x23aa3b0a1ca807c7de4724944aa46f19bb0530198bf5ac77196f2c877337c65e,
  valid: false}
- checks:
    head: {slot: 12, root: '0xe43498e53d60a08019406571637ae1b63122e3b9e2a7e97a2c7378dcacef6c55'}
- block_hash: '0xe874f491ac490606d848657cc80c48f92975820c35d81e9ece632fc3702ac378'
  payload_status: {status: INVALID, latest_valid_hash: '0xb9a07089b65517ade3874fc891cc6206dfe5e753ac3bfb8dbf2cf0fe514b2a98',
    validation_error: invalid}
- {block: block_0x0fd4c324301e908366d2e53a0b9f601ffb4c33f5726939a975a9d4d5fa73a293,
  valid: false}
- checks:
    head: {slot: 12, root: '0x44155297210f07a144d61d57e239dc1908d08ae42edcee79a9e3d8b803ea0877'}

mkalinin · 2022-09-05T15:00:34Z

I think we should remove valid: false from blocks with NOT_VALIDATED EE status but valid with respect to CL. Otherwise, some CL test runners won't event apply this blocks to their fork choice (as these blocks are invalid), cc @potuz.

potuz · 2022-09-05T16:17:36Z

I think we should remove valid: false from blocks with NOT_VALIDATED EE status but valid with respect to CL. Otherwise, some CL test runners won't event apply this blocks to their fork choice (as these blocks are invalid), cc @potuz.

yeah, I would rather see for these blocks valid: true. So just to emphasize, what I would want to request is that valid: false only happens when we are guaranteed that the block is invalid, this can happen either if the block fails CL validation, or if the block is deemed INVALID by the EE. All other cases I would want them to be valid: true

mkalinin · 2022-09-06T10:41:38Z

I think we should remove valid: false from blocks with NOT_VALIDATED EE status but valid with respect to CL. Otherwise, some CL test runners won't event apply this blocks to their fork choice (as these blocks are invalid), cc @potuz.

yeah, I would rather see for these blocks valid: true. So just to emphasize, what I would want to request is that valid: false only happens when we are guaranteed that the block is invalid, this can happen either if the block fails CL validation, or if the block is deemed INVALID by the EE. All other cases I would want them to be valid: true

I would make valid even more stricter in the context of these tests, i.e. valid: false means that a block is invalid from CL point of view. If the block is deemed INVALID by the EE then CL will reject it in the middle of state_transition function

potuz · 2022-09-06T10:44:49Z

I would make valid even more stricter in the context of these tests, i.e. valid: false means that a block is invalid from CL point of view. If the block is deemed INVALID by the EE then CL will reject it in the middle of state_transition function

I think this is exactly the same as my request isn't it? in any case I'm pleased if this interpretation is taken. I manually changed the steps file and we pass with those conditions, without any code changes.

mkalinin · 2022-09-07T05:22:09Z

I would make valid even more stricter in the context of these tests, i.e. valid: false means that a block is invalid from CL point of view. If the block is deemed INVALID by the EE then CL will reject it in the middle of state_transition function

I think this is exactly the same as my request isn't it? in any case I'm pleased if this interpretation is taken. I manually changed the steps file and we pass with those conditions, without any code changes.

My suggestion is to have valid: true even in the case when "the block is deemed INVALID by the EE"

michaelsproul · 2022-09-07T08:04:12Z

I've just got the test to pass in Lighthouse by changing all valid: false to true and adding an allowance for execution payload errors when valid=true. I agree with @mkalinin that it's simplest to have valid refer only to CL block validity, as it's useful to import blocks as SYNCING and mark them invalid from the EL's perspective later. Presently our test runner would fail if it optimistically imported a block which was marked invalid.

potuz · 2022-09-07T10:48:49Z

I can live with valid: true even when the EL returns INVALID, but my argument to not have it this way is that it will require us to change code to pass the already existing forkchoice tests. I don't see any reason why these new tests would need to affect the previous ones, specially if we can avoid this at this stage

EDIT: Also notice that from the spec perspective, a block that fails validation of the EE is in fact invalid on the CL side, so it makes really little sense to have valid: true for such blocks. For optimistically synced blocks that pass CL validation I agree that it's a coin flip, but I would want them to be valid: true to not break previous tests.

mkalinin · 2022-09-08T12:10:12Z

After clarification in Discord, my suggestion for valid flag values is as follows:

`state_transition` without `notify_new_payload`	Payload Status	valid
passes	VALID	true
passes	NOT_VALIDATED	true
passes	INVALIDATED	false
fails	ANY	false

cc @potuz @michaelsproul

Justification: valid is false whenever on_block is expected to return error, it happens when state_transition errors including the case when notify_new_payload returns false (i.e. payload status is INVALIDATED). When the state_transition doesn't return error including the case when payload is NOT_VALIDATED even if the payload is eventually turns to be INVALID the valid is true.

hwwhww · 2022-09-08T14:50:07Z

Thank you for the feedback!

2e73091 implmeneted #2982 (comment) suggestions

Sync test vectors updates

20220908 updated
- mainnet-sync-20220908.tar.gz
- minimal-sync-20220908.tar.gz

`minimal/bellatrix/sync/optimistic/pyspec_tests/from_syncing_to_invalid/steps.yaml`

Removed valid=False for the SYNCING blocks [B1', B2', B3']

- {tick: 7728}
- block_hash: '0xb9a07089b65517ade3874fc891cc6206dfe5e753ac3bfb8dbf2cf0fe514b2a98'
  payload_status: {status: VALID, latest_valid_hash: '0xb9a07089b65517ade3874fc891cc6206dfe5e753ac3bfb8dbf2cf0fe514b2a98',
    validation_error: null}
- {block: block_0x3d1241fb9fa6d442d45e1f2153abdb1158cfe90f273978b6212b80d15053d1d8}
- checks:
    head: {slot: 9, root: '0x30b495d00fb542d2c513a464bba858295b884ab2451953e5876804a049afc2ef'}
- block_hash: '0xcfa86982c4ca3a0c429d223ee3000f64305e7bbb6ed57f12748a3ca1bb73e2ff'
  payload_status: {status: VALID, latest_valid_hash: '0xcfa86982c4ca3a0c429d223ee3000f64305e7bbb6ed57f12748a3ca1bb73e2ff',
    validation_error: null}
- {block: block_0x2a06b3082961d053d0b59c1fd232b298acc0353cdd58480c0d2dab16403f85fb}
- checks:
    head: {slot: 10, root: '0x1486942d4953830f9bbf43141838db41686edc43ca870d5b22c500e5e0c0898b'}
- block_hash: '0xb051302363f1bdb6ef94a54cee5958539aedda44229baaf1dae3e8db08baa5de'
  payload_status: {status: VALID, latest_valid_hash: '0xb051302363f1bdb6ef94a54cee5958539aedda44229baaf1dae3e8db08baa5de',
    validation_error: null}
- {block: block_0xeb2300a9028f0dc91cd026d74010f7a0122a48984d2b207071e7015698cf7887}
- checks:
    head: {slot: 11, root: '0xfadf15c33151200546709d8bdeed7f00de88e15f93d9ffed48d8c1673f257c52'}
- block_hash: '0x6b5e520e48f6736070911079162157dd87cc473da78b328a5a77d18069d223b2'
  payload_status: {status: VALID, latest_valid_hash: '0x6b5e520e48f6736070911079162157dd87cc473da78b328a5a77d18069d223b2',
    validation_error: null}
- {block: block_0x28cfd6113e88cfdd3903c576bada1f79aa7f3955e2ef30604e63091680a8b625}
- checks:
    head: {slot: 12, root: '0x44155297210f07a144d61d57e239dc1908d08ae42edcee79a9e3d8b803ea0877'}
- block_hash: '0x13d0f34b2c411f286db9f8164333a97e20793ac64313a5f68fb20a7decc1487d'
  payload_status: {status: SYNCING, latest_valid_hash: null, validation_error: null}
- {block: block_0xa1e8cd339e0ac129a0892197507f7d49d731d1a3db08d32036f926ac1dbaf335}
- checks:
    head: {slot: 10, root: '0x3f6acbc5be90c353052aad42117cac2c9577c103adea76d1775fcec45b4c0c72'}
- block_hash: '0xfa8e3d85ee065a9350cd480ac76de90f6d8238d1c93f1f5fb7339d05e280d832'
  payload_status: {status: SYNCING, latest_valid_hash: null, validation_error: null}
- {block: block_0x3c131a495076f7af107cb3a02f84f7c8fff22ccd5d9204e50e95f4c1ed640269}
- checks:
    head: {slot: 11, root: '0x55dd6957eea9d4af18165761974233976808c88b64cbcd9f53328e514f1c5cca'}
- block_hash: '0x3b99e4259065cc75f1c8d658d3c37a4d63cb712a263fb410b93771b109aeb47f'
  payload_status: {status: SYNCING, latest_valid_hash: null, validation_error: null}
- {block: block_0x23aa3b0a1ca807c7de4724944aa46f19bb0530198bf5ac77196f2c877337c65e}
- checks:
    head: {slot: 12, root: '0xe43498e53d60a08019406571637ae1b63122e3b9e2a7e97a2c7378dcacef6c55'}
- block_hash: '0xe874f491ac490606d848657cc80c48f92975820c35d81e9ece632fc3702ac378'
  payload_status: {status: INVALID, latest_valid_hash: '0xb9a07089b65517ade3874fc891cc6206dfe5e753ac3bfb8dbf2cf0fe514b2a98',
    validation_error: invalid}
- {block: block_0x0fd4c324301e908366d2e53a0b9f601ffb4c33f5726939a975a9d4d5fa73a293,
  valid: false}
- checks:
    head: {slot: 12, root: '0x44155297210f07a144d61d57e239dc1908d08ae42edcee79a9e3d8b803ea0877'}

potuz · 2022-09-11T15:26:12Z

Thanks for the new format, confirming Prysm passes those vectors without any changes to the runner

mkalinin

Looks good to me! 🚀

## Issue Addressed Implements new optimistic sync test format from ethereum/consensus-specs#2982. ## Proposed Changes - Add parsing and runner support for the new test format. - Extend the mock EL with a set of canned responses keyed by block hash. Although this doubles up on some of the existing functionality I think it's really nice to use compared to the `preloaded_responses` or static responses. I think we could write novel new opt sync tests using these primtives much more easily than the previous ones. Forks are natively supported, and different responses to `forkchoiceUpdated` and `newPayload` are also straight-forward. ## Additional Info Blocked on merge of the spec PR and release of new test vectors.

Implements new optimistic sync test format from ethereum/consensus-specs#2982. - Add parsing and runner support for the new test format. - Extend the mock EL with a set of canned responses keyed by block hash. Although this doubles up on some of the existing functionality I think it's really nice to use compared to the `preloaded_responses` or static responses. I think we could write novel new opt sync tests using these primtives much more easily than the previous ones. Forks are natively supported, and different responses to `forkchoiceUpdated` and `newPayload` are also straight-forward. Blocked on merge of the spec PR and release of new test vectors.

hwwhww added the testing CI, actions, tests, testing infra label Aug 23, 2022

Add basic test case

f8d92c3

hwwhww force-pushed the optimistic-sync-tests branch from 4ed2249 to f8d92c3 Compare August 24, 2022 15:20

mkalinin reviewed Aug 31, 2022

View reviewed changes

mkalinin mentioned this pull request Aug 31, 2022

Extend fork_choice test format with on_payload_info #2965

Merged

Address PR feedback from @mkalinin

ac717b1

This comment was marked as spam.

Sign in to view

hwwhww added the general:RFC label Sep 1, 2022

potuz mentioned this pull request Sep 1, 2022

Implement optimistic sync spectests OffchainLabs/prysm#11391

Merged

Apply PR feedback from @michaelsproul

0f8b5ae

hwwhww force-pushed the optimistic-sync-tests branch from b9cb5a5 to 0f8b5ae Compare September 2, 2022 12:46

ajsutton mentioned this pull request Sep 2, 2022

Add support for optimistic sync ref tests Consensys/teku#6153

Closed

mkalinin mentioned this pull request Sep 8, 2022

Consensus-layer Call 95 ethereum/pm#618

Closed

Add get_valid_flag_value helper to determine the valid flag value

2e73091

hwwhww changed the title ~~[WIP] Add optimistic sync tests~~ Add optimistic sync tests Sep 8, 2022

hwwhww marked this pull request as ready for review September 8, 2022 14:52

hwwhww requested a review from mkalinin September 10, 2022 18:33

michaelsproul mentioned this pull request Sep 12, 2022

[Merged by Bors] - Optimistic sync spec tests (v1.2.0) sigp/lighthouse#3564

Closed

mkalinin approved these changes Sep 12, 2022

View reviewed changes

hwwhww merged commit f2c2656 into dev Sep 12, 2022

hwwhww deleted the optimistic-sync-tests branch September 12, 2022 10:06

This was referenced Sep 12, 2022

Optimistic Sync tests proposal ethereum/consensus-spec-tests#28

Closed

Add more optimistic sync tests ethereum/consensus-spec-tests#30

Open

g11tech mentioned this pull request Sep 13, 2022

Spec v1.2.0-rc.3 completion tracker ChainSafe/lodestar#4455

Closed

29 tasks

hwwhww mentioned this pull request Sep 20, 2022

Update BLS test format: output null for invalid case #3001

Merged

Conversation

hwwhww commented Aug 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Description

Example

minimal/bellatrix/sync/optimistic/pyspec_tests/from_syncing_to_invalid/steps.yaml (20220908 fixed)

Testing tool implementation

Sync test vectors:

TODOs:

Uh oh!

mkalinin Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

hwwhww Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

mkalinin commented Aug 31, 2022

Uh oh!

hwwhww commented Aug 31, 2022

Uh oh!

hwwhww commented Aug 31, 2022

Sync test vectors updates

Uh oh!

mkalinin commented Sep 1, 2022

Uh oh!

This comment was marked as spam.

potuz commented Sep 1, 2022

Uh oh!

michaelsproul commented Sep 2, 2022

Uh oh!

hwwhww commented Sep 2, 2022

Uh oh!

mkalinin commented Sep 5, 2022

Uh oh!

potuz commented Sep 5, 2022

Uh oh!

mkalinin commented Sep 6, 2022

Uh oh!

potuz commented Sep 6, 2022

Uh oh!

mkalinin commented Sep 7, 2022

Uh oh!

michaelsproul commented Sep 7, 2022

Uh oh!

potuz commented Sep 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkalinin commented Sep 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hwwhww commented Sep 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sync test vectors updates

minimal/bellatrix/sync/optimistic/pyspec_tests/from_syncing_to_invalid/steps.yaml

Uh oh!

potuz commented Sep 11, 2022

Uh oh!

mkalinin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hwwhww commented Aug 23, 2022 •

edited

Loading

`minimal/bellatrix/sync/optimistic/pyspec_tests/from_syncing_to_invalid/steps.yaml` (20220908 fixed)

potuz commented Sep 7, 2022 •

edited

Loading

mkalinin commented Sep 8, 2022 •

edited

Loading

hwwhww commented Sep 8, 2022 •

edited

Loading

`minimal/bellatrix/sync/optimistic/pyspec_tests/from_syncing_to_invalid/steps.yaml`