Feat: Require l1Config Validation

[IdrisHanafi]

What's new? and Motivation

The current approach would allow for empty L1 configs that are required for important aspects. Instead of allowing the code to run and use default type values, we should enforce the values to be set.

For example, let's assume the following l1Config in genesis.json (note: missing polygonZkEVMGlobalExitRootAddress value and incorrectly spelled supernets2Address):

{
  ...
  "l1Config" : {
    "chainId": 420,
    "supernets2Addresssss": "0xc949254d682d8c9ad5682521675b8f43b102aec4",
    "maticTokenAddress": "0xc949254d682d8c9ad5682521675b8f43b102aec4",
    "supernets2DataCommitteeContract": "0x2279B7A0a67DB372996a5FaB50D91eAA73d2eBe6"
  },
  ...
}

The synchronizer will continue to poll for smart contract logs and use default values of 0x0000000000000000000000000000000000000000 for the addresses:

{"jsonrpc":"2.0","id":766,"method":"eth_getLogs","params":[{"address":["0x0000000000000000000000000000000000000000","0x0000000000000000000000000000000000000000"],"fromBlock":"0x2cdb","toBlock":"0x2d3f","topics":null}]}

This will obviously return nothing and cause a lot of issues such as bridging, etc. In other words, filter queries in etherman.go (such as this) will not function properly.

• Also added formatting to Makefile and ran the formatter

Testing

$ go test
2023-08-15T19:59:22.397Z        INFO    config/config.go:162    config file not found   {"pid": 1185384, "version": "v0.1.0"}
2023-08-15T19:59:22.402Z        INFO    config/config.go:162    config file not found   {"pid": 1185384, "version": "v0.1.0"}
PASS
ok      github.com/0xPolygon/supernets2-node/config     0.261s

and running in a devnet without a valid l1Config:

$ ./dist/supernets2-node run --network custom --custom-network-file /etc/zkevm/genesis.json --cfg /etc/zkevm/config.toml --components "rpc,synchronizer"
panic: failed to load genesis configuration from file. Error: Key: 'L1Config.GlobalExitRootManagerAddr' Error:Field validation for 'GlobalExitRootManagerAddr' failed on the 'required' tag

goroutine 1 [running]:
github.com/0xPolygonHermez/zkevm-node/config.(*Config).loadNetworkConfig(0xc000988e00, 0x13fa740?)
        /opt/zkevm-node/config/network.go:78 +0x1ee
github.com/0xPolygonHermez/zkevm-node/config.Load(0x3932353238343932?, 0x1)
        /opt/zkevm-node/config/config.go:181 +0x41d
main.start(0xc000a20e40)
        /opt/zkevm-node/cmd/run.go:46 +0x45
github.com/urfave/cli/v2.(*Command).Run(0xc0000e06e0, 0xc000a20e40, {0xc0004227e0, 0x9, 0x9})
        /root/go/pkg/mod/github.com/urfave/cli/[email protected]/command.go:274 +0xa42
github.com/urfave/cli/v2.(*Command).Run(0xc0000e1a20, 0xc000a20d00, {0xc000000140, 0xa, 0xa})
        /root/go/pkg/mod/github.com/urfave/cli/[email protected]/command.go:267 +0xc97
github.com/urfave/cli/v2.(*App).RunContext(0xc0008581e0, {0x1a34668?, 0xc000040070}, {0xc000000140, 0xa, 0xa})
        /root/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:332 +0x616
github.com/urfave/cli/v2.(*App).Run(...)
        /root/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:309
main.main()
        /opt/zkevm-node/cmd/main.go:191 +0xcdf

Open Question

This is just a small example of where the use case of enforcing the L1 values is required.

• Are there cases where we do not need to enforce L1 Config values in the zkEVM node?

Future Tasks

If this is a feature to be merged, I suspect the following will have to be done:

• Merge same logic into zkevm-node repo
• Update usage docs

[arnaubennassar]

@IdrisHanafi sorry for taking long time to check this PR. I think it makes sense, but since it's not related to the CDK / Validium, I'd suggest to open this PR on the upstream repo so we can all benefit from it

Maybe the argument is that there are modes of running where L1 config is not required, and this prevents config from being parsed in those situations?

Actually this is a valid point, not sure if there is such a situation where the network config file is required but not the L1 config in particular. TBH, if such a case exists, it's most likely a poor implementation.

But anyway, I do agree that if the config is wrong the app would crash quite soon actually