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.

Sign up for GitHub

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

replace "enable_l2_blob" with "l2BlobTime" #60

Merged
merged 3 commits into from
Sep 18, 2024
Merged

replace "enable_l2_blob" with "l2BlobTime" #60

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e117904
replace "enable_l2_blob" with "l2BlobTime"
blockchaindevsh Sep 17, 2024
cfc688c
move l2BlobTime to ChainConfig
blockchaindevsh Sep 18, 2024
bf91815
mod comment for L2GenesisBlobTimeOffset
blockchaindevsh Sep 18, 2024
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
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ require (
rsc.io/tmplfunc v0.0.3 // indirect
)

replace github.com/ethereum/go-ethereum v1.13.15 => github.com/blockchaindevsh/op-geth v0.0.0-20240711035602-dfbe1492069c
replace github.com/ethereum/go-ethereum v1.13.15 => github.com/ethstorage/op-geth v0.0.0-20240918005936-a18d1c99aecb

//replace github.com/ethereum/go-ethereum v1.13.9 => ../op-geth

Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
github.com/bits-and-blooms/bitset v1.10.0 h1:ePXTeiPEazB5+opbv5fr8umg2R/1NlzgDsyepwsSr88=
github.com/bits-and-blooms/bitset v1.10.0/go.mod h1:7hO7Gc7Pp1vODcmWvKMRA9BNmbv6a/7QIWpPxHddWR8=
github.com/blockchaindevsh/op-geth v0.0.0-20240711035602-dfbe1492069c h1:uSibLxYF1JVhapdx6xszr5f/Bzm2RU3l66mag3KdZoY=
github.com/blockchaindevsh/op-geth v0.0.0-20240711035602-dfbe1492069c/go.mod h1:8tQ6r0e1NNJbSVHzYKafQqf62gV9BzZR+SKkXRckjLM=
github.com/boltdb/bolt v1.3.1 h1:JQmyP4ZBrce+ZQu0dY660FMfatumYDLun9hBCUVIkF4=
github.com/boltdb/bolt v1.3.1/go.mod h1:clJnj/oiGkjum5o1McbSZDSLxVThjynRyGBgiAx27Ps=
github.com/bradfitz/go-smtpd v0.0.0-20170404230938-deb6d6237625/go.mod h1:HYsPBTaaSFSlLx/70C2HPIMNZpVV8+vt/A+FMnYP11g=
Expand Down Expand Up @@ -183,6 +181,8 @@ github.com/ethereum/c-kzg-4844 v0.4.0 h1:3MS1s4JtA868KpJxroZoepdV0ZKBp3u/O5HcZ7R
github.com/ethereum/c-kzg-4844 v0.4.0/go.mod h1:VewdlzQmpT5QSrVhbBuGoCdFJkpaJlO1aQputP83wc0=
github.com/ethstorage/da-server v0.0.0-20240628094857-ed2ee4ff52d9 h1:yRJRj81rP83R5dZEsYitAFcqRagK/e+UxhiSSK4/Wds=
github.com/ethstorage/da-server v0.0.0-20240628094857-ed2ee4ff52d9/go.mod h1:YeveZNzQFdseeqS0mQavqfyMI+U9l1GQu0CEwezTZA8=
github.com/ethstorage/op-geth v0.0.0-20240918005936-a18d1c99aecb h1:f076fgZk2/vUmrpHewhKOJgLXjQqfzsRlU/gKVPiKbg=
github.com/ethstorage/op-geth v0.0.0-20240918005936-a18d1c99aecb/go.mod h1:8tQ6r0e1NNJbSVHzYKafQqf62gV9BzZR+SKkXRckjLM=
github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
github.com/fatih/color v1.15.0 h1:kOqh6YHBtK8aywxGerMG2Eq3H6Qgoqeo13Bk2Mv/nBs=
github.com/fatih/color v1.15.0/go.mod h1:0h5ZqXfHYED7Bhv2ZJamyIOUej9KtShiJESRwBDUSsw=
Expand Down
19 changes: 15 additions & 4 deletions op-chain-ops/genesis/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,8 @@ type DeployConfig struct {
// UseInterop is a flag that indicates if the system is using interop
UseInterop bool `json:"useInterop,omitempty"`
// l2 blob related configs
blockchaindevsh marked this conversation as resolved.
Show resolved Hide resolved
EnableL2Blob bool `json:"enable_l2_blob,omitempty"`
DACURLS []string `json:"dac_urls,omitempty"`
L2GenesisBlobTimeOffset *hexutil.Uint64 `json:"l2GenesisBlobTimeOffset,omitempty"`
DACURLS []string `json:"dac_urls,omitempty"`
Copy link

Choose a reason for hiding this comment

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

As I understand it, we might want to remove the DACURLS from the rollup configuration, or do we want to do it in a seprate PR?

Copy link
Collaborator Author

@blockchaindevsh blockchaindevsh Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, there's a separate issue for that(no PR yet).


// UseSoulGasToken is a flag that indicates if the system is using SoulGasToken
UseSoulGasToken bool `json:"useSoulGasToken"`
Expand Down Expand Up @@ -612,6 +612,17 @@ func (d *DeployConfig) InteropTime(genesisTime uint64) *uint64 {
return &v
}

func (d *DeployConfig) L2BlobTime(genesisTime uint64) *uint64 {
if d.L2GenesisBlobTimeOffset == nil {
return nil
}
v := uint64(0)
if offset := *d.L2GenesisBlobTimeOffset; offset > 0 {
v = genesisTime + uint64(offset)
}
return &v
}

// RollupConfig converts a DeployConfig to a rollup.Config. If Ecotone is active at genesis, the
// Overhead value is considered a noop.
func (d *DeployConfig) RollupConfig(l1StartBlock *types.Block, l2GenesisBlockHash common.Hash, l2GenesisBlockNumber uint64) (*rollup.Config, error) {
Expand All @@ -631,9 +642,9 @@ func (d *DeployConfig) RollupConfig(l1StartBlock *types.Block, l2GenesisBlockHas
}

var l2BlobConfig *rollup.L2BlobConfig
if d.EnableL2Blob {
if d.L2GenesisBlobTimeOffset != nil {
l2BlobConfig = &rollup.L2BlobConfig{
EnableL2Blob: true,
L2BlobTime: d.L2BlobTime(l1StartBlock.Time()),
}
if len(d.DACURLS) > 0 {
l2BlobConfig.DACConfig = &rollup.DACConfig{URLS: d.DACURLS}
Expand Down
2 changes: 1 addition & 1 deletion op-chain-ops/genesis/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ func NewL2Genesis(config *DeployConfig, block *types.Block) (*core.Genesis, erro
EcotoneTime: config.EcotoneTime(block.Time()),
FjordTime: config.FjordTime(block.Time()),
InteropTime: config.InteropTime(block.Time()),
L2BlobTime: config.L2BlobTime(block.Time()),
Optimism: &params.OptimismConfig{
EIP1559Denominator: eip1559Denom,
EIP1559Elasticity: eip1559Elasticity,
EIP1559DenominatorCanyon: eip1559DenomCanyon,
EnableL2Blob: config.EnableL2Blob,
IsSoulBackedByNative: config.IsSoulBackedByNative,
UseSoulGasToken: config.UseSoulGasToken,
},
Expand Down
6 changes: 3 additions & 3 deletions op-node/p2p/gossip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func BuildBlocksValidator(log log.Logger, cfg *rollup.Config, runCfg GossipRunti
now := uint64(time.Now().Unix())

// [REJECT] if the `payload.timestamp` is older than 60 seconds in the past
if uint64(payload.Timestamp) < now-60 {
if uint64(payload.Timestamp) < now-60 || uint64(payload.Timestamp) < cfg.BlockTime /* ensure timestamp>=BlockTime since we'll do subtraction below */ {
log.Warn("payload is too old", "timestamp", uint64(payload.Timestamp))
return pubsub.ValidationReject
}
Expand Down Expand Up @@ -362,13 +362,13 @@ func BuildBlocksValidator(log log.Logger, cfg *rollup.Config, runCfg GossipRunti

if blockVersion.HasBlobProperties() {
// [REJECT] if the block is on a topic >= V3 and has a blob gas used value that is not zero
if payload.BlobGasUsed == nil || (!cfg.IsL2BlobEnabled() && *payload.BlobGasUsed != 0) {
if payload.BlobGasUsed == nil || (!cfg.IsL2Blob(uint64(payload.Timestamp)-cfg.BlockTime) && *payload.BlobGasUsed != 0) {
Copy link

Choose a reason for hiding this comment

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

Why we need to subtract the blockTime (2s) here?

Copy link
Collaborator Author

@blockchaindevsh blockchaindevsh Sep 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Because the geth side is always checking against the parent block to test HF activation of the current block. That's why the parameter for IsL2Blob is named parentTime.

log.Warn("payload is on v3 topic, but has non-zero blob gas used", "bad_hash", payload.BlockHash.String(), "blob_gas_used", payload.BlobGasUsed)
return pubsub.ValidationReject
}

// [REJECT] if the block is on a topic >= V3 and has an excess blob gas value that is not zero
if payload.ExcessBlobGas == nil || (!cfg.IsL2BlobEnabled() && *payload.ExcessBlobGas != 0) {
if payload.ExcessBlobGas == nil || (!cfg.IsL2Blob(uint64(payload.Timestamp)-cfg.BlockTime) && *payload.ExcessBlobGas != 0) {
log.Warn("payload is on v3 topic, but has non-zero excess blob gas", "bad_hash", payload.BlockHash.String(), "excess_blob_gas", payload.ExcessBlobGas)
return pubsub.ValidationReject
}
Expand Down
9 changes: 5 additions & 4 deletions op-node/rollup/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ type Config struct {
}

type L2BlobConfig struct {
DACConfig *DACConfig `json:"dac_config,omitempty"`
EnableL2Blob bool `json:"enable_l2_blob,omitempty"`
DACConfig *DACConfig `json:"dac_config,omitempty"`
L2BlobTime *uint64 `json:"l2BlobTime,omitempty"`
}
type DACConfig struct {
URLS []string `json:"urls,omitempty"`
Expand All @@ -165,8 +165,9 @@ func (dacConfig *DACConfig) Client() DACClient {
return client.New(dacConfig.URLS)
}

func (cfg *Config) IsL2BlobEnabled() bool {
return cfg.L2BlobConfig != nil && cfg.L2BlobConfig.EnableL2Blob
// IsL2Blob returns whether l2 blob is enabled
func (cfg *Config) IsL2Blob(parentTime uint64) bool {
return cfg.L2BlobConfig != nil && *cfg.L2BlobConfig.L2BlobTime <= parentTime
}

func (cfg *Config) DACConfig() *DACConfig {
Expand Down