Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: pass coinbase from consensus node #179

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions consensus/l2/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ var (
errInvalidNonce = errors.New("invalid nonce")
errInvalidUncleHash = errors.New("invalid uncle hash")
errInvalidTimestamp = errors.New("invalid timestamp")
errInvalidCoinbase = errors.New("invalid coinbase")
)

type Consensus struct {
Expand Down Expand Up @@ -142,9 +141,6 @@ func (l2 *Consensus) verifyHeader(chain consensus.ChainHeaderReader, header, par
return errInvalidUncleHash
}

if l2.config.Morph.FeeVaultEnabled() && header.Coinbase != types.EmptyAddress {
return errInvalidCoinbase
}
// Verify the timestamp
if header.Time <= parent.Time {
return errInvalidTimestamp
Expand Down Expand Up @@ -187,10 +183,6 @@ func (l2 *Consensus) Prepare(chain consensus.ChainHeaderReader, header *types.He
header.Nonce = l2Nonce
header.UncleHash = types.EmptyUncleHash
header.Extra = []byte{} // disable extra field filling with bytes
// set coinbase to empty address, if feeVault is enabled
if l2.config.Morph.FeeVaultEnabled() {
header.Coinbase = types.EmptyAddress
}
return nil
}

Expand Down
5 changes: 3 additions & 2 deletions eth/catalyst/api_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ type assembleBlockParamsMarshaling struct {
//go:generate go run github.com/fjl/gencodec -type AssembleL2BlockParams -field-override assembleL2BlockParamsMarshaling -out gen_l2blockparams.go

type AssembleL2BlockParams struct {
Number uint64 `json:"number" gencodec:"required"`
Transactions [][]byte `json:"transactions"`
Number uint64 `json:"number" gencodec:"required"`
Coinbase common.Address `json:"coinbase"`
Transactions [][]byte `json:"transactions"`
}

// JSON type overrides for assembleL2BlockParams.
Expand Down
7 changes: 7 additions & 0 deletions eth/catalyst/gen_l2blockparams.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions eth/catalyst/l2_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ type executionResult struct {
}

func (api *l2ConsensusAPI) AssembleL2Block(params AssembleL2BlockParams) (*ExecutableL2Data, error) {
log.Info("Assembling block", "block number", params.Number)
log.Info("Assembling block", "block number", params.Number, "coinbase", params.Coinbase)
parent := api.eth.BlockChain().CurrentHeader()
expectedBlockNumber := parent.Number.Uint64() + 1
if params.Number != expectedBlockNumber {
Expand All @@ -81,7 +81,7 @@ func (api *l2ConsensusAPI) AssembleL2Block(params AssembleL2BlockParams) (*Execu
}

start := time.Now()
newBlockResult, err := api.eth.Miner().BuildBlock(parent.Hash(), time.Now(), transactions)
newBlockResult, err := api.eth.Miner().BuildBlock(parent.Hash(), time.Now(), params.Coinbase, transactions)
if err != nil {
return nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions eth/catalyst/l2_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func TestValidateL2Block(t *testing.T) {
// generic case
err = sendTransfer(config, ethService)
require.NoError(t, err)
ret, err := ethService.Miner().BuildBlock(ethService.BlockChain().CurrentHeader().Hash(), time.Now(), nil)
ret, err := ethService.Miner().BuildBlock(ethService.BlockChain().CurrentHeader().Hash(), time.Now(), common.Address{}, nil)
require.NoError(t, err)
block := ret.Block
l2Data := ExecutableL2Data{
Expand Down Expand Up @@ -170,7 +170,7 @@ func TestNewL2Block(t *testing.T) {

err := sendTransfer(config, ethService)
require.NoError(t, err)
ret, err := ethService.Miner().BuildBlock(ethService.BlockChain().CurrentHeader().Hash(), time.Now(), nil)
ret, err := ethService.Miner().BuildBlock(ethService.BlockChain().CurrentHeader().Hash(), time.Now(), common.Address{}, nil)
block := ret.Block
require.NoError(t, err)
l2Data := ExecutableL2Data{
Expand Down Expand Up @@ -223,7 +223,7 @@ func TestNewSafeL2Block(t *testing.T) {

err := sendTransfer(config, ethService)
require.NoError(t, err)
ret, err := ethService.Miner().BuildBlock(ethService.BlockChain().CurrentHeader().Hash(), time.Now(), nil)
ret, err := ethService.Miner().BuildBlock(ethService.BlockChain().CurrentHeader().Hash(), time.Now(), common.Address{}, nil)
require.NoError(t, err)
block := ret.Block
l2Data := SafeL2Data{
Expand Down
3 changes: 2 additions & 1 deletion ethclient/authclient/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

// AssembleL2Block assembles L2 Block used for L2 sequencer to propose a block in L2 consensus progress
func (ec *Client) AssembleL2Block(ctx context.Context, number *big.Int, transactions types.Transactions) (*catalyst.ExecutableL2Data, error) {
func (ec *Client) AssembleL2Block(ctx context.Context, number *big.Int, coinbase common.Address, transactions types.Transactions) (*catalyst.ExecutableL2Data, error) {
txs := make([][]byte, 0, len(transactions))
for i, tx := range transactions {
bz, err := tx.MarshalBinary()
Expand All @@ -24,6 +24,7 @@ func (ec *Client) AssembleL2Block(ctx context.Context, number *big.Int, transact
var result catalyst.ExecutableL2Data
err := ec.c.CallContext(ctx, &result, "engine_assembleL2Block", &catalyst.AssembleL2BlockParams{
Number: number.Uint64(),
Coinbase: coinbase,
Transactions: txs,
})
return &result, err
Expand Down
3 changes: 2 additions & 1 deletion miner/miner.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,11 @@ func (miner *Miner) getSealingBlockAndState(params *generateParams) (*NewBlockRe
}
}

func (miner *Miner) BuildBlock(parentHash common.Hash, timestamp time.Time, transactions types.Transactions) (*NewBlockResult, error) {
func (miner *Miner) BuildBlock(parentHash common.Hash, timestamp time.Time, coinbase common.Address, transactions types.Transactions) (*NewBlockResult, error) {
return miner.getSealingBlockAndState(&generateParams{
timestamp: uint64(timestamp.Unix()),
parentHash: parentHash,
coinbase: coinbase,
transactions: transactions,
timeout: miner.newBlockTimeout,
})
Expand Down
3 changes: 2 additions & 1 deletion miner/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ func (miner *Miner) prepareWork(genParams *generateParams) (*environment, error)
timestamp = parent.Time() + 1
}
var coinBase common.Address
if genParams.coinbase != (common.Address{}) {
if miner.chainConfig.IsMorph204(timestamp) && // only when Morph204 is activated can we set the coinbase
genParams.coinbase != (common.Address{}) {
coinBase = genParams.coinbase
}
header, err := miner.makeHeader(parent, timestamp, coinBase)
Expand Down
5 changes: 5 additions & 0 deletions params/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@ type ChainConfig struct {
BernoulliBlock *big.Int `json:"bernoulliBlock,omitempty"` // Bernoulli switch block (nil = no fork, 0 = already on bernoulli)
CurieBlock *big.Int `json:"curieBlock,omitempty"` // Curie switch block (nil = no fork, 0 = already on curie)
Morph203Time *uint64 `json:"morph203Time,omitempty"` // Morph203Time switch time (nil = no fork, 0 = already on morph203)
Morph204Time *uint64 `json:"morph204Time,omitempty"`

// TerminalTotalDifficulty is the amount of total difficulty reached by
// the network that triggers the consensus upgrade.
Expand Down Expand Up @@ -748,6 +749,10 @@ func (c *ChainConfig) IsMorph203(now uint64) bool {
return isForkedTime(now, c.Morph203Time)
}

func (c *ChainConfig) IsMorph204(now uint64) bool {
return isForkedTime(now, c.Morph204Time)
}
Comment on lines +752 to +754
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

❓ Verification inconclusive

Consistent implementation for new fork activation check

The IsMorph204 method follows the established pattern for fork detection by leveraging the existing isForkedTime helper function, maintaining codebase consistency.

Note that you'll need to update the CheckConfigForkOrder method to include this new fork in the sequence to properly enforce fork ordering.


🏁 Script executed:

#!/bin/bash
# Check if Morph204Time has been added to the fork ordering check
grep -A 30 "CheckConfigForkOrder.*func" params/config.go | grep -B 5 -A 15 "for.*range.*fork" | grep "morph204Time"

Length of output: 115


Action Required: Update Fork Ordering in CheckConfigForkOrder

The new fork activation check in IsMorph204 is implemented consistently using the existing isForkedTime helper. However, our verification indicates that CheckConfigForkOrder has not yet been updated to include Morph204Time. Please update the fork ordering logic in CheckConfigForkOrder (located in params/config.go) to incorporate this new fork, ensuring that fork activation is enforced in the intended sequence.


// IsTerminalPoWBlock returns whether the given block is the last block of PoW stage.
func (c *ChainConfig) IsTerminalPoWBlock(parentTotalDiff *big.Int, totalDiff *big.Int) bool {
if c.TerminalTotalDifficulty == nil {
Expand Down