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

Upgrade optimism to v1.9.5 #316

Merged
merged 14 commits into from
Dec 19, 2024
Merged
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
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ test-all:

.PHONY: e2e
e2e:
$(GO_WRAPPER) test -v ./e2e
mkdir -p $(E2E_ARTIFACTS_PATH)
$(GO_WRAPPER) test -v ./e2e > $(E2E_ARTIFACTS_PATH)/stdout 2> $(E2E_ARTIFACTS_PATH)/stderr

.PHONY: install-golangci-lint
install-golangci-lint:
Expand Down
5 changes: 4 additions & 1 deletion adapters.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/ethereum-optimism/optimism/op-node/rollup/derive"
"github.com/ethereum/go-ethereum/common/hexutil"
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/polymerdao/monomer/utils"
rolluptypes "github.com/polymerdao/monomer/x/rollup/types"
)

Expand All @@ -28,7 +29,9 @@ func AdaptPayloadTxsToCosmosTxs(ethTxs []hexutil.Bytes) (bfttypes.Txs, error) {
if ethL1AttributesTx.Type() != ethtypes.DepositTxType {
return nil, errors.New("first transaction is not a deposit transaction")
}
l1BlockInfo, err := derive.L1BlockInfoFromBytes(&rollup.Config{}, uint64(time.Now().Unix()), ethL1AttributesTx.Data())
l1BlockInfo, err := derive.L1BlockInfoFromBytes(&rollup.Config{
EcotoneTime: utils.Ptr(uint64(0)), // TODO: this is a hack, but it works for now.
natebeauregard marked this conversation as resolved.
Show resolved Hide resolved
}, uint64(time.Now().Unix()), ethL1AttributesTx.Data())
Comment on lines +32 to +34
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential integer overflow in time conversion

Converting time.Now().Unix() (int64) to uint64 could cause overflow for negative timestamps.

Consider this fix:

-uint64(time.Now().Unix())
+uint64(max(0, time.Now().Unix()))

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 golangci-lint (1.62.2)

[high] 34-34: G115: integer overflow conversion int64 -> uint64

(gosec)


💡 Codebase verification

⚠️ Potential issue

EcotoneTime Temporary Implementation Requires Attention

The hardcoded EcotoneTime: utils.Ptr(uint64(0)) is used in three locations:

  • adapters.go: Main implementation marked as a hack
  • e2e/stack_test.go: Test implementation marked as hacky
  • testutils/utils.go: Test utility implementation

This confirms that the temporary solution is used consistently across the codebase, but it's primarily in the main implementation and test code. Given this is marked as a hack and appears in production code, this needs to be properly implemented before going to production.

🔗 Analysis chain

Address the temporary EcotoneTime solution

The current implementation uses a hardcoded EcotoneTime: utils.Ptr(uint64(0)) with a TODO comment. This should be properly implemented before the code goes to production.

Let's check if there are other instances of this pattern:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other temporary EcotoneTime implementations
rg -l "EcotoneTime.*TODO|TODO.*EcotoneTime"

Length of output: 75


Script:

#!/bin/bash
# Let's check the actual content of these files to understand the EcotoneTime usage
rg -A 5 -B 5 "EcotoneTime.*TODO|TODO.*EcotoneTime"

Length of output: 1348


Script:

#!/bin/bash
# Let's check what EcotoneTime is and its usage in the codebase
rg -A 3 -B 3 "EcotoneTime" --type go

Length of output: 1380

🧰 Tools
🪛 golangci-lint (1.62.2)

[high] 34-34: G115: integer overflow conversion int64 -> uint64

(gosec)

if err != nil {
return nil, fmt.Errorf("l1 block info from bytes: %v", err)
}
Expand Down
22 changes: 13 additions & 9 deletions builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,11 @@ type Payload struct {
// from the consensus layer that must be included in the block.
InjectedTransactions bfttypes.Txs
// TODO: make the gas limit actually be enforced. Need to translate between cosmos and op gas limit.
GasLimit uint64
Timestamp uint64
NoTxPool bool
GasLimit uint64
Timestamp uint64
NoTxPool bool
ParentBeaconRoot *common.Hash
Coinbase common.Address
}

func (b *Builder) Build(ctx context.Context, payload *Payload) (*monomer.Block, error) {
Expand Down Expand Up @@ -131,11 +133,13 @@ func (b *Builder) Build(ctx context.Context, payload *Payload) (*monomer.Block,
return nil, fmt.Errorf("header by height: %v", err)
}
header := &monomer.Header{
ChainID: b.chainID,
Height: currentHeader.Height + 1,
Time: payload.Timestamp,
ParentHash: currentHeader.Hash,
GasLimit: payload.GasLimit,
ChainID: b.chainID,
Height: currentHeader.Height + 1,
Time: payload.Timestamp,
ParentHash: currentHeader.Hash,
GasLimit: payload.GasLimit,
ParentBeaconRoot: payload.ParentBeaconRoot,
Coinbase: payload.Coinbase,
}

cometHeader := header.ToComet()
Expand All @@ -160,7 +164,7 @@ func (b *Builder) Build(ctx context.Context, payload *Payload) (*monomer.Block,
return nil, fmt.Errorf("commit: %v", err)
}

ethState, err := state.New(currentHeader.StateRoot, b.ethstatedb, nil)
ethState, err := state.New(currentHeader.StateRoot, b.ethstatedb)
if err != nil {
return nil, fmt.Errorf("create ethereum state: %v", err)
}
Expand Down
34 changes: 18 additions & 16 deletions builder/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,19 +151,20 @@ func TestBuild(t *testing.T) {

ethStateRoot := gotBlock.Header.StateRoot
header := &monomer.Header{
ChainID: env.g.ChainID,
Height: uint64(postBuildInfo.GetLastBlockHeight()),
Time: payload.Timestamp,
ParentHash: genesisHeader.Header.Hash,
StateRoot: ethStateRoot,
GasLimit: payload.GasLimit,
ChainID: env.g.ChainID,
Height: uint64(postBuildInfo.GetLastBlockHeight()),
Time: payload.Timestamp,
ParentHash: genesisHeader.Header.Hash,
StateRoot: ethStateRoot,
GasLimit: payload.GasLimit,
ParentBeaconRoot: payload.ParentBeaconRoot,
}
wantBlock, err := monomer.MakeBlock(header, bfttypes.ToTxs(allTxs))
require.NoError(t, err)
verifyBlockContent(t, wantBlock, gotBlock, builtBlock)

// Eth state db.
ethState, err := state.New(ethStateRoot, env.ethstatedb, nil)
ethState, err := state.New(ethStateRoot, env.ethstatedb)
require.NoError(t, err)
appHash, err := getAppHashFromEVM(ethState, header)
require.NoError(t, err)
Expand Down Expand Up @@ -244,7 +245,7 @@ func TestRollback(t *testing.T) {
require.NoError(t, env.blockStore.UpdateLabels(block.Header.Hash, block.Header.Hash, block.Header.Hash))

// Eth state db before rollback.
ethState, err := state.New(block.Header.StateRoot, env.ethstatedb, nil)
ethState, err := state.New(block.Header.StateRoot, env.ethstatedb)
require.NoError(t, err)
require.NotEqual(t, ethState.GetStorageRoot(contracts.L2ApplicationStateRootProviderAddr), gethtypes.EmptyRootHash)

Expand All @@ -267,7 +268,7 @@ func TestRollback(t *testing.T) {
// We trust that the other parts of a block store rollback were done as well.

// Eth state db after rollback.
ethState, err = state.New(genesisHeader.StateRoot, env.ethstatedb, nil)
ethState, err = state.New(genesisHeader.StateRoot, env.ethstatedb)
require.NoError(t, err)
require.Equal(t, ethState.GetStorageRoot(contracts.L2ApplicationStateRootProviderAddr), gethtypes.EmptyRootHash)

Expand Down Expand Up @@ -378,12 +379,13 @@ func TestBuildRollupTxs(t *testing.T) {

ethStateRoot := gotBlock.Header.StateRoot
header := monomer.Header{
ChainID: env.g.ChainID,
Height: uint64(postBuildInfo.GetLastBlockHeight()),
Time: payload.Timestamp,
ParentHash: genesisBlock.Header.Hash,
StateRoot: ethStateRoot,
GasLimit: payload.GasLimit,
ChainID: env.g.ChainID,
Height: uint64(postBuildInfo.GetLastBlockHeight()),
Time: payload.Timestamp,
ParentHash: genesisBlock.Header.Hash,
StateRoot: ethStateRoot,
GasLimit: payload.GasLimit,
ParentBeaconRoot: payload.ParentBeaconRoot,
}
wantBlock, err := monomer.MakeBlock(&header, txs)
require.NoError(t, err)
Expand Down Expand Up @@ -416,7 +418,7 @@ func setupTestEnvironment(t *testing.T) testEnvironment {
pool := mempool.New(testutils.NewMemDB(t))
blockStore := testutils.NewLocalMemDB(t)
txStore := txstore.NewTxStore(testutils.NewCometMemDB(t))
ethstatedb := testutils.NewEthStateDB(t)
ethstatedb := state.NewDatabaseForTesting()

app := testapp.NewTest(t, chainID.String())
appState := testapp.MakeGenesisAppState(t, app)
Expand Down
Binary file modified cmd/monogen/testapp.zip
Binary file not shown.
Loading
Loading