op-e2e: Always run subtests on the parent executor. (#9593)

* op-e2e: Always run subtests on the parent executor.

* op-e2e: Fix TestMixedWithdrawalValidity by not cancelling its own context.

Moves timeouts to be handled in helpers to reduce complexity of the test.

* op-e2e: Fix TestMixedWithdrawalValidity for fault proofs

* op-e2e: Evaluate test options for subtests

Ensures that tests are skipped correctly when using conditional options like UsesCannon even if not specified in the parent case.

Add InitParallel back to cannon tests so they can execute in parallel.

op-e2e/e2eutils/challenger/helper.go

@@ -66,6 +66,25 @@ func WithPollInterval(pollInterval time.Duration) Option {
 	}
 }
 
+// findMonorepoRoot finds the relative path to the monorepo root
+// Different tests might be nested in subdirectories of the op-e2e dir.
+func findMonorepoRoot(t *testing.T) string {
+	path := "./"
+	// Only search up 5 directories
+	// Avoids infinite recursion if the root isn't found for some reason
+	for i := 0; i < 5; i++ {
+		_, err := os.Stat(path + "op-e2e")
+		if errors.Is(err, os.ErrNotExist) {
+			path = path + "../"
+			continue
+		}
+		require.NoErrorf(t, err, "Failed to stat %v even though it existed", path)
+		return path
+	}
+	t.Fatalf("Could not find monorepo root, trying up to %v", path)
+	return ""
+}
+
 func applyCannonConfig(
 	c *config.Config,
 	t *testing.T,
@@ -75,9 +94,10 @@ func applyCannonConfig(
 ) {
 	require := require.New(t)
 	c.CannonL2 = l2Endpoint
-	c.CannonBin = "../../cannon/bin/cannon"
-	c.CannonServer = "../../op-program/bin/op-program"
-	c.CannonAbsolutePreState = "../../op-program/bin/prestate.json"
+	root := findMonorepoRoot(t)
+	c.CannonBin = root + "cannon/bin/cannon"
+	c.CannonServer = root + "op-program/bin/op-program"
+	c.CannonAbsolutePreState = root + "op-program/bin/prestate.json"
 	c.CannonSnapshotFreq = 10_000_000
 
 	genesisBytes, err := json.Marshal(l2Genesis)

op-e2e/e2eutils/wait/withdrawals.go

@@ -13,13 +13,14 @@ import (
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/ethclient"
-	"github.com/ethereum/go-ethereum/log"
 )
 
 // ForOutputRootPublished waits until there is an output published for an L2 block number larger than the supplied l2BlockNumber
 // This function polls and can block for a very long time if used on mainnet.
 // This returns the block number to use for proof generation.
 func ForOutputRootPublished(ctx context.Context, client *ethclient.Client, l2OutputOracleAddr common.Address, l2BlockNumber *big.Int) (uint64, error) {
+	ctx, cancel := context.WithTimeout(ctx, 2*time.Minute)
+	defer cancel()
 	l2BlockNumber = new(big.Int).Set(l2BlockNumber) // Don't clobber caller owned l2BlockNumber
 	opts := &bind.CallOpts{Context: ctx}
 
@@ -76,6 +77,8 @@ func ForFinalizationPeriod(ctx context.Context, client *ethclient.Client, l1Prov
 
 // ForGamePublished waits until a game is published on L1 for the given l2BlockNumber.
 func ForGamePublished(ctx context.Context, client *ethclient.Client, optimismPortalAddr common.Address, disputeGameFactoryAddr common.Address, l2BlockNumber *big.Int) (uint64, error) {
+	ctx, cancel := context.WithTimeout(ctx, 2*time.Minute)
+	defer cancel()
 	l2BlockNumber = new(big.Int).Set(l2BlockNumber) // Don't clobber caller owned l2BlockNumber
 
 	optimismPortal2Contract, err := bindingspreview.NewOptimismPortal2Caller(optimismPortalAddr, client)
@@ -108,6 +111,8 @@ func ForGamePublished(ctx context.Context, client *ethclient.Client, optimismPor
 
 // ForWithdrawalCheck waits until the withdrawal check in the portal succeeds.
 func ForWithdrawalCheck(ctx context.Context, client *ethclient.Client, withdrawal crossdomain.Withdrawal, optimismPortalAddr common.Address) error {
+	ctx, cancel := context.WithTimeout(ctx, 2*time.Minute)
+	defer cancel()
 	opts := &bind.CallOpts{Context: ctx}
 	portal, err := bindingspreview.NewOptimismPortal2Caller(optimismPortalAddr, client)
 	if err != nil {
@@ -115,14 +120,12 @@ func ForWithdrawalCheck(ctx context.Context, client *ethclient.Client, withdrawa
 	}
 
 	return For(ctx, time.Second, func() (bool, error) {
-		log.Warn("checking withdrawal!")
 		wdHash, err := withdrawal.Hash()
 		if err != nil {
 			return false, fmt.Errorf("hash withdrawal: %w", err)
 		}
 
 		err = portal.CheckWithdrawal(opts, wdHash)
-		log.Warn("checking withdrawal", "hash", wdHash, "err", err)
 		return err == nil, nil
 	})
 }

op-e2e/faultproofs/output_cannon_test.go

@@ -101,6 +101,7 @@ func TestOutputCannon_ChallengeAllZeroClaim(t *testing.T) {
 }
 
 func TestOutputCannon_PublishCannonRootClaim(t *testing.T) {
+	op_e2e.InitParallel(t, op_e2e.UsesCannon)
 	tests := []struct {
 		disputeL2BlockNumber uint64
 	}{
@@ -129,6 +130,7 @@ func TestOutputCannon_PublishCannonRootClaim(t *testing.T) {
 }
 
 func TestOutputCannonDisputeGame(t *testing.T) {
+	op_e2e.InitParallel(t, op_e2e.UsesCannon)
 	tests := []struct {
 		name             string
 		defendClaimDepth types.Depth
@@ -254,6 +256,7 @@ func TestOutputCannonStepWithLargePreimage(t *testing.T) {
 }
 
 func TestOutputCannonStepWithPreimage(t *testing.T) {
+	op_e2e.InitParallel(t, op_e2e.UsesCannon)
 	testPreimageStep := func(t *testing.T, preimageType cannon.PreimageOpt, preloadPreimage bool) {
 		op_e2e.InitParallel(t, op_e2e.UsesCannon)
 
@@ -296,6 +299,7 @@ func TestOutputCannonStepWithPreimage(t *testing.T) {
 
 func TestOutputCannonStepWithKZGPointEvaluation(t *testing.T) {
 	t.Skip("TODO: Fix flaky test")
+	op_e2e.InitParallel(t, op_e2e.UsesCannon)
 
 	testPreimageStep := func(t *testing.T, preloadPreimage bool) {
 		op_e2e.InitParallel(t, op_e2e.UsesCannon)
@@ -337,6 +341,7 @@ func TestOutputCannonStepWithKZGPointEvaluation(t *testing.T) {
 }
 
 func TestOutputCannonProposedOutputRootValid(t *testing.T) {
+	op_e2e.InitParallel(t, op_e2e.UsesCannon)
 	// honestStepsFail attempts to perform both an attack and defend step using the correct trace.
 	honestStepsFail := func(ctx context.Context, game *disputegame.OutputCannonGameHelper, correctTrace *disputegame.OutputHonestHelper, parentClaimIdx int64) {
 		// Attack step should fail

op-e2e/faultproofs/precompile_test.go

@@ -27,6 +27,7 @@ import (
 )
 
 func TestPrecompiles(t *testing.T) {
+	op_e2e.InitParallel(t, op_e2e.UsesCannon)
 	// precompile test vectors copied from go-ethereum
 	tests := []struct {
 		name    string

op-e2e/helper.go

@@ -4,65 +4,61 @@ import (
 	"crypto/md5"
 	"os"
 	"strconv"
+	"strings"
 
 	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils"
 )
 
 var enableParallelTesting bool = os.Getenv("OP_E2E_DISABLE_PARALLEL") != "true"
 
-type testopts struct {
-	executor uint64
-}
-
-func InitParallel(t e2eutils.TestingBase, args ...func(t e2eutils.TestingBase, opts *testopts)) {
+func InitParallel(t e2eutils.TestingBase, args ...func(t e2eutils.TestingBase)) {
 	t.Helper()
 	if enableParallelTesting {
 		t.Parallel()
 	}
+	for _, arg := range args {
+		arg(t)
+	}
+	autoAllocateExecutor(t)
+}
+
+// isSubTest determines if the test is a sub-test or top level test.
+// It does this by checking if the test name contains /
+// This is not a particularly great way check, but appears to be the only option currently.
+func isSubTest(t e2eutils.TestingBase) bool {
+	return strings.Contains(t.Name(), "/")
+}
 
+func autoAllocateExecutor(t e2eutils.TestingBase) {
+	if isSubTest(t) {
+		// Always run subtests, they only start on the same executor as their parent.
+		return
+	}
 	info := getExecutorInfo(t)
 	tName := t.Name()
 	tHash := md5.Sum([]byte(tName))
 	executor := uint64(tHash[0]) % info.total
-	opts := &testopts{
-		executor: executor,
-	}
-	for _, arg := range args {
-		arg(t, opts)
-	}
-	checkExecutor(t, info, opts.executor)
+	checkExecutor(t, info, executor)
 }
 
-func UsesCannon(t e2eutils.TestingBase, opts *testopts) {
+func UsesCannon(t e2eutils.TestingBase) {
 	if os.Getenv("OP_E2E_CANNON_ENABLED") == "false" {
 		t.Skip("Skipping cannon test")
 	}
 }
 
-func SkipOnFPAC(t e2eutils.TestingBase, opts *testopts) {
+func SkipOnFPAC(t e2eutils.TestingBase) {
 	if e2eutils.UseFPAC() {
 		t.Skip("Skipping test for FPAC")
 	}
 }
 
-func SkipOnNotFPAC(t e2eutils.TestingBase, opts *testopts) {
+func SkipOnNotFPAC(t e2eutils.TestingBase) {
 	if !e2eutils.UseFPAC() {
 		t.Skip("Skipping test for non-FPAC")
 	}
 }
 
-//	UseExecutor allows manually splitting tests between circleci executors
-//
-// Tests default to run on the first executor but can be moved to the second with:
-// InitParallel(t, UseExecutor(1))
-// Any tests assigned to an executor greater than the number available automatically use the last executor.
-// Executor indexes start from 0
-func UseExecutor(assignedIdx uint64) func(t e2eutils.TestingBase, opts *testopts) {
-	return func(t e2eutils.TestingBase, opts *testopts) {
-		opts.executor = assignedIdx
-	}
-}
-
 type executorInfo struct {
 	total      uint64
 	idx        uint64

op-e2e/system_tob_test.go

@@ -15,6 +15,8 @@ import (
 	"github.com/ethereum-optimism/optimism/op-bindings/predeploys"
 	"github.com/ethereum-optimism/optimism/op-chain-ops/crossdomain"
 	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils"
+	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/challenger"
+	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/disputegame"
 	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/geth"
 	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/wait"
 	"github.com/ethereum-optimism/optimism/op-node/withdrawals"
@@ -546,7 +548,6 @@ func TestMixedWithdrawalValidity(t *testing.T) {
 			transactor.ExpectedL2Nonce = transactor.ExpectedL2Nonce + 1
 
 			// Wait for the finalization period, then we can finalize this withdrawal.
-			ctx, withdrawalCancel := context.WithTimeout(context.Background(), 60*time.Duration(cfg.DeployConfig.L1BlockTime)*time.Second)
 			require.NotEqual(t, cfg.L1Deployments.L2OutputOracleProxy, common.Address{})
 			var blockNumber uint64
 			if e2eutils.UseFPAC() {
@@ -554,12 +555,9 @@ func TestMixedWithdrawalValidity(t *testing.T) {
 			} else {
 				blockNumber, err = wait.ForOutputRootPublished(ctx, l1Client, cfg.L1Deployments.L2OutputOracleProxy, receipt.BlockNumber)
 			}
-			withdrawalCancel()
 			require.Nil(t, err)
 
-			ctx, txCancel = context.WithTimeout(context.Background(), txTimeoutDuration)
 			header, err = l2Verif.HeaderByNumber(ctx, new(big.Int).SetUint64(blockNumber))
-			txCancel()
 			require.Nil(t, err)
 
 			rpcClient, err := rpc.Dial(sys.EthInstances["verifier"].WSEndpoint())
@@ -658,14 +656,19 @@ func TestMixedWithdrawalValidity(t *testing.T) {
 			} else {
 				require.NoError(t, err)
 
+				if e2eutils.UseFPAC() {
+					// Start a challenger to resolve claims and games once the clock expires
+					factoryHelper := disputegame.NewFactoryHelper(t, ctx, sys)
+					factoryHelper.StartChallenger(ctx, "Challenger",
+						challenger.WithCannon(t, sys.RollupConfig, sys.L2GenesisCfg, sys.RollupEndpoint("sequencer"), sys.NodeEndpoint("sequencer")),
+						challenger.WithPrivKey(sys.Cfg.Secrets.Mallory))
+				}
 				receipt, err = wait.ForReceiptOK(ctx, l1Client, tx.Hash())
-				require.Nil(t, err, "finalize withdrawal")
+				require.NoError(t, err, "finalize withdrawal")
 
 				// Verify balance after withdrawal
-				ctx, txCancel = context.WithTimeout(context.Background(), txTimeoutDuration)
 				header, err = l1Client.HeaderByNumber(ctx, receipt.BlockNumber)
-				txCancel()
-				require.Nil(t, err)
+				require.NoError(t, err)
 
 				// Ensure that withdrawal - gas fees are added to the L1 balance
 				// Fun fact, the fee is greater than the withdrawal amount
@@ -676,17 +679,17 @@ func TestMixedWithdrawalValidity(t *testing.T) {
 
 				// Ensure that our withdrawal was proved successfully
 				_, err := wait.ForReceiptOK(ctx, l1Client, tx.Hash())
-				require.Nil(t, err, "prove withdrawal")
+				require.NoError(t, err, "prove withdrawal")
 
 				// Wait for finalization and then create the Finalized Withdrawal Transaction
 				ctx, withdrawalCancel := context.WithTimeout(context.Background(), 60*time.Duration(cfg.DeployConfig.L1BlockTime)*time.Second)
 				defer withdrawalCancel()
 				if e2eutils.UseFPAC() {
 					err = wait.ForWithdrawalCheck(ctx, l1Client, withdrawal, cfg.L1Deployments.OptimismPortalProxy)
-					require.Nil(t, err)
+					require.NoError(t, err)
 				} else {
 					err = wait.ForFinalizationPeriod(ctx, l1Client, header.Number, cfg.L1Deployments.L2OutputOracleProxy)
-					require.Nil(t, err)
+					require.NoError(t, err)
 				}
 
 				// Finalize withdrawal
@@ -700,39 +703,27 @@ func TestMixedWithdrawalValidity(t *testing.T) {
 			// At the end, assert our account balance/nonce states.
 
 			// Obtain the L2 sequencer account balance
-			ctx, txCancel = context.WithTimeout(context.Background(), txTimeoutDuration)
 			endL1Balance, err := l1Client.BalanceAt(ctx, transactor.Account.L1Opts.From, nil)
-			txCancel()
 			require.NoError(t, err)
 
 			// Obtain the L1 account nonce
-			ctx, txCancel = context.WithTimeout(context.Background(), txTimeoutDuration)
 			endL1Nonce, err := l1Client.NonceAt(ctx, transactor.Account.L1Opts.From, nil)
-			txCancel()
 			require.NoError(t, err)
 
 			// Obtain the L2 sequencer account balance
-			ctx, txCancel = context.WithTimeout(context.Background(), txTimeoutDuration)
 			endL2SeqBalance, err := l2Seq.BalanceAt(ctx, transactor.Account.L1Opts.From, nil)
-			txCancel()
 			require.NoError(t, err)
 
 			// Obtain the L2 sequencer account nonce
-			ctx, txCancel = context.WithTimeout(context.Background(), txTimeoutDuration)
 			endL2SeqNonce, err := l2Seq.NonceAt(ctx, transactor.Account.L1Opts.From, nil)
-			txCancel()
 			require.NoError(t, err)
 
 			// Obtain the L2 verifier account balance
-			ctx, txCancel = context.WithTimeout(context.Background(), txTimeoutDuration)
 			endL2VerifBalance, err := l2Verif.BalanceAt(ctx, transactor.Account.L1Opts.From, nil)
-			txCancel()
 			require.NoError(t, err)
 
 			// Obtain the L2 verifier account nonce
-			ctx, txCancel = context.WithTimeout(context.Background(), txTimeoutDuration)
 			endL2VerifNonce, err := l2Verif.NonceAt(ctx, transactor.Account.L1Opts.From, nil)
-			txCancel()
 			require.NoError(t, err)
 
 			// TODO: Check L1 balance as well here. We avoided this due to time constraints as it seems L1 fees