Merge pull request #10069 from filecoin-project/asr/fix-tsk

fix: chain: put tipsetkey upon expansion of tipset

.circleci/config.yml

@@ -730,6 +730,11 @@ workflows:
           suite: itest-eth_balance
           target: "./itests/eth_balance_test.go"
 
+      - test:
+          name: test-itest-eth_block_hash
+          suite: itest-eth_block_hash
+          target: "./itests/eth_block_hash_test.go"
+
       - test:
           name: test-itest-eth_deploy
           suite: itest-eth_deploy

chain/store/store.go

@@ -384,7 +384,19 @@ func (cs *ChainStore) PutTipSet(ctx context.Context, ts *types.TipSet) error {
 	if err != nil {
 		return xerrors.Errorf("errored while expanding tipset: %w", err)
 	}
-	log.Debugf("expanded %s into %s\n", ts.Cids(), expanded.Cids())
+
+	if expanded.Key() != ts.Key() {
+		log.Debugf("expanded %s into %s\n", ts.Cids(), expanded.Cids())
+
+		tsBlk, err := expanded.Key().ToStorageBlock()
+		if err != nil {
+			return xerrors.Errorf("failed to get tipset key block: %w", err)
+		}
+
+		if err = cs.chainLocalBlockstore.Put(ctx, tsBlk); err != nil {
+			return xerrors.Errorf("failed to put tipset key block: %w", err)
+		}
+	}
 
 	if err := cs.MaybeTakeHeavierTipSet(ctx, expanded); err != nil {
 		return xerrors.Errorf("MaybeTakeHeavierTipSet failed in PutTipSet: %w", err)

itests/deals_padding_test.go

@@ -33,7 +33,7 @@ func TestDealPadding(t *testing.T) {
 	dh := kit.NewDealHarness(t, client, miner, miner)
 
 	ctx := context.Background()
-	client.WaitTillChain(ctx, kit.BlockMinedBy(miner.ActorAddr))
+	client.WaitTillChain(ctx, kit.BlocksMinedByAll(miner.ActorAddr))
 
 	// Create a random file, would originally be a 256-byte sector
 	res, inFile := client.CreateImportFile(ctx, 1, 200)

itests/deals_power_test.go

@@ -52,7 +52,7 @@ func TestFirstDealEnablesMining(t *testing.T) {
 	providerMined := make(chan struct{})
 
 	go func() {
-		_ = client.WaitTillChain(ctx, kit.BlockMinedBy(provider.ActorAddr))
+		_ = client.WaitTillChain(ctx, kit.BlocksMinedByAll(provider.ActorAddr))
 		close(providerMined)
 	}()
 

itests/eth_block_hash_test.go

@@ -0,0 +1,65 @@
+package itests
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/filecoin-project/go-state-types/abi"
+
+	"github.com/filecoin-project/lotus/itests/kit"
+)
+
+// TestEthBlockHashesCorrect_MultiBlockTipset validates that blocks retrieved through
+// EthGetBlockByNumber are identical to blocks retrieved through
+// EthGetBlockByHash, when using the block hash returned by the former.
+//
+// Specifically, it checks the system behaves correctly with multiblock tipsets.
+//
+// Catches regressions around https://github.com/filecoin-project/lotus/issues/10061.
+func TestEthBlockHashesCorrect_MultiBlockTipset(t *testing.T) {
+	// miner is connected to the first node, and we want to observe the chain
+	// from the second node.
+	blocktime := 100 * time.Millisecond
+	n1, m1, m2, ens := kit.EnsembleOneTwo(t,
+		kit.MockProofs(),
+		kit.ThroughRPC(),
+	)
+	ens.InterconnectAll().BeginMining(blocktime)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	n1.WaitTillChain(ctx, kit.HeightAtLeast(abi.ChainEpoch(25)))
+	defer cancel()
+
+	var n2 kit.TestFullNode
+	ens.FullNode(&n2, kit.ThroughRPC()).Start().Connect(n2, n1)
+
+	// find the first tipset where all miners mined a block.
+	ctx, cancel = context.WithTimeout(context.Background(), 1*time.Minute)
+	n2.WaitTillChain(ctx, kit.BlocksMinedByAll(m1.ActorAddr, m2.ActorAddr))
+	defer cancel()
+
+	head, err := n2.ChainHead(context.Background())
+	require.NoError(t, err)
+
+	// let the chain run a little bit longer to minimise the chance of reorgs
+	n2.WaitTillChain(ctx, kit.HeightAtLeast(head.Height()+50))
+
+	head, err = n2.ChainHead(context.Background())
+	require.NoError(t, err)
+
+	for i := 1; i <= int(head.Height()); i++ {
+		hex := fmt.Sprintf("0x%x", i)
+
+		ethBlockA, err := n2.EthGetBlockByNumber(ctx, hex, true)
+		require.NoError(t, err)
+
+		ethBlockB, err := n2.EthGetBlockByHash(ctx, ethBlockA.Hash, true)
+		require.NoError(t, err)
+
+		require.Equal(t, ethBlockA, ethBlockB)
+	}
+}

itests/eth_deploy_test.go

@@ -98,6 +98,7 @@ func TestDeployment(t *testing.T) {
 
 	mpoolTx, err := client.EthGetTransactionByHash(ctx, &hash)
 	require.NoError(t, err)
+	require.NotNil(t, mpoolTx)
 
 	// require that the hashes are identical
 	require.Equal(t, hash, mpoolTx.Hash)

itests/kit/node_full.go

@@ -135,13 +135,21 @@ func HeightAtLeast(target abi.ChainEpoch) ChainPredicate {
 	}
 }
 
-// BlockMinedBy returns a ChainPredicate that is satisfied when we observe the
-// first block mined by the specified miner.
-func BlockMinedBy(miner address.Address) ChainPredicate {
+// BlocksMinedByAll returns a ChainPredicate that is satisfied when we observe a
+// tipset including blocks from all the specified miners, in no particular order.
+func BlocksMinedByAll(miner ...address.Address) ChainPredicate {
 	return func(ts *types.TipSet) bool {
+		seen := make([]bool, len(miner))
+		var done int
 		for _, b := range ts.Blocks() {
-			if b.Miner == miner {
-				return true
+			for i, m := range miner {
+				if b.Miner != m || seen[i] {
+					continue
+				}
+				seen[i] = true
+				if done++; done == len(miner) {
+					return true
+				}
 			}
 		}
 		return false