From 5f5d9c8de93144874e47af3b0c56fb911e1f8de6 Mon Sep 17 00:00:00 2001 From: Gabriel de Quadros Ligneul Date: Wed, 11 Sep 2024 15:21:36 -0300 Subject: [PATCH 1/5] Fix DAS test by setting the default config The batch-poster wasn't using the DAS because the max retention period wasn't being set correctly. --- system_tests/das_test.go | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/system_tests/das_test.go b/system_tests/das_test.go index 9f4d153b6f..593058eaac 100644 --- a/system_tests/das_test.go +++ b/system_tests/das_test.go @@ -44,18 +44,13 @@ func startLocalDASServer( pubkey, _, err := das.GenerateAndStoreKeys(keyDir) Require(t, err) - config := das.DataAvailabilityConfig{ - Enable: true, - Key: das.KeyConfig{ - KeyDir: keyDir, - }, - LocalFileStorage: das.LocalFileStorageConfig{ - Enable: true, - DataDir: dataDir, - }, - ParentChainNodeURL: "none", - RequestTimeout: 5 * time.Second, - } + config := das.DefaultDataAvailabilityConfig + config.Enable = true + config.Key = das.KeyConfig{KeyDir: keyDir} + config.ParentChainNodeURL = "none" + config.LocalFileStorage = das.DefaultLocalFileStorageConfig + config.LocalFileStorage.Enable = true + config.LocalFileStorage.DataDir = dataDir storageService, lifecycleManager, err := das.CreatePersistentStorageService(ctx, &config) defer lifecycleManager.StopAndWaitUntil(time.Second) From 919003894b09dfacdcaa0ba55b589217b8b74fdd Mon Sep 17 00:00:00 2001 From: Gabriel de Quadros Ligneul Date: Wed, 11 Sep 2024 17:53:30 -0300 Subject: [PATCH 2/5] Test manual batch-poster fallback for DAS This test checks whether the batch-poster manual fallback for DAS works correctly. Here are the steps of the test: * Setup a L2 chain using a DAS * Sends a batch using the DAS * Shutdown the DAS * Fail to send a batch because the fallback is disabled * Enable the fallback * Verify the batch was sent with the fallback --- system_tests/das_test.go | 77 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/system_tests/das_test.go b/system_tests/das_test.go index 593058eaac..4f9bc8ab9f 100644 --- a/system_tests/das_test.go +++ b/system_tests/das_test.go @@ -322,3 +322,80 @@ func initTest(t *testing.T) { enableLogging(logLvl) } } + +func TestDASBatchPosterFallback(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Setup L1 + builder := NewNodeBuilder(ctx).DefaultConfig(t, true) + builder.chainConfig = params.ArbitrumDevTestDASChainConfig() + builder.BuildL1(t) + l1client := builder.L1.Client + l1info := builder.L1Info + + // Setup DAS server + dasDataDir := t.TempDir() + dasRpcServer, pubkey, backendConfig, _, restServerUrl := startLocalDASServer( + t, ctx, dasDataDir, l1client, builder.addresses.SequencerInbox) + authorizeDASKeyset(t, ctx, pubkey, l1info, l1client) + + // Setup sequence/batch-poster L2 node + builder.nodeConfig.DataAvailability.Enable = true + builder.nodeConfig.DataAvailability.RPCAggregator = aggConfigForBackend(backendConfig) + builder.nodeConfig.DataAvailability.RestAggregator = das.DefaultRestfulClientAggregatorConfig + builder.nodeConfig.DataAvailability.RestAggregator.Enable = true + builder.nodeConfig.DataAvailability.RestAggregator.Urls = []string{restServerUrl} + builder.nodeConfig.DataAvailability.ParentChainNodeURL = "none" + builder.nodeConfig.BatchPoster.DisableDapFallbackStoreDataOnChain = true // Disable DAS fallback + builder.nodeConfig.BatchPoster.ErrorDelay = time.Millisecond * 250 // Increase error delay because we expect errors + builder.L2Info = NewArbTestInfo(t, builder.chainConfig.ChainID) + builder.L2Info.GenerateAccount("User2") + cleanup := builder.BuildL2OnL1(t) + defer cleanup() + l2client := builder.L2.Client + l2info := builder.L2Info + + // Setup secondary L2 node + nodeConfigB := arbnode.ConfigDefaultL1NonSequencerTest() + nodeConfigB.BlockValidator.Enable = false + nodeConfigB.DataAvailability.Enable = true + nodeConfigB.DataAvailability.RestAggregator = das.DefaultRestfulClientAggregatorConfig + nodeConfigB.DataAvailability.RestAggregator.Enable = true + nodeConfigB.DataAvailability.RestAggregator.Urls = []string{restServerUrl} + nodeConfigB.DataAvailability.ParentChainNodeURL = "none" + nodeBParams := SecondNodeParams{ + nodeConfig: nodeConfigB, + initData: &l2info.ArbInitData, + } + l2B, cleanupB := builder.Build2ndNode(t, &nodeBParams) + defer cleanupB() + + // Check batch posting using the DAS + checkBatchPosting(t, ctx, l1client, l2client, l1info, l2info, big.NewInt(1e12), l2B.Client) + + // Shutdown the DAS + err := dasRpcServer.Shutdown(ctx) + Require(t, err) + + // Send 2nd transaction and check it doesn't arrive on second node + tx, _ := TransferBalanceTo(t, "Owner", l2info.GetAddress("User2"), big.NewInt(1e12), l2info, l2client, ctx) + _, err = WaitForTx(ctx, l2B.Client, tx.Hash(), time.Second*3) + if err == nil { + Fatal(t, "expected error but got nil") + } + + // Enable the DAP fallback and check the transaction on the second node. + // (We don't need to restart the node because of the hot-reload.) + builder.nodeConfig.BatchPoster.DisableDapFallbackStoreDataOnChain = false + _, err = WaitForTx(ctx, l2B.Client, tx.Hash(), time.Second*3) + Require(t, err) + l2balance, err := l2B.Client.BalanceAt(ctx, l2info.GetAddress("User2"), nil) + Require(t, err) + if l2balance.Cmp(big.NewInt(2e12)) != 0 { + Fatal(t, "Unexpected balance:", l2balance) + } + + // Send another transaction with fallback on + checkBatchPosting(t, ctx, l1client, l2client, l1info, l2info, big.NewInt(3e12), l2B.Client) +} From beca92d21bfa6d1666f64cadd31334244e8fd98a Mon Sep 17 00:00:00 2001 From: Gabriel de Quadros Ligneul Date: Thu, 12 Sep 2024 11:43:15 -0300 Subject: [PATCH 3/5] Check for context-deadline exceeded error --- system_tests/das_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/system_tests/das_test.go b/system_tests/das_test.go index 4f9bc8ab9f..d59fca3e47 100644 --- a/system_tests/das_test.go +++ b/system_tests/das_test.go @@ -6,6 +6,7 @@ package arbtest import ( "context" "encoding/base64" + "errors" "io" "math/big" "net" @@ -381,8 +382,8 @@ func TestDASBatchPosterFallback(t *testing.T) { // Send 2nd transaction and check it doesn't arrive on second node tx, _ := TransferBalanceTo(t, "Owner", l2info.GetAddress("User2"), big.NewInt(1e12), l2info, l2client, ctx) _, err = WaitForTx(ctx, l2B.Client, tx.Hash(), time.Second*3) - if err == nil { - Fatal(t, "expected error but got nil") + if err == nil || !errors.Is(err, context.DeadlineExceeded) { + Fatal(t, "expected context-deadline exceeded error, but got:", err) } // Enable the DAP fallback and check the transaction on the second node. From f0f5af5798d3bd6f4f6be5a1c16c3cba82567458 Mon Sep 17 00:00:00 2001 From: Gabriel de Quadros Ligneul Date: Mon, 16 Sep 2024 14:31:25 -0300 Subject: [PATCH 4/5] Use positive in config name (disable -> enable) Rename --node.batch-poster.disable-dap-fallback-store-data-on-chain to --node.batch-poster.enable-dap-fallback-store-data-on-chain --- arbnode/batch_poster.go | 12 ++++++------ arbstate/daprovider/writer.go | 6 +++--- system_tests/das_test.go | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 44b360e76e..4e4a8d8572 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -141,8 +141,8 @@ type BatchPosterDangerousConfig struct { } type BatchPosterConfig struct { - Enable bool `koanf:"enable"` - DisableDapFallbackStoreDataOnChain bool `koanf:"disable-dap-fallback-store-data-on-chain" reload:"hot"` + Enable bool `koanf:"enable"` + EnableDapFallbackStoreDataOnChain bool `koanf:"enable-dap-fallback-store-data-on-chain" reload:"hot"` // Max batch size. MaxSize int `koanf:"max-size" reload:"hot"` // Maximum 4844 blob enabled batch size. @@ -205,7 +205,7 @@ type BatchPosterConfigFetcher func() *BatchPosterConfig func BatchPosterConfigAddOptions(prefix string, f *pflag.FlagSet) { f.Bool(prefix+".enable", DefaultBatchPosterConfig.Enable, "enable posting batches to l1") - f.Bool(prefix+".disable-dap-fallback-store-data-on-chain", DefaultBatchPosterConfig.DisableDapFallbackStoreDataOnChain, "If unable to batch to DA provider, disable fallback storing data on chain") + f.Bool(prefix+".enable-dap-fallback-store-data-on-chain", DefaultBatchPosterConfig.EnableDapFallbackStoreDataOnChain, "If unable to batch to DA provider, enable fallback storing data on chain") f.Int(prefix+".max-size", DefaultBatchPosterConfig.MaxSize, "maximum batch size") f.Int(prefix+".max-4844-batch-size", DefaultBatchPosterConfig.Max4844BatchSize, "maximum 4844 blob enabled batch size") f.Duration(prefix+".max-delay", DefaultBatchPosterConfig.MaxDelay, "maximum batch posting delay") @@ -231,8 +231,8 @@ func BatchPosterConfigAddOptions(prefix string, f *pflag.FlagSet) { } var DefaultBatchPosterConfig = BatchPosterConfig{ - Enable: false, - DisableDapFallbackStoreDataOnChain: false, + Enable: false, + EnableDapFallbackStoreDataOnChain: true, // This default is overridden for L3 chains in applyChainParameters in cmd/nitro/nitro.go MaxSize: 100000, // Try to fill 3 blobs per batch @@ -1366,7 +1366,7 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) return false, fmt.Errorf("%w: nonce changed from %d to %d while creating batch", storage.ErrStorageRace, nonce, gotNonce) } // #nosec G115 - sequencerMsg, err = b.dapWriter.Store(ctx, sequencerMsg, uint64(time.Now().Add(config.DASRetentionPeriod).Unix()), config.DisableDapFallbackStoreDataOnChain) + sequencerMsg, err = b.dapWriter.Store(ctx, sequencerMsg, uint64(time.Now().Add(config.DASRetentionPeriod).Unix()), config.EnableDapFallbackStoreDataOnChain) if err != nil { batchPosterDAFailureCounter.Inc(1) return false, err diff --git a/arbstate/daprovider/writer.go b/arbstate/daprovider/writer.go index a26e53c94d..1d83c0348f 100644 --- a/arbstate/daprovider/writer.go +++ b/arbstate/daprovider/writer.go @@ -17,7 +17,7 @@ type Writer interface { ctx context.Context, message []byte, timeout uint64, - disableFallbackStoreDataOnChain bool, + enableFallbackStoreDataOnChain bool, ) ([]byte, error) } @@ -31,10 +31,10 @@ type writerForDAS struct { dasWriter DASWriter } -func (d *writerForDAS) Store(ctx context.Context, message []byte, timeout uint64, disableFallbackStoreDataOnChain bool) ([]byte, error) { +func (d *writerForDAS) Store(ctx context.Context, message []byte, timeout uint64, enableFallbackStoreDataOnChain bool) ([]byte, error) { cert, err := d.dasWriter.Store(ctx, message, timeout) if errors.Is(err, ErrBatchToDasFailed) { - if disableFallbackStoreDataOnChain { + if !enableFallbackStoreDataOnChain { return nil, errors.New("unable to batch to DAS and fallback storing data on chain is disabled") } log.Warn("Falling back to storing data on chain", "err", err) diff --git a/system_tests/das_test.go b/system_tests/das_test.go index d59fca3e47..b16a97720b 100644 --- a/system_tests/das_test.go +++ b/system_tests/das_test.go @@ -348,7 +348,7 @@ func TestDASBatchPosterFallback(t *testing.T) { builder.nodeConfig.DataAvailability.RestAggregator.Enable = true builder.nodeConfig.DataAvailability.RestAggregator.Urls = []string{restServerUrl} builder.nodeConfig.DataAvailability.ParentChainNodeURL = "none" - builder.nodeConfig.BatchPoster.DisableDapFallbackStoreDataOnChain = true // Disable DAS fallback + builder.nodeConfig.BatchPoster.EnableDapFallbackStoreDataOnChain = false // Disable DAS fallback builder.nodeConfig.BatchPoster.ErrorDelay = time.Millisecond * 250 // Increase error delay because we expect errors builder.L2Info = NewArbTestInfo(t, builder.chainConfig.ChainID) builder.L2Info.GenerateAccount("User2") @@ -388,7 +388,7 @@ func TestDASBatchPosterFallback(t *testing.T) { // Enable the DAP fallback and check the transaction on the second node. // (We don't need to restart the node because of the hot-reload.) - builder.nodeConfig.BatchPoster.DisableDapFallbackStoreDataOnChain = false + builder.nodeConfig.BatchPoster.EnableDapFallbackStoreDataOnChain = true _, err = WaitForTx(ctx, l2B.Client, tx.Hash(), time.Second*3) Require(t, err) l2balance, err := l2B.Client.BalanceAt(ctx, l2info.GetAddress("User2"), nil) From 143c68be28db76d12c71fa5f9c4d9c2e303b2f66 Mon Sep 17 00:00:00 2001 From: Gabriel de Quadros Ligneul Date: Mon, 16 Sep 2024 16:00:25 -0300 Subject: [PATCH 5/5] Enable DAP fallback on test config --- arbnode/batch_poster.go | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 4e4a8d8572..8c80d65009 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -267,26 +267,27 @@ var DefaultBatchPosterL1WalletConfig = genericconf.WalletConfig{ } var TestBatchPosterConfig = BatchPosterConfig{ - Enable: true, - MaxSize: 100000, - Max4844BatchSize: DefaultBatchPosterConfig.Max4844BatchSize, - PollInterval: time.Millisecond * 10, - ErrorDelay: time.Millisecond * 10, - MaxDelay: 0, - WaitForMaxDelay: false, - CompressionLevel: 2, - DASRetentionPeriod: daprovider.DefaultDASRetentionPeriod, - GasRefunderAddress: "", - ExtraBatchGas: 10_000, - Post4844Blobs: true, - IgnoreBlobPrice: false, - DataPoster: dataposter.TestDataPosterConfig, - ParentChainWallet: DefaultBatchPosterL1WalletConfig, - L1BlockBound: "", - L1BlockBoundBypass: time.Hour, - UseAccessLists: true, - GasEstimateBaseFeeMultipleBips: arbmath.OneInUBips * 3 / 2, - CheckBatchCorrectness: true, + Enable: true, + EnableDapFallbackStoreDataOnChain: true, + MaxSize: 100000, + Max4844BatchSize: DefaultBatchPosterConfig.Max4844BatchSize, + PollInterval: time.Millisecond * 10, + ErrorDelay: time.Millisecond * 10, + MaxDelay: 0, + WaitForMaxDelay: false, + CompressionLevel: 2, + DASRetentionPeriod: daprovider.DefaultDASRetentionPeriod, + GasRefunderAddress: "", + ExtraBatchGas: 10_000, + Post4844Blobs: true, + IgnoreBlobPrice: false, + DataPoster: dataposter.TestDataPosterConfig, + ParentChainWallet: DefaultBatchPosterL1WalletConfig, + L1BlockBound: "", + L1BlockBoundBypass: time.Hour, + UseAccessLists: true, + GasEstimateBaseFeeMultipleBips: arbmath.OneInUBips * 3 / 2, + CheckBatchCorrectness: true, } type BatchPosterOpts struct {