Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
BedrockSquirrel committed Sep 12, 2023
1 parent a9a46cf commit e72096f
Show file tree
Hide file tree
Showing 20 changed files with 69 additions and 82 deletions.
53 changes: 20 additions & 33 deletions go/ethadapter/geth_rpc_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,46 +31,33 @@ const (

// gethRPCClient implements the EthClient interface and allows connection to a real ethereum node
type gethRPCClient struct {
client *ethclient.Client // the underlying eth rpc client
l2ID gethcommon.Address // the address of the Obscuro node this client is dedicated to
timeout time.Duration // the timeout for connecting to, or communicating with, the L1 node
logger gethlog.Logger
rpcAddress string
client *ethclient.Client // the underlying eth rpc client
l2ID gethcommon.Address // the address of the Obscuro node this client is dedicated to
timeout time.Duration // the timeout for connecting to, or communicating with, the L1 node
logger gethlog.Logger
rpcURL string
}

// NewEthClientFromAddress instantiates a new ethadapter.EthClient that connects to an ethereum node
func NewEthClientFromAddress(rpcAddress string, timeout time.Duration, l2ID gethcommon.Address, logger gethlog.Logger) (EthClient, error) {
client, err := connect(rpcAddress, timeout)
// NewEthClientFromURL instantiates a new ethadapter.EthClient that connects to an ethereum node
func NewEthClientFromURL(rpcURL string, timeout time.Duration, l2ID gethcommon.Address, logger gethlog.Logger) (EthClient, error) {
client, err := connect(rpcURL, timeout)
if err != nil {
return nil, fmt.Errorf("unable to connect to the eth node (%s) - %w", rpcAddress, err)
return nil, fmt.Errorf("unable to connect to the eth node (%s) - %w", rpcURL, err)
}

logger.Trace(fmt.Sprintf("Initialized eth node connection - addr: %s", rpcAddress))
logger.Trace(fmt.Sprintf("Initialized eth node connection - addr: %s", rpcURL))
return &gethRPCClient{
client: client,
l2ID: l2ID,
timeout: timeout,
logger: logger,
rpcAddress: rpcAddress,
client: client,
l2ID: l2ID,
timeout: timeout,
logger: logger,
rpcURL: rpcURL,
}, nil
}

// NewEthClient instantiates a new ethadapter.EthClient that connects to an ethereum node
func NewEthClient(ipaddress string, port uint, timeout time.Duration, l2ID gethcommon.Address, logger gethlog.Logger) (EthClient, error) {
rpcAddress := fmt.Sprintf("ws://%s:%d", ipaddress, port)
client, err := connect(rpcAddress, timeout)
if err != nil {
return nil, fmt.Errorf("unable to connect to the eth node (%s) - %w", rpcAddress, err)
}

logger.Trace(fmt.Sprintf("Initialized eth node connection - addr: %s port: %d", ipaddress, port))
return &gethRPCClient{
client: client,
l2ID: l2ID,
timeout: timeout,
logger: logger.New(log.PackageKey, "gethrpcclient"),
rpcAddress: rpcAddress,
}, nil
return NewEthClientFromURL(fmt.Sprintf("ws://%s:%d", ipaddress, port), timeout, l2ID, logger)
}

func (e *gethRPCClient) FetchHeadBlock() (*types.Block, error) {
Expand Down Expand Up @@ -271,9 +258,9 @@ func (e *gethRPCClient) ReconnectIfClosed() error {
}
e.client.Close()

client, err := connect(e.rpcAddress, e.timeout)
client, err := connect(e.rpcURL, e.timeout)
if err != nil {
return fmt.Errorf("unable to connect to the eth node (%s) - %w", e.rpcAddress, err)
return fmt.Errorf("unable to connect to the eth node (%s) - %w", e.rpcURL, err)
}
e.client = client
return nil
Expand All @@ -289,11 +276,11 @@ func (e *gethRPCClient) Alive() bool {
return err == nil
}

func connect(rpcAddress string, connectionTimeout time.Duration) (*ethclient.Client, error) {
func connect(rpcURL string, connectionTimeout time.Duration) (*ethclient.Client, error) {
var err error
var c *ethclient.Client
for start := time.Now(); time.Since(start) < connectionTimeout; time.Sleep(time.Second) {
c, err = ethclient.Dial(rpcAddress)
c, err = ethclient.Dial(rpcURL)
if err == nil {
break
}
Expand Down
2 changes: 1 addition & 1 deletion go/host/container/host_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func NewHostContainerFromConfig(parsedConfig *config.HostInputConfig, logger get
ethWallet := wallet.NewInMemoryWalletFromConfig(cfg.PrivateKeyString, cfg.L1ChainID, log.New("wallet", cfg.LogLevel, cfg.LogPath))

fmt.Println("Connecting to L1 network...")
l1Client, err := ethadapter.NewEthClientFromAddress(cfg.L1WebsocketURL, cfg.L1RPCTimeout, cfg.ID, logger)
l1Client, err := ethadapter.NewEthClientFromURL(cfg.L1WebsocketURL, cfg.L1RPCTimeout, cfg.ID, logger)
if err != nil {
logger.Crit("could not create Ethereum client.", log.ErrKey, err)
}
Expand Down
4 changes: 2 additions & 2 deletions integration/networktest/env/network_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ func DevTestnet() networktest.Environment {
return &testnetEnv{connector}
}

func LongRunningLocalNetwork(l1RPCAddress string) networktest.Environment {
func LongRunningLocalNetwork(l1WSURL string) networktest.Environment {
connector := NewTestnetConnectorWithFaucetAccount(
"http://127.0.0.1:37800",
[]string{"http://127.0.0.1:37801", "http://127.0.0.1:37802"},
genesis.TestnetPrefundedPK,
l1RPCAddress,
l1WSURL,
)
return &testnetEnv{connector}
}
Expand Down
10 changes: 5 additions & 5 deletions integration/networktest/env/testnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ type testnetConnector struct {
seqRPCAddress string
validatorRPCAddresses []string
faucetHTTPAddress string
l1RPCAddress string
l1WSURL string
faucetWallet *userwallet.UserWallet
}

func NewTestnetConnector(seqRPCAddr string, validatorRPCAddressses []string, faucetHTTPAddress string, l1RPCAddress string) networktest.NetworkConnector {
func NewTestnetConnector(seqRPCAddr string, validatorRPCAddressses []string, faucetHTTPAddress string, l1WSURL string) networktest.NetworkConnector {
return &testnetConnector{
seqRPCAddress: seqRPCAddr,
validatorRPCAddresses: validatorRPCAddressses,
faucetHTTPAddress: faucetHTTPAddress,
l1RPCAddress: l1RPCAddress,
l1WSURL: l1WSURL,
}
}

Expand All @@ -50,7 +50,7 @@ func NewTestnetConnectorWithFaucetAccount(seqRPCAddr string, validatorRPCAddress
seqRPCAddress: seqRPCAddr,
validatorRPCAddresses: validatorRPCAddressses,
faucetWallet: userwallet.NewUserWallet(ecdsaKey, validatorRPCAddressses[0], testlog.Logger(), userwallet.WithChainID(big.NewInt(integration.ObscuroChainID))),
l1RPCAddress: l1RPCAddress,
l1WSURL: l1RPCAddress,
}
}

Expand Down Expand Up @@ -94,7 +94,7 @@ func (t *testnetConnector) NumValidators() int {
}

func (t *testnetConnector) GetL1Client() (ethadapter.EthClient, error) {
client, err := ethadapter.NewEthClientFromAddress(t.l1RPCAddress, time.Minute, gethcommon.Address{}, testlog.Logger())
client, err := ethadapter.NewEthClientFromURL(t.l1WSURL, time.Minute, gethcommon.Address{}, testlog.Logger())
if err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestRunLocalNetworkAgainstSepolia(t *testing.T) {
}

func checkBalance(walDesc string, wal wallet.Wallet, rpcAddress string) {
client, err := ethadapter.NewEthClientFromAddress(rpcAddress, 20*time.Second, common.HexToAddress("0x0"), testlog.Logger())
client, err := ethadapter.NewEthClientFromURL(rpcAddress, 20*time.Second, common.HexToAddress("0x0"), testlog.Logger())
if err != nil {
panic("unable to create live L1 eth client, err=" + err.Error())
}
Expand Down
4 changes: 2 additions & 2 deletions integration/noderunner/noderunner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,11 @@ func TestCanStartStandaloneObscuroHostAndEnclave(t *testing.T) {
}

// we create the node RPC client
rpcAddress := fmt.Sprintf("ws://127.0.0.1:%d", _startPort+integration.DefaultGethWSPortOffset)
wsURL := fmt.Sprintf("ws://127.0.0.1:%d", _startPort+integration.DefaultGethWSPortOffset)
var obscuroClient rpc.Client
wait := 30 // max wait in seconds
for {
obscuroClient, err = rpc.NewNetworkClient(rpcAddress)
obscuroClient, err = rpc.NewNetworkClient(wsURL)
if err == nil {
break
}
Expand Down
2 changes: 1 addition & 1 deletion integration/simulation/devnetwork/live_l1.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type liveL1Network struct {
func (l *liveL1Network) Prepare() {
// nothing to do really, sanity check the L1 connection
logger := testlog.Logger()
client, err := ethadapter.NewEthClientFromAddress(l.rpcAddress, 20*time.Second, common.HexToAddress("0x0"), logger)
client, err := ethadapter.NewEthClientFromURL(l.rpcAddress, 20*time.Second, common.HexToAddress("0x0"), logger)
if err != nil {
panic("unable to create live L1 eth client, err=" + err.Error())
}
Expand Down
8 changes: 4 additions & 4 deletions integration/simulation/network/obscuro_node_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,19 +135,19 @@ func StopObscuroNodes(clients []rpc.Client) {
}

// CheckHostRPCServersStopped checks whether the hosts' RPC server addresses have been freed up.
func CheckHostRPCServersStopped(hostRPCAddresses []string) {
func CheckHostRPCServersStopped(hostWSURLS []string) {
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
defer cancel()
eg, _ := errgroup.WithContext(ctx)

for _, hostRPCAddress := range hostRPCAddresses {
rpcAddress := hostRPCAddress
for _, hostWSURL := range hostWSURLS {
url := hostWSURL
// We cannot stop the RPC server synchronously. This is because the host itself is being stopped by an RPC
// call, so there is a deadlock. The RPC server is waiting for all connections to close, but a single
// connection remains open, waiting for the RPC server to close. Instead, we check whether the RPC port
// becomes free.
eg.Go(func() error {
for !isAddressAvailable(rpcAddress) {
for !isAddressAvailable(url) {
time.Sleep(100 * time.Millisecond)
}
return nil
Expand Down
10 changes: 5 additions & 5 deletions integration/simulation/network/socket.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import (

// creates Obscuro nodes with their own enclave servers that communicate with peers via sockets, wires them up, and populates the network objects
type networkOfSocketNodes struct {
l2Clients []rpc.Client
hostRPCAddresses []string
l2Clients []rpc.Client
hostWebsocketURLs []string

// geth
eth2Network eth2network.Eth2Network
Expand Down Expand Up @@ -127,13 +127,13 @@ func (n *networkOfSocketNodes) TearDown() {
// Stop the Obscuro nodes first (each host will attempt to shut down its enclave as part of shutdown).
StopObscuroNodes(n.l2Clients)
StopEth2Network(n.gethClients, n.eth2Network)
CheckHostRPCServersStopped(n.hostRPCAddresses)
CheckHostRPCServersStopped(n.hostWebsocketURLs)
}

func (n *networkOfSocketNodes) createConnections(simParams *params.SimParams) error {
// create the clients in the structs
n.l2Clients = make([]rpc.Client, simParams.NumberOfNodes)
n.hostRPCAddresses = make([]string, simParams.NumberOfNodes)
n.hostWebsocketURLs = make([]string, simParams.NumberOfNodes)
n.obscuroClients = make([]*obsclient.ObsClient, simParams.NumberOfNodes)

for i := 0; i < simParams.NumberOfNodes; i++ {
Expand All @@ -153,7 +153,7 @@ func (n *networkOfSocketNodes) createConnections(simParams *params.SimParams) er
}

n.l2Clients[i] = client
n.hostRPCAddresses[i] = fmt.Sprintf("ws://%s:%d", Localhost, simParams.StartPort+integration.DefaultHostRPCWSOffset+i)
n.hostWebsocketURLs[i] = fmt.Sprintf("ws://%s:%d", Localhost, simParams.StartPort+integration.DefaultHostRPCWSOffset+i)
}

for idx, l2Client := range n.l2Clients {
Expand Down
8 changes: 4 additions & 4 deletions testnet/launcher/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (t *Testnet) Start() error {

l2ContractDeployer, err := l2cd.NewDockerContractDeployer(
l2cd.NewContractDeployerConfig(
l2cd.WithL1HTTPRPCAddress("http://eth2network:8025"),
l2cd.WithL1HTTPURL("http://eth2network:8025"),
l2cd.WithL2Host("sequencer-host"),
l2cd.WithL2WSPort(81),
l2cd.WithL1PrivateKey("f52e5418e349dccdda29b6ac8b0abe6576bb7713886aa85abea6181ba731f9bb"),
Expand Down Expand Up @@ -182,7 +182,7 @@ func startEth2Network() error {
func deployL1Contracts() (*node.NetworkConfig, error) {
l1ContractDeployer, err := l1cd.NewDockerContractDeployer(
l1cd.NewContractDeployerConfig(
l1cd.WithL1HTTPRPCAddress("http://eth2network:8025"),
l1cd.WithL1HTTPURL("http://eth2network:8025"),
l1cd.WithPrivateKey("f52e5418e349dccdda29b6ac8b0abe6576bb7713886aa85abea6181ba731f9bb"),
l1cd.WithDockerImage("testnetobscuronet.azurecr.io/obscuronet/hardhatdeployer:latest"),
),
Expand All @@ -208,11 +208,11 @@ func deployL1Contracts() (*node.NetworkConfig, error) {
func waitForHealthyNode(port int) error { // todo: hook the cfg
timeStart := time.Now()

hostRPCAddress := fmt.Sprintf("http://localhost:%d", port)
hostURL := fmt.Sprintf("http://localhost:%d", port)
fmt.Println("Waiting for Obscuro node to be healthy...")
err := retry.Do(
func() error {
client, err := rpc.NewNetworkClient(hostRPCAddress)
client, err := rpc.NewNetworkClient(hostURL)
if err != nil {
return err
}
Expand Down
6 changes: 3 additions & 3 deletions testnet/launcher/l1contractdeployer/cmd/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (

// L1ContractDeployerConfigCLI represents the configurations passed into the deployer over CLI
type L1ContractDeployerConfigCLI struct {
l1HTTPRPCAddr string
l1HTTPURL string
privateKey string
dockerImage string
contractsEnvFile string
Expand All @@ -17,13 +17,13 @@ func ParseConfigCLI() *L1ContractDeployerConfigCLI {
cfg := &L1ContractDeployerConfigCLI{}
flagUsageMap := getFlagUsageMap()

l1HTTPRPCAddr := flag.String(l1HTTPRPCAddressFlag, "http://eth2network:8025", flagUsageMap[l1HTTPRPCAddressFlag])
l1HTTPURL := flag.String(l1HTTPURLFlag, "http://eth2network:8025", flagUsageMap[l1HTTPURLFlag])
privateKey := flag.String(privateKeyFlag, "", flagUsageMap[privateKeyFlag])
dockerImage := flag.String(dockerImageFlag, "testnetobscuronet.azurecr.io/obscuronet/hardhatdeployer:latest", flagUsageMap[dockerImageFlag])
contractsEnvFile := flag.String(contractsEnvFileFlag, "", flagUsageMap[contractsEnvFileFlag])
flag.Parse()

cfg.l1HTTPRPCAddr = *l1HTTPRPCAddr
cfg.l1HTTPURL = *l1HTTPURL
cfg.privateKey = *privateKey
cfg.dockerImage = *dockerImage
cfg.contractsEnvFile = *contractsEnvFile
Expand Down
4 changes: 2 additions & 2 deletions testnet/launcher/l1contractdeployer/cmd/cli_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package main

// Flag names.
const (
l1HTTPRPCAddressFlag = "l1_ws_url"
l1HTTPURLFlag = "l1_ws_url"
privateKeyFlag = "private_key"
dockerImageFlag = "docker_image"
contractsEnvFileFlag = "contracts_env_file"
Expand All @@ -12,7 +12,7 @@ const (
// While we could just use constants instead of a map, this approach allows us to test that all the expected flags are defined.
func getFlagUsageMap() map[string]string {
return map[string]string{
l1HTTPRPCAddressFlag: "Layer 1 network http RPC addr",
l1HTTPURLFlag: "Layer 1 network http RPC addr",
privateKeyFlag: "L1 and L2 private key used in the node",
dockerImageFlag: "Docker image to run",
contractsEnvFileFlag: "If set, it will write the contract addresses to the file",
Expand Down
6 changes: 3 additions & 3 deletions testnet/launcher/l1contractdeployer/cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ func main() {

l1ContractDeployer, err := l1cd.NewDockerContractDeployer(
l1cd.NewContractDeployerConfig(
l1cd.WithL1HTTPRPCAddress(cliConfig.l1HTTPRPCAddr), // "http://eth2network:8025"
l1cd.WithPrivateKey(cliConfig.privateKey), //"f52e5418e349dccdda29b6ac8b0abe6576bb7713886aa85abea6181ba731f9bb"),
l1cd.WithDockerImage(cliConfig.dockerImage), //"testnetobscuronet.azurecr.io/obscuronet/hardhatdeployer:latest"
l1cd.WithL1HTTPURL(cliConfig.l1HTTPURL), // "http://eth2network:8025"
l1cd.WithPrivateKey(cliConfig.privateKey), //"f52e5418e349dccdda29b6ac8b0abe6576bb7713886aa85abea6181ba731f9bb"),
l1cd.WithDockerImage(cliConfig.dockerImage), //"testnetobscuronet.azurecr.io/obscuronet/hardhatdeployer:latest"
),
)
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions testnet/launcher/l1contractdeployer/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ type Option = func(c *Config)

// Config holds the properties that configure the package
type Config struct {
l1HTTPRPCAddress string
privateKey string
dockerImage string
l1HTTPURL string
privateKey string
dockerImage string
}

func NewContractDeployerConfig(opts ...Option) *Config {
Expand All @@ -20,9 +20,9 @@ func NewContractDeployerConfig(opts ...Option) *Config {
return defaultConfig
}

func WithL1HTTPRPCAddress(s string) Option {
func WithL1HTTPURL(s string) Option {
return func(c *Config) {
c.l1HTTPRPCAddress = s
c.l1HTTPURL = s
}
}

Expand Down
2 changes: 1 addition & 1 deletion testnet/launcher/l1contractdeployer/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (n *ContractDeployer) Start() error {
"accounts": [ "%s" ]
}
}
`, n.cfg.l1HTTPRPCAddress, n.cfg.privateKey),
`, n.cfg.l1HTTPURL, n.cfg.privateKey),
}

containerID, err := docker.StartNewContainer("hh-l1-deployer", n.cfg.dockerImage, cmds, nil, envs, nil, nil)
Expand Down
6 changes: 3 additions & 3 deletions testnet/launcher/l2contractdeployer/cmd/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (

// L2ContractDeployerConfigCLI represents the configurations passed into the deployer over CLI
type L2ContractDeployerConfigCLI struct {
l1HTTPRPCAddr string
l1HTTPURL string
privateKey string
dockerImage string
l2Host string
Expand All @@ -22,7 +22,7 @@ func ParseConfigCLI() *L2ContractDeployerConfigCLI {
cfg := &L2ContractDeployerConfigCLI{}
flagUsageMap := getFlagUsageMap()

l1HTTPRPCAddr := flag.String(l1HTTPRPCAddressFlag, "http://eth2network:8025", flagUsageMap[l1HTTPRPCAddressFlag])
l1HTTPURL := flag.String(l1HTTPURLFlag, "http://eth2network:8025", flagUsageMap[l1HTTPURLFlag])
privateKey := flag.String(privateKeyFlag, "", flagUsageMap[privateKeyFlag])
dockerImage := flag.String(dockerImageFlag, "testnetobscuronet.azurecr.io/obscuronet/hardhatdeployer:latest", flagUsageMap[dockerImageFlag])
l2Host := flag.String(l2HostFlag, "", flagUsageMap[l2HostFlag])
Expand All @@ -34,7 +34,7 @@ func ParseConfigCLI() *L2ContractDeployerConfigCLI {

flag.Parse()

cfg.l1HTTPRPCAddr = *l1HTTPRPCAddr
cfg.l1HTTPURL = *l1HTTPURL
cfg.privateKey = *privateKey
cfg.dockerImage = *dockerImage
cfg.l2Host = *l2Host
Expand Down
Loading

0 comments on commit e72096f

Please sign in to comment.