fix: use proper type for engine_newPayloadV2

[Rjected]

Should fix #4628

The spec we linked in docs says to use ExecutionPayloadV2:

engine_newPayloadV2

Request

• method: engine_newPayloadV2
• params:
  1. ExecutionPayloadV2

This is incorrect as noted in the issue. We already had a type for the correct version, which was used as the return value for engine_getPayloadV2, but it was not used as the input for engine_newPayloadV2.

Will look into developing some testing for this, since we've had similar bugs that were not regression tested.

[codecov]

Codecov Report

Merging #4630 (3d4c5c5) into main (f7b28e5) will decrease coverage by 0.03%.
Report is 7 commits behind head on main.
The diff coverage is 88.75%.

Files Changed	Coverage Δ
crates/rpc/rpc-api/src/engine.rs	50.00% <ø> (ø)
crates/rpc/rpc-types/src/eth/engine/payload.rs	65.04% <88.46%> (-3.89%)	⬇️
crates/rpc/rpc-engine-api/src/engine_api.rs	79.34% <100.00%> (+0.17%)	⬆️

... and 26 files with indirect coverage changes

Flag	Coverage Δ
integration-tests	16.95% <16.25%> (-0.05%)	⬇️
unit-tests	63.21% <73.75%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
reth binary	31.95% <ø> (+0.58%)	⬆️
blockchain tree	83.59% <ø> (ø)
pipeline	88.54% <ø> (ø)
storage (db)	73.47% <ø> (+0.03%)	⬆️
trie	94.73% <ø> (ø)
txpool	49.42% <ø> (-0.24%)	⬇️
networking	77.19% <ø> (-0.07%)	⬇️
rpc	57.29% <88.75%> (-0.09%)	⬇️
consensus	62.42% <ø> (ø)
revm	19.66% <ø> (-0.02%)	⬇️
payload builder	9.06% <ø> (-0.03%)	⬇️
primitives	86.42% <ø> (-0.04%)	⬇️

[0xTylerHolmes]

This appears to fix pre-shanghai but now we have an issue with all CL clients making it across to capella. Looking into this now.

[0xTylerHolmes]

Now we get a rejection on an request with an empty withdrawals entry.

Request

{
  "jsonrpc": "2.0",
  "method": "engine_newPayloadV2",
  "params": [
    {
      "baseFeePerGas": "0x173b30b3",
      "blockHash": "0x99d486755fd046ad0bbb60457bac93d4856aa42fa00629cc7e4a28b65b5f8164",
      "blockNumber": "0xb",
      "extraData": "0xd883010d01846765746888676f312e32302e33856c696e7578",
      "feeRecipient": "0x0000000000000000000000000000000000000000",
      "gasLimit": "0x405829",
      "gasUsed": "0x3f0ca0",
      "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
      "parentHash": "0xfe34aaa2b869c66a727783ee5ad3e3983b6ef22baf24a1e502add94e7bcac67a",
      "prevRandao": "0x74132c32fe3ab9a470a8352544514d21b6969e7749f97742b53c18a1b22b396c",
      "receiptsRoot": "0x6a5c41dc55a1bd3e74e7f6accc799efb08b00c36c15265058433fcea6323e95f",
      "stateRoot": "0xde3b357f5f099e4c33d0343c9e9d204d663d7bd9c65020a38e5d0b2a9ace78a2",
      "timestamp": "0x6507d6b4",
      "transactions": [
        "0xf86d0a8458b20efd825208946177843db3138ae69679a54b95cf345ed759450d8806f3e8d87878800080820a95a0f8bddb1dcc4558b532ff747760a6f547dd275afdbe7bdecc90680e71de105757a014f34ba38c180913c0543b0ac2eccfb77cc3f801a535008dc50e533fbe435f53",
        "0xf86d0b8458b20efd82520894687704db07e902e9a8b3754031d168d46e3d586e8806f3e8d87878800080820a95a0e3108f710902be662d5c978af16109961ffaf2ac4f88522407d40949a9574276a0205719ed21889b42ab5c1026d40b759a507c12d92db0d100fa69e1ac79137caa",
        "0xf86d0c8458b20efd8252089415e6a5a2e131dd5467fa1ff3acd104f45ee5940b8806f3e8d87878800080820a96a0af556ba9cda1d686239e08c24e169dece7afa7b85e0948eaa8d457c0561277fca029da03d3af0978322e54ac7e8e654da23934e0dd839804cb0430f8aaafd732dc",
  "0xf8521784565adcb7830186a0808080820a96a0ec782872a673a9fe4eff028a5bdb30d6b8b7711f58a187bf55d3aec9757cb18ea001796d373da76f2b0aeda72183cce0ad070a4f03aa3e6fee4c757a9444245206",

...
        "0xf8521284565adcb7830186a0808080820a95a08a0ea89028eff02596b385a10e0bd6ae098f3b281be2c95a9feb1685065d7384a06239d48a72e4be767bd12f317dd54202f5623a33e71e25a87cb25dd781aa2fc8",
      "0xf8521384565adcb7830186a0808080820a95a0784dbd311a82f822184a46f1677a428cbe3a2b88a798fb8ad1370cdbc06429e8a07a7f6a0efd428e3d822d1de9a050b8a883938b632185c254944dd3e40180eb79"
      ],
      "withdrawals": []
    }
  ],
  "id": 1
}

Response

{
  "jsonrpc": "2.0",
  "error": {
    "code": -32602,
    "message": "no withdrawals post-shanghai"
  },
  "id": 1
}

I second the regression testing. I've added reth instances into my testing workflow at least.

[mattsse]

nice catch, this change did not fix it entirely, due to how deserialization now works.

fixing asap

[mattsse]

lgtm, missing withdrawals post shanghai are caught during validation against timestmap

[0xTylerHolmes]

LGTM