diff --git a/vms/saevm/sae/BUILD.bazel b/vms/saevm/sae/BUILD.bazel index 7622ac89b3c4..2e3d97e05cf5 100644 --- a/vms/saevm/sae/BUILD.bazel +++ b/vms/saevm/sae/BUILD.bazel @@ -2,6 +2,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") load("//.bazel:defs.bzl", "go_test") # gazelle:exclude accept_block_test.go +# gazelle:exclude worstcase_test.go go_library( name = "sae", @@ -77,7 +78,6 @@ go_test( "rpc_test.go", "tx_test.go", "vm_test.go", - "worstcase_test.go", ], embed = [":sae"], deps = [ @@ -104,7 +104,6 @@ go_test( "//vms/saevm/gastime", "//vms/saevm/hook", "//vms/saevm/hook/hookstest", - "//vms/saevm/intmath", "//vms/saevm/params", "//vms/saevm/sae/rpc", "//vms/saevm/saedb", @@ -112,7 +111,6 @@ go_test( "//vms/saevm/saetest/escrow", "//vms/saevm/txgossip/txgossiptest", "//vms/saevm/types", - "//vms/saevm/worstcase", "@com_github_arr4n_shed//testerr", "@com_github_ava_labs_libevm//:libevm", "@com_github_ava_labs_libevm//common", @@ -145,7 +143,7 @@ go_test( ) go_test( - name = "accept_block_test", + name = "flaky_tests", srcs = [ "accept_block_test.go", "always_test.go", @@ -160,9 +158,9 @@ go_test( "vm_test.go", "worstcase_test.go", ], - args = ["-test.run=TestAcceptBlock"], + args = ["-test.run=TestAcceptBlock|TestWorstCase"], embed = [":sae"], - env = {"SAEVM_TEST_ACCEPT_BLOCK": "1"}, + env = {"SAEVM_TEST_FLAKY": "1"}, flaky = True, deps = [ "//database", diff --git a/vms/saevm/sae/accept_block_test.go b/vms/saevm/sae/accept_block_test.go index 83b32c325c39..6d96c1c389a7 100644 --- a/vms/saevm/sae/accept_block_test.go +++ b/vms/saevm/sae/accept_block_test.go @@ -24,8 +24,8 @@ func TestAcceptBlock(t *testing.T) { // TODO(JonathanOppenheimer): determine whether this test is actually flaky // or whether there's a bug in the test. This test is enabled in Bazel and // disabled in go test. - if os.Getenv("SAEVM_TEST_ACCEPT_BLOCK") == "" { - t.Skip("FLAKY: set SAEVM_TEST_ACCEPT_BLOCK to run") + if os.Getenv("SAEVM_TEST_FLAKY") == "" { + t.Skip("FLAKY: set SAEVM_TEST_FLAKY to run") } // We use a generous timeout because GC finalizers from previous tests take diff --git a/vms/saevm/sae/rpc_test.go b/vms/saevm/sae/rpc_test.go index b616a0769cd4..7e2c87ac0369 100644 --- a/vms/saevm/sae/rpc_test.go +++ b/vms/saevm/sae/rpc_test.go @@ -789,27 +789,45 @@ func TestGetReceipts(t *testing.T) { timeOpt, vmTime := withVMTime(t, time.Unix(saeparams.TauSeconds, 0)) precompileOpt, unblock := withBlockingPrecompile(blockingPrecompile) - ctx, sut := newSUT(t, 1, timeOpt, precompileOpt) + ctx, sut := newSUT(t, 2, timeOpt, precompileOpt) t.Cleanup(unblock) var ( txs []*types.Transaction want []*types.Receipt ) - for range 6 { - tx := sut.wallet.SetNonceAndSign(t, 0, &types.LegacyTx{ + // The mempool cannot be relied on to mark a transaction as pending + // if there is already a pending transaction with the same account. + // To avoid this, we use two different accounts and price the + // transactions such that the builder will maintain ordering. + for range 3 { + tx1 := sut.wallet.SetNonceAndSign(t, 0, &types.LegacyTx{ To: &zeroAddr, Gas: params.TxGas, - GasPrice: big.NewInt(1), + GasPrice: big.NewInt(2), // ensure this tx is first in block }) - txs = append(txs, tx) - want = append(want, &types.Receipt{ - TxHash: tx.Hash(), - Status: types.ReceiptStatusSuccessful, - GasUsed: params.TxGas, - EffectiveGasPrice: big.NewInt(1), - Logs: []*types.Log{}, + tx2 := sut.wallet.SetNonceAndSign(t, 1, &types.LegacyTx{ + To: &zeroAddr, + Gas: params.TxGas, + GasPrice: big.NewInt(1), }) + txs = append(txs, tx1, tx2) + want = append(want, []*types.Receipt{ + { + TxHash: tx1.Hash(), + Status: types.ReceiptStatusSuccessful, + GasUsed: params.TxGas, + EffectiveGasPrice: big.NewInt(2), + Logs: []*types.Log{}, + }, + { + TxHash: tx2.Hash(), + Status: types.ReceiptStatusSuccessful, + GasUsed: params.TxGas, + EffectiveGasPrice: big.NewInt(1), + Logs: []*types.Log{}, + }, + }...) } slice := func(t *testing.T, from, to int) (*blocks.Block, []*types.Receipt) { @@ -834,7 +852,7 @@ func TestGetReceipts(t *testing.T) { settled, wantSettled := slice(t, 2, 4) vmTime.advanceToSettle(ctx, t, settled) unsettled, wantUnsettled := slice(t, 4, 6) - require.NoError(t, unsettled.WaitUntilExecuted(ctx), "WaitUntilExecuted()") + require.NoErrorf(t, unsettled.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", unsettled) pending := sut.runConsensusLoop(t, sut.wallet.SetNonceAndSign(t, 0, &types.LegacyTx{ To: &blockingPrecompile, diff --git a/vms/saevm/sae/vm_test.go b/vms/saevm/sae/vm_test.go index f4fc32ad47e1..b475cf98d78d 100644 --- a/vms/saevm/sae/vm_test.go +++ b/vms/saevm/sae/vm_test.go @@ -7,7 +7,6 @@ import ( "context" "encoding/json" "errors" - "flag" "math/big" "net/http/httptest" "os" @@ -64,9 +63,6 @@ import ( ) func TestMain(m *testing.M) { - createWorstCaseFuzzFlags(flag.CommandLine) - flag.Parse() - log.SetDefault(log.NewLogger(log.NewTerminalHandlerWithLevel(os.Stderr, log.LevelError, true))) goleak.VerifyTestMain( @@ -362,6 +358,10 @@ func (s *SUT) mustSendTx(tb testing.TB, txs ...*types.Transaction) { // sendTxsAndWaitUntilPending sends all `txs` to the mempool, and waits for // each to be marked as pending. +// +// WARNING: if there is a block executing concurrently with this method, +// the pending state of the transactions may not be accurately reflected, +// resulting in a timeout. func (s *SUT) sendTxsAndWaitUntilPending(tb testing.TB, txs ...*types.Transaction) { tb.Helper() @@ -369,6 +369,11 @@ func (s *SUT) sendTxsAndWaitUntilPending(tb testing.TB, txs ...*types.Transactio s.waitUntilTxsPending(tb, txs...) } +// waitUntilTxsPending waits until all `txs` are marked as pending in the mempool. +// +// WARNING: if there is a block executing concurrently with this method, +// the pending state of the transactions may not be accurately reflected, +// resulting in a timeout. func (s *SUT) waitUntilTxsPending(tb testing.TB, txs ...*types.Transaction) { tb.Helper() diff --git a/vms/saevm/sae/worstcase_test.go b/vms/saevm/sae/worstcase_test.go index 215f2f5c0d0e..75deaab87a33 100644 --- a/vms/saevm/sae/worstcase_test.go +++ b/vms/saevm/sae/worstcase_test.go @@ -10,6 +10,7 @@ import ( "math" "math/big" "math/rand/v2" + "os" "runtime" "testing" "time" @@ -31,7 +32,7 @@ import ( saeparams "github.com/ava-labs/avalanchego/vms/saevm/params" ) -var worstCaseFuzzFlags struct { +type worstCaseFlags struct { numAccounts uint balance uint256.Int parallel uint @@ -42,12 +43,13 @@ var worstCaseFuzzFlags struct { rngSeed uint64 } -func createWorstCaseFuzzFlags(set *flag.FlagSet) { +func parseWorstCaseFlags() *worstCaseFlags { + set := flag.NewFlagSet("worstcase", flag.ContinueOnError) + fs := &worstCaseFlags{} + name := func(n string) string { return "worstcase.fuzz." + n } - fs := &worstCaseFuzzFlags - set.UintVar(&fs.numAccounts, name("num_eoa"), 10, "Number of EOAs to send funds between") set.TextVar(&fs.balance, name("eoa_balance"), uint256.NewInt(params.Ether), "Starting balance of EOAs") set.UintVar(&fs.parallel, name("parallel"), uint(runtime.GOMAXPROCS(0)), "Number of parallel tests to run; defaults to GOMAXPROCS") //#nosec G115 -- Known to be positive @@ -56,6 +58,12 @@ func createWorstCaseFuzzFlags(set *flag.FlagSet) { set.Uint64Var(&fs.maxGasLimit, name("max_gas_limit"), 60e6, "Maximum gas limit per transaction (uniform distribution)") set.Uint64Var(&fs.maxTxValue, name("max_tx_value"), params.Ether/1000, "Maximum tx value to send per transaction (uniform distribution)") set.Uint64Var(&fs.rngSeed, name("rng_seed"), 0, "Seed for random-number generator; ignored if zero") + + // Parse returns an error because the testing harness provides additional + // unregistered flags. [flag.ContinueOnError] allows the expected flags to + // be parsed anyways. + _ = set.Parse(os.Args[1:]) + return fs } // A guzzler is both a [params.ChainConfigHooks] and [params.RulesHooks]. When @@ -110,7 +118,15 @@ func (*guzzler) guzzle(env vm.PrecompileEnvironment, input []byte) ([]byte, erro } func TestWorstCase(t *testing.T) { - flags := worstCaseFuzzFlags + // TODO(alarso16): This test flakes due to a race in the legacypool. When + // a block executes, it sends an event to the pool, which causes an + // incorrect nonce update if the pool already had a pending transaction from + // the same account. + if os.Getenv("SAEVM_TEST_FLAKY") == "" { + t.Skip("FLAKY: set SAEVM_TEST_FLAKY to run") + } + + flags := parseWorstCaseFlags() t.Logf("Flags: %+v", flags) guzzle := common.Address{'g', 'u', 'z', 'z', 'l', 'e'}