engine-api support #5

MoonShiesty · 2023-09-20T16:52:47Z

This PR adds support to specular-geth to support deterministic execution of transactions via the engine-api.

To support this we need to make a few changes to geth:

extend PayloadAttributes with Transactions, a list of transaction to force include in the block
extend PayloadAttributes with NoTxPool, if true no transactions are taken out of the tx-pool, only transactions from the above Transactions list will be included. to validate existing blocks we will exclude transactions from the mempool, to sequence new blocks we will include transactions from the mempool.
allow proposers to reorg their own chain

Specular changes differ from optimism/superchain engine-api spec, because we don't use deposit/system transactions:

https://op-geth.optimism.io/

https://github.com/ethereum-optimism/optimism/blob/develop/specs/exec-engine.md#engine-api

beacon/engine/types.go

MoonShiesty · 2023-09-30T00:19:54Z

@ujvl ready for review

I added an L2EngineApi param to the ChainConfig, which enables our L2 engine ap modifications. This change really helped reduce the specular diff.

Right now, L2EngineApi can be enabled/disabled with the genesis.json file. If it makes more sense to use a cli flag, it looks like we need to move this parameter to a different config.

I also cleaned up how we disable the ConsensusAPI heartbeat.

LEAFERx

What does Optimism's RollupComputePendingBlock option mean? Why don't we need it?
Can you explain several inconsistent changes w/ optimism:
1. missing changes in https://github.com/ethereum-optimism/op-geth/blob/optimism/miner/worker.go#L559
2. missing changes in https://github.com/ethereum-optimism/op-geth/blob/optimism/miner/worker.go#L1059
The signature of getSealingBlock is different. Is this caused by different geth versions?

LEAFERx · 2023-10-02T03:58:37Z

beacon/engine/types.go

+	Random                common.Hash    `json:"prevRandao"            gencodec:"required"`
+	SuggestedFeeRecipient common.Address `json:"suggestedFeeRecipient" gencodec:"required"`
+	// <specular modification>
+	Withdrawals []*types.Withdrawal `json:"withdrawals,omitempty" gencodec:"optional"`


why it is marked as optional?

i have the same q

added this by mistake, fixed

LEAFERx · 2023-10-02T04:07:13Z

miner/payload_building.go

 	if err != nil {
 		return nil, err
 	}
 	// Construct a payload object for return.
 	payload := newPayload(empty, args.Id())
+	// <specular modification>
+	if args.NoTxPool { // don't start the background payload updating job if there is no tx pool to pull from
+		return payload, nil


do you need to mark as full here? ref https://github.com/ethereum-optimism/op-geth/blob/optimism/miner/payload_building.go#L221

LEAFERx · 2023-10-02T04:09:39Z

miner/worker.go

 		header.Extra = w.extra
 	}
+	// <specular modification>


missing /

ujvl · 2023-10-02T18:48:18Z

params/config.go

@@ -304,6 +304,9 @@ type ChainConfig struct {
 	Ethash    *EthashConfig `json:"ethash,omitempty"`
 	Clique    *CliqueConfig `json:"clique,omitempty"`
 	IsDevMode bool          `json:"isDev,omitempty"`
+	// <specular modification>
+	L2EngineApi bool `json:"l2EngineApi,omitempty"`


nit rename to

Suggested change

L2EngineApi bool `json:"l2EngineApi,omitempty"`

EnableL2EngineAPI bool `json:"l2EngineApi,omitempty"`

ujvl · 2023-10-02T18:54:37Z

miner/worker_test.go

@@ -468,7 +470,9 @@ func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine co
 	// This API should work even when the automatic sealing is enabled
 	w.start()
 	for _, c := range cases {
-		block, _, err := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, nil, false)
+		// <specular modification>
+		block, _, err := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, nil, false, nil, nil)


might be misreading but is this setting a bool to nil (2nd to last param)

i was actually passing an extra param. I did run make test to confirm all the tests pass but it must not have built this file. I'll look into how to actually build and run this test.

If we update to the latest geth, the new getSealingBlock api means we can eliminate the specular diff in this file.

ujvl · 2023-10-02T18:58:46Z

beacon/engine/types.go

+	Random                common.Hash    `json:"prevRandao"            gencodec:"required"`
+	SuggestedFeeRecipient common.Address `json:"suggestedFeeRecipient" gencodec:"required"`
+	// <specular modification>
+	Withdrawals []*types.Withdrawal `json:"withdrawals,omitempty" gencodec:"optional"`


i have the same q

MoonShiesty · 2023-10-04T16:54:32Z

What does Optimism's RollupComputePendingBlock option mean? Why don't we need it?

Can you explain several inconsistent changes w/ optimism:

missing changes in https://github.com/ethereum-optimism/op-geth/blob/optimism/miner/worker.go#L559

missing changes in https://github.com/ethereum-optimism/op-geth/blob/optimism/miner/worker.go#L1059

The signature of getSealingBlock is different. Is this caused by different geth versions?

RollupComputePendingBlock determines whether the miner computes a pending block: i.e. whether eth.getBlock('pending') returns the 'latest' block or a 'pending' block using the mempool. This shouldn't affect the engine-api payload/engine_forkchoiceUpdatedV1 changes in the PR.
https://github.com/ethereum-optimism/op-geth/blob/optimism/miner/worker.go#L559 I didn't implement RollupComputePendingBlock in this PR.
https://github.com/ethereum-optimism/op-geth/blob/optimism/miner/worker.go#L1059
Optimism passes a GasLimit as a part of the engine-api payload, I didn't implement the GasLimit parameter or make any changes to calculating a block's GasLimit as part of this PR.
op-geth updated to a later version of geth since I opened this PR. it might be easier to review these changes if I update to the latest geth.

MoonShiesty · 2023-10-04T18:00:31Z

cmd/utils/flags.go

+	EnableL2EngineApiFlag = &cli.BoolFlag{
+		Name:     "enableL2EngineApi",
+		Usage:    "Enable L2 Engine API",
+		Category: flags.EthCategory,


wasn't sure if this should go in the miner catagory instead

eth is good for now

siosw reviewed Sep 25, 2023

View reviewed changes

beacon/engine/types.go Show resolved Hide resolved

MoonShiesty force-pushed the engine-api branch 3 times, most recently from d5ae9f3 to eaf968a Compare September 29, 2023 22:36

MoonShiesty marked this pull request as ready for review September 30, 2023 00:11

LEAFERx reviewed Oct 2, 2023

View reviewed changes

ujvl reviewed Oct 2, 2023

View reviewed changes

MoonShiesty changed the title ~~[draft] engine-api support~~ engine-api support Oct 2, 2023

MoonShiesty commented Oct 4, 2023

View reviewed changes

MoonShiesty force-pushed the engine-api branch 2 times, most recently from 5d76f54 to 8e35e3a Compare October 4, 2023 18:44

LEAFERx approved these changes Oct 4, 2023

View reviewed changes

MoonShiesty force-pushed the engine-api branch 4 times, most recently from 8e35e3a to ebc427e Compare October 9, 2023 16:33

ujvl approved these changes Oct 9, 2023

View reviewed changes

MoonShiesty force-pushed the engine-api branch from ebc427e to c92471a Compare October 9, 2023 19:54

MoonShiesty changed the base branch from specular to rebase-geth-1.13.2 October 9, 2023 19:57

MoonShiesty force-pushed the engine-api branch from c92471a to c8eb311 Compare October 9, 2023 20:01

support deterministic execution via engine-api

50901bb

MoonShiesty force-pushed the engine-api branch from c8eb311 to 50901bb Compare October 9, 2023 20:02

MoonShiesty merged this pull request into fabriqnetwork:rebase-geth-1.13.2 Oct 10, 2023

MoonShiesty deleted the engine-api branch October 10, 2023 20:22

MoonShiesty restored the engine-api branch October 10, 2023 20:24

MoonShiesty mentioned this pull request Oct 10, 2023

Engine api #7

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

engine-api support #5

engine-api support #5

MoonShiesty commented Sep 20, 2023 •

edited

Loading

MoonShiesty commented Sep 30, 2023

LEAFERx left a comment •

edited

Loading

LEAFERx Oct 2, 2023

ujvl Oct 2, 2023

MoonShiesty Oct 4, 2023

LEAFERx Oct 2, 2023

MoonShiesty Oct 4, 2023

LEAFERx Oct 2, 2023

MoonShiesty Oct 4, 2023

ujvl Oct 2, 2023

ujvl Oct 2, 2023

MoonShiesty Oct 4, 2023 •

edited

Loading

MoonShiesty Oct 4, 2023

ujvl Oct 2, 2023

MoonShiesty commented Oct 4, 2023

MoonShiesty Oct 4, 2023 •

edited

Loading

LEAFERx Oct 4, 2023

	L2EngineApi bool `json:"l2EngineApi,omitempty"`
	EnableL2EngineAPI bool `json:"l2EngineApi,omitempty"`

engine-api support #5

engine-api support #5

Conversation

MoonShiesty commented Sep 20, 2023 • edited Loading

MoonShiesty commented Sep 30, 2023

LEAFERx left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MoonShiesty Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MoonShiesty commented Oct 4, 2023

MoonShiesty Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MoonShiesty commented Sep 20, 2023 •

edited

Loading

LEAFERx left a comment •

edited

Loading

MoonShiesty Oct 4, 2023 •

edited

Loading

MoonShiesty Oct 4, 2023 •

edited

Loading