feat: Completed implementation of customizable anvil hosts #264

s29papi · 2024-11-13T20:43:42Z

Description

Adding customizable host configuration options for anvil nodes. This allows users to specify custom host addresses for L1 and L2 instances.

✅ Implementation Complete

Features:

Added L1Host and L2Host configuration flags
Implemented host configuration in chain and network configs
Added host validation and default fallbacks
Integrated with anvil startup and orchestrator

Tests verify:

Default host fallback (127.0.0.1)
Custom host configurations (0.0.0.0)
Mixed host scenarios
Host validation
Integration with anvil and orchestrator

Additional context

This implementation allows users to run anvil instances on different host addresses, enabling:

Custom network configurations
Flexible deployment scenarios
Better container support
Improved network isolation options

Metadata

Closes #262

WIP: Adding host configuration options to CLI - Added L1Host and L2Host flags - Added initial config structure changes - TODO: Complete host validation - TODO: Integrate with anvil startup Refs ethereum-optimism#262

- Add Host field to ChainConfig struct - Add L2Host field to NetworkConfig struct - Enable custom host configuration for anvil instances Part of ethereum-optimism#262

- Add host configuration tests in anvil_test.go - Add host configuration tests in orchestrator_test.go - Implement default host (127.0.0.1) fallbacks in: - opsimulator.go - orchestrator.go - anvil.go Part of ethereum-optimism#262

s29papi · 2024-11-14T08:20:54Z

@jakim929 The customizable host implementation is now complete!

You can now configure host addresses using the CLI flags

jakim929

Thank you for the contribution! This looks pretty close - just some changes requested to how the config is passed down from the CLI flags, and also some test changes

jakim929 · 2024-11-15T16:18:41Z

orchestrator/orchestrator_test.go

+	wantErr bool
+}
+
+func createTestSuite(t *testing.T, tc *TestCase) *TestSuite {


think it will be more flexible to just pass it a networkConfig as a param

func createTestSuite(t *testing.T, networkConfig *NetworkConfig) *TestSuite {

jakim929 · 2024-11-15T16:19:22Z

orchestrator/orchestrator_test.go

+	l1Host := defaultHost
+	l2Host := defaultHost
+	if tc != nil {
+		l1Host = tc.l1Host
+		l2Host = tc.l2Host
+	}
+
+	networkConfig.L1Config.Host = l1Host
+	networkConfig.L1Config.Port = 9471
+	for i := range networkConfig.L2Configs {
+		networkConfig.L2Configs[i].Host = l2Host
+		networkConfig.L2StartingPort = 9473
+	}


can we leave the starting ports to remain default 0? The test run in parallel so it will clash on port usage.

jakim929 · 2024-11-15T16:21:29Z

orchestrator/orchestrator_test.go

-	testSuite := createTestSuite(t)
+	testSuite := createTestSuite(t, nil)
+	verifyChainConfigs(t, testSuite)
+}
+
+func TestOrchestratorWithHosts(t *testing.T) {
+	t.Run("default configuration", func(t *testing.T) {
+		testSuite := createTestSuite(t, nil)
+		verifyChainConfigs(t, testSuite)
+	})
+	testCases := []TestCase{
+		{
+			name:    "custom hosts",
+			l1Host:  "0.0.0.0",
+			l2Host:  "0.0.0.0",
+			wantErr: false,
+		},
+		{
+			name:    "mixed custom hosts",
+			l1Host:  defaultHost,
+			l2Host:  "0.0.0.0",
+			wantErr: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Logf("Testing with L1 Host: %s, L2 Host: %s", tc.l1Host, tc.l2Host)
+
+			testSuite := createTestSuite(t, &tc)
+			if !tc.wantErr {
+				require.NotNil(t, testSuite)
+				verifyChainConfigs(t, testSuite)
+			}
+		})
+	}
+}


the createTestSuite should ideally not have any tests in it.

If you want to test for the orchestrator failing to start I would add another test that runs

orchestrator, err := NewOrchestrator(testlog, closeApp, &networkConfig)

and checks for error with out using createTestSuite

jakim929 · 2024-11-15T16:22:18Z

orchestrator/orchestrator_test.go

+type TestCase struct {
+	name    string
+	l1Host  string
+	l2Host  string
+	wantErr bool
+}
+


to remove - see comment below - these can just be individual tests that don't use createTestSuite

jakim929 · 2024-11-15T16:23:23Z

orchestrator/orchestrator.go

+		if cfg.Host == "" {
+			cfg.Host = defaultHost
+		}


by default the cli flags will fall back to 127.0.0.1 so this can be removed, no need to have defaultHost

jakim929 · 2024-11-15T16:23:58Z

anvil/anvil.go

@@ -36,7 +36,7 @@ var (
 )

 const (
-	host                 = "127.0.0.1"
+	defaultHost          = "127.0.0.1"


this can be removed, since host will default to "127.0.0.1" based on the default CLI flag value

jakim929 · 2024-11-15T16:26:13Z

orchestrator/orchestrator.go

+const defaultHost = "127.0.0.1"
+


jakim929 · 2024-11-15T16:31:11Z

orchestrator/orchestrator.go

+		if cfg.Host == "" {
+			cfg.Host = defaultHost
+		}


right now the CLI flags are not passed to the L2 chains. I think you need to pass something like

cfg.Host = networkConfig.L2Host

jakim929 · 2024-11-15T16:32:01Z

orchestrator/orchestrator.go

@@ -49,7 +51,10 @@ func NewOrchestrator(log log.Logger, closeApp context.CancelCauseFunc, networkCo
 	// Sping up OpSim to fornt the L2 instances
 	for i := range networkConfig.L2Configs {
 		cfg := networkConfig.L2Configs[i]
-		l2OpSims[cfg.ChainID] = opsimulator.New(log, closeApp, nextL2Port, l1Anvil, l2Anvils[cfg.ChainID], l2Anvils, networkConfig.InteropDelay)
+		if cfg.Host == "" {


right now the CLI flags are not passed to the chains for the L1 chain as well. I think you need to pass something like

cfg.Host = networkConfig.L1Host

Hey man thanks for the reviews.

Based on my understanding of this, I see the

// Forward set ports. Setting `0` will work to allocate a random port networkConfig.L1Config.Port = cliConfig.L1Port networkConfig.L2StartingPort = cliConfig.L2StartingPort

// Forward interop config networkConfig.InteropAutoRelay = cliConfig.InteropAutoRelay networkConfig.InteropDelay = cliConfig.InteropDelay

cliConfig params are forwaded to the network config from outside the orchestrator so i decided to follow same

// Forward host config networkConfig.L1Config.Host = cliConfig.L1Host for i := range networkConfig.L2Configs { networkConfig.L2Configs[i].Host = cliConfig.L2Host }

But I might be wrong, My thinking is that the l1Chain is handled by this

// Spin up L1 anvil instance l1Anvil := anvil.New(log, closeApp, &networkConfig.L1Config)

jakim929 · 2024-11-15T16:32:49Z

opsimulator/opsimulator.go

@@ -33,7 +33,7 @@ import (
 )

 const (
-	host                        = "127.0.0.1"
+	defaultHost                 = "127.0.0.1"


remove default host, make it always passed down

…ble-anvil-hosts

…ts in orchestrator

s29papi · 2024-11-16T06:53:19Z

@jakim929

Changes:

Removed default hosts
Removed checks in createsuite
Seperates host tests on orchestrator

Would appreciate if you clarify the comment "cli flags are not passed down", they are currently being forwarded from supersim and also the test suite, and printing to the console within anvil, and opsim also suggests this.

jakim929

This looks correct and apologize for the delay.

by cli flag not being passed down I just meant that we don't need a default value to be compared against at every level - the implementation looks correct now

anvil/anvil_test.go

feat: begin implementation of customizable anvil hosts

6da7e67

WIP: Adding host configuration options to CLI - Added L1Host and L2Host flags - Added initial config structure changes - TODO: Complete host validation - TODO: Integrate with anvil startup Refs ethereum-optimism#262

s29papi requested a review from a team as a code owner November 13, 2024 20:43

feat: add host configuration to chain and network configs

ec4b77e

- Add Host field to ChainConfig struct - Add L2Host field to NetworkConfig struct - Enable custom host configuration for anvil instances Part of ethereum-optimism#262

s29papi changed the title ~~feat: begin implementation of customizable anvil hosts~~ feat: progress on implementation of customizable anvil hosts Nov 13, 2024

s29papi added 4 commits November 14, 2024 08:15

test: add host configuration tests and default fallbacks

21fd5d0

- Add host configuration tests in anvil_test.go - Add host configuration tests in orchestrator_test.go - Implement default host (127.0.0.1) fallbacks in: - opsimulator.go - orchestrator.go - anvil.go Part of ethereum-optimism#262

test: add host configuration tests and default fallbacks

0134ec5

- Add host configuration tests in anvil_test.go - Add host configuration tests in orchestrator_test.go - Implement default host (127.0.0.1) fallbacks in: - opsimulator.go - orchestrator.go - anvil.go Part of ethereum-optimism#262

Merge branch 'main' into enhancement/configurable-anvil-hosts

11b54c5

merged with interopDelay

6ee0655

s29papi changed the title ~~feat: progress on implementation of customizable anvil hosts~~ feat: Completed implementation of customizable anvil hosts Nov 14, 2024

jakim929 requested changes Nov 15, 2024

View reviewed changes

s29papi added 2 commits November 15, 2024 20:52

Merge remote-tracking branch 'origin/main' into enhancement/configura…

87d7554

…ble-anvil-hosts

removed default hosts, removed checks from createSuite, seperates tes…

1d31aff

…ts in orchestrator

jakim929 approved these changes Nov 19, 2024

View reviewed changes

jakim929 reviewed Nov 19, 2024

View reviewed changes

anvil/anvil_test.go Outdated Show resolved Hide resolved

Update anvil_test.go

4cb6c03

jakim929 merged commit 31bf6be into ethereum-optimism:main Nov 19, 2024
3 of 4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Completed implementation of customizable anvil hosts #264

feat: Completed implementation of customizable anvil hosts #264

s29papi commented Nov 13, 2024 •

edited

Loading

s29papi commented Nov 14, 2024

jakim929 left a comment

jakim929 Nov 15, 2024

jakim929 Nov 15, 2024

jakim929 Nov 15, 2024

jakim929 Nov 15, 2024

jakim929 Nov 15, 2024

jakim929 Nov 15, 2024

jakim929 Nov 15, 2024

jakim929 Nov 15, 2024

jakim929 Nov 15, 2024

s29papi Nov 16, 2024 •

edited

Loading

jakim929 Nov 15, 2024

s29papi commented Nov 16, 2024

jakim929 left a comment

feat: Completed implementation of customizable anvil hosts #264

feat: Completed implementation of customizable anvil hosts #264

Conversation

s29papi commented Nov 13, 2024 • edited Loading

s29papi commented Nov 14, 2024

jakim929 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s29papi Nov 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s29papi commented Nov 16, 2024

jakim929 left a comment

Choose a reason for hiding this comment

s29papi commented Nov 13, 2024 •

edited

Loading

s29papi Nov 16, 2024 •

edited

Loading