Merge pull request #1523 from filecoin-project/feat/safe-chain-weights

Limit Weight Precision #1483

api/impl/mining.go

@@ -39,18 +39,18 @@ func (api *nodeMining) Once(ctx context.Context) (*types.Block, error) {
 	getState := func(ctx context.Context, ts consensus.TipSet) (state.Tree, error) {
 		return getStateByKey(ctx, ts.String())
 	}
-	getWeight := func(ctx context.Context, ts consensus.TipSet) (uint64, uint64, error) {
+	getWeight := func(ctx context.Context, ts consensus.TipSet) (uint64, error) {
 		parent, err := ts.Parents()
 		if err != nil {
-			return uint64(0), uint64(0), err
+			return uint64(0), err
 		}
 		// TODO handle genesis cid more gracefully
 		if parent.Len() == 0 {
 			return nd.Consensus.Weight(ctx, ts, nil)
 		}
 		pSt, err := getStateByKey(ctx, parent.String())
 		if err != nil {
-			return uint64(0), uint64(0), err
+			return uint64(0), err
 		}
 		return nd.Consensus.Weight(ctx, ts, pSt)
 	}

chain/default_syncer.go

@@ -232,26 +232,15 @@ func (syncer *DefaultSyncer) syncOne(ctx context.Context, parent, next consensus
 		}
 	}
 
-	cmp, err := syncer.consensus.IsHeavier(ctx, next, syncer.chainStore.Head(), nextParentSt, headParentSt)
-	if err != nil {
-		return err
-	}
-	nextH, err := next.Height()
-	if err != nil {
-		return err
-	}
-	headH, err := syncer.chainStore.Head().Height()
+	heavier, err := syncer.consensus.IsHeavier(ctx, next, syncer.chainStore.Head(), nextParentSt, headParentSt)
 	if err != nil {
 		return err
 	}
 
-	if cmp > 0 {
-		logSyncer.Debugf("New TS %s (h=%d) is new heaviest over %s (h=%d), update head", next.String(), nextH, syncer.chainStore.Head(), headH)
+	if heavier {
 		if err = syncer.chainStore.SetHead(ctx, next); err != nil {
 			return err
 		}
-	} else {
-		logSyncer.Debugf("New TS %s (h=%d) is not heavier than %s (h=%d), no head update", next.String(), nextH, syncer.chainStore.Head().String(), headH)
 	}
 
 	return nil

chain/default_syncer_test.go

@@ -738,11 +738,10 @@ func TestTipSetWeightDeep(t *testing.T) {
 	//  w({f1b1, f2b1})   = sw + 0   + 11 * 2  = sw + 22
 	//  w({f1b2a, f1b2b}) = sw + 11  + 11 * 2  = sw + 33
 	//  w({f2b2})         = sw + 11  + 108 	   = sw + 119
-	startingWeightN, startingWeightD, err := con.Weight(ctx, baseTS, pSt)
+	startingWeight, err := con.Weight(ctx, baseTS, pSt)
 	require.NoError(err)
-	require.Equal(uint64(1), startingWeightD)
 
-	wFun := func(ts consensus.TipSet) (uint64, uint64, error) {
+	wFun := func(ts consensus.TipSet) (uint64, error) {
 		// No power-altering messages processed from here on out.
 		// And so bootstrapSt correctly retrives power table for all
 		// test blocks.
@@ -759,10 +758,9 @@ func TestTipSetWeightDeep(t *testing.T) {
 	err = syncer.HandleNewBlocks(ctx, sharedCids)
 	require.NoError(err)
 	assertHead(assert, chain, tsShared)
-	measuredWeight, denom, err := wFun(chain.Head())
+	measuredWeight, err := wFun(chain.Head())
 	require.NoError(err)
-	require.Equal(uint64(1), denom)
-	expectedWeight := startingWeightN + uint64(22)
+	expectedWeight := startingWeight + uint64(22000)
 	assert.Equal(expectedWeight, measuredWeight)
 
 	// fork 1 is heavier than the old head.
@@ -775,10 +773,9 @@ func TestTipSetWeightDeep(t *testing.T) {
 	err = syncer.HandleNewBlocks(ctx, f1Cids)
 	require.NoError(err)
 	assertHead(assert, chain, f1)
-	measuredWeight, denom, err = wFun(chain.Head())
+	measuredWeight, err = wFun(chain.Head())
 	require.NoError(err)
-	require.Equal(uint64(1), denom)
-	expectedWeight = startingWeightN + uint64(33)
+	expectedWeight = startingWeight + uint64(33000)
 	assert.Equal(expectedWeight, measuredWeight)
 
 	// fork 2 has heavier weight because of addr3's power even though there
@@ -790,9 +787,8 @@ func TestTipSetWeightDeep(t *testing.T) {
 	err = syncer.HandleNewBlocks(ctx, f2Cids)
 	require.NoError(err)
 	assertHead(assert, chain, f2)
-	measuredWeight, denom, err = wFun(chain.Head())
+	measuredWeight, err = wFun(chain.Head())
 	require.NoError(err)
-	require.Equal(uint64(1), denom)
-	expectedWeight = startingWeightN + uint64(119)
+	expectedWeight = startingWeight + uint64(119000)
 	assert.Equal(expectedWeight, measuredWeight)
 }

chain/testing.go

@@ -53,17 +53,17 @@ func MkFakeChild(parent consensus.TipSet, genCid cid.Cid, stateRoot cid.Cid, non
 
 // MkFakeChildWithCon creates a chain with the given consensus weight function.
 func MkFakeChildWithCon(parent consensus.TipSet, genCid cid.Cid, stateRoot cid.Cid, nonce uint64, nullBlockCount uint64, con consensus.Protocol) (*types.Block, error) {
-	wFun := func(ts consensus.TipSet) (uint64, uint64, error) {
+	wFun := func(ts consensus.TipSet) (uint64, error) {
 		return con.Weight(context.Background(), parent, nil)
 	}
 	return MkFakeChildCore(parent, genCid, stateRoot, nonce, nullBlockCount, wFun)
 }
 
 // MkFakeChildCore houses shared functionality between MkFakeChildWithCon and MkFakeChild.
-func MkFakeChildCore(parent consensus.TipSet, genCid cid.Cid, stateRoot cid.Cid, nonce uint64, nullBlockCount uint64, wFun func(consensus.TipSet) (uint64, uint64, error)) (*types.Block, error) {
+func MkFakeChildCore(parent consensus.TipSet, genCid cid.Cid, stateRoot cid.Cid, nonce uint64, nullBlockCount uint64, wFun func(consensus.TipSet) (uint64, error)) (*types.Block, error) {
 	// State can be nil because it doesn't it is assumed consensus uses a
 	// power table view that does not access the state.
-	nW, dW, err := wFun(parent)
+	w, err := wFun(parent)
 	if err != nil {
 		return nil, err
 	}
@@ -82,12 +82,11 @@ func MkFakeChildCore(parent consensus.TipSet, genCid cid.Cid, stateRoot cid.Cid,
 	}
 
 	return &types.Block{
-		Parents:           pIDs,
-		Height:            types.Uint64(height),
-		ParentWeightNum:   types.Uint64(nW),
-		ParentWeightDenom: types.Uint64(dW),
-		Nonce:             types.Uint64(nonce),
-		StateRoot:         stateRoot,
+		Parents:      pIDs,
+		Height:       types.Uint64(height),
+		ParentWeight: types.Uint64(w),
+		Nonce:        types.Uint64(nonce),
+		StateRoot:    stateRoot,
 	}, nil
 }
 
@@ -108,7 +107,7 @@ func RequireMkFakeChildWithCon(require *require.Assertions, parent consensus.Tip
 
 // RequireMkFakeChildCore wraps MkFakeChildCore with a requirement that
 // it does not errror.
-func RequireMkFakeChildCore(require *require.Assertions, parent consensus.TipSet, genCid cid.Cid, stateRoot cid.Cid, nonce uint64, nullBlockCount uint64, wFun func(consensus.TipSet) (uint64, uint64, error)) *types.Block {
+func RequireMkFakeChildCore(require *require.Assertions, parent consensus.TipSet, genCid cid.Cid, stateRoot cid.Cid, nonce uint64, nullBlockCount uint64, wFun func(consensus.TipSet) (uint64, error)) *types.Block {
 	child, err := MkFakeChildCore(parent, genCid, stateRoot, nonce, nullBlockCount, wFun)
 	require.NoError(err)
 	return child
@@ -225,7 +224,7 @@ func RequireMineOnce(ctx context.Context, t *testing.T, syncer Syncer, cst *hamt
 	// Sync the block.
 	c, err := cst.Put(ctx, b)
 	require.NoError(err)
-	fmt.Printf("new block parent weight num: %v, parent weight den: %v\n", b.ParentWeightNum, b.ParentWeightDenom)
+	fmt.Printf("new block parent weight: %v\n", b.ParentWeight)
 	err = syncer.HandleNewBlocks(ctx, []cid.Cid{c})
 	require.NoError(err)
 

commands/block.go

@@ -24,7 +24,7 @@ var showCmd = &cmds.Command{
 var showBlockCmd = &cmds.Command{
 	Helptext: cmdkit.HelpText{
 		Tagline: "Show a filecoin block by its CID",
-		ShortDescription: `Prints the miner, parent weight numerator, parent weight denominator, height,
+		ShortDescription: `Prints the miner, parent weight, height,
 and nonce of a given block. If JSON encoding is specified with the --enc flag,
 all other block properties will be included as well.`,
 	},
@@ -47,16 +47,19 @@ all other block properties will be included as well.`,
 	Type: types.Block{},
 	Encoders: cmds.EncoderMap{
 		cmds.Text: cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, block *types.Block) error {
-			_, err := fmt.Fprintf(w, `Block Details
-Miner:       %s
-Numerator:   %s
-Denominator: %s
-Height:      %s
-Nonce:       %s
+			wStr, err := types.FixedStr(uint64(block.ParentWeight))
+			if err != nil {
+				return err
+			}
+
+			_, err = fmt.Fprintf(w, `Block Details
+Miner:  %s
+Weight: %s
+Height: %s
+Nonce:  %s
 `,
 				block.Miner,
-				strconv.FormatUint(uint64(block.ParentWeightNum), 10),
-				strconv.FormatUint(uint64(block.ParentWeightDenom), 10),
+				wStr,
 				strconv.FormatUint(uint64(block.Height), 10),
 				strconv.FormatUint(uint64(block.Nonce), 10),
 			)

commands/block_daemon_test.go

@@ -27,10 +27,9 @@ func TestBlockDaemon(t *testing.T) {
 		output := d.RunSuccess("show", "block", minedBlockCidStr).ReadStdoutTrimNewlines()
 
 		assert.Contains(output, "Block Details")
-		assert.Contains(output, "Numerator:   0")
-		assert.Contains(output, "Denominator: 1")
-		assert.Contains(output, "Height:      1")
-		assert.Contains(output, "Nonce:       0")
+		assert.Contains(output, "Weight: 0")
+		assert.Contains(output, "Height: 1")
+		assert.Contains(output, "Nonce:  0")
 	})
 
 	t.Run("show block <cid-of-genesis-block> --enc json returns JSON for a filecoin block", func(t *testing.T) {

commands/schema/filecoin_block.schema.json

@@ -44,10 +44,7 @@
             "null"
           ]
         },
-        "parentWeightNumerator": {
-          "type": "string"
-        },
-        "parentWeightDenominator": {
+        "parentWeight": {
           "type": "string"
         },
         "proof": {

consensus/expected.go

@@ -113,110 +113,94 @@ func (c *Expected) validateBlockStructure(ctx context.Context, b *types.Block) e
 	return nil
 }
 
-// weight returns the EC weight of the given tipset in a format (big Rational)
-// suitable for internal use in the consensus package.
-// TODO: this implementation needs to handle precision of long chains correctly,
-// see issue #655.
-func (c *Expected) weight(ctx context.Context, ts TipSet, pSt state.Tree) (*big.Rat, error) {
-	ctx = log.Start(ctx, "Expected.weight")
+// Weight returns the EC weight of this TipSet in uint64 encoded fixed point
+// representation.
+func (c *Expected) Weight(ctx context.Context, ts TipSet, pSt state.Tree) (uint64, error) {
+	ctx = log.Start(ctx, "Expected.Weight")
 	log.LogKV(ctx, "Weight", ts.String())
 	if len(ts) == 1 && ts.ToSlice()[0].Cid().Equals(c.genesisCid) {
-		return big.NewRat(int64(0), int64(1)), nil
+		return uint64(0), nil
 	}
 	// Compute parent weight.
-	wNum, wDenom, err := ts.ParentWeight()
+	parentW, err := ts.ParentWeight()
 	if err != nil {
-		return nil, err
-	}
-	if wDenom == uint64(0) {
-		return nil, errors.New("storage market with 0 bytes stored not handled")
+		return uint64(0), err
 	}
-	w := big.NewRat(int64(wNum), int64(wDenom))
 
+	w, err := types.FixedToBig(parentW)
+	if err != nil {
+		return uint64(0), err
+	}
 	// Each block in the tipset adds ECV + ECPrm * miner_power to parent weight.
 	totalBytes, err := c.PwrTableView.Total(ctx, pSt, c.bstore)
 	if err != nil {
-		return nil, err
+		return uint64(0), err
 	}
-	ratECV := big.NewRat(int64(ECV), int64(1))
+	floatTotalBytes := new(big.Float).SetInt64(int64(totalBytes))
+	floatECV := new(big.Float).SetInt64(int64(ECV))
+	floatECPrM := new(big.Float).SetInt64(int64(ECPrM))
 	for _, blk := range ts.ToSlice() {
 		minerBytes, err := c.PwrTableView.Miner(ctx, pSt, c.bstore, blk.Miner)
 		if err != nil {
-			return nil, err
+			return uint64(0), err
 		}
-		wNumBlk := int64(ECPrM * minerBytes)
-		wBlk := big.NewRat(wNumBlk, int64(totalBytes)) // power added for each block
-		wBlk.Add(wBlk, ratECV)                         // constant added for each block
+		floatOwnBytes := new(big.Float).SetInt64(int64(minerBytes))
+		wBlk := new(big.Float)
+		wBlk.Quo(floatOwnBytes, floatTotalBytes)
+		wBlk.Mul(wBlk, floatECPrM) // Power addition
+		wBlk.Add(wBlk, floatECV)   // Constant addition
 		w.Add(w, wBlk)
 	}
-	return w, nil
+	return types.BigToFixed(w)
 }
 
-// Weight returns the EC weight of this TipSet
-// TODO: this implementation needs to handle precision of long chains correctly,
-// see issue #655.
-func (c *Expected) Weight(ctx context.Context, ts TipSet, pSt state.Tree) (uint64, uint64, error) {
-	w, err := c.weight(ctx, ts, pSt)
-	if err != nil {
-		return uint64(0), uint64(0), err
-	}
-	wNum := w.Num()
-	if !wNum.IsUint64() {
-		return uint64(0), uint64(0), errors.New("weight numerator cannot be repr by uint64")
-	}
-	wDenom := w.Denom()
-	if !wDenom.IsUint64() {
-		return uint64(0), uint64(0), errors.New("weight denominator cannot be repr by uint64")
-	}
-	return wNum.Uint64(), wDenom.Uint64(), nil
-}
-
-// IsHeavier returns an integer comparing two tipsets by weight.  The
-// result will be -1 if W(a) < W(b), and 1 if W(a) > W(b).  In the rare
-// case where two tipsets have the same weight, ties are broken by taking
-// the tipset with the smallest ticket.  In the event that tickets
-// are the same, IsHeavier will break ties by comparing the concatenation
-// of block cids in the tipset.
+// IsHeavier returns true if tipset a is heavier than tipset b, and false
+// vice versa.  In the rare case where two tipsets have the same weight ties
+// are broken by taking the tipset with the smallest ticket.  In the event that
+// tickets are the same, IsHeavier will break ties by comparing the
+// concatenation of block cids in the tipset.
 // TODO BLOCK CID CONCAT TIE BREAKER IS NOT IN THE SPEC AND SHOULD BE
 // EVALUATED BEFORE GETTING TO PRODUCTION.
-func (c *Expected) IsHeavier(ctx context.Context, a, b TipSet, aSt, bSt state.Tree) (int, error) {
-	aW, err := c.weight(ctx, a, aSt)
+func (c *Expected) IsHeavier(ctx context.Context, a, b TipSet, aSt, bSt state.Tree) (bool, error) {
+	aW, err := c.Weight(ctx, a, aSt)
 	if err != nil {
-		return 0, err
+		return false, err
 	}
-	bW, err := c.weight(ctx, b, bSt)
+	bW, err := c.Weight(ctx, b, bSt)
 	if err != nil {
-		return 0, err
+		return false, err
 	}
 
 	// Without ties pass along the comparison.
-	cmp := aW.Cmp(bW)
-	if cmp != 0 {
-		return cmp, nil
+	if aW != bW {
+		return aW > bW, nil
 	}
 
 	// To break ties compare the min tickets.
 	aTicket, err := a.MinTicket()
 	if err != nil {
-		return 0, err
+		return false, err
 	}
 	bTicket, err := b.MinTicket()
 	if err != nil {
-		return 0, err
+		return false, err
 	}
 
-	cmp = bytes.Compare(bTicket, aTicket)
+	cmp := bytes.Compare(bTicket, aTicket)
 	if cmp != 0 {
-		return cmp, nil
+		// a is heavier if b's ticket is greater than a's ticket.
+		return cmp == 1, nil
 	}
 
 	// Tie break on cid ids.
+	// TODO: I think this is drastically impacted by number of blocks in tipset
+	// i.e. bigger tipset is always heavier.  Not sure if this is ok, need to revist.
 	cmp = strings.Compare(a.String(), b.String())
 	if cmp == 0 {
 		// Caller is mistakenly calling on two identical tipsets.
-		return 0, ErrUnorderedTipSets
+		return false, ErrUnorderedTipSets
 	}
-	return cmp, nil
+	return cmp == 1, nil
 }
 
 // RunStateTransition is the chain transition function that goes from a

consensus/protocol.go

@@ -29,10 +29,10 @@ type Protocol interface {
 	// blocks were mined according to protocol rules (RunStateTransition does these checks).
 	NewValidTipSet(ctx context.Context, blks []*types.Block) (TipSet, error)
 	// Weight returns the weight given to the input ts by this consensus protocol.
-	Weight(ctx context.Context, ts TipSet, pSt state.Tree) (uint64, uint64, error)
+	Weight(ctx context.Context, ts TipSet, pSt state.Tree) (uint64, error)
 	// IsHeaver returns 1 if tipset a is heavier than tipset b and -1 if
 	// tipset b is heavier than tipset a.
-	IsHeavier(ctx context.Context, a, b TipSet, aSt, bSt state.Tree) (int, error)
+	IsHeavier(ctx context.Context, a, b TipSet, aSt, bSt state.Tree) (bool, error)
 	// RunStateTransition returns the state resulting from applying the input ts to the parent
 	// state pSt.  It returns an error if the transition is invalid.
 	RunStateTransition(ctx context.Context, ts TipSet, parentTs TipSet, pSt state.Tree) (state.Tree, error)