eth/catalyst: add catalyst API prototype #22641

gballet · 2021-04-09T12:17:00Z

Remove all the superfluous code from #22468 to prepare for rayonism:

Remove RANDAOMIX and BEACONSTATEROOT instructions
Rename API calls and change inputs

…ructions Co-Authored-By: Mikhail Kalinin <noblesse.knight@gmail.com>

rjl493456442 · 2021-04-13T05:32:49Z

eth/eth2.go

+		return nil, fmt.Errorf("child timestamp lower than parent's: %d >= %d", parent.Time(), params.Timestamp)
+	}
+	// this will ensure we're not going off too far in the future
+	if now := uint64(time.Now().Unix()); params.Timestamp > now+1 {


I'm not sure the sanity check is needed here. I image it's possible in the future that users put beacon node and geth node into two separate machines and connect them via RPC. So it's possible that the geth node machine is time delayed and return the block with long time sleeping(e.g. 5 second).

IIUC, in the eth2, the block has to be purposed in the given time limit. Maybe it would make more sense to do the sleeping in the eth2 part?

Just throw out the thoughts.

rjl493456442 · 2021-04-13T05:41:49Z

eth/eth2.go

+	if err != nil {
+		return nil, err
+	}
+	signer := types.NewEIP155Signer(bc.Config().ChainID)


Please use types.LatestSigner()

rjl493456442 · 2021-04-13T05:42:38Z

eth/eth2.go

+}
+
+func (api *Eth2API) commitTransaction(tx *types.Transaction, coinbase common.Address) error {
+	//snap := eth2rpc.current.state.Snapshot()


Any particular reason to remove this "snapshot" mechanism?

yes: it didn't work and wasn't needed for the prototype

I don't think it makes sense to try activating it now, this part will be replaced with your PR soon enough.

rjl493456442 · 2021-04-13T05:46:19Z

eth/eth2.go

+		// Update the block hash in all logs since it is now available and not when the
+		// receipt/log of individual transactions were created.
+		for _, log := range receipt.Logs {
+			log.BlockHash = hash


QQ: Looks like the log.BlockHash is set to "application-layer" block hash. Do we have any spec for defining the expected behavior?

Although the eth2 block hash is not available here. I guess the application layer block hash is the only option here.

rjl493456442 · 2021-04-13T05:48:17Z

eth/eth2.go

+	Timestamp              uint64         `json:"timestamp"`
+	RecentBeaconBlockRoots []common.Hash  `json:"recent_beacon_block_roots"`
+	ExecutableData         ExecutableData `json:"executable_data"`
+}


RandaoMix, Slot and RecentBeaconBlockRoots seems unused. I guess they are removed from the spec?

These are useful to have when implementing the opcodes (which were present in previous catalyst versions). It was indeed removed from the latest spec for simplicity of the first devnet.

You can find the latest spec for RPC here: https://notes.ethereum.org/@n0ble/rayonism-the-merge-spec

Right, according to the spec there is also ExecutionPayload object in the params of this method, https://notes.ethereum.org/@n0ble/rayonism-the-merge-spec#Parameters1

holiman · 2021-04-14T09:05:04Z

eth/eth2.go

+func (api *Eth2API) SetHead(newHead common.Hash) error {
+	//oldBlock := api.eth.BlockChain().CurrentBlock()
+
+	//if oldBlock.Hash() == newHead {
+	//return nil
+	//}
+
+	//newBlock := api.eth.BlockChain().GetBlockByHash(newHead)
+
+	//err := api.eth.BlockChain().Reorg(oldBlock, newBlock)
+	//if err != nil {
+	//return err
+	//}
+	//api.head = newHead
+	return nil
+}


Isn't this needed?

in a previous version of the spec, yes. But not anymore. I kept it in case people decide they need it after all. It will be replaced with a call to Gary's PR soon enough.In the mean time, it's just a placeholder.

protolambda · 2021-04-14T10:34:19Z

eth/backend.go

@@ -337,6 +342,11 @@ func (s *Ethereum) APIs() []rpc.API {
 			Version:   "1.0",
 			Service:   s.netRPCService,
 			Public:    true,
+		}, {
+			Namespace: "eth2",


See https://notes.ethereum.org/@n0ble/rayonism-the-merge-spec
This was renamed to "consensus". @mkalinin

The preference is to use consensus name for this namespace. Should not be difficult to update, right?

mkalinin · 2021-04-15T07:32:35Z

eth/eth2.go

+	BlockHash    common.Hash          `json:"block_hash"`
+	ParentHash   common.Hash          `json:"parent_hash"`
+	Difficulty   *big.Int             `json:"difficulty"`
+}


There is a slightly different field set for the ExecutionPayload in the Rayonism spec. Please, take a look here https://notes.ethereum.org/@n0ble/rayonism-the-merge-spec#Parameters1.

mkalinin · 2021-04-15T07:35:41Z

eth/eth2.go

+	block := insertBlockParamsToBlock(params, number)
+	_, err := api.eth.BlockChain().InsertChainWithoutSealVerification(types.Blocks([]*types.Block{block}))
+
+	return (err == nil), err


May a result object be returned here with valid flag in it (https://notes.ethereum.org/@n0ble/rayonism-the-merge-spec#Returns1)? The idea is to make this object extensible to allow for adding additional info in return like the reason of block invalidity (might be useful for debugging).

mkalinin · 2021-04-15T07:37:17Z

eth/eth2.go

+	//return err
+	//}
+	//api.head = newHead
+	return nil


You might want to adjust the result with the spec (https://notes.ethereum.org/@n0ble/rayonism-the-merge-spec#Return). The same is for FinalizeBlock method.

holiman · 2021-04-16T10:25:37Z

core/blockchain.go

@@ -1693,6 +1693,44 @@ func (bc *BlockChain) InsertChain(chain types.Blocks) (int, error) {
 	return n, err
 }

+// InsertChainWithoutSealVerification works exactly the same
+// except for seal verification, seal verification is omitted
+func (bc *BlockChain) InsertChainWithoutSealVerification(chain types.Blocks) (int, error) {


This is ever called with a slice containing one block, AFAICT. Might just make it take a single block on input, and throw away the whole sanity check loop

gballet · 2021-04-16T10:26:39Z

eth/eth2.go

+		return &NewBlockReturn{false}, fmt.Errorf("could not find parent %x", params.ParentHash)
+	}
+	number := big.NewInt(0)
+	number.Add(parent.Number(), big.NewInt(1))


don't compute the number

holiman · 2021-04-16T10:33:12Z

eth/eth2.go

+	return &Eth2API{eth: eth}
+}
+
+type eth2bpenv struct {


rename to something meaningful

holiman · 2021-04-16T10:35:03Z

eth/eth2.go

+	//oldBlock := api.eth.BlockChain().CurrentBlock()
+
+	//if oldBlock.Hash() == newHead {
+	//return nil
+	//}
+
+	//newBlock := api.eth.BlockChain().GetBlockByHash(newHead)
+
+	//err := api.eth.BlockChain().Reorg(oldBlock, newBlock)
+	//if err != nil {
+	//return err
+	//}
+	//api.head = newHead


Perhaps remove the dead commented code?

… one block

This makes the API object encoding adhere to the spec as written: all integers are hex string QUANTITY, all bytes data is hex string DATA, and transactions are hex strings instead of JSON objects.

types.LatestSigner is for code that doesn't know the block number. When creating the new block in catalyst, the number is known, and MakeSigner should be used.

This makes 'env' local to AssembleBlock instead of keeping it in the api object. Doing this avoids races for concurrent calls of AssembleBlock.

In receipts and logs, the block location fields are not part of consensus and don't need to be derived because they will be thrown away. Also remove pointless assignment of the ReceiptHash after this. Block.Header() returns a copy of the header, so modifying the receipt hash doesn't do anything.

This change adds the --catalyst flag, enabling an RPC API for eth2 integration. In this initial version, catalyst mode also disables all peer-to-peer networking. Co-authored-by: Mikhail Kalinin <noblesse.knight@gmail.com> Co-authored-by: Felix Lange <fjl@twurst.com>

gballet and others added 3 commits April 8, 2021 16:35

eth2: introduce RPC apis, catalyst command line option and extra inst…

d9916d1

…ructions Co-Authored-By: Mikhail Kalinin <noblesse.knight@gmail.com>

Enable catalyst with option --catalyst

fad2587

Fix rebase and merge errors

9a35a22

gballet requested review from holiman, karalabe and rjl493456442 as code owners April 9, 2021 12:17

gballet added 2 commits April 9, 2021 14:19

remove the code that isn't needed in rayonism

ccfb737

Fix unit tests

26ee92a

gballet force-pushed the catalyst-for-rayonism branch from 19fe89f to 26ee92a Compare April 9, 2021 13:15

rjl493456442 reviewed Apr 13, 2021

View reviewed changes

holiman reviewed Apr 14, 2021

View reviewed changes

protolambda reviewed Apr 14, 2021

View reviewed changes

integrate review feedback

dfdff92

mkalinin reviewed Apr 15, 2021

View reviewed changes

gballet added 6 commits April 15, 2021 18:23

Change difficulty calculations to accept 1 for each block

dae9d1a

Bring API up to spec

de593d0

Fix unit tests

ce8fff9

Add a success field to the response in GinalizeBlock and SetHead

0602ca7

quell linter warning

8e3adf9

fix console test breakage because of the namespace change

78e41ff

holiman reviewed Apr 16, 2021

View reviewed changes

gballet commented Apr 16, 2021

View reviewed changes

holiman reviewed Apr 16, 2021

View reviewed changes

eth/eth2.go Outdated

return &Eth2API{eth: eth}

}

type eth2bpenv struct {

Copy link

Contributor

holiman Apr 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to something meaningful

holiman reviewed Apr 16, 2021

View reviewed changes

fjl changed the title ~~eth/catalyst: add catalyst prototype for rayonism~~ eth/catalyst: add catalyst API prototype Apr 16, 2021

gballet and others added 13 commits April 16, 2021 15:18

Change signature of InsertChainWithoutSealVerification to only accept…

71165f4

… one block

Reuse the block number from ExecutableData in NewBlock

16be944

Review feedback: a more descriptive name for the execution environment

33c465b

remove commented code, it won't work out of the box

09e3867

eth/catalyst: remove zeroNonce

2c22b5c

eth/catalyst: unexport API types

7adf70b

eth/catalyst: fix encoding of API objects

01d16e5

This makes the API object encoding adhere to the spec as written: all integers are hex string QUANTITY, all bytes data is hex string DATA, and transactions are hex strings instead of JSON objects.

eth/catalyst: fix name of parenthash parameter

0eda836

eth/catalyst: make all params required

0a5bf45

Finalize and SetHead stubs return success upon call

43b88ae

Remove outdated comment

558649e

eth/catalyst: move Register to api.go

7b0a22d

eth/catalyst: improve error for misconfigured catalystBlock

77e5444

fjl added this to the 1.10.3 milestone Apr 16, 2021

fjl approved these changes Apr 16, 2021

View reviewed changes

fjl added 7 commits April 16, 2021 19:31

eth/catalyst: remove debug print

b75abfe

eth/catalyst: fix tx signer creation

be735b8

types.LatestSigner is for code that doesn't know the block number. When creating the new block in catalyst, the number is known, and MakeSigner should be used.

eth/catalyst: avoid keeping block env

b311fc8

This makes 'env' local to AssembleBlock instead of keeping it in the api object. Doing this avoids races for concurrent calls of AssembleBlock.

eth/catalyst: check error of sender derivation

dc8295a

eth/catalyst: skip the sender error check again, and explain why

5521b0a

eth/catalyst: remove more weird comments

7eea1cf

fjl merged commit f79cce5 into ethereum:master Apr 16, 2021

gballet mentioned this pull request Apr 20, 2021

eth2: catalyst RPC calls and command line options #22468

Closed

quorumbot mentioned this pull request Jul 14, 2022

[Upgrade] Go-Ethereum release v1.10.3 Consensys/quorum#1449

Closed

9 tasks

quorumbot mentioned this pull request Aug 5, 2022

[Upgrade] Go-Ethereum release v1.10.3 Consensys/quorum#1469

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

eth/catalyst: add catalyst API prototype #22641

eth/catalyst: add catalyst API prototype #22641

gballet commented Apr 9, 2021 •

edited

Loading

rjl493456442 Apr 13, 2021 •

edited

Loading

rjl493456442 Apr 13, 2021

rjl493456442 Apr 13, 2021

gballet Apr 14, 2021

gballet Apr 14, 2021

rjl493456442 Apr 13, 2021

rjl493456442 Apr 13, 2021

rjl493456442 Apr 13, 2021

protolambda Apr 14, 2021

mkalinin Apr 15, 2021

holiman Apr 14, 2021

gballet Apr 14, 2021 •

edited

Loading

protolambda Apr 14, 2021

mkalinin Apr 14, 2021

mkalinin Apr 15, 2021

mkalinin Apr 15, 2021

mkalinin Apr 15, 2021

holiman Apr 16, 2021

gballet Apr 16, 2021

holiman Apr 16, 2021

holiman Apr 16, 2021

eth/catalyst: add catalyst API prototype #22641

eth/catalyst: add catalyst API prototype #22641

Conversation

gballet commented Apr 9, 2021 • edited Loading

rjl493456442 Apr 13, 2021 • 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

Choose a reason for hiding this comment

gballet Apr 14, 2021 • 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

gballet commented Apr 9, 2021 •

edited

Loading

rjl493456442 Apr 13, 2021 •

edited

Loading

gballet Apr 14, 2021 •

edited

Loading