e2e: Diagnose and fix e2e test flakes (#1941)

Author: maru-ava

tests/colors.go

@@ -4,7 +4,7 @@
 package tests
 
 import (
-	"fmt"
+	ginkgo "github.com/onsi/ginkgo/v2"
 
 	"github.com/onsi/ginkgo/v2/formatter"
 )
@@ -20,5 +20,11 @@ import (
 // for an exhaustive list of color options.
 func Outf(format string, args ...interface{}) {
 	s := formatter.F(format, args...)
-	fmt.Fprint(formatter.ColorableStdOut, s)
+	// Use GinkgoWriter to ensure that output from this function is
+	// printed sequentially within other test output produced with
+	// GinkgoWriter (e.g. `STEP:...`) when tests are run in
+	// parallel. ginkgo collects and writes stdout separately from
+	// GinkgoWriter during parallel execution and the resulting output
+	// can be confusing.
+	ginkgo.GinkgoWriter.Print(s)
 }

tests/e2e/banff/suites.go

@@ -29,7 +29,7 @@ var _ = ginkgo.Describe("[Banff]", func() {
 		),
 		func() {
 			keychain := e2e.Env.NewKeychain(1)
-			wallet := e2e.Env.NewWallet(keychain)
+			wallet := e2e.Env.NewWallet(keychain, e2e.Env.GetRandomNodeURI())
 
 			// Get the P-chain and the X-chain wallets
 			pWallet := wallet.P()

tests/e2e/c/interchain_workflow.go

@@ -32,6 +32,13 @@ var _ = e2e.DescribeCChain("[Interchain Workflow]", func() {
 	)
 
 	ginkgo.It("should ensure that funds can be transferred from the C-Chain to the X-Chain and the P-Chain", func() {
+		ginkgo.By("initializing a new eth client")
+		// Select a random node URI to use for both the eth client and
+		// the wallet to avoid having to verify that all nodes are at
+		// the same height before initializing the wallet.
+		nodeURI := e2e.Env.GetRandomNodeURI()
+		ethClient := e2e.Env.NewEthClient(nodeURI)
+
 		ginkgo.By("allocating a pre-funded key to send from and a recipient key to deliver to")
 		senderKey := e2e.Env.AllocateFundedKey()
 		senderEthAddress := evm.GetEthAddress(senderKey)
@@ -40,18 +47,11 @@ var _ = e2e.DescribeCChain("[Interchain Workflow]", func() {
 		require.NoError(err)
 		recipientEthAddress := evm.GetEthAddress(recipientKey)
 
-		// Select a random node URI to use for both the eth client and
-		// the wallet to avoid having to verify that all nodes are at
-		// the same height before initializing the wallet.
-		nodeURI := e2e.Env.GetRandomNodeURI()
-		ethClient := e2e.Env.NewEthClientForURI(nodeURI)
-
 		ginkgo.By("sending funds from one address to another on the C-Chain", func() {
 			// Create transaction
 			acceptedNonce, err := ethClient.AcceptedNonceAt(e2e.DefaultContext(), senderEthAddress)
 			require.NoError(err)
-			gasPrice, err := ethClient.SuggestGasPrice(e2e.DefaultContext())
-			require.NoError(err)
+			gasPrice := e2e.SuggestGasPrice(ethClient)
 			tx := types.NewTransaction(
 				acceptedNonce,
 				recipientEthAddress,
@@ -68,7 +68,7 @@ var _ = e2e.DescribeCChain("[Interchain Workflow]", func() {
 			signedTx, err := types.SignTx(tx, signer, senderKey.ToECDSA())
 			require.NoError(err)
 
-			require.NoError(ethClient.SendTransaction(e2e.DefaultContext(), signedTx))
+			_ = e2e.SendEthTransaction(ethClient, signedTx)
 
 			ginkgo.By("waiting for the C-Chain recipient address to have received the sent funds")
 			e2e.Eventually(func() bool {
@@ -83,7 +83,7 @@ var _ = e2e.DescribeCChain("[Interchain Workflow]", func() {
 		// matches on-chain state.
 		ginkgo.By("initializing a keychain and associated wallet")
 		keychain := secp256k1fx.NewKeychain(senderKey, recipientKey)
-		baseWallet := e2e.Env.NewWalletForURI(keychain, nodeURI)
+		baseWallet := e2e.Env.NewWallet(keychain, nodeURI)
 		xWallet := baseWallet.X()
 		cWallet := baseWallet.C()
 		pWallet := baseWallet.P()
@@ -115,6 +115,7 @@ var _ = e2e.DescribeCChain("[Interchain Workflow]", func() {
 				xWallet.BlockchainID(),
 				exportOutputs,
 				e2e.WithDefaultContext(),
+				e2e.WithSuggestedGasPrice(ethClient),
 			)
 			require.NoError(err)
 		})
@@ -141,6 +142,7 @@ var _ = e2e.DescribeCChain("[Interchain Workflow]", func() {
 				constants.PlatformChainID,
 				exportOutputs,
 				e2e.WithDefaultContext(),
+				e2e.WithSuggestedGasPrice(ethClient),
 			)
 			require.NoError(err)
 		})

tests/e2e/e2e.go

@@ -7,7 +7,9 @@ package e2e
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"math/big"
 	"math/rand"
 	"strings"
 	"time"
@@ -16,8 +18,11 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/ava-labs/coreth/core/types"
 	"github.com/ava-labs/coreth/ethclient"
+	"github.com/ava-labs/coreth/interfaces"
 
+	"github.com/ava-labs/avalanchego/ids"
 	"github.com/ava-labs/avalanchego/tests"
 	"github.com/ava-labs/avalanchego/tests/fixture"
 	"github.com/ava-labs/avalanchego/tests/fixture/testnet"
@@ -58,7 +63,7 @@ type TestEnvironment struct {
 	// The directory where the test network configuration is stored
 	NetworkDir string
 	// URIs used to access the API endpoints of nodes of the network
-	URIs []string
+	URIs []testnet.NodeURI
 	// The URI used to access the http server that allocates test data
 	TestDataServerURI string
 
@@ -76,9 +81,11 @@ func InitTestEnvironment(envBytes []byte) {
 
 // Retrieve a random URI to naively attempt to spread API load across
 // nodes.
-func (te *TestEnvironment) GetRandomNodeURI() string {
+func (te *TestEnvironment) GetRandomNodeURI() testnet.NodeURI {
 	r := rand.New(rand.NewSource(time.Now().Unix())) //#nosec G404
-	return te.URIs[r.Intn(len(te.URIs))]
+	nodeURI := te.URIs[r.Intn(len(te.URIs))]
+	tests.Outf("{{blue}} targeting node %s with URI: %s{{/}}\n", nodeURI.NodeID, nodeURI.URI)
+	return nodeURI
 }
 
 // Retrieve the network to target for testing.
@@ -92,6 +99,7 @@ func (te *TestEnvironment) GetNetwork() testnet.Network {
 func (te *TestEnvironment) AllocateFundedKeys(count int) []*secp256k1.PrivateKey {
 	keys, err := fixture.AllocateFundedKeys(te.TestDataServerURI, count)
 	te.require.NoError(err)
+	tests.Outf("{{blue}} allocated funded key(s): %+v{{/}}\n", keys)
 	return keys
 }
 
@@ -102,36 +110,33 @@ func (te *TestEnvironment) AllocateFundedKey() *secp256k1.PrivateKey {
 
 // Create a new keychain with the specified number of test keys.
 func (te *TestEnvironment) NewKeychain(count int) *secp256k1fx.Keychain {
-	tests.Outf("{{blue}} initializing keychain with %d keys {{/}}\n", count)
 	keys := te.AllocateFundedKeys(count)
 	return secp256k1fx.NewKeychain(keys...)
 }
 
-// Create a new wallet for the provided keychain against a random node URI.
-func (te *TestEnvironment) NewWallet(keychain *secp256k1fx.Keychain) primary.Wallet {
-	return te.NewWalletForURI(keychain, te.GetRandomNodeURI())
-}
-
 // Create a new wallet for the provided keychain against the specified node URI.
-func (te *TestEnvironment) NewWalletForURI(keychain *secp256k1fx.Keychain, uri string) primary.Wallet {
-	tests.Outf("{{blue}} initializing a new wallet {{/}}\n")
-	wallet, err := primary.MakeWallet(DefaultContext(), &primary.WalletConfig{
-		URI:          uri,
+func (te *TestEnvironment) NewWallet(keychain *secp256k1fx.Keychain, nodeURI testnet.NodeURI) primary.Wallet {
+	tests.Outf("{{blue}} initializing a new wallet for node %s with URI: %s {{/}}\n", nodeURI.NodeID, nodeURI.URI)
+	baseWallet, err := primary.MakeWallet(DefaultContext(), &primary.WalletConfig{
+		URI:          nodeURI.URI,
 		AVAXKeychain: keychain,
 		EthKeychain:  keychain,
 	})
 	te.require.NoError(err)
-	return wallet
-}
-
-// Create a new eth client targeting a random node.
-func (te *TestEnvironment) NewEthClient() ethclient.Client {
-	return te.NewEthClientForURI(te.GetRandomNodeURI())
+	return primary.NewWalletWithOptions(
+		baseWallet,
+		common.WithPostIssuanceFunc(
+			func(id ids.ID) {
+				tests.Outf(" issued transaction with ID: %s\n", id)
+			},
+		),
+	)
 }
 
 // Create a new eth client targeting the specified node URI.
-func (te *TestEnvironment) NewEthClientForURI(nodeURI string) ethclient.Client {
-	nodeAddress := strings.Split(nodeURI, "//")[1]
+func (te *TestEnvironment) NewEthClient(nodeURI testnet.NodeURI) ethclient.Client {
+	tests.Outf("{{blue}} initializing a new eth client for node %s with URI: %s {{/}}\n", nodeURI.NodeID, nodeURI.URI)
+	nodeAddress := strings.Split(nodeURI.URI, "//")[1]
 	uri := fmt.Sprintf("ws://%s/ext/bc/C/ws", nodeAddress)
 	client, err := ethclient.Dial(uri)
 	te.require.NoError(err)
@@ -198,3 +203,47 @@ func WaitForHealthy(node testnet.Node) {
 	defer cancel()
 	require.NoError(ginkgo.GinkgoT(), testnet.WaitForHealthy(ctx, node))
 }
+
+// Sends an eth transaction, waits for the transaction receipt to be issued
+// and checks that the receipt indicates success.
+func SendEthTransaction(ethClient ethclient.Client, signedTx *types.Transaction) *types.Receipt {
+	require := require.New(ginkgo.GinkgoT())
+
+	txID := signedTx.Hash()
+	tests.Outf(" sending eth transaction with ID: %s\n", txID)
+
+	require.NoError(ethClient.SendTransaction(DefaultContext(), signedTx))
+
+	// Wait for the receipt
+	var receipt *types.Receipt
+	Eventually(func() bool {
+		var err error
+		receipt, err = ethClient.TransactionReceipt(DefaultContext(), txID)
+		if errors.Is(err, interfaces.NotFound) {
+			return false // Transaction is still pending
+		}
+		require.NoError(err)
+		return true
+	}, DefaultTimeout, DefaultPollingInterval, "failed to see transaction acceptance before timeout")
+
+	require.Equal(receipt.Status, types.ReceiptStatusSuccessful)
+	return receipt
+}
+
+// Determines the suggested gas price for the configured client that will
+// maximize the chances of transaction acceptance.
+func SuggestGasPrice(ethClient ethclient.Client) *big.Int {
+	gasPrice, err := ethClient.SuggestGasPrice(DefaultContext())
+	require.NoError(ginkgo.GinkgoT(), err)
+	// Double the suggested gas price to maximize the chances of
+	// acceptance. Maybe this can be revisited pending resolution of
+	// https://github.com/ava-labs/coreth/issues/314.
+	gasPrice.Add(gasPrice, gasPrice)
+	return gasPrice
+}
+
+// Helper simplifying use via an option of a gas price appropriate for testing.
+func WithSuggestedGasPrice(ethClient ethclient.Client) common.Option {
+	baseFee := SuggestGasPrice(ethClient)
+	return common.WithBaseFee(baseFee)
+}

tests/e2e/p/permissionless_subnets.go

@@ -35,17 +35,18 @@ var _ = e2e.DescribePChain("[Permissionless Subnets]", func() {
 			"permissionless-subnets",
 		),
 		func() {
+			nodeURI := e2e.Env.GetRandomNodeURI()
+
 			keychain := e2e.Env.NewKeychain(1)
-			baseWallet := e2e.Env.NewWallet(keychain)
+			baseWallet := e2e.Env.NewWallet(keychain, nodeURI)
 
-			nodeURI := e2e.Env.GetRandomNodeURI()
 			pWallet := baseWallet.P()
 			xWallet := baseWallet.X()
 			xChainID := xWallet.BlockchainID()
 
 			var validatorID ids.NodeID
 			ginkgo.By("retrieving the node ID of a primary network validator", func() {
-				pChainClient := platformvm.NewClient(nodeURI)
+				pChainClient := platformvm.NewClient(nodeURI.URI)
 				ctx, cancel := context.WithTimeout(context.Background(), e2e.DefaultTimeout)
 				validatorIDs, err := pChainClient.SampleValidators(ctx, constants.PrimaryNetworkID, 1)
 				cancel()

tests/e2e/p/workflow.go

@@ -44,12 +44,12 @@ var _ = e2e.DescribePChain("[Workflow]", func() {
 		func() {
 			nodeURI := e2e.Env.GetRandomNodeURI()
 			keychain := e2e.Env.NewKeychain(2)
-			baseWallet := e2e.Env.NewWallet(keychain)
+			baseWallet := e2e.Env.NewWallet(keychain, nodeURI)
 
 			pWallet := baseWallet.P()
 			avaxAssetID := baseWallet.P().AVAXAssetID()
 			xWallet := baseWallet.X()
-			pChainClient := platformvm.NewClient(nodeURI)
+			pChainClient := platformvm.NewClient(nodeURI.URI)
 
 			tests.Outf("{{blue}} fetching minimal stake amounts {{/}}\n")
 			ctx, cancel := context.WithTimeout(context.Background(), e2e.DefaultWalletCreationTimeout)
@@ -60,7 +60,7 @@ var _ = e2e.DescribePChain("[Workflow]", func() {
 			tests.Outf("{{green}} minimal delegator stake: %d {{/}}\n", minDelStake)
 
 			tests.Outf("{{blue}} fetching tx fee {{/}}\n")
-			infoClient := info.NewClient(nodeURI)
+			infoClient := info.NewClient(nodeURI.URI)
 			ctx, cancel = context.WithTimeout(context.Background(), e2e.DefaultWalletCreationTimeout)
 			fees, err := infoClient.GetTxFee(ctx)
 			cancel()

tests/e2e/static-handlers/suites.go

@@ -110,7 +110,7 @@ var _ = ginkgo.Describe("[StaticHandlers]", func() {
 					},
 				},
 			}
-			staticClient := avm.NewStaticClient(e2e.Env.GetRandomNodeURI())
+			staticClient := avm.NewStaticClient(e2e.Env.GetRandomNodeURI().URI)
 			ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
 			resp, err := staticClient.BuildGenesis(ctx, &avmArgs)
 			cancel()
@@ -181,7 +181,7 @@ var _ = ginkgo.Describe("[StaticHandlers]", func() {
 			Encoding:      formatting.Hex,
 		}
 
-		staticClient := api.NewStaticClient(e2e.Env.GetRandomNodeURI())
+		staticClient := api.NewStaticClient(e2e.Env.GetRandomNodeURI().URI)
 		ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
 		resp, err := staticClient.BuildGenesis(ctx, &buildGenesisArgs)
 		cancel()

tests/e2e/x/interchain_workflow.go

@@ -29,13 +29,15 @@ var _ = e2e.DescribeXChain("[Interchain Workflow]", func() {
 	const transferAmount = 10 * units.Avax
 
 	ginkgo.It("should ensure that funds can be transferred from the X-Chain to the C-Chain and the P-Chain", func() {
+		nodeURI := e2e.Env.GetRandomNodeURI()
+
 		ginkgo.By("creating wallet with a funded key to send from and recipient key to deliver to")
 		factory := secp256k1.Factory{}
 		recipientKey, err := factory.NewPrivateKey()
 		require.NoError(err)
 		keychain := e2e.Env.NewKeychain(1)
 		keychain.Add(recipientKey)
-		baseWallet := e2e.Env.NewWallet(keychain)
+		baseWallet := e2e.Env.NewWallet(keychain, nodeURI)
 		xWallet := baseWallet.X()
 		cWallet := baseWallet.C()
 		pWallet := baseWallet.P()
@@ -101,17 +103,20 @@ var _ = e2e.DescribeXChain("[Interchain Workflow]", func() {
 			require.NoError(err)
 		})
 
+		ginkgo.By("initializing a new eth client")
+		ethClient := e2e.Env.NewEthClient(nodeURI)
+
 		ginkgo.By("importing AVAX from the X-Chain to the C-Chain", func() {
 			_, err := cWallet.IssueImportTx(
 				xWallet.BlockchainID(),
 				recipientEthAddress,
 				e2e.WithDefaultContext(),
+				e2e.WithSuggestedGasPrice(ethClient),
 			)
 			require.NoError(err)
 		})
 
 		ginkgo.By("checking that the recipient address has received imported funds on the C-Chain")
-		ethClient := e2e.Env.NewEthClient()
 		e2e.Eventually(func() bool {
 			balance, err := ethClient.BalanceAt(e2e.DefaultContext(), recipientEthAddress, nil)
 			require.NoError(err)

tests/e2e/x/transfer/virtuous.go

@@ -44,7 +44,10 @@ var _ = e2e.DescribeXChainSerial("[Virtuous Transfer Tx AVAX]", func() {
 			"virtuous-transfer-tx-avax",
 		),
 		func() {
-			rpcEps := e2e.Env.URIs
+			rpcEps := make([]string, len(e2e.Env.URIs))
+			for i, nodeURI := range e2e.Env.URIs {
+				rpcEps[i] = nodeURI.URI
+			}
 
 			// Waiting for ongoing blocks to have completed before starting this
 			// test avoids the case of a previous test having initiated block
@@ -84,7 +87,7 @@ var _ = e2e.DescribeXChainSerial("[Virtuous Transfer Tx AVAX]", func() {
 				}
 
 				keychain := secp256k1fx.NewKeychain(testKeys...)
-				baseWallet := e2e.Env.NewWallet(keychain)
+				baseWallet := e2e.Env.NewWallet(keychain, e2e.Env.GetRandomNodeURI())
 				avaxAssetID := baseWallet.X().AVAXAssetID()
 
 				wallets := make([]primary.Wallet, len(testKeys))

tests/fixture/testnet/config.go

@@ -157,6 +157,12 @@ func (c *NetworkConfig) EnsureGenesis(networkID uint32, validatorIDs []ids.NodeI
 	return nil
 }
 
+// NodeURI associates a node ID with its API URI.
+type NodeURI struct {
+	NodeID ids.NodeID
+	URI    string
+}
+
 // NodeConfig defines configuration for an AvalancheGo node.
 type NodeConfig struct {
 	NodeID ids.NodeID