From 97514570379207f24c0707e629b7bd6dfb70ee42 Mon Sep 17 00:00:00 2001 From: Aman Sanghi Date: Thu, 1 Aug 2024 23:15:03 +0530 Subject: [PATCH 1/6] Remove fast confirmer config --- staker/staker.go | 52 +++++++++++++++++-------------- system_tests/fast_confirm_test.go | 3 -- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/staker/staker.go b/staker/staker.go index e3dd11dc07..b3770cf9d1 100644 --- a/staker/staker.go +++ b/staker/staker.go @@ -23,6 +23,7 @@ import ( "github.com/offchainlabs/nitro/arbnode/dataposter" "github.com/offchainlabs/nitro/arbutil" "github.com/offchainlabs/nitro/cmd/genericconf" + "github.com/offchainlabs/nitro/solgen/go/rollupgen" "github.com/offchainlabs/nitro/staker/txbuilder" "github.com/offchainlabs/nitro/util" "github.com/offchainlabs/nitro/util/arbmath" @@ -90,8 +91,6 @@ type L1ValidatorConfig struct { ExtraGas uint64 `koanf:"extra-gas" reload:"hot"` Dangerous DangerousConfig `koanf:"dangerous"` ParentChainWallet genericconf.WalletConfig `koanf:"parent-chain-wallet"` - EnableFastConfirmation bool `koanf:"enable-fast-confirmation"` - FastConfirmSafeAddress string `koanf:"fast-confirm-safe-address"` LogQueryBatchSize uint64 `koanf:"log-query-batch-size" reload:"hot"` strategy StakerStrategy @@ -159,8 +158,6 @@ var DefaultL1ValidatorConfig = L1ValidatorConfig{ ExtraGas: 50000, Dangerous: DefaultDangerousConfig, ParentChainWallet: DefaultValidatorL1WalletConfig, - EnableFastConfirmation: false, - FastConfirmSafeAddress: "", LogQueryBatchSize: 0, } @@ -182,8 +179,6 @@ var TestL1ValidatorConfig = L1ValidatorConfig{ ExtraGas: 50000, Dangerous: DefaultDangerousConfig, ParentChainWallet: DefaultValidatorL1WalletConfig, - EnableFastConfirmation: false, - FastConfirmSafeAddress: "", LogQueryBatchSize: 0, } @@ -214,8 +209,6 @@ func L1ValidatorConfigAddOptions(prefix string, f *flag.FlagSet) { dataposter.DataPosterConfigAddOptions(prefix+".data-poster", f, dataposter.DefaultDataPosterConfigForValidator) DangerousConfigAddOptions(prefix+".dangerous", f) genericconf.WalletConfigAddOptions(prefix+".parent-chain-wallet", f, DefaultL1ValidatorConfig.ParentChainWallet.Pathname) - f.Bool(prefix+".enable-fast-confirmation", DefaultL1ValidatorConfig.EnableFastConfirmation, "enable fast confirmation") - f.String(prefix+".fast-confirm-safe-address", DefaultL1ValidatorConfig.FastConfirmSafeAddress, "safe address for fast confirmation") } type DangerousConfig struct { @@ -263,6 +256,7 @@ type Staker struct { statelessBlockValidator *StatelessBlockValidator fatalErr chan<- error fastConfirmSafe *FastConfirmSafe + fastConfirmer common.Address } type ValidatorWalletInterface interface { @@ -313,18 +307,13 @@ func NewStaker( stakedNotifiers = append(stakedNotifiers, blockValidator) } var fastConfirmSafe *FastConfirmSafe - if config.EnableFastConfirmation && config.FastConfirmSafeAddress != "" { - fastConfirmSafe, err = NewFastConfirmSafe( - callOpts, - common.HexToAddress(config.FastConfirmSafeAddress), - val.builder, - wallet, - config.gasRefunder, - l1Reader, - ) - if err != nil { - return nil, err - } + rollup, err := rollupgen.NewRollupUserLogic(wallet.RollupAddress(), l1Reader.Client()) + if err != nil { + return nil, err + } + fastConfirmer, err := rollup.AnyTrustFastConfirmer(&bind.CallOpts{}) + if err != nil { + return nil, err } return &Staker{ L1Validator: val, @@ -339,6 +328,7 @@ func NewStaker( statelessBlockValidator: statelessBlockValidator, fatalErr: fatalErr, fastConfirmSafe: fastConfirmSafe, + fastConfirmer: fastConfirmer, }, nil } @@ -372,7 +362,7 @@ func (s *Staker) Initialize(ctx context.Context) error { } func (s *Staker) tryFastConfirmationNodeNumber(ctx context.Context, number uint64) error { - if !s.config.EnableFastConfirmation { + if s.fastConfirmer == (common.Address{}) { return nil } nodeInfo, err := s.rollup.LookupNode(ctx, number) @@ -383,10 +373,26 @@ func (s *Staker) tryFastConfirmationNodeNumber(ctx context.Context, number uint6 } func (s *Staker) tryFastConfirmation(ctx context.Context, blockHash common.Hash, sendRoot common.Hash) error { - if !s.config.EnableFastConfirmation { + if s.fastConfirmer == (common.Address{}) { return nil } - if s.fastConfirmSafe != nil { + // Only use gnosis safe fast confirmation, if the safe address is different from the wallet address, else it's not a safe contract. + if s.fastConfirmer != *s.wallet.Address() { + // If we don't have a fastConfirmSafe, create one + if s.fastConfirmSafe == nil { + fastConfirmSafe, err := NewFastConfirmSafe( + s.baseCallOpts, + s.fastConfirmer, + s.builder, + s.wallet, + s.config.gasRefunder, + s.l1Reader, + ) + if err != nil { + return err + } + s.fastConfirmSafe = fastConfirmSafe + } return s.fastConfirmSafe.tryFastConfirmation(ctx, blockHash, sendRoot) } auth, err := s.builder.Auth(ctx) diff --git a/system_tests/fast_confirm_test.go b/system_tests/fast_confirm_test.go index d780f80414..02f9dd7938 100644 --- a/system_tests/fast_confirm_test.go +++ b/system_tests/fast_confirm_test.go @@ -122,7 +122,6 @@ func TestFastConfirmation(t *testing.T) { Require(t, err) valConfig := staker.TestL1ValidatorConfig - valConfig.EnableFastConfirmation = true parentChainID, err := builder.L1.Client.ChainID(ctx) if err != nil { t.Fatalf("Failed to get parent chain id: %v", err) @@ -324,8 +323,6 @@ func TestFastConfirmationWithSafe(t *testing.T) { Require(t, err) valConfig := staker.TestL1ValidatorConfig - valConfig.EnableFastConfirmation = true - valConfig.FastConfirmSafeAddress = safeAddress.String() parentChainID, err := builder.L1.Client.ChainID(ctx) if err != nil { From 87e837fc6d1d433883bd88632096b86434ec78b8 Mon Sep 17 00:00:00 2001 From: Aman Sanghi Date: Thu, 1 Aug 2024 23:20:20 +0530 Subject: [PATCH 2/6] Keep the option to disable fast confirmation --- staker/staker.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/staker/staker.go b/staker/staker.go index b3770cf9d1..8de660a4c4 100644 --- a/staker/staker.go +++ b/staker/staker.go @@ -92,6 +92,7 @@ type L1ValidatorConfig struct { Dangerous DangerousConfig `koanf:"dangerous"` ParentChainWallet genericconf.WalletConfig `koanf:"parent-chain-wallet"` LogQueryBatchSize uint64 `koanf:"log-query-batch-size" reload:"hot"` + EnableFastConfirmation bool `koanf:"enable-fast-confirmation"` strategy StakerStrategy gasRefunder common.Address @@ -159,6 +160,7 @@ var DefaultL1ValidatorConfig = L1ValidatorConfig{ Dangerous: DefaultDangerousConfig, ParentChainWallet: DefaultValidatorL1WalletConfig, LogQueryBatchSize: 0, + EnableFastConfirmation: true, } var TestL1ValidatorConfig = L1ValidatorConfig{ @@ -180,6 +182,7 @@ var TestL1ValidatorConfig = L1ValidatorConfig{ Dangerous: DefaultDangerousConfig, ParentChainWallet: DefaultValidatorL1WalletConfig, LogQueryBatchSize: 0, + EnableFastConfirmation: true, } var DefaultValidatorL1WalletConfig = genericconf.WalletConfig{ @@ -209,6 +212,7 @@ func L1ValidatorConfigAddOptions(prefix string, f *flag.FlagSet) { dataposter.DataPosterConfigAddOptions(prefix+".data-poster", f, dataposter.DefaultDataPosterConfigForValidator) DangerousConfigAddOptions(prefix+".dangerous", f) genericconf.WalletConfigAddOptions(prefix+".parent-chain-wallet", f, DefaultL1ValidatorConfig.ParentChainWallet.Pathname) + f.Bool(prefix+".enable-fast-confirmation", DefaultL1ValidatorConfig.EnableFastConfirmation, "enable fast confirmation") } type DangerousConfig struct { @@ -362,7 +366,7 @@ func (s *Staker) Initialize(ctx context.Context) error { } func (s *Staker) tryFastConfirmationNodeNumber(ctx context.Context, number uint64) error { - if s.fastConfirmer == (common.Address{}) { + if s.fastConfirmer == (common.Address{}) || !s.config.EnableFastConfirmation { return nil } nodeInfo, err := s.rollup.LookupNode(ctx, number) @@ -373,7 +377,7 @@ func (s *Staker) tryFastConfirmationNodeNumber(ctx context.Context, number uint6 } func (s *Staker) tryFastConfirmation(ctx context.Context, blockHash common.Hash, sendRoot common.Hash) error { - if s.fastConfirmer == (common.Address{}) { + if s.fastConfirmer == (common.Address{}) || !s.config.EnableFastConfirmation { return nil } // Only use gnosis safe fast confirmation, if the safe address is different from the wallet address, else it's not a safe contract. From 9cfdfe917c8c42d96ede791f2b474ffc24bdeb54 Mon Sep 17 00:00:00 2001 From: Aman Sanghi Date: Thu, 1 Aug 2024 23:28:20 +0530 Subject: [PATCH 3/6] Changes based on PR comments --- staker/staker.go | 32 +++++++++++++++---------------- system_tests/fast_confirm_test.go | 8 ++++---- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/staker/staker.go b/staker/staker.go index 8de660a4c4..870c0657a3 100644 --- a/staker/staker.go +++ b/staker/staker.go @@ -319,6 +319,20 @@ func NewStaker( if err != nil { return nil, err } + // Only use gnosis safe fast confirmation, if the safe address is different from the wallet address, else it's not a safe contract. + if fastConfirmer != (common.Address{}) && config.EnableFastConfirmation && wallet.AddressOrZero() != fastConfirmer { + fastConfirmSafe, err = NewFastConfirmSafe( + callOpts, + fastConfirmer, + val.builder, + wallet, + config.gasRefunder, + l1Reader, + ) + if err != nil { + return nil, err + } + } return &Staker{ L1Validator: val, l1Reader: l1Reader, @@ -380,23 +394,7 @@ func (s *Staker) tryFastConfirmation(ctx context.Context, blockHash common.Hash, if s.fastConfirmer == (common.Address{}) || !s.config.EnableFastConfirmation { return nil } - // Only use gnosis safe fast confirmation, if the safe address is different from the wallet address, else it's not a safe contract. - if s.fastConfirmer != *s.wallet.Address() { - // If we don't have a fastConfirmSafe, create one - if s.fastConfirmSafe == nil { - fastConfirmSafe, err := NewFastConfirmSafe( - s.baseCallOpts, - s.fastConfirmer, - s.builder, - s.wallet, - s.config.gasRefunder, - s.l1Reader, - ) - if err != nil { - return err - } - s.fastConfirmSafe = fastConfirmSafe - } + if s.fastConfirmSafe != nil { return s.fastConfirmSafe.tryFastConfirmation(ctx, blockHash, sendRoot) } auth, err := s.builder.Auth(ctx) diff --git a/system_tests/fast_confirm_test.go b/system_tests/fast_confirm_test.go index 02f9dd7938..44adbe8119 100644 --- a/system_tests/fast_confirm_test.go +++ b/system_tests/fast_confirm_test.go @@ -359,6 +359,8 @@ func TestFastConfirmationWithSafe(t *testing.T) { Require(t, err) err = statelessA.Start(ctx) Require(t, err) + err = valWalletA.Initialize(ctx) + Require(t, err) stakerA, err := staker.NewStaker( l2nodeA.L1Reader, valWalletA, @@ -374,8 +376,6 @@ func TestFastConfirmationWithSafe(t *testing.T) { Require(t, err) err = stakerA.Initialize(ctx) Require(t, err) - err = valWalletA.Initialize(ctx) - Require(t, err) cfg := arbnode.ConfigDefaultL1NonSequencerTest() signerCfg, err := externalSignerTestCfg(srv.Address, srv.URL()) if err != nil { @@ -409,6 +409,8 @@ func TestFastConfirmationWithSafe(t *testing.T) { Require(t, err) err = statelessB.Start(ctx) Require(t, err) + err = valWalletB.Initialize(ctx) + Require(t, err) stakerB, err := staker.NewStaker( l2nodeB.L1Reader, valWalletB, @@ -424,8 +426,6 @@ func TestFastConfirmationWithSafe(t *testing.T) { Require(t, err) err = stakerB.Initialize(ctx) Require(t, err) - err = valWalletB.Initialize(ctx) - Require(t, err) builder.L2Info.GenerateAccount("BackgroundUser") tx = builder.L2Info.PrepareTx("Faucet", "BackgroundUser", builder.L2Info.TransferGas, balance, nil) From ec62067aecb3b21630d7d4d0d3d37ea7599be4c1 Mon Sep 17 00:00:00 2001 From: Aman Sanghi Date: Fri, 2 Aug 2024 00:01:51 +0530 Subject: [PATCH 4/6] Confirm validator is part of owners --- staker/staker.go | 9 ++++++++- system_tests/fast_confirm_test.go | 6 ++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/staker/staker.go b/staker/staker.go index 870c0657a3..353541847f 100644 --- a/staker/staker.go +++ b/staker/staker.go @@ -320,7 +320,7 @@ func NewStaker( return nil, err } // Only use gnosis safe fast confirmation, if the safe address is different from the wallet address, else it's not a safe contract. - if fastConfirmer != (common.Address{}) && config.EnableFastConfirmation && wallet.AddressOrZero() != fastConfirmer { + if fastConfirmer != (common.Address{}) && config.EnableFastConfirmation && wallet.AddressOrZero() != (common.Address{}) && wallet.AddressOrZero() != fastConfirmer { fastConfirmSafe, err = NewFastConfirmSafe( callOpts, fastConfirmer, @@ -332,6 +332,13 @@ func NewStaker( if err != nil { return nil, err } + isOwner, err := fastConfirmSafe.safe.IsOwner(&callOpts, wallet.AddressOrZero()) + if err != nil { + return nil, err + } + if !isOwner { + log.Info("fast confirmer is not part of owners of safe", "fastConfirmer", fastConfirmer, "wallet", wallet.AddressOrZero()) + } } return &Staker{ L1Validator: val, diff --git a/system_tests/fast_confirm_test.go b/system_tests/fast_confirm_test.go index 44adbe8119..b967cb1ddc 100644 --- a/system_tests/fast_confirm_test.go +++ b/system_tests/fast_confirm_test.go @@ -157,6 +157,8 @@ func TestFastConfirmation(t *testing.T) { Require(t, err) err = stateless.Start(ctx) Require(t, err) + err = valWallet.Initialize(ctx) + Require(t, err) stakerA, err := staker.NewStaker( l2node.L1Reader, valWallet, @@ -171,10 +173,6 @@ func TestFastConfirmation(t *testing.T) { ) Require(t, err) err = stakerA.Initialize(ctx) - if stakerA.Strategy() != staker.WatchtowerStrategy { - err = valWallet.Initialize(ctx) - Require(t, err) - } Require(t, err) cfg := arbnode.ConfigDefaultL1NonSequencerTest() signerCfg, err := externalSignerTestCfg(srv.Address, srv.URL()) From 7ff6554246e6a1a184a3ed65884b92c8a16aae7e Mon Sep 17 00:00:00 2001 From: Aman Sanghi Date: Wed, 7 Aug 2024 15:17:06 +0530 Subject: [PATCH 5/6] Changes based on PR comments --- staker/staker.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/staker/staker.go b/staker/staker.go index be88d88430..9203156fc1 100644 --- a/staker/staker.go +++ b/staker/staker.go @@ -344,7 +344,11 @@ func NewStaker( return nil, err } if !isOwner { - log.Info("fast confirmer is not part of owners of safe", "fastConfirmer", fastConfirmer, "wallet", wallet.AddressOrZero()) + // If the wallet is not an owner of the safe, we can't use it for fast confirmation + // So disable fast confirmation. + fastConfirmer = common.Address{} + fastConfirmSafe = nil + log.Info("Staker wallet address is not part of owners of safe so cannot use it for fast confirmation", "fastConfirmer", fastConfirmer, "wallet", wallet.AddressOrZero()) } } inactiveValidatedNodes := btree.NewG(2, func(a, b validatedNode) bool { From 8530c658929860e146cb17cd5c540d832ee25e9a Mon Sep 17 00:00:00 2001 From: Aman Sanghi Date: Thu, 8 Aug 2024 21:04:00 +0530 Subject: [PATCH 6/6] Changes based on PR comments --- staker/staker.go | 55 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/staker/staker.go b/staker/staker.go index 9203156fc1..109f0ea7be 100644 --- a/staker/staker.go +++ b/staker/staker.go @@ -328,27 +328,46 @@ func NewStaker( } // Only use gnosis safe fast confirmation, if the safe address is different from the wallet address, else it's not a safe contract. if fastConfirmer != (common.Address{}) && config.EnableFastConfirmation && wallet.AddressOrZero() != (common.Address{}) && wallet.AddressOrZero() != fastConfirmer { - fastConfirmSafe, err = NewFastConfirmSafe( - callOpts, - fastConfirmer, - val.builder, - wallet, - config.gasRefunder, - l1Reader, - ) - if err != nil { - return nil, err - } - isOwner, err := fastConfirmSafe.safe.IsOwner(&callOpts, wallet.AddressOrZero()) + codeAt, err := client.CodeAt(context.Background(), fastConfirmer, nil) if err != nil { - return nil, err + return nil, fmt.Errorf("getting code at fast confirmer address: %w", err) } - if !isOwner { - // If the wallet is not an owner of the safe, we can't use it for fast confirmation - // So disable fast confirmation. + if len(codeAt) == 0 { + // The fast confirmer address is an EOA address, but it does not match the wallet address so cannot enable fast confirmation. fastConfirmer = common.Address{} - fastConfirmSafe = nil - log.Info("Staker wallet address is not part of owners of safe so cannot use it for fast confirmation", "fastConfirmer", fastConfirmer, "wallet", wallet.AddressOrZero()) + log.Info("Fast confirmer address is an EOA address which does not match the wallet address so cannot enable fast confirmation", "fastConfirmer", fastConfirmer, "wallet", wallet.AddressOrZero()) + } else { + // The fast confirmer address is a contract address, not sure if it's a safe contract yet. + fastConfirmSafe, err = NewFastConfirmSafe( + callOpts, + fastConfirmer, + val.builder, + wallet, + config.gasRefunder, + l1Reader, + ) + if err != nil && headerreader.ExecutionRevertedRegexp.MatchString(err.Error()) { + // If the safe is not a safe contract, we can't use it for fast confirmation + fastConfirmer = common.Address{} + fastConfirmSafe = nil + log.Warn("Fast confirmer address is not a safe contract so cannot enable fast confirmation", "fastConfirmer", fastConfirmer, "wallet", wallet.AddressOrZero()) + } else if err != nil { + // Unknown while loading the safe contract. + return nil, fmt.Errorf("loading fast confirm safe: %w", err) + } else { + // Fast confirmer address is a safe contract. + isOwner, err := fastConfirmSafe.safe.IsOwner(&callOpts, wallet.AddressOrZero()) + if err != nil { + return nil, fmt.Errorf("checking if wallet is owner of safe: %w", err) + } + if !isOwner { + // If the wallet is not an owner of the safe, we can't use it for fast confirmation + // So disable fast confirmation. + fastConfirmer = common.Address{} + fastConfirmSafe = nil + log.Info("Staker wallet address is not part of owners of safe so cannot use it for fast confirmation", "fastConfirmer", fastConfirmer, "wallet", wallet.AddressOrZero()) + } + } } } inactiveValidatedNodes := btree.NewG(2, func(a, b validatedNode) bool {