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

ops-bedrock: Beacon-chain devnet with Dencun + Ecotone upgrade #9117

Open
wants to merge 35 commits into
base: develop
Choose a base branch
from

Conversation

sebastianst
Copy link
Member

@sebastianst sebastianst commented Jan 19, 2024

Description

This PR makes the local devnet to use a real lighthouse Beacon node.

It also enables Delta by default on all e2e tests, plus L1 enables Cancun after a few blocks, and Ecotone a shortly after.

In a follow up PR, Ecotone and Cancun will be enabled at genesis.

The PR also improves fork selection in individual tests and fixes from fork tests (e.g. one Ecotone test didn't call its activation function).

The PR introduces a new package op-e2e/devnet to run e2e tests against the local devnet. It provides a minimal System abstraction that works with the withdrawals test, which got extracted and made into a reusable test.

ℹ️ For reviewers: the files in ops-bedrock/data are auto-generated.

Tests

Many fixed.

Additional context

Explicit fork selection in tests is somewhat improved, but is still brittle. We should completely remove fork selection by setting individual <Fork>TimeOffset fields in the deploy config and do it by a modular constructor of the configs using the With... option pattern.

Open TODOs are tracked at #10968

Metadata

Copy link

semgrep-app bot commented Jan 22, 2024

Semgrep found 6 sol-style-return-arg-fmt findings:

Named return arguments to functions must be appended with an underscore (_)

Ignore this finding from sol-style-return-arg-fmt.

Copy link
Contributor

github-actions bot commented Feb 7, 2024

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@protolambda
Copy link
Contributor

Squashed and rebased on latest develop, in case anyone wants to run this or experiment with Ecotone locally.

@protolambda protolambda changed the title ops-bedrock: add lighthouse L1 node to local devnet ops-bedrock: Beacon-chain devnet with Dencun + Ecotone upgrade Feb 7, 2024
@tynes
Copy link
Contributor

tynes commented Feb 12, 2024

Wondering if we plan on moving forward with this as we will be needing a devnet for interop development soon

@protolambda
Copy link
Contributor

@tynes yes, moving forward with this. Currently blocked on 2 things:

  • L1/L2 genesis refactor completion
  • eth2-testnet-genesis tool update, to generate a L1 dencun genesis, instead of a shapella one and having to schedule an upgrade. Working on updating this (implementing dencun support in underlying libs now).

@protolambda
Copy link
Contributor

eth2-testnet-genesis supports Dencun at genesis now

Copy link
Contributor

github-actions bot commented Mar 6, 2024

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added Stale and removed Stale labels Mar 6, 2024
Copy link
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added Stale and removed Stale labels Mar 22, 2024
Copy link
Contributor

github-actions bot commented Apr 8, 2024

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

Copy link
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

Copy link

semgrep-app bot commented May 3, 2024

Semgrep found 3 sol-style-return-arg-fmt findings:

  • packages/contracts-bedrock/scripts/Deployer.sol
  • packages/contracts-bedrock/scripts/L2Genesis.s.sol
  • packages/contracts-bedrock/scripts/Config.sol

Named return arguments to functions must be appended with an underscore (_)

Ignore this finding from sol-style-return-arg-fmt.

Copy link
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label May 21, 2024
@tynes
Copy link
Contributor

tynes commented May 21, 2024

How is the progress on this coming along?

@github-actions github-actions bot removed the Stale label May 22, 2024
@sebastianst
Copy link
Member Author

How is the progress on this coming along?

@tynes Blocked by a bunch of other more urgent work unfortunately. Hope to get to it on Friday or Monday.

Copy link
Contributor

github-actions bot commented Jun 6, 2024

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Jun 6, 2024
@sebastianst sebastianst removed the Stale label Jun 10, 2024
Copy link

semgrep-app bot commented Jun 10, 2024

Semgrep found 9 golang_fmt_errorf_no_params findings:

No fmt.Errorf invocations without fmt arguments allowed

Ignore this finding from golang_fmt_errorf_no_params.

Semgrep found 3 writable-filesystem-service findings:

Service 'grafana' is running with a writable root filesystem. This may allow malicious applications to download and run additional payloads, or modify container files. If an application inside a container has to save something temporarily consider using a tmpfs. Add 'read_only: true' to this service to prevent this.

Ignore this finding from writable-filesystem-service.

Semgrep found 3 no-new-privileges findings:

Service 'grafana' allows for privilege escalation via setuid or setgid binaries. Add 'no-new-privileges:true' in 'security_opt' to prevent this.

Ignore this finding from no-new-privileges.

Semgrep found 3 port-all-interfaces findings:

Service port is exposed on all interfaces

Ignore this finding from port-all-interfaces.

Semgrep found 1 gorm-hardcoded-secret finding:

A secret is hard-coded in the application. Secrets stored in source code, such as credentials, identifiers, and other types of sensitive data, can be leaked and used by internal or external malicious actors. It is recommended to rotate the secret and retrieve them from a secure secret vault or Hardware Security Module (HSM), alternatively environment variables can be used if allowed by your company policy.

View Dataflow Graph
flowchart LR
    classDef invis fill:white, stroke: none
    classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none

    subgraph File0["<b>indexer/database/db.go</b>"]
        direction LR
        %% Source

        subgraph Source
            direction LR

            v0["<a href=https://github.com/ethereum-optimism/optimism/blob/ab31d2b0485f84a9868d9aa6fffbdace836333bc/indexer/database/db.go#L46 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 46] &quot; password=%s&quot;</a>"]
        end
        %% Intermediate

        subgraph Traces0[Traces]
            direction TB

            v2["<a href=https://github.com/ethereum-optimism/optimism/blob/ab31d2b0485f84a9868d9aa6fffbdace836333bc/indexer/database/db.go#L46 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 46] dsn</a>"]
        end
        %% Sink

        subgraph Sink
            direction LR

            v1["<a href=https://github.com/ethereum-optimism/optimism/blob/ab31d2b0485f84a9868d9aa6fffbdace836333bc/indexer/database/db.go#L64 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 64] gorm.Open(postgres.Open(dsn), &gormConfig)</a>"]
        end
    end
    %% Class Assignment
    Source:::invis
    Sink:::invis

    Traces0:::invis
    File0:::invis

    %% Connections

    Source --> Traces0
    Traces0 --> Sink

Ignore this finding from gorm-hardcoded-secret.

Semgrep found 1 gorm-empty-password finding:

The application uses an empty credential. This can lead to unauthorized access by either an internal or external malicious actor. It is recommended to rotate the secret and retrieve them from a secure secret vault or Hardware Security Module (HSM), alternatively environment variables can be used if allowed by your company policy.

View Dataflow Graph
flowchart LR
    classDef invis fill:white, stroke: none
    classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none

    subgraph File0["<b>indexer/database/db.go</b>"]
        direction LR
        %% Source

        subgraph Source
            direction LR

            v0["<a href=https://github.com/ethereum-optimism/optimism/blob/ab31d2b0485f84a9868d9aa6fffbdace836333bc/indexer/database/db.go#L46 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 46] &quot; password=%s&quot;</a>"]
        end
        %% Intermediate

        subgraph Traces0[Traces]
            direction TB

            v2["<a href=https://github.com/ethereum-optimism/optimism/blob/ab31d2b0485f84a9868d9aa6fffbdace836333bc/indexer/database/db.go#L46 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 46] dsn</a>"]
        end
        %% Sink

        subgraph Sink
            direction LR

            v1["<a href=https://github.com/ethereum-optimism/optimism/blob/ab31d2b0485f84a9868d9aa6fffbdace836333bc/indexer/database/db.go#L64 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 64] gorm.Open(postgres.Open(dsn), &gormConfig)</a>"]
        end
    end
    %% Class Assignment
    Source:::invis
    Sink:::invis

    Traces0:::invis
    File0:::invis

    %% Connections

    Source --> Traces0
    Traces0 --> Sink

Ignore this finding from gorm-empty-password.

Semgrep found 2 no-direct-write-to-responsewriter findings:

Detected directly writing or similar in 'http.ResponseWriter.write()'. This bypasses HTML escaping that prevents cross-site scripting vulnerabilities. Instead, use the 'html/template' package and render data using 'template.Execute()'.

Ignore this finding from no-direct-write-to-responsewriter.

Semgrep found 2 unsafe-reflect-by-name findings:

If an attacker can supply values that the application then uses to determine which method or field to invoke, the potential exists for the attacker to create control flow paths through the application that were not intended by the application developers. This attack vector may allow the attacker to bypass authentication or access control checks or otherwise cause the application to behave in an unexpected manner.

Ignore this finding from unsafe-reflect-by-name.

Semgrep found 2 gosql-sqli findings:

Detected string concatenation with a non-literal variable in a "database/sql" Go SQL statement. This could lead to SQL injection if the variable is user-controlled and not properly sanitized. In order to prevent SQL injection, use parameterized queries or prepared statements instead. You can use prepared statements with the 'Prepare' and 'PrepareContext' calls.

Ignore this finding from gosql-sqli.

Comment on lines 338 to +342
run_commands([
CommandPreset('erc20-test',
['npx', 'hardhat', 'deposit-erc20', '--network', 'devnetL1',
'--l1-contracts-json-path', paths.addresses_json_path, '--signer-index', '14'],
cwd=paths.tasks_dir, timeout=8*60),
# CommandPreset('erc20-test',
# ['npx', 'hardhat', 'deposit-erc20', '--network', 'devnetL1',
# '--l1-contracts-json-path', paths.addresses_json_path, '--signer-index', '14'],
# cwd=paths.tasks_dir, timeout=8*60)
Copy link
Member Author

Choose a reason for hiding this comment

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

Didn't mean to commit this diff. If reviewers are happy with the op-e2e devnet test, I will just remove this altogether. Can then remove the devnet-tasks in a follow up.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to remove these devnet tasks, perhaps its a better idea to see them pass in CI against the new devnet before removing them

Copy link
Member Author

Choose a reason for hiding this comment

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

@tynes They don't pass and I cannot get them to work. I wasted a lot of time on this. This is the reason I switched to Go tests.

@@ -189,3 +192,14 @@ func (a *Addresses) All() []common.Address {
a.Mallory,
}
}

func (s *Secrets) AccountAtIdx(idx int) *ecdsa.PrivateKey {
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps uint because negative values don't make sense here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's common to use ints in Go for indices that don't make sense to be negative. So I just followed that convention. E.g. see stdlib, it uses int indices everywhere. The user of this API is usually an expert who wouldn't put in negative numbers. So I'd just leave it as is.

@tynes
Copy link
Contributor

tynes commented Jun 20, 2024

We are moving towards removing the L2OO and being a fault proofs only sort of system, just wanna make sure that we are on the same page and that this takes that into account, ie no deep assumptions about L2OO in this diff

@sebastianst
Copy link
Member Author

We are moving towards removing the L2OO and being a fault proofs only sort of system, just wanna make sure that we are on the same page and that this takes that into account, ie no deep assumptions about L2OO in this diff

@tynes yes, this PR just adds lighthouse, the default local devnet is still FP enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants