Skip to content

Mock Engine API req/resp in tests - take 2#2639

Closed
hwwhww wants to merge 3 commits intodevfrom
engine-api-tests-take-2
Closed

Mock Engine API req/resp in tests - take 2#2639
hwwhww wants to merge 3 commits intodevfrom
engine-api-tests-take-2

Conversation

@hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Oct 2, 2021

Replace #2637
It may be better to start with small changes rather than writing overcomplicated scripts.

1. Use validator guide helpers to generate the execution payload for on_merge_block tests

See run_get_execution_payload_with_mock_engine_get_payload.

  • Comparing to [WIP] Mock Engine API req/resp in tests #2637, this PR doesn't output the block production API calls logs.
  • Using validator guide get_execution_payload would improve our test coverage a bit. 🙂 We can also create validator guide unit tests later in other PRs.

2. Fork choice rule test format updates

This PR changes the existing fork choice tests output:

  1. Add one more layer to the execution steps.
    • e.g., previous {'attestation': attestation_0x<attestation_root>} is now {'on_attestation': {'attestation': 'attestation_0x<attestation_root>'}}
    • it allows us to add more description to the on_block step.
  2. Change the pow_block output
    • make it only shows when get_pow_block helper is called.
    • e.g.,
      - on_block:
          ...
          block: block_0x6f0e0d6d65481fc1142bde4234b8537a502facc3b26168046cfcecd95e6bdc8a
          get_pow_block:
            input: {block_hash: 0x922ef34b6ca455c5d13de0ac5db0f6d7c1c26adfa026141a2e086fb155deb6b4}
            output: {result: pow_block_0x006a9edb9f8316403009d1cf96c21907ddfafe86c5701c49cd0d57abab2a8a7b}
  3. Output mocking engine_executePayload request and response with on_block
    • e.g., if an on_block step triggers process_execution_payload and engine_executePayload:
      - on_block:
          ...
          process_execution_payload:
            engine_api:
              request:
                method: engine_executePayload
                params:
                  parentHash: '0x006a9edb9f8316403009d1cf96c21907ddfafe86c5701c49cd0d57abab2a8a7b'
                  coinbase: '0x1212121212121212121212121212121212121212'
                  stateRoot: '0x0000000000000000000000000000000000000000000000000000000000000000'
                  receiptRoot: '0x6e6f207265636569707473206865726500000000000000000000000000000000'
                  logsBloom: '0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000'
                  random: '0xdadadadadadadadadadadadadadadadadadadadadadadadadadadadadadadada'
                  blockNumber: 1
                  gasLimit: 0
                  gasUsed: 0
                  timestamp: 12
                  extraData: 0x
                  baseFeePerGas: 0
                  blockHash: '0x15c1774a93c7a216ca89fb27245b0dec9c6a8a8ebab162ffcbe80861de87e4fc'
                  transactions: []
              response:
                error: false
                result: {status: VALID}
    • Client team can use the request fields to verify if the request is as expected.
    • Client team can use the response fields as the return value of the given APIs.

Minor fixes

  • Fix default prepare_payload parameters in setup.py
  • Refactor build_empty_execution_payload

@hwwhww hwwhww requested a review from mkalinin October 2, 2021 14:05
@hwwhww hwwhww added the testing CI, actions, tests, testing infra label Oct 2, 2021
@hwwhww hwwhww requested a review from djrtwo October 2, 2021 14:51
@hwwhww hwwhww force-pushed the engine-api-tests-take-2 branch from c170705 to 99bb719 Compare October 18, 2021 05:21
@mkalinin
Copy link
Contributor

mkalinin commented Nov 5, 2021

IMO, we should not test the Engine API client implementations via CL tests. This should rather be done via Hive. Recalling my comment from the original PR and your reply:

Do you mean like disposable TestEngine in test_process_execution_payload tests? Or a more reusable and general ExecutionEngine cross consensus pyspec tests?

I think the disposable TestEngine should be pretty much enough for CL tests.

@hwwhww hwwhww closed this May 10, 2022
@jtraglia jtraglia deleted the engine-api-tests-take-2 branch January 22, 2025 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing CI, actions, tests, testing infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants