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

Sepolia prep: add batch + rollup interval CLI options #1530

Merged
merged 3 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/manual-deploy-testnet-l2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ jobs:
RESOURCE_TESTNET_NAME: ${{ steps.outputVars.outputs.RESOURCE_TESTNET_NAME }}
L1_WS_URL: ${{ steps.outputVars.outputs.L1_WS_URL }}
L1_HTTP_URL: ${{ steps.outputVars.outputs.L1_HTTP_URL }}
BATCH_INTERVAL: ${{ steps.deployL2Contracts.outputs.BATCH_INTERVAL }}
ROLLUP_INTERVAL: ${{ steps.deployL2Contracts.outputs.ROLLUP_INTERVAL }}


steps:
Expand All @@ -65,6 +67,8 @@ jobs:
echo "RESOURCE_TESTNET_NAME=testnet" >> $GITHUB_ENV
echo "L1_WS_URL=ws://testnet-eth2network.uksouth.cloudapp.azure.com:9000" >> $GITHUB_ENV
echo "L1_HTTP_URL=http://testnet-eth2network.uksouth.cloudapp.azure.com:8025" >> $GITHUB_ENV
echo "BATCH_INTERVAL=1s" >> $GITHUB_ENV
echo "ROLLUP_INTERVAL=10s" >> $GITHUB_ENV

- name: 'Sets env vars for dev-testnet'
if: ${{ (github.event.inputs.testnet_type == 'dev-testnet') }}
Expand All @@ -77,6 +81,8 @@ jobs:
echo "RESOURCE_TESTNET_NAME=devtestnet" >> $GITHUB_ENV
echo "L1_WS_URL=ws://dev-testnet-eth2network.uksouth.cloudapp.azure.com:9000" >> $GITHUB_ENV
echo "L1_HTTP_URL=http://dev-testnet-eth2network.uksouth.cloudapp.azure.com:8025" >> $GITHUB_ENV
echo "BATCH_INTERVAL=1s" >> $GITHUB_ENV
echo "ROLLUP_INTERVAL=10s" >> $GITHUB_ENV

- name: 'Output env vars'
id: outputVars
Expand All @@ -89,6 +95,8 @@ jobs:
echo "RESOURCE_TESTNET_NAME=${{env.RESOURCE_TESTNET_NAME}}" >> $GITHUB_OUTPUT
echo "L1_WS_URL=${{env.L1_WS_URL}}" >> $GITHUB_OUTPUT
echo "L1_HTTP_URL=${{env.L1_HTTP_URL}}" >> $GITHUB_OUTPUT
echo "BATCH_INTERVAL=${{env.BATCH_INTERVAL}}" >> $GITHUB_OUTPUT
echo "ROLLUP_INTERVAL=${{env.ROLLUP_INTERVAL}}" >> $GITHUB_OUTPUT

- name: 'Login to Azure docker registry'
uses: azure/docker-login@v1
Expand Down Expand Up @@ -268,6 +276,8 @@ jobs:
-host_docker_image=${{needs.build.outputs.L2_HOST_DOCKER_BUILD_TAG}} \
-is_debug_namespace_enabled=true \
-log_level=${{ github.event.inputs.log_level }} \
-batch_interval=${{needs.build.outputs.BATCH_INTERVAL}} \
-rollup_interval=${{needs.build.outputs.ROLLUP_INTERVAL}} \
start'


Expand Down
10 changes: 10 additions & 0 deletions .github/workflows/manual-upgrade-testnet-l2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ jobs:
RESOURCE_TESTNET_NAME: ${{ steps.outputVars.outputs.RESOURCE_TESTNET_NAME }}
L1_WS_URL: ${{ steps.outputVars.outputs.L1_WS_URL }}
VM_BUILD_NUMBER: ${{ steps.outputVars.outputs.VM_BUILD_NUMBER }}
BATCH_INTERVAL: ${{ steps.deployL2Contracts.outputs.BATCH_INTERVAL }}
ROLLUP_INTERVAL: ${{ steps.deployL2Contracts.outputs.ROLLUP_INTERVAL }}

steps:
- uses: actions/checkout@v3
Expand All @@ -63,6 +65,8 @@ jobs:
echo "RESOURCE_STARTING_NAME=T" >> $GITHUB_ENV
echo "RESOURCE_TESTNET_NAME=testnet" >> $GITHUB_ENV
echo "L1_WS_URL=ws://testnet-eth2network.uksouth.cloudapp.azure.com:9000" >> $GITHUB_ENV
echo "BATCH_INTERVAL=1s" >> $GITHUB_ENV
echo "ROLLUP_INTERVAL=10s" >> $GITHUB_ENV

- name: 'Sets env vars for dev-testnet'
if: ${{ (github.event.inputs.testnet_type == 'dev-testnet') || (github.event_name == 'schedule') }}
Expand All @@ -73,6 +77,8 @@ jobs:
echo "RESOURCE_STARTING_NAME=D" >> $GITHUB_ENV
echo "RESOURCE_TESTNET_NAME=devtestnet" >> $GITHUB_ENV
echo "L1_WS_URL=ws://dev-testnet-eth2network.uksouth.cloudapp.azure.com:9000" >> $GITHUB_ENV
echo "BATCH_INTERVAL=1s" >> $GITHUB_ENV
echo "ROLLUP_INTERVAL=10s" >> $GITHUB_ENV

- name: 'Fetch latest VM hostnames by env tag and extract build number'
id: fetch_hostnames
Expand All @@ -97,6 +103,8 @@ jobs:
echo "RESOURCE_TESTNET_NAME=${{env.RESOURCE_TESTNET_NAME}}" >> $GITHUB_OUTPUT
echo "L1_WS_URL=${{env.L1_WS_URL}}" >> $GITHUB_OUTPUT
echo "VM_BUILD_NUMBER=${{env.VM_BUILD_NUMBER}}" >> $GITHUB_OUTPUT
echo "BATCH_INTERVAL=${{env.BATCH_INTERVAL}}" >> $GITHUB_OUTPUT
echo "ROLLUP_INTERVAL=${{env.ROLLUP_INTERVAL}}" >> $GITHUB_OUTPUT

- name: 'Login to Azure docker registry'
uses: azure/docker-login@v1
Expand Down Expand Up @@ -182,6 +190,8 @@ jobs:
-enclave_docker_image=${{needs.build.outputs.L2_ENCLAVE_DOCKER_BUILD_TAG}} \
-host_docker_image=${{needs.build.outputs.L2_HOST_DOCKER_BUILD_TAG}} \
-log_level=${{ github.event.inputs.log_level }} \
-batch_interval=${{needs.build.outputs.BATCH_INTERVAL}} \
-rollup_interval=${{needs.build.outputs.ROLLUP_INTERVAL}} \
upgrade'

check-obscuro-is-healthy:
Expand Down
15 changes: 15 additions & 0 deletions go/node/cmd/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"os"
"strings"
"time"
)

var (
Expand Down Expand Up @@ -41,6 +42,8 @@ type NodeConfigCLI struct {
isDebugNamespaceEnabled bool
logLevel int
isInboundP2PDisabled bool
batchInterval string // format like 500ms or 2s (any time parsable by time.ParseDuration())
rollupInterval string // format like 500ms or 2s (any time parsable by time.ParseDuration())
}

// ParseConfigCLI returns a NodeConfigCLI based the cli params and defaults.
Expand Down Expand Up @@ -73,6 +76,8 @@ func ParseConfigCLI() *NodeConfigCLI {
isDebugNamespaceEnabled := flag.Bool(isDebugNamespaceEnabledFlag, false, flagUsageMap[isDebugNamespaceEnabledFlag])
logLevel := flag.Int(logLevelFlag, 3, flagUsageMap[logLevelFlag])
isInboundP2PDisabled := flag.Bool(isInboundP2PDisabledFlag, false, flagUsageMap[isInboundP2PDisabledFlag])
batchInterval := flag.String(batchIntervalFlag, "1s", flagUsageMap[batchIntervalFlag])
rollupInterval := flag.String(rollupIntervalFlag, "3s", flagUsageMap[rollupIntervalFlag])

flag.Parse()
cfg.nodeName = *nodeName
Expand Down Expand Up @@ -100,6 +105,16 @@ func ParseConfigCLI() *NodeConfigCLI {
cfg.isDebugNamespaceEnabled = *isDebugNamespaceEnabled
cfg.logLevel = *logLevel
cfg.isInboundP2PDisabled = *isInboundP2PDisabled
cfg.batchInterval = *batchInterval
if _, err := time.ParseDuration(cfg.batchInterval); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the node validated the time parsing and this mechanism only took strings and simple types ?

fmt.Printf("invalid batch interval: %s\n", err)
os.Exit(1)
}
cfg.rollupInterval = *rollupInterval
if _, err := time.ParseDuration(cfg.rollupInterval); err != nil {
fmt.Printf("invalid rollup interval: %s\n", err)
os.Exit(1)
}

cfg.nodeAction = flag.Arg(0)
if !validateNodeAction(cfg.nodeAction) {
Expand Down
4 changes: 4 additions & 0 deletions go/node/cmd/cli_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ const (
isDebugNamespaceEnabledFlag = "is_debug_namespace_enabled"
logLevelFlag = "log_level"
isInboundP2PDisabledFlag = "is_inbound_p2p_disabled"
batchIntervalFlag = "batch_interval"
rollupIntervalFlag = "rollup_interval"
)

// Returns a map of the flag usages.
Expand Down Expand Up @@ -58,5 +60,7 @@ func getFlagUsageMap() map[string]string {
isDebugNamespaceEnabledFlag: "Enables the debug namespace for both enclave and host",
logLevelFlag: "Sets the log level 1-Error, 5-Trace",
isInboundP2PDisabledFlag: "Disables inbound p2p (for testing)",
batchIntervalFlag: "Duration between each batch. Can be formatted like 500ms or 1s",
rollupIntervalFlag: "Duration between each rollup. Can be formatted like 500ms or 1s",
}
}
8 changes: 8 additions & 0 deletions go/node/cmd/main.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
package main

import (
"time"

"github.com/obscuronet/go-obscuro/go/node"
)

func main() {
cliConfig := ParseConfigCLI()
// todo (#1618) - allow for multiple operation (start, stop, status)

// we already validated the config in ParseConfigCLI, so we can safely ignore the error here
batchInterval, _ := time.ParseDuration(cliConfig.batchInterval)
rollupInterval, _ := time.ParseDuration(cliConfig.rollupInterval)
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the node validated the time parsing and this mechanism only took strings and simple types ?


nodeCfg := node.NewNodeConfig(
node.WithNodeName(cliConfig.nodeName),
node.WithNodeType(cliConfig.nodeType),
Expand All @@ -33,6 +39,8 @@ func main() {
node.WithDebugNamespaceEnabled(cliConfig.isDebugNamespaceEnabled), // false
node.WithLogLevel(cliConfig.logLevel),
node.WithInboundP2PDisabled(cliConfig.isInboundP2PDisabled),
node.WithBatchInterval(batchInterval),
node.WithRollupInterval(rollupInterval),
)

dockerNode := node.NewDockerNode(nodeCfg)
Expand Down
14 changes: 14 additions & 0 deletions go/node/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ type Config struct {
logLevel int
isInboundP2PDisabled bool
l1BlockTime time.Duration
batchInterval time.Duration
rollupInterval time.Duration
}

func NewNodeConfig(opts ...Option) *Config {
Expand Down Expand Up @@ -286,3 +288,15 @@ func WithL1BlockTime(d time.Duration) Option {
c.l1BlockTime = d
}
}

func WithBatchInterval(d time.Duration) Option {
return func(c *Config) {
c.batchInterval = d
}
}

func WithRollupInterval(d time.Duration) Option {
return func(c *Config) {
c.rollupInterval = d
}
}
4 changes: 2 additions & 2 deletions go/node/docker_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ func (d *DockerNode) startHost() error {
fmt.Sprintf("-useInMemoryDB=%t", d.cfg.hostInMemDB),
fmt.Sprintf("-debugNamespaceEnabled=%t", d.cfg.debugNamespaceEnabled),
// todo (@stefan): once the limiter is in, increase it back to 5 or 10s
"-batchInterval=1s",
"-rollupInterval=3s",
fmt.Sprintf("-batchInterval=%s", d.cfg.batchInterval),
fmt.Sprintf("-rollupInterval=%s", d.cfg.rollupInterval),
fmt.Sprintf("-logLevel=%d", d.cfg.logLevel),
fmt.Sprintf("-isInboundP2PDisabled=%t", d.cfg.isInboundP2PDisabled),
}
Expand Down