From 4be9bbb019fde7e6103304f266a85e0622f3b7b8 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Fri, 1 Sep 2023 13:24:48 -0500 Subject: [PATCH 1/4] remove precompilesgen dependency from headerreader --- arbnode/node.go | 7 +++++-- cmd/daserver/daserver.go | 5 ++++- cmd/nitro/nitro.go | 5 ++++- system_tests/das_test.go | 4 +++- util/headerreader/header_reader.go | 17 +++++++++-------- 5 files changed, 25 insertions(+), 13 deletions(-) diff --git a/arbnode/node.go b/arbnode/node.go index e6960a3f22..e3e9223b1d 100644 --- a/arbnode/node.go +++ b/arbnode/node.go @@ -40,6 +40,7 @@ import ( "github.com/offchainlabs/nitro/solgen/go/bridgegen" "github.com/offchainlabs/nitro/solgen/go/challengegen" "github.com/offchainlabs/nitro/solgen/go/ospgen" + "github.com/offchainlabs/nitro/solgen/go/precompilesgen" "github.com/offchainlabs/nitro/solgen/go/rollupgen" "github.com/offchainlabs/nitro/staker" "github.com/offchainlabs/nitro/util/contracts" @@ -235,7 +236,8 @@ func GenerateRollupConfig(prod bool, wasmModuleRoot common.Hash, rollupOwner com } func DeployOnL1(ctx context.Context, l1client arbutil.L1Interface, deployAuth *bind.TransactOpts, batchPoster common.Address, authorizeValidators uint64, readerConfig headerreader.ConfigFetcher, config rollupgen.Config) (*chaininfo.RollupAddresses, error) { - l1Reader, err := headerreader.New(ctx, l1client, readerConfig) + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) + l1Reader, err := headerreader.New(ctx, l1client, readerConfig, arbSys) if err != nil { return nil, err } @@ -611,7 +613,8 @@ func createNodeImpl( var l1Reader *headerreader.HeaderReader if config.ParentChainReader.Enable { - l1Reader, err = headerreader.New(ctx, l1client, func() *headerreader.Config { return &configFetcher.Get().ParentChainReader }) + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) + l1Reader, err = headerreader.New(ctx, l1client, func() *headerreader.Config { return &configFetcher.Get().ParentChainReader }, arbSys) if err != nil { return nil, err } diff --git a/cmd/daserver/daserver.go b/cmd/daserver/daserver.go index 7cdfc39915..335aba6a1b 100644 --- a/cmd/daserver/daserver.go +++ b/cmd/daserver/daserver.go @@ -17,6 +17,7 @@ import ( flag "github.com/spf13/pflag" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/metrics/exp" @@ -24,6 +25,7 @@ import ( "github.com/offchainlabs/nitro/cmd/genericconf" "github.com/offchainlabs/nitro/cmd/util/confighelpers" "github.com/offchainlabs/nitro/das" + "github.com/offchainlabs/nitro/solgen/go/precompilesgen" "github.com/offchainlabs/nitro/util/headerreader" ) @@ -196,7 +198,8 @@ func startup() error { if err != nil { return err } - l1Reader, err = headerreader.New(ctx, l1Client, func() *headerreader.Config { return &headerreader.DefaultConfig }) // TODO: config + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1Client) + l1Reader, err = headerreader.New(ctx, l1Client, func() *headerreader.Config { return &headerreader.DefaultConfig }, arbSys) // TODO: config if err != nil { return err } diff --git a/cmd/nitro/nitro.go b/cmd/nitro/nitro.go index 407ed0afe7..dd26fea46f 100644 --- a/cmd/nitro/nitro.go +++ b/cmd/nitro/nitro.go @@ -28,6 +28,7 @@ import ( "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/arbitrum" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" _ "github.com/ethereum/go-ethereum/eth/tracers/js" _ "github.com/ethereum/go-ethereum/eth/tracers/native" @@ -48,6 +49,7 @@ import ( "github.com/offchainlabs/nitro/cmd/util" "github.com/offchainlabs/nitro/cmd/util/confighelpers" _ "github.com/offchainlabs/nitro/nodeInterface" + "github.com/offchainlabs/nitro/solgen/go/precompilesgen" "github.com/offchainlabs/nitro/staker" "github.com/offchainlabs/nitro/util/colors" "github.com/offchainlabs/nitro/util/headerreader" @@ -354,7 +356,8 @@ func mainImpl() int { flag.Usage() log.Crit("--node.validator.only-create-wallet-contract requires --node.validator.use-smart-contract-wallet") } - l1Reader, err := headerreader.New(ctx, l1Client, func() *headerreader.Config { return &liveNodeConfig.Get().Node.ParentChainReader }) + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1Client) + l1Reader, err := headerreader.New(ctx, l1Client, func() *headerreader.Config { return &liveNodeConfig.Get().Node.ParentChainReader }, arbSys) if err != nil { log.Crit("failed to get L1 headerreader", "error", err) diff --git a/system_tests/das_test.go b/system_tests/das_test.go index 7952120933..8889d2d53d 100644 --- a/system_tests/das_test.go +++ b/system_tests/das_test.go @@ -28,6 +28,7 @@ import ( "github.com/offchainlabs/nitro/cmd/genericconf" "github.com/offchainlabs/nitro/das" "github.com/offchainlabs/nitro/solgen/go/bridgegen" + "github.com/offchainlabs/nitro/solgen/go/precompilesgen" "github.com/offchainlabs/nitro/util/headerreader" "github.com/offchainlabs/nitro/util/signature" ) @@ -233,7 +234,8 @@ func TestDASComplexConfigAndRestMirror(t *testing.T) { chainConfig := params.ArbitrumDevTestDASChainConfig() l1info, l1client, _, l1stack := createTestL1BlockChain(t, nil) defer requireClose(t, l1stack) - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }) + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys) Require(t, err) l1Reader.Start(ctx) defer l1Reader.StopAndWait() diff --git a/util/headerreader/header_reader.go b/util/headerreader/header_reader.go index e5807224c0..8487ccd54b 100644 --- a/util/headerreader/header_reader.go +++ b/util/headerreader/header_reader.go @@ -18,17 +18,20 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rpc" "github.com/offchainlabs/nitro/arbutil" - "github.com/offchainlabs/nitro/solgen/go/precompilesgen" "github.com/offchainlabs/nitro/util/stopwaiter" flag "github.com/spf13/pflag" ) +type ArbSysInterface interface { + ArbBlockNumber(*bind.CallOpts) (*big.Int, error) +} + type HeaderReader struct { stopwaiter.StopWaiter config ConfigFetcher client arbutil.L1Interface isParentChainArbitrum bool - arbSys *precompilesgen.ArbSys + arbSys ArbSysInterface chanMutex sync.RWMutex // All fields below require the chanMutex @@ -91,25 +94,23 @@ var TestConfig = Config{ UseFinalityData: false, } -func New(ctx context.Context, client arbutil.L1Interface, config ConfigFetcher) (*HeaderReader, error) { +func New(ctx context.Context, client arbutil.L1Interface, config ConfigFetcher, arbSysPrecompile ArbSysInterface) (*HeaderReader, error) { isParentChainArbitrum := false - var arbSys *precompilesgen.ArbSys codeAt, err := client.CodeAt(ctx, types.ArbSysAddress, nil) if err != nil { return nil, err } if len(codeAt) != 0 { isParentChainArbitrum = true - arbSys, err = precompilesgen.NewArbSys(types.ArbSysAddress, client) - if err != nil { - return nil, err + if arbSysPrecompile == nil { + return nil, errors.New("unable to create ArbSys from precompilesgen") } } return &HeaderReader{ client: client, config: config, isParentChainArbitrum: isParentChainArbitrum, - arbSys: arbSys, + arbSys: arbSysPrecompile, outChannels: make(map[chan<- *types.Header]struct{}), outChannelsBehind: make(map[chan<- *types.Header]struct{}), safe: cachedHeader{rpcBlockNum: big.NewInt(rpc.SafeBlockNumber.Int64())}, From 57f25acccc14d0633502136bacc4e637dc89ea02 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Fri, 1 Sep 2023 14:23:19 -0500 Subject: [PATCH 2/4] modify implementation to handle CodeAt errors in lb --- arbnode/node.go | 4 ++-- cmd/daserver/daserver.go | 2 +- cmd/nitro/nitro.go | 2 +- system_tests/das_test.go | 2 +- util/headerreader/header_reader.go | 20 +++++++++++--------- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/arbnode/node.go b/arbnode/node.go index e3e9223b1d..7fde929771 100644 --- a/arbnode/node.go +++ b/arbnode/node.go @@ -237,7 +237,7 @@ func GenerateRollupConfig(prod bool, wasmModuleRoot common.Hash, rollupOwner com func DeployOnL1(ctx context.Context, l1client arbutil.L1Interface, deployAuth *bind.TransactOpts, batchPoster common.Address, authorizeValidators uint64, readerConfig headerreader.ConfigFetcher, config rollupgen.Config) (*chaininfo.RollupAddresses, error) { arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err := headerreader.New(ctx, l1client, readerConfig, arbSys) + l1Reader, err := headerreader.New(ctx, l1client, readerConfig, arbSys, true) if err != nil { return nil, err } @@ -614,7 +614,7 @@ func createNodeImpl( var l1Reader *headerreader.HeaderReader if config.ParentChainReader.Enable { arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err = headerreader.New(ctx, l1client, func() *headerreader.Config { return &configFetcher.Get().ParentChainReader }, arbSys) + l1Reader, err = headerreader.New(ctx, l1client, func() *headerreader.Config { return &configFetcher.Get().ParentChainReader }, arbSys, true) if err != nil { return nil, err } diff --git a/cmd/daserver/daserver.go b/cmd/daserver/daserver.go index 335aba6a1b..6b874f4639 100644 --- a/cmd/daserver/daserver.go +++ b/cmd/daserver/daserver.go @@ -199,7 +199,7 @@ func startup() error { return err } arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1Client) - l1Reader, err = headerreader.New(ctx, l1Client, func() *headerreader.Config { return &headerreader.DefaultConfig }, arbSys) // TODO: config + l1Reader, err = headerreader.New(ctx, l1Client, func() *headerreader.Config { return &headerreader.DefaultConfig }, arbSys, true) // TODO: config if err != nil { return err } diff --git a/cmd/nitro/nitro.go b/cmd/nitro/nitro.go index dd26fea46f..e404733b1e 100644 --- a/cmd/nitro/nitro.go +++ b/cmd/nitro/nitro.go @@ -357,7 +357,7 @@ func mainImpl() int { log.Crit("--node.validator.only-create-wallet-contract requires --node.validator.use-smart-contract-wallet") } arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1Client) - l1Reader, err := headerreader.New(ctx, l1Client, func() *headerreader.Config { return &liveNodeConfig.Get().Node.ParentChainReader }, arbSys) + l1Reader, err := headerreader.New(ctx, l1Client, func() *headerreader.Config { return &liveNodeConfig.Get().Node.ParentChainReader }, arbSys, true) if err != nil { log.Crit("failed to get L1 headerreader", "error", err) diff --git a/system_tests/das_test.go b/system_tests/das_test.go index 8889d2d53d..e79b993d03 100644 --- a/system_tests/das_test.go +++ b/system_tests/das_test.go @@ -235,7 +235,7 @@ func TestDASComplexConfigAndRestMirror(t *testing.T) { l1info, l1client, _, l1stack := createTestL1BlockChain(t, nil) defer requireClose(t, l1stack) arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys, true) Require(t, err) l1Reader.Start(ctx) defer l1Reader.StopAndWait() diff --git a/util/headerreader/header_reader.go b/util/headerreader/header_reader.go index 8487ccd54b..c7fa937385 100644 --- a/util/headerreader/header_reader.go +++ b/util/headerreader/header_reader.go @@ -94,16 +94,18 @@ var TestConfig = Config{ UseFinalityData: false, } -func New(ctx context.Context, client arbutil.L1Interface, config ConfigFetcher, arbSysPrecompile ArbSysInterface) (*HeaderReader, error) { +func New(ctx context.Context, client arbutil.L1Interface, config ConfigFetcher, arbSysPrecompile ArbSysInterface, usePrecompilesgen bool) (*HeaderReader, error) { isParentChainArbitrum := false - codeAt, err := client.CodeAt(ctx, types.ArbSysAddress, nil) - if err != nil { - return nil, err - } - if len(codeAt) != 0 { - isParentChainArbitrum = true - if arbSysPrecompile == nil { - return nil, errors.New("unable to create ArbSys from precompilesgen") + if usePrecompilesgen { + codeAt, err := client.CodeAt(ctx, types.ArbSysAddress, nil) + if err != nil { + return nil, err + } + if len(codeAt) != 0 { + isParentChainArbitrum = true + if arbSysPrecompile == nil { + return nil, errors.New("unable to create ArbSys from precompilesgen") + } } } return &HeaderReader{ From bf0c01a1e98a846801288d1893f3a171ff96cb35 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Wed, 13 Sep 2023 15:27:04 -0500 Subject: [PATCH 3/4] fix failing test --- cmd/deploy/deploy.go | 5 ++++- system_tests/common_test.go | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/deploy/deploy.go b/cmd/deploy/deploy.go index 17725a7a4c..3e698749d5 100644 --- a/cmd/deploy/deploy.go +++ b/cmd/deploy/deploy.go @@ -14,10 +14,12 @@ import ( "github.com/offchainlabs/nitro/cmd/chaininfo" "github.com/offchainlabs/nitro/cmd/genericconf" + "github.com/offchainlabs/nitro/solgen/go/precompilesgen" "github.com/offchainlabs/nitro/util/headerreader" "github.com/offchainlabs/nitro/validator/server_common" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/ethclient" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/params" @@ -127,7 +129,8 @@ func main() { panic(fmt.Errorf("failed to deserialize chain config: %w", err)) } - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerReaderConfig }) + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerReaderConfig }, arbSys, true) if err != nil { panic(fmt.Errorf("failed to create header reader: %w", err)) } diff --git a/system_tests/common_test.go b/system_tests/common_test.go index b92fbf7578..a643b1b719 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -476,7 +476,8 @@ func DeployOnTestL1( serializedChainConfig, err := json.Marshal(chainConfig) Require(t, err) - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }) + arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys, true) Require(t, err) l1Reader.Start(ctx) defer l1Reader.StopAndWait() From 6a9fd710cd6611ec6adcdadc56f8307afe13ecce Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Wed, 13 Sep 2023 19:42:33 -0500 Subject: [PATCH 4/4] revert impl --- arbnode/node.go | 2 +- cmd/daserver/daserver.go | 2 +- cmd/deploy/deploy.go | 2 +- cmd/nitro/nitro.go | 2 +- system_tests/common_test.go | 2 +- system_tests/das_test.go | 2 +- util/headerreader/header_reader.go | 11 +++++------ 7 files changed, 11 insertions(+), 12 deletions(-) diff --git a/arbnode/node.go b/arbnode/node.go index a40c84fa94..5bdc716264 100644 --- a/arbnode/node.go +++ b/arbnode/node.go @@ -606,7 +606,7 @@ func createNodeImpl( var l1Reader *headerreader.HeaderReader if config.ParentChainReader.Enable { arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err = headerreader.New(ctx, l1client, func() *headerreader.Config { return &configFetcher.Get().ParentChainReader }, arbSys, true) + l1Reader, err = headerreader.New(ctx, l1client, func() *headerreader.Config { return &configFetcher.Get().ParentChainReader }, arbSys) if err != nil { return nil, err } diff --git a/cmd/daserver/daserver.go b/cmd/daserver/daserver.go index 6b874f4639..335aba6a1b 100644 --- a/cmd/daserver/daserver.go +++ b/cmd/daserver/daserver.go @@ -199,7 +199,7 @@ func startup() error { return err } arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1Client) - l1Reader, err = headerreader.New(ctx, l1Client, func() *headerreader.Config { return &headerreader.DefaultConfig }, arbSys, true) // TODO: config + l1Reader, err = headerreader.New(ctx, l1Client, func() *headerreader.Config { return &headerreader.DefaultConfig }, arbSys) // TODO: config if err != nil { return err } diff --git a/cmd/deploy/deploy.go b/cmd/deploy/deploy.go index 3e698749d5..d687821e8b 100644 --- a/cmd/deploy/deploy.go +++ b/cmd/deploy/deploy.go @@ -130,7 +130,7 @@ func main() { } arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerReaderConfig }, arbSys, true) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerReaderConfig }, arbSys) if err != nil { panic(fmt.Errorf("failed to create header reader: %w", err)) } diff --git a/cmd/nitro/nitro.go b/cmd/nitro/nitro.go index f2cb3a37d2..a7dc7f26f9 100644 --- a/cmd/nitro/nitro.go +++ b/cmd/nitro/nitro.go @@ -357,7 +357,7 @@ func mainImpl() int { log.Crit("--node.validator.only-create-wallet-contract requires --node.validator.use-smart-contract-wallet") } arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1Client) - l1Reader, err := headerreader.New(ctx, l1Client, func() *headerreader.Config { return &liveNodeConfig.Get().Node.ParentChainReader }, arbSys, true) + l1Reader, err := headerreader.New(ctx, l1Client, func() *headerreader.Config { return &liveNodeConfig.Get().Node.ParentChainReader }, arbSys) if err != nil { log.Crit("failed to get L1 headerreader", "error", err) } diff --git a/system_tests/common_test.go b/system_tests/common_test.go index a643b1b719..9fd002bd94 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -477,7 +477,7 @@ func DeployOnTestL1( Require(t, err) arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys, true) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys) Require(t, err) l1Reader.Start(ctx) defer l1Reader.StopAndWait() diff --git a/system_tests/das_test.go b/system_tests/das_test.go index e79b993d03..8889d2d53d 100644 --- a/system_tests/das_test.go +++ b/system_tests/das_test.go @@ -235,7 +235,7 @@ func TestDASComplexConfigAndRestMirror(t *testing.T) { l1info, l1client, _, l1stack := createTestL1BlockChain(t, nil) defer requireClose(t, l1stack) arbSys, _ := precompilesgen.NewArbSys(types.ArbSysAddress, l1client) - l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys, true) + l1Reader, err := headerreader.New(ctx, l1client, func() *headerreader.Config { return &headerreader.TestConfig }, arbSys) Require(t, err) l1Reader.Start(ctx) defer l1Reader.StopAndWait() diff --git a/util/headerreader/header_reader.go b/util/headerreader/header_reader.go index c7fa937385..befd54ace3 100644 --- a/util/headerreader/header_reader.go +++ b/util/headerreader/header_reader.go @@ -94,25 +94,24 @@ var TestConfig = Config{ UseFinalityData: false, } -func New(ctx context.Context, client arbutil.L1Interface, config ConfigFetcher, arbSysPrecompile ArbSysInterface, usePrecompilesgen bool) (*HeaderReader, error) { +func New(ctx context.Context, client arbutil.L1Interface, config ConfigFetcher, arbSysPrecompile ArbSysInterface) (*HeaderReader, error) { isParentChainArbitrum := false - if usePrecompilesgen { + var arbSys ArbSysInterface + if arbSysPrecompile != nil { codeAt, err := client.CodeAt(ctx, types.ArbSysAddress, nil) if err != nil { return nil, err } if len(codeAt) != 0 { isParentChainArbitrum = true - if arbSysPrecompile == nil { - return nil, errors.New("unable to create ArbSys from precompilesgen") - } + arbSys = arbSysPrecompile } } return &HeaderReader{ client: client, config: config, isParentChainArbitrum: isParentChainArbitrum, - arbSys: arbSysPrecompile, + arbSys: arbSys, outChannels: make(map[chan<- *types.Header]struct{}), outChannelsBehind: make(map[chan<- *types.Header]struct{}), safe: cachedHeader{rpcBlockNum: big.NewInt(rpc.SafeBlockNumber.Int64())},