From d7deb9a3ebb922cf81c235179c62462c055fa766 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 4 May 2023 12:42:01 -0700 Subject: [PATCH] fix: itest: fix eth deploy test flake This fixes the flakyness by: 1. Disconnecting the client from the miner before submitting the message. That way, we force it to get stuck in the message pool. 2. Removing the logic that asks lotus for the "latest" block. We have other tests that exercise "latest". fixes #10824 --- itests/eth_deploy_test.go | 70 +++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/itests/eth_deploy_test.go b/itests/eth_deploy_test.go index bbdadb1d5..ce4c94a28 100644 --- a/itests/eth_deploy_test.go +++ b/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)