From fb47037ed2a473ea4ac6d56db8b573526aa10881 Mon Sep 17 00:00:00 2001 From: Tristan Wilson Date: Fri, 21 Jun 2024 10:56:08 -0700 Subject: [PATCH 1/3] Automatically determine signer mask for backends --- das/rpc_aggregator.go | 5 ++--- das/rpc_test.go | 1 - system_tests/common_test.go | 1 - system_tests/das_test.go | 2 -- 4 files changed, 2 insertions(+), 7 deletions(-) diff --git a/das/rpc_aggregator.go b/das/rpc_aggregator.go index 7e363c6179..ca8561b831 100644 --- a/das/rpc_aggregator.go +++ b/das/rpc_aggregator.go @@ -25,7 +25,6 @@ import ( type BackendConfig struct { URL string `json:"url"` PubKeyBase64Encoded string `json:"pubkey"` - SignerMask uint64 `json:"signermask"` } func NewRPCAggregator(ctx context.Context, config DataAvailabilityConfig, signer signature.DataSignerFunc) (*Aggregator, error) { @@ -61,7 +60,7 @@ func ParseServices(config AggregatorConfig, signer signature.DataSignerFunc) ([] var services []ServiceDetails - for _, b := range cs { + for i, b := range cs { url, err := url.Parse(b.URL) if err != nil { return nil, err @@ -78,7 +77,7 @@ func ParseServices(config AggregatorConfig, signer signature.DataSignerFunc) ([] return nil, err } - d, err := NewServiceDetails(service, *pubKey, b.SignerMask, metricName) + d, err := NewServiceDetails(service, *pubKey, 1< Date: Fri, 21 Jun 2024 17:31:06 -0700 Subject: [PATCH 2/3] Fix das backends field json parsing Previously the das backends configuration fields: `--keyset.backends` for datool `--node.data-availability.rpc-aggregator.backends` for nitro needed to be an escaped JSON string if they were in the JSON configuration file, eg '"backends": "[{\"url\":\"http://...'. Now it can be normal JSON just like the surrounding file, eg ``` "backends": [ { "url": "http://...", "pubkey": "..." }, { "url": "http://...", ``` etc The backends can also be specified on the command line as JSON, eg `datool dumpkeyset --keyset.backends '[{"url":"http://", ...}]'` --- cmd/datool/datool.go | 6 +++- cmd/nitro/nitro.go | 5 +++ das/aggregator.go | 14 +++++---- das/rpc_aggregator.go | 62 +++++++++++++++++++++++++++++++------ das/rpc_test.go | 12 +++---- system_tests/common_test.go | 4 +-- system_tests/das_test.go | 13 +++----- 7 files changed, 82 insertions(+), 34 deletions(-) diff --git a/cmd/datool/datool.go b/cmd/datool/datool.go index cdea134bcb..91619999b8 100644 --- a/cmd/datool/datool.go +++ b/cmd/datool/datool.go @@ -316,6 +316,10 @@ func parseDumpKeyset(args []string) (*DumpKeysetConfig, error) { return nil, err } + if err = das.FixKeysetCLIParsing("keyset.backends", k); err != nil { + return nil, err + } + var config DumpKeysetConfig if err := confighelpers.EndCommonParse(k, &config); err != nil { return nil, err @@ -334,7 +338,7 @@ func parseDumpKeyset(args []string) (*DumpKeysetConfig, error) { if config.Keyset.AssumedHonest == 0 { return nil, errors.New("--keyset.assumed-honest must be set") } - if config.Keyset.Backends == "" { + if config.Keyset.Backends == nil { return nil, errors.New("--keyset.backends must be set") } diff --git a/cmd/nitro/nitro.go b/cmd/nitro/nitro.go index b2eb07f69c..e94b27f36d 100644 --- a/cmd/nitro/nitro.go +++ b/cmd/nitro/nitro.go @@ -51,6 +51,7 @@ import ( "github.com/offchainlabs/nitro/cmd/genericconf" "github.com/offchainlabs/nitro/cmd/util" "github.com/offchainlabs/nitro/cmd/util/confighelpers" + "github.com/offchainlabs/nitro/das" "github.com/offchainlabs/nitro/execution/gethexec" _ "github.com/offchainlabs/nitro/execution/nodeInterface" "github.com/offchainlabs/nitro/solgen/go/bridgegen" @@ -874,6 +875,10 @@ func ParseNode(ctx context.Context, args []string) (*NodeConfig, *genericconf.Wa return nil, nil, err } + if err = das.FixKeysetCLIParsing("node.data-availability.rpc-aggregator.backends", k); err != nil { + return nil, nil, err + } + var nodeConfig NodeConfig if err := confighelpers.EndCommonParse(k, &nodeConfig); err != nil { return nil, nil, err diff --git a/das/aggregator.go b/das/aggregator.go index 25db73a76e..b237096df6 100644 --- a/das/aggregator.go +++ b/das/aggregator.go @@ -27,22 +27,24 @@ import ( ) type AggregatorConfig struct { - Enable bool `koanf:"enable"` - AssumedHonest int `koanf:"assumed-honest"` - Backends string `koanf:"backends"` - MaxStoreChunkBodySize int `koanf:"max-store-chunk-body-size"` + Enable bool `koanf:"enable"` + AssumedHonest int `koanf:"assumed-honest"` + Backends BackendConfigList `koanf:"backends"` + MaxStoreChunkBodySize int `koanf:"max-store-chunk-body-size"` } var DefaultAggregatorConfig = AggregatorConfig{ AssumedHonest: 0, - Backends: "", + Backends: nil, MaxStoreChunkBodySize: 512 * 1024, } +var parsedBackendsConf BackendConfigList + func AggregatorConfigAddOptions(prefix string, f *flag.FlagSet) { f.Bool(prefix+".enable", DefaultAggregatorConfig.Enable, "enable storage of sequencer batch data from a list of RPC endpoints; this should only be used by the batch poster and not in combination with other DAS storage types") f.Int(prefix+".assumed-honest", DefaultAggregatorConfig.AssumedHonest, "Number of assumed honest backends (H). If there are N backends, K=N+1-H valid responses are required to consider an Store request to be successful.") - f.String(prefix+".backends", DefaultAggregatorConfig.Backends, "JSON RPC backend configuration") + f.Var(&parsedBackendsConf, prefix+".backends", "JSON RPC backend configuration") f.Int(prefix+".max-store-chunk-body-size", DefaultAggregatorConfig.MaxStoreChunkBodySize, "maximum HTTP POST body size to use for individual batch chunks, including JSON RPC overhead and an estimated overhead of 512B of headers") } diff --git a/das/rpc_aggregator.go b/das/rpc_aggregator.go index ca8561b831..24a470be5b 100644 --- a/das/rpc_aggregator.go +++ b/das/rpc_aggregator.go @@ -12,6 +12,8 @@ import ( "math/bits" "net/url" + "github.com/knadh/koanf" + "github.com/knadh/koanf/providers/confmap" "github.com/offchainlabs/nitro/arbstate/daprovider" "github.com/offchainlabs/nitro/blsSignatures" "github.com/offchainlabs/nitro/solgen/go/bridgegen" @@ -23,8 +25,54 @@ import ( ) type BackendConfig struct { - URL string `json:"url"` - PubKeyBase64Encoded string `json:"pubkey"` + URL string `koanf:"url" json:"url"` + Pubkey string `koanf:"pubkey" json:"pubkey"` +} + +type BackendConfigList []BackendConfig + +func (l *BackendConfigList) String() string { + b, _ := json.Marshal(*l) + return string(b) +} + +func (l *BackendConfigList) Set(value string) error { + return l.UnmarshalJSON([]byte(value)) +} + +func (l *BackendConfigList) UnmarshalJSON(data []byte) error { + var tmp []BackendConfig + if err := json.Unmarshal(data, &tmp); err != nil { + return err + } + *l = tmp + return nil +} + +func (l *BackendConfigList) Type() string { + return "backendConfigList" +} + +func FixKeysetCLIParsing(path string, k *koanf.Koanf) error { + rawBackends := k.Get(path) + if bk, ok := rawBackends.(string); ok { + err := parsedBackendsConf.UnmarshalJSON([]byte(bk)) + if err != nil { + return err + } + + // Create a map with the parsed backend configurations + tempMap := map[string]interface{}{ + path: parsedBackendsConf, + } + + // Load the map into koanf + if err = k.Load(confmap.Provider(tempMap, "."), nil); err != nil { + return err + } + + } + return nil } func NewRPCAggregator(ctx context.Context, config DataAvailabilityConfig, signer signature.DataSignerFunc) (*Aggregator, error) { @@ -52,15 +100,9 @@ func NewRPCAggregatorWithSeqInboxCaller(config DataAvailabilityConfig, seqInboxC } func ParseServices(config AggregatorConfig, signer signature.DataSignerFunc) ([]ServiceDetails, error) { - var cs []BackendConfig - err := json.Unmarshal([]byte(config.Backends), &cs) - if err != nil { - return nil, err - } - var services []ServiceDetails - for i, b := range cs { + for i, b := range config.Backends { url, err := url.Parse(b.URL) if err != nil { return nil, err @@ -72,7 +114,7 @@ func ParseServices(config AggregatorConfig, signer signature.DataSignerFunc) ([] return nil, err } - pubKey, err := DecodeBase64BLSPublicKey([]byte(b.PubKeyBase64Encoded)) + pubKey, err := DecodeBase64BLSPublicKey([]byte(b.Pubkey)) if err != nil { return nil, err } diff --git a/das/rpc_test.go b/das/rpc_test.go index bc445681f7..e11120b454 100644 --- a/das/rpc_test.go +++ b/das/rpc_test.go @@ -8,7 +8,6 @@ import ( "context" "encoding/base64" "encoding/hex" - "encoding/json" "net" "sync" "testing" @@ -73,17 +72,16 @@ func testRpcImpl(t *testing.T, size, times int, concurrent bool) { } }() testhelpers.RequireImpl(t, err) - beConfig := BackendConfig{ - URL: "http://" + lis.Addr().String(), - PubKeyBase64Encoded: blsPubToBase64(pubkey), - } + beConfigs := BackendConfigList{BackendConfig{ + URL: "http://" + lis.Addr().String(), + Pubkey: blsPubToBase64(pubkey), + }} - backendsJsonByte, err := json.Marshal([]BackendConfig{beConfig}) testhelpers.RequireImpl(t, err) aggConf := DataAvailabilityConfig{ RPCAggregator: AggregatorConfig{ AssumedHonest: 1, - Backends: string(backendsJsonByte), + Backends: beConfigs, MaxStoreChunkBodySize: (chunkSize * 2) + len(sendChunkJSONBoilerplate), }, RequestTimeout: time.Minute, diff --git a/system_tests/common_test.go b/system_tests/common_test.go index bfcb8c718e..1c45802b54 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -1144,8 +1144,8 @@ func setupConfigWithDAS( Require(t, err) beConfigA := das.BackendConfig{ - URL: "http://" + rpcLis.Addr().String(), - PubKeyBase64Encoded: blsPubToBase64(dasSignerKey), + URL: "http://" + rpcLis.Addr().String(), + Pubkey: blsPubToBase64(dasSignerKey), } l1NodeConfigA.DataAvailability.RPCAggregator = aggConfigForBackend(t, beConfigA) l1NodeConfigA.DataAvailability.Enable = true diff --git a/system_tests/das_test.go b/system_tests/das_test.go index 55daba24d9..fc0f17cecc 100644 --- a/system_tests/das_test.go +++ b/system_tests/das_test.go @@ -6,7 +6,6 @@ package arbtest import ( "context" "encoding/base64" - "encoding/json" "io" "math/big" "net" @@ -80,8 +79,8 @@ func startLocalDASServer( restServer, err := das.NewRestfulDasServerOnListener(restLis, genericconf.HTTPServerTimeoutConfigDefault, storageService, storageService) Require(t, err) beConfig := das.BackendConfig{ - URL: "http://" + rpcLis.Addr().String(), - PubKeyBase64Encoded: blsPubToBase64(pubkey), + URL: "http://" + rpcLis.Addr().String(), + Pubkey: blsPubToBase64(pubkey), } return rpcServer, pubkey, beConfig, restServer, "http://" + restLis.Addr().String() } @@ -94,12 +93,10 @@ func blsPubToBase64(pubkey *blsSignatures.PublicKey) string { } func aggConfigForBackend(t *testing.T, backendConfig das.BackendConfig) das.AggregatorConfig { - backendsJsonByte, err := json.Marshal([]das.BackendConfig{backendConfig}) - Require(t, err) return das.AggregatorConfig{ Enable: true, AssumedHonest: 1, - Backends: string(backendsJsonByte), + Backends: das.BackendConfigList{backendConfig}, MaxStoreChunkBodySize: 512 * 1024, } } @@ -300,8 +297,8 @@ func TestDASComplexConfigAndRestMirror(t *testing.T) { RequestTimeout: 5 * time.Second, } beConfigA := das.BackendConfig{ - URL: "http://" + rpcLis.Addr().String(), - PubKeyBase64Encoded: blsPubToBase64(pubkey), + URL: "http://" + rpcLis.Addr().String(), + Pubkey: blsPubToBase64(pubkey), } l1NodeConfigA.DataAvailability.RPCAggregator = aggConfigForBackend(t, beConfigA) l1NodeConfigA.DataAvailability.RestAggregator = das.DefaultRestfulClientAggregatorConfig From b1ef8c9c29e4febb7b2efb28726c222dc74d1ce0 Mon Sep 17 00:00:00 2001 From: Tristan Wilson Date: Mon, 24 Jun 2024 16:23:01 -0700 Subject: [PATCH 3/3] Fix config parsing tests, explain backends cli opt --- cmd/nitro/config_test.go | 5 ++++- das/aggregator.go | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cmd/nitro/config_test.go b/cmd/nitro/config_test.go index d76dd1b7b9..f94f941e0b 100644 --- a/cmd/nitro/config_test.go +++ b/cmd/nitro/config_test.go @@ -16,6 +16,7 @@ import ( "github.com/offchainlabs/nitro/cmd/genericconf" "github.com/offchainlabs/nitro/cmd/util/confighelpers" + "github.com/offchainlabs/nitro/das" "github.com/offchainlabs/nitro/util/colors" "github.com/offchainlabs/nitro/util/testhelpers" @@ -28,6 +29,8 @@ func TestEmptyCliConfig(t *testing.T) { NodeConfigAddOptions(f) k, err := confighelpers.BeginCommonParse(f, []string{}) Require(t, err) + err = das.FixKeysetCLIParsing("node.data-availability.rpc-aggregator.backends", k) + Require(t, err) var emptyCliNodeConfig NodeConfig err = confighelpers.EndCommonParse(k, &emptyCliNodeConfig) Require(t, err) @@ -57,7 +60,7 @@ func TestValidatorConfig(t *testing.T) { } func TestAggregatorConfig(t *testing.T) { - args := strings.Split("--persistent.chain /tmp/data --init.dev-init --node.parent-chain-reader.enable=false --parent-chain.id 5 --chain.id 421613 --node.batch-poster.parent-chain-wallet.pathname /l1keystore --node.batch-poster.parent-chain-wallet.password passphrase --http.addr 0.0.0.0 --ws.addr 0.0.0.0 --node.sequencer --execution.sequencer.enable --node.feed.output.enable --node.feed.output.port 9642 --node.data-availability.enable --node.data-availability.rpc-aggregator.backends {[\"url\":\"http://localhost:8547\",\"pubkey\":\"abc==\",\"signerMask\":0x1]}", " ") + args := strings.Split("--persistent.chain /tmp/data --init.dev-init --node.parent-chain-reader.enable=false --parent-chain.id 5 --chain.id 421613 --node.batch-poster.parent-chain-wallet.pathname /l1keystore --node.batch-poster.parent-chain-wallet.password passphrase --http.addr 0.0.0.0 --ws.addr 0.0.0.0 --node.sequencer --execution.sequencer.enable --node.feed.output.enable --node.feed.output.port 9642 --node.data-availability.enable --node.data-availability.rpc-aggregator.backends [{\"url\":\"http://localhost:8547\",\"pubkey\":\"abc==\"}]", " ") _, _, err := ParseNode(context.Background(), args) Require(t, err) } diff --git a/das/aggregator.go b/das/aggregator.go index b237096df6..24bde9cece 100644 --- a/das/aggregator.go +++ b/das/aggregator.go @@ -44,7 +44,7 @@ var parsedBackendsConf BackendConfigList func AggregatorConfigAddOptions(prefix string, f *flag.FlagSet) { f.Bool(prefix+".enable", DefaultAggregatorConfig.Enable, "enable storage of sequencer batch data from a list of RPC endpoints; this should only be used by the batch poster and not in combination with other DAS storage types") f.Int(prefix+".assumed-honest", DefaultAggregatorConfig.AssumedHonest, "Number of assumed honest backends (H). If there are N backends, K=N+1-H valid responses are required to consider an Store request to be successful.") - f.Var(&parsedBackendsConf, prefix+".backends", "JSON RPC backend configuration") + f.Var(&parsedBackendsConf, prefix+".backends", "JSON RPC backend configuration. This can be specified on the command line as a JSON array, eg: [{\"url\": \"...\", \"pubkey\": \"...\"},...], or as a JSON array in the config file.") f.Int(prefix+".max-store-chunk-body-size", DefaultAggregatorConfig.MaxStoreChunkBodySize, "maximum HTTP POST body size to use for individual batch chunks, including JSON RPC overhead and an estimated overhead of 512B of headers") }