Optimism feature-flag in chain config of Go-ethereum fork

[trianglesphere]

It currently has the same name as upstream Geth. Renaming it would allow us to more easily import it into projects. Renaming the module path does incur some ongoing maintenance, but it is easily scripted. When doing a normal merge, it is pretty easy to deal with. Otherwise if we want to keep rebasing, we probably need more scripts and manual steps to help with dealing with the path renames.

[protolambda]

It currently has the same name as upstream Geth. Renaming it would allow us to more easily import it into projects.

It sounds nice, and this is what optimism monorepo did, but I'm very much NOT a fan of this! The diff would get polluted with import path changes and we want to make collaboration with the geth team as frictionless as possible. Currently the geth version in the monorepo rewrote all import-paths, removed all git-history, and is super-outdated: reviewing the diff with Geth is a nightmare, nevermind porting over changes (no simple git merges etc.).

[trianglesphere]

I agree that we should not wipe the git history.

On the matter of the path rename, we can discuss a strategy to make the overhead as low as possible: It will likely be scripted and we may maintain a separate branch that has the rename. Maybe it's something done only in release versions?

Whatever it is, I think this is something that will need to be done. It's absolutely worth figuring out how to limit the friction, but as it stands, the path requires a replace directive and prohibits importing from both go-ethereum and our reference implementation in a single project.

[norswap]

Could we wrap what we need to access in the L2 node in a different package? Or is there still a clash if the dependency is indirect?

[protolambda]

IMHO we should keep the L1 and L2 geth implementations compatible. Our goal is to make the diff as small as possible, the library should be interchangeable aside from a chain-config flag that enables rollup features (similar to the flag that enables PoS Merge features).

That way we get the best of everything:

• Simplicity with imports (no duplicate geth library, no two different logging packages, etc.)
• Minimum diff, maximum ability to upstream/downstream changes in geth, no code-gen/transformer tooling required

The only real differences right now are:

• Deposit transaction type (if it's not present in a block, there's no difference. Controlling it with a flag would be a nice improvement though)
• Backwards-compatible change to the engine API to insert transactions for block production

[trianglesphere]

I do like the idea of having a single feature flag that controls rollup mode or normal Geth. That goes a long way to integrating with both L1 and L2 nodes from the same piece of code.

That still leaves the replace directive, but I guess we can just deal with it.

With a full path rename, one option is to have a develop branch that doesn't have it, and a release branch that does have the rename applied on top. That allows programatic access to the new path without making dev too hard.

[protolambda]

Starting with a feature flag in the config and the replace directive would improve our ability to contribute back and forth with L1 a lot, very valuable long-term. I'll rename this issue to reflect the feature-flag approach.