Commit dcdf044f authored by Matthew Slipper's avatar Matthew Slipper Committed by GitHub

op-e2e: Reduce flakiness in p2p tests (#9333)

P2P tests were flaking sometimes due to the checks asserting that everything received was published. `require.ElementsMatch` requires that lists be equal, sans duplicates. Sometimes, however, messages would get sent multiple times/not be picked up in time for the assertions leading to errors like this following:

```
listA:
([]string) (len=29) {
 ... elided
 (string) (len=68) "0xd770621d3b0a196087ace7efcb0a8a61b353736526f6095946c714ec134b1648:1",
 (string) (len=69) "0x71a364bf88d4e7e049cfe1635e95c31c9d93435b03a4d20c8863637fc0eb319f:28",
 (string) (len=68) "0xd770621d3b0a196087ace7efcb0a8a61b353736526f6095946c714ec134b1648:1"
}

listB:
([]string) (len=29) {
 (string) (len=68) "0xd770621d3b0a196087ace7efcb0a8a61b353736526f6095946c714ec134b1648:1",
 ... elided
 (string) (len=69) "0xf84acde28fb7b630f9e5d91bddc7c2d4d2e1f4ce29a7d1a4551fee9ac828b10f:29"
}

extra elements in list A:
([]interface {}) (len=1) {
 (string) (len=68) "0xd770621d3b0a196087ace7efcb0a8a61b353736526f6095946c714ec134b1648:1"
}

extra elements in list B:
([]interface {}) (len=1) {
 (string) (len=69) "0xf84acde28fb7b630f9e5d91bddc7c2d4d2e1f4ce29a7d1a4551fee9ac828b10f:29"
}
```

This PR refactors to use `require.Subset` which handles this case while still asserting that everything received was in fact published.
parent e4b6585f
...@@ -622,13 +622,16 @@ func TestSystemMockP2P(t *testing.T) { ...@@ -622,13 +622,16 @@ func TestSystemMockP2P(t *testing.T) {
// Verify that everything that was received was published // Verify that everything that was received was published
require.GreaterOrEqual(t, len(published), len(received)) require.GreaterOrEqual(t, len(published), len(received))
require.ElementsMatch(t, received, published[:len(received)]) require.Subset(t, published, received)
// Verify that the tx was received via p2p // Verify that the tx was received via p2p
require.Contains(t, received, receiptSeq.BlockHash) require.Contains(t, received, receiptSeq.BlockHash)
} }
func TestSystemP2PAltSync(t *testing.T) { func TestSystemP2PAltSync(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
InitParallel(t) InitParallel(t)
cfg := DefaultSystemConfig(t) cfg := DefaultSystemConfig(t)
...@@ -736,10 +739,13 @@ func TestSystemP2PAltSync(t *testing.T) { ...@@ -736,10 +739,13 @@ func TestSystemP2PAltSync(t *testing.T) {
configureL2(syncNodeCfg, syncerL2Engine, cfg.JWTSecret) configureL2(syncNodeCfg, syncerL2Engine, cfg.JWTSecret)
syncerNode, err := rollupNode.New(context.Background(), syncNodeCfg, cfg.Loggers["syncer"], snapLog, "", metrics.NewMetrics("")) syncerNode, err := rollupNode.New(ctx, syncNodeCfg, cfg.Loggers["syncer"], snapLog, "", metrics.NewMetrics(""))
require.NoError(t, err) require.NoError(t, err)
err = syncerNode.Start(context.Background()) err = syncerNode.Start(ctx)
require.NoError(t, err) require.NoError(t, err)
defer func() {
require.NoError(t, syncerNode.Stop(ctx))
}()
// connect alice and bob to our new syncer node // connect alice and bob to our new syncer node
_, err = sys.Mocknet.ConnectPeers(sys.RollupNodes["alice"].P2P().Host().ID(), syncerNode.P2P().Host().ID()) _, err = sys.Mocknet.ConnectPeers(sys.RollupNodes["alice"].P2P().Host().ID(), syncerNode.P2P().Host().ID())
...@@ -751,7 +757,7 @@ func TestSystemP2PAltSync(t *testing.T) { ...@@ -751,7 +757,7 @@ func TestSystemP2PAltSync(t *testing.T) {
l2Verif := ethclient.NewClient(rpc) l2Verif := ethclient.NewClient(rpc)
// It may take a while to sync, but eventually we should see the sequenced data show up // It may take a while to sync, but eventually we should see the sequenced data show up
receiptVerif, err := geth.WaitForTransaction(receiptSeq.TxHash, l2Verif, 100*time.Duration(sys.RollupConfig.BlockTime)*time.Second) receiptVerif, err := wait.ForReceiptOK(ctx, l2Verif, receiptSeq.TxHash)
require.Nil(t, err, "Waiting for L2 tx on verifier") require.Nil(t, err, "Waiting for L2 tx on verifier")
require.Equal(t, receiptSeq, receiptVerif) require.Equal(t, receiptSeq, receiptVerif)
...@@ -761,7 +767,7 @@ func TestSystemP2PAltSync(t *testing.T) { ...@@ -761,7 +767,7 @@ func TestSystemP2PAltSync(t *testing.T) {
// Verify that everything that was received was published // Verify that everything that was received was published
require.GreaterOrEqual(t, len(published), len(syncedPayloads)) require.GreaterOrEqual(t, len(published), len(syncedPayloads))
require.ElementsMatch(t, syncedPayloads, published[:len(syncedPayloads)]) require.Subset(t, published, syncedPayloads)
} }
// TestSystemDenseTopology sets up a dense p2p topology with 3 verifier nodes and 1 sequencer node. // TestSystemDenseTopology sets up a dense p2p topology with 3 verifier nodes and 1 sequencer node.
......
...@@ -8,6 +8,8 @@ import ( ...@@ -8,6 +8,8 @@ import (
"testing" "testing"
"time" "time"
"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/wait"
"github.com/ethereum-optimism/optimism/op-bindings/bindings" "github.com/ethereum-optimism/optimism/op-bindings/bindings"
"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/geth" "github.com/ethereum-optimism/optimism/op-e2e/e2eutils/geth"
"github.com/ethereum-optimism/optimism/op-e2e/e2eutils/transactions" "github.com/ethereum-optimism/optimism/op-e2e/e2eutils/transactions"
...@@ -112,13 +114,13 @@ func SendL2Tx(t *testing.T, cfg SystemConfig, l2Client *ethclient.Client, privKe ...@@ -112,13 +114,13 @@ func SendL2Tx(t *testing.T, cfg SystemConfig, l2Client *ethclient.Client, privKe
err := l2Client.SendTransaction(ctx, tx) err := l2Client.SendTransaction(ctx, tx)
require.NoError(t, err, "Sending L2 tx") require.NoError(t, err, "Sending L2 tx")
receipt, err := geth.WaitForTransaction(tx.Hash(), l2Client, 10*time.Duration(cfg.DeployConfig.L2BlockTime)*time.Second) receipt, err := wait.ForReceiptOK(ctx, l2Client, tx.Hash())
require.NoError(t, err, "Waiting for L2 tx") require.NoError(t, err, "Waiting for L2 tx")
require.Equal(t, opts.ExpectedStatus, receipt.Status, "TX should have expected status") require.Equal(t, opts.ExpectedStatus, receipt.Status, "TX should have expected status")
for i, client := range opts.VerifyClients { for i, client := range opts.VerifyClients {
t.Logf("Waiting for tx %v on verification client %d", tx.Hash(), i) t.Logf("Waiting for tx %v on verification client %d", tx.Hash(), i)
receiptVerif, err := geth.WaitForTransaction(tx.Hash(), client, 10*time.Duration(cfg.DeployConfig.L2BlockTime)*time.Second) receiptVerif, err := wait.ForReceiptOK(ctx, client, tx.Hash())
require.NoErrorf(t, err, "Waiting for L2 tx on verification client %d", i) require.NoErrorf(t, err, "Waiting for L2 tx on verification client %d", i)
require.Equalf(t, receipt, receiptVerif, "Receipts should be the same on sequencer and verification client %d", i) require.Equalf(t, receipt, receiptVerif, "Receipts should be the same on sequencer and verification client %d", i)
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment