[WIP] Merge interop spec #23607

MariusVanDerWijden · 2021-09-20T11:49:37Z

This PR contains a first implementation of the interop spec rebased on current master

to test:

./build/bin/geth --dev --catalyst --http --ws -http.api "engine" console

curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"engine_preparePayload","params":[{"parentHash":"0x582ea3ee1731029756c8bd211647e02fb68528a1d0a18a99a413d8b6afbe20b2", "timestamp":"0x1", "random":"0x0000000000000000000000000000000000000000000000000000000000000000", "feeRecipient":"0x0000000000000000000000000000000000000000"}],"id":67}' http://localhost:8545
{"jsonrpc":"2.0","id":67,"result":{"payloadId":"0x0"}}


curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"engine_getPayload","params":["0x0"],"id":67}' http://localhost:8545
{"jsonrpc":"2.0","id":67,"result":{"blockHash":"0xca6fc9b73939ad40c0ebfcab5b93ca81548f690b62d3c54b0fd0bdcbb2ce5872","parentHash":"0x582ea3ee1731029756c8bd211647e02fb68528a1d0a18a99a413d8b6afbe20b2","coinbase":"0xcd3f20468170e3094f09f16ae987e6325634ada1","stateRoot":"0x6cb6d72150ab4f942667c2f3426091b7de8095e08c5e8fc5c92edeb2d804729b","receiptsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","random":"0x0000000000000000000000000000000000000000000000000000000000000000","number":"0x1","gasLimit":"0xaf79e0","gasUsed":"0x0","timestamp":"0x1","extraData":null,"baseFeePerGas":"0x0","transactions":[]}}



 curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"engine_executePayload","params":[{"blockHash":"0xca6fc9b73939ad40c0ebfcab5b93ca81548f690b62d3c54b0fd0bdcbb2ce5872","parentHash":"0x582ea3ee1731029756c8bd211647e02fb68528a1d0a18a99a413d8b6afbe20b2","coinbase":"0xcd3f20468170e3094f09f16ae987e6325634ada1","stateRoot":"0x6cb6d72150ab4f942667c2f3426091b7de8095e08c5e8fc5c92edeb2d804729b","receiptsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","random":"0x0000000000000000000000000000000000000000000000000000000000000000","number":"0x1","gasLimit":"0xaf79e0","gasUsed":"0x0","timestamp":"0x1","extraData":"","baseFeePerGas":"0x0","transactions":[]}],"id":67}' http://localhost:8545
{"jsonrpc":"2.0","id":67,"result":{"status":"VALID"}}


 curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"engine_consensusValidated","params":[{"blockHash":"0xca6fc9b73939ad40c0ebfcab5b93ca81548f690b62d3c54b0fd0bdcbb2ce5872", "status":"VALID"}],"id":67}' http://localhost:8545
{"jsonrpc":"2.0","id":67,"result":null}


curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"engine_forkChoiceUpdated","params":[{"headBlockHash":"0xca6fc9b73939ad40c0ebfcab5b93ca81548f690b62d3c54b0fd0bdcbb2ce5872", "finalizedBlockHash":"0xca6fc9b73939ad40c0ebfcab5b93ca81548f690b62d3c54b0fd0bdcbb2ce5872"}],"id":67}' http://localhost:8545
{"jsonrpc":"2.0","id":67,"result":null}

eth/catalyst/api_types.go

consensus/beacon/consensus.go

eth/catalyst/api.go

eth/catalyst/api_types.go

eth/catalyst/api.go

paulhauner · 2021-09-30T01:23:18Z

Hi there, thanks for publishing the test vectors. It's much appreciated!

I have been working on these vectors and have found some discrepancies. I've raised them on TG/Discord, but I thought it might be helpful to have them collated on Github.

Here are the discrepancies:

For the ExecutionPayload object, the test vectors use receiptsRoot whilst the spec uses receiptRoot.
Also for ExecutionPayload, the test vectors use number whilst the spec uses blockNumber.
For engine_preparePayload, the test vectors state {"jsonrpc":"2.0","id":67,"result":{"payloadId":"0x0"} whist I understand that the spec declares {"jsonrpc":"2.0","id":67,"result":"0x0"} (return a QUANTITY not an Object).
- Credit to @tersec for first raising this, who reports that Geth returns as per the spec, not as per the vectors.
- As per the main branch of the specs, engine_preparePayload returns an Object with a payloadId field. This means the test vectors are indeed correct.
Test test vectors use engine_forkChoiceUpdated whilst the spec declares engine_forkchoiceUpdated (no capital C).

MariusVanDerWijden · 2021-09-30T08:07:28Z

Thank you for looking into it! Most of the issues were already fixed in code.
I've now updated the test vectors to be in line with the implementation.
And I've updated engine_forkChoiceUpdated -> engine_forkchoiceUpdated

paulhauner · 2021-10-01T04:55:55Z

I just ran the Lighthouse tests against Geth and I can confirm all those previous issues have been fixed. Thank you 🎉

There are still two things remaining:

Geth doesn't return an Object for the engine_preparePayload as per the v1.0.0.alpha.2 changes. That's totally understandable, it was only released hours ago. I thought I'd just make a note of it.
Geth doesn't accept "finalizedBlockHash":"0x0000000000000000000000000000000000000000000000000000000000000000" on engine_forkchoiceUpdated. (More below)

According to EIP-3675:

A POS_FORKCHOICE_UPDATED event contains references to the head of the canonical chain and to the most recent finalized block. Before the first finalized block occurs in the system the finalized block hash provided by this event is stubbed with 0x0000000000000000000000000000000000000000000000000000000000000000.

This issue can be recreated with a modified version of the test vectors:

Request:

curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"engine_forkchoiceUpdated","params":[{"headBlockHash":"0xb084c10440f05f5a23a55d1d7ebcb1b3892935fb56f23cdc9a7f42c348eed174", "finalizedBlockHash":"0x0000000000000000000000000000000000000000000000000000000000000000"}],"id":67}'

Response:

http://localhost:8545 {"jsonrpc":"2.0","id":67,"error":{"code":4,"message":"unknown header"}}

eth: reqid dispatcher, nuke fast sync, add beacon sync

eth: minor sync polishes

holiman · 2021-10-04T12:15:30Z

eth/catalyst/api.go

+	}
+	parent := api.eth.BlockChain().GetBlockByHash(params.ParentHash)
+	if parent == nil {
+		return INVALID, fmt.Errorf("could not find parent %x", params.ParentHash)


I don't think this will work. Our RPC server implementation, AFAIK, when dealing with a non-nil return and a non-nil error, will discard the return vaue and return only the error. This might even be a 'feature' of json rpc: either you return a response XOR an error. Our server chooses the error.

core, eth: various tiny fixups integrating sync

jwasinger · 2021-10-05T09:25:21Z

core/blockchain.go

 	vmConfig   vm.Config

-	shouldPreserve func(*types.Block) bool // Function used to determine whether should preserve the given block.
+	shouldPreserve  func(*types.Block) bool        // Function used to determine whether should preserve the given block.
+	terminateInsert func(common.Hash, uint64) bool // Testing hook used to terminate ancient receipt chain insertion.


stale code? I don't see any references to this elsewhere.

eth: disable legacy syncing on the merge interop pr for now

core: overwrite head header and fast block too on block sethead

eth/downloader: restart the downloader after completion on new head

eth: only sync with peers below the ttd

blazejkrzak · 2021-11-15T16:05:49Z

consensus/beacon/consensus.go

+// (b) the timestamp is not verified anymore
+func (beacon *Beacon) verifyHeader(chain consensus.ChainHeaderReader, header, parent *types.Header) error {
+	// Ensure that the header's extra-data section is of a reasonable size
+	if len(header.Extra) > 32 {


32 should go to enum or const

blazejkrzak · 2021-11-15T16:06:25Z

consensus/beacon/consensus.go

+		return fmt.Errorf("invalid difficulty: have %v, want %v", header.Difficulty, beaconDifficulty)
+	}
+	// Verify that the gas limit is <= 2^63-1
+	cap := uint64(0x7fffffffffffffff)


same here, it could be extracted outside the function

blazejkrzak · 2021-11-15T16:11:54Z

core/chain_makers.go

+		// to a chain, so the difficulty will be left unset (nil). Set it here to the
+		// correct value.
+		if b.header.Difficulty == nil {
+			b.header.Difficulty = big.NewInt(2)


2 could be extracted to enum/const with proper name

MariusVanDerWijden mentioned this pull request Sep 20, 2021

Engine API subset for the Merge Interop ethereum/execution-apis#74

Merged

rjl493456442 mentioned this pull request Sep 21, 2021

all: rework the chain for eth1/2 merging #22827

Closed

lightclient reviewed Sep 28, 2021

View reviewed changes

eth/catalyst/api_types.go Outdated Show resolved Hide resolved

lightclient reviewed Sep 28, 2021

View reviewed changes

eth/catalyst/api_types.go Outdated Show resolved Hide resolved

lightclient reviewed Sep 28, 2021

View reviewed changes

consensus/beacon/consensus.go Outdated Show resolved Hide resolved

lightclient reviewed Sep 28, 2021

View reviewed changes

eth/catalyst/api.go Outdated Show resolved Hide resolved

lightclient reviewed Sep 29, 2021

View reviewed changes

eth/catalyst/api_types.go Show resolved Hide resolved

lightclient reviewed Sep 29, 2021

View reviewed changes

eth/catalyst/api.go Show resolved Hide resolved

Nashatyrev mentioned this pull request Oct 1, 2021

[Merge] Make Teku working with Geth EE Consensys/teku#4431

Merged

2 tasks

MariusVanDerWijden and others added 18 commits October 1, 2021 16:45

consensus/clique: prevent 0 len extradata from panicing

25b336d

all: work for eth1/2 transtition

50a145c

consensus/beacon, eth: change beacon difficulty to 0

49a4891

eth: updates

bc08e01

all: add terminalBlockDifficulty config

a1645a4

miner/stress/beacon: fix stress test

d460aa4

core: rebased on master

150a1eb

eth: implemented merge interop spec

cb89e26

eth: fix tests, remove unused methods

37ddc08

eth: fix stress-test

881613d

eth/catalyst: regenerate api types

bdb6070

eth, core: remove unusued code, fix typos

034da5a

eth: added api for exporting and importing blocks

4e12000

core: updated stale comment

6cde534

eth/catalyst: better types, testing, first working protype

dd773c7

eth/catalyst: added terminal difficulty check

adb8626

eth/catalyst: use custom errors

4d4c298

eth/catalyst: renamed api types, better adherence to spec

b4aaa07

MariusVanDerWijden added 3 commits October 3, 2021 16:15

eth/catalyst: verify that totalDiff is reached on executeBlock

4a8425d

eth/catalyst: prevent nil panic in checkTerminalTotalDifficulty

ad5da55

eth/catalyst: prevent nil panic in checkTerminalTotalDifficulty

b798cbd

paulhauner mentioned this pull request Oct 3, 2021

Merge Interop Milestone Tracking hyperledger/besu#2830

Closed

6 tasks

karalabe and others added 5 commits October 4, 2021 09:51

eth: reqid dispatcher, nuke fast sync, add beacon sync

54a5b2f

Merge pull request #21 from karalabe/merge-interop-spec-sync2

00f3688

eth: reqid dispatcher, nuke fast sync, add beacon sync

miner: add totalDifficultyOverride flag

a094be5

eth: minor sync polishes

b1ff642

Merge pull request #22 from karalabe/merge-interop-spec-sync3

716ff95

eth: minor sync polishes

holiman reviewed Oct 4, 2021

View reviewed changes

karalabe and others added 3 commits October 4, 2021 17:15

core, eth: various tiny fixups integrating sync

7834589

Merge pull request #23 from karalabe/merge-interop-spec-sync4

56bebd7

core, eth: various tiny fixups integrating sync

eth/catalyst: fixed some tiny issues/nitpicks in api

a0fe68b

jwasinger reviewed Oct 5, 2021

View reviewed changes

karalabe and others added 10 commits October 5, 2021 14:29

eth: disable legacy syncing on the merge interop pr for now

deaa555

Merge pull request #24 from karalabe/merge-interop-spec-sync5

1216abb

eth: disable legacy syncing on the merge interop pr for now

core: overwrite head header and fast block too on block sethead

9562d10

Merge pull request #25 from karalabe/merge-interop-spec-sync6

1758b1a

core: overwrite head header and fast block too on block sethead

miner: remove superflous log message

0b0427d

eth/downloader: restart the downloader after completion on new head

e90ede6

Merge pull request #27 from karalabe/merge-interop-spec-sync7

efad795

eth/downloader: restart the downloader after completion on new head

eth: only sync with beers below the ttd

4b9ea81

Merge pull request #28 from karalabe/merge-interop-spec-sync8

d28b16f

eth: only sync with peers below the ttd

eth/catalyst: added GetHead endpoint for fuzzing

ac736f9

MariusVanDerWijden mentioned this pull request Oct 18, 2021

Minimized changes for the merge #23761

Merged

blazejkrzak reviewed Nov 15, 2021

View reviewed changes

MariusVanDerWijden closed this Nov 30, 2021

MariusVanDerWijden deleted the merge-interop-spec branch November 30, 2021 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Merge interop spec #23607

[WIP] Merge interop spec #23607

MariusVanDerWijden commented Sep 20, 2021 •

edited

Loading

paulhauner commented Sep 30, 2021 •

edited

Loading

MariusVanDerWijden commented Sep 30, 2021

paulhauner commented Oct 1, 2021

holiman Oct 4, 2021

jwasinger Oct 5, 2021

blazejkrzak Nov 15, 2021

blazejkrzak Nov 15, 2021

blazejkrzak Nov 15, 2021

[WIP] Merge interop spec #23607

[WIP] Merge interop spec #23607

Conversation

MariusVanDerWijden commented Sep 20, 2021 • edited Loading

paulhauner commented Sep 30, 2021 • edited Loading

MariusVanDerWijden commented Sep 30, 2021

paulhauner commented Oct 1, 2021

holiman Oct 4, 2021

Choose a reason for hiding this comment

jwasinger Oct 5, 2021

Choose a reason for hiding this comment

blazejkrzak Nov 15, 2021

Choose a reason for hiding this comment

blazejkrzak Nov 15, 2021

Choose a reason for hiding this comment

blazejkrzak Nov 15, 2021

Choose a reason for hiding this comment

MariusVanDerWijden commented Sep 20, 2021 •

edited

Loading

paulhauner commented Sep 30, 2021 •

edited

Loading