Merge pull request #10829 from filecoin-project/steb/fix-deploy-flake

fix: itest: fix eth deploy test flake

itests/eth_deploy_test.go

@@ -29,18 +29,13 @@ import (
 // TestDeployment smoke tests the deployment of a contract via the
 // Ethereum JSON-RPC endpoint, from an EEOA.
 func TestDeployment(t *testing.T) {
-	// TODO::FVM @raulk the contract installation and invocation can be lifted into utility methods
-	// He who writes the second test, shall do that.
-	// kit.QuietMiningLogs()
-
-	// reasonable blocktime so that the tx sits in the mpool for a bit during the test.
-	// although this is non-deterministic...
-	blockTime := 1 * time.Second
+	blockTime := 100 * time.Millisecond
 	client, _, ens := kit.EnsembleMinimal(
 		t,
 		kit.MockProofs(),
 		kit.ThroughRPC())
-	ens.InterconnectAll().BeginMining(blockTime)
+
+	miners := ens.InterconnectAll().BeginMining(blockTime)
 
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 	defer cancel()
@@ -93,6 +88,11 @@ func TestDeployment(t *testing.T) {
 	pendingFilter, err := client.EthNewPendingTransactionFilter(ctx)
 	require.NoError(t, err)
 
+	// Pause so we can test that everything works while the message is in the message pool.
+	for _, miner := range miners {
+		miner.Pause()
+	}
+
 	hash := client.EVM().SubmitTransaction(ctx, &tx)
 
 	mpoolTx, err := client.EthGetTransactionByHash(ctx, &hash)
@@ -103,31 +103,40 @@ func TestDeployment(t *testing.T) {
 	require.Equal(t, hash, mpoolTx.Hash)
 
 	// these fields should be nil because the tx hasn't landed on chain.
-	// TODO::FVM @raulk We can either skip the assertion if the msg has already
-	//  landed, or pause mining between the embryo creation and this assertion.
 	require.Nil(t, mpoolTx.BlockNumber)
 	require.Nil(t, mpoolTx.BlockHash)
 	require.Nil(t, mpoolTx.TransactionIndex)
 
+	// We should be able to get the message CID immediately.
+	mCid, err := client.EthGetMessageCidByTransactionHash(ctx, &hash)
+	require.NoError(t, err)
+	require.NotNil(t, mCid)
+
+	// ... and it should map back to the transaction hash.
+	mHash, err := client.EthGetTransactionHashByCid(ctx, *mCid)
+	require.NoError(t, err)
+	require.NotNil(t, mHash)
+	require.Equal(t, hash, *mHash)
+
 	changes, err := client.EthGetFilterChanges(ctx, pendingFilter)
 	require.NoError(t, err)
 	require.Len(t, changes.Results, 1)
 	require.Equal(t, hash.String(), changes.Results[0])
 
-	var receipt *api.EthTxReceipt
-	for i := 0; i < 20; i++ {
-		// TODO::FVM @raulk The right time to exit this loop isn't after
-		//  20 iterations, but when StateWaitMsg returns -- let's wait on that
-		//  event while continuing to make this assertion
-		receipt, err = client.EthGetTransactionReceipt(ctx, hash)
-		if err != nil || receipt == nil {
-			time.Sleep(500 * time.Millisecond)
-			continue
-		}
-		break
+	// Unpause mining.
+	for _, miner := range miners {
+		miner.Restart()
 	}
+
+	// Wait for the message to land.
+	_, err = client.StateWaitMsg(ctx, *mCid, 3, api.LookbackNoLimit, false)
+	require.NoError(t, err)
+
+	// Then lookup the receipt.
+	receipt, err := client.EthGetTransactionReceipt(ctx, hash)
 	require.NoError(t, err)
 	require.NotNil(t, receipt)
+
 	// logs must be an empty array, not a nil value, to avoid tooling compatibility issues
 	require.Empty(t, receipt.Logs)
 	// a correctly formed logs bloom, albeit empty, has 256 zeroes
@@ -173,21 +182,16 @@ func TestDeployment(t *testing.T) {
 	require.Nil(t, err)
 	require.True(t, reflect.DeepEqual(block1, block2))
 
-	// should be able to get the block using latest as well
-	block3, err := client.EthGetBlockByNumber(ctx, "latest", false)
-	require.Nil(t, err)
-	require.True(t, reflect.DeepEqual(block2, block3))
-
 	// verify that the block contains full tx objects
-	block4, err := client.EthGetBlockByHash(ctx, *chainTx.BlockHash, true)
+	block3, err := client.EthGetBlockByHash(ctx, *chainTx.BlockHash, true)
 	require.Nil(t, err)
-	require.Equal(t, block4.Hash, *chainTx.BlockHash)
-	require.Equal(t, block4.Number, *chainTx.BlockNumber)
+	require.Equal(t, block3.Hash, *chainTx.BlockHash)
+	require.Equal(t, block3.Number, *chainTx.BlockNumber)
 
 	// the call went through json-rpc and the response was unmarshaled
 	// into map[string]interface{}, so it has to be converted into ethtypes.EthTx
 	var foundTx *ethtypes.EthTx
-	for _, obj := range block4.Transactions {
+	for _, obj := range block3.Transactions {
 		j, err := json.Marshal(obj)
 		require.Nil(t, err)
 
@@ -202,10 +206,10 @@ func TestDeployment(t *testing.T) {
 	require.NotNil(t, foundTx)
 	require.True(t, reflect.DeepEqual(*foundTx, *chainTx))
 
-	// make sure the block got from EthGetBlockByNumber is the same
-	block5, err := client.EthGetBlockByNumber(ctx, blkNum, true)
+	// make sure the _full_ block got from EthGetBlockByNumber is the same
+	block4, err := client.EthGetBlockByNumber(ctx, blkNum, true)
 	require.Nil(t, err)
-	require.True(t, reflect.DeepEqual(block4, block5))
+	require.True(t, reflect.DeepEqual(block3, block4))
 
 	// Verify that the deployer is now an account.
 	client.AssertActorType(ctx, deployer, manifest.EthAccountKey)