Merge pull request #4350 from filecoin-project/asr/improve-fork

Avoid having the same message CID show up in execution traces

chain/stmgr/forks.go

@@ -168,7 +168,7 @@ func (sm *StateManager) hasExpensiveFork(ctx context.Context, height abi.ChainEp
 	return ok
 }
 
-func doTransfer(cb ExecCallback, tree types.StateTree, from, to address.Address, amt abi.TokenAmount) error {
+func doTransfer(tree types.StateTree, from, to address.Address, amt abi.TokenAmount, cb func(trace types.ExecutionTrace)) error {
 	fromAct, err := tree.GetActor(from)
 	if err != nil {
 		return xerrors.Errorf("failed to get 'from' actor for transfer: %w", err)
@@ -201,7 +201,6 @@ func doTransfer(cb ExecCallback, tree types.StateTree, from, to address.Address,
 			From:  from,
 			To:    to,
 			Value: amt,
-			Nonce: math.MaxUint64,
 		}
 		fakeRct := &types.MessageReceipt{
 			ExitCode: 0,
@@ -209,22 +208,14 @@ func doTransfer(cb ExecCallback, tree types.StateTree, from, to address.Address,
 			GasUsed:  0,
 		}
 
-		if err := cb(fakeMsg.Cid(), fakeMsg, &vm.ApplyRet{
+		cb(types.ExecutionTrace{
-			MessageReceipt: *fakeRct,
+			Msg:        fakeMsg,
-			ActorErr:       nil,
+			MsgRct:     fakeRct,
-			ExecutionTrace: types.ExecutionTrace{
+			Error:      "",
-				Msg:        fakeMsg,
+			Duration:   0,
-				MsgRct:     fakeRct,
+			GasCharges: nil,
-				Error:      "",
+			Subcalls:   nil,
-				Duration:   0,
+		})
-				GasCharges: nil,
-				Subcalls:   nil,
-			},
-			Duration: 0,
-			GasCosts: vm.ZeroGasOutputs(),
-		}); err != nil {
-			return xerrors.Errorf("recording transfer: %w", err)
-		}
 	}
 
 	return nil
@@ -277,6 +268,10 @@ func UpgradeFaucetBurnRecovery(ctx context.Context, sm *StateManager, cb ExecCal
 	}
 
 	var transfers []transfer
+	subcalls := make([]types.ExecutionTrace, 0)
+	transferCb := func(trace types.ExecutionTrace) {
+		subcalls = append(subcalls, trace)
+	}
 
 	// Take all excess funds away, put them into the reserve account
 	err = tree.ForEach(func(addr address.Address, act *types.Actor) error {
@@ -312,11 +307,13 @@ func UpgradeFaucetBurnRecovery(ctx context.Context, sm *StateManager, cb ExecCal
 				available = st.GetAvailableBalance(act.Balance)
 			}
 
-			transfers = append(transfers, transfer{
+			if !available.IsZero() {
-				From: addr,
+				transfers = append(transfers, transfer{
-				To:   builtin.ReserveAddress,
+					From: addr,
-				Amt:  available,
+					To:   builtin.ReserveAddress,
-			})
+					Amt:  available,
+				})
+			}
 		}
 		return nil
 	})
@@ -326,7 +323,7 @@ func UpgradeFaucetBurnRecovery(ctx context.Context, sm *StateManager, cb ExecCal
 
 	// Execute transfers from previous step
 	for _, t := range transfers {
-		if err := doTransfer(cb, tree, t.From, t.To, t.Amt); err != nil {
+		if err := doTransfer(tree, t.From, t.To, t.Amt, transferCb); err != nil {
 			return cid.Undef, xerrors.Errorf("transfer %s %s->%s failed: %w", t.Amt, t.From, t.To, err)
 		}
 	}
@@ -429,7 +426,7 @@ func UpgradeFaucetBurnRecovery(ctx context.Context, sm *StateManager, cb ExecCal
 	}
 
 	for _, t := range transfersBack {
-		if err := doTransfer(cb, tree, t.From, t.To, t.Amt); err != nil {
+		if err := doTransfer(tree, t.From, t.To, t.Amt, transferCb); err != nil {
 			return cid.Undef, xerrors.Errorf("transfer %s %s->%s failed: %w", t.Amt, t.From, t.To, err)
 		}
 	}
@@ -439,7 +436,7 @@ func UpgradeFaucetBurnRecovery(ctx context.Context, sm *StateManager, cb ExecCal
 	if err != nil {
 		return cid.Undef, xerrors.Errorf("failed to load burnt funds actor: %w", err)
 	}
-	if err := doTransfer(cb, tree, builtin0.BurntFundsActorAddr, builtin.ReserveAddress, burntAct.Balance); err != nil {
+	if err := doTransfer(tree, builtin0.BurntFundsActorAddr, builtin.ReserveAddress, burntAct.Balance, transferCb); err != nil {
 		return cid.Undef, xerrors.Errorf("failed to unburn funds: %w", err)
 	}
 
@@ -455,7 +452,7 @@ func UpgradeFaucetBurnRecovery(ctx context.Context, sm *StateManager, cb ExecCal
 	}
 
 	difference := types.BigSub(DesiredReimbursementBalance, reimb.Balance)
-	if err := doTransfer(cb, tree, builtin.ReserveAddress, reimbAddr, difference); err != nil {
+	if err := doTransfer(tree, builtin.ReserveAddress, reimbAddr, difference, transferCb); err != nil {
 		return cid.Undef, xerrors.Errorf("failed to top up reimbursement account: %w", err)
 	}
 
@@ -474,6 +471,39 @@ func UpgradeFaucetBurnRecovery(ctx context.Context, sm *StateManager, cb ExecCal
 		return cid.Undef, xerrors.Errorf("resultant state tree account balance was not correct: %s", total)
 	}
 
+	if cb != nil {
+		// record the transfer in execution traces
+
+		fakeMsg := &types.Message{
+			From:  builtin.SystemActorAddr,
+			To:    builtin.SystemActorAddr,
+			Value: big.Zero(),
+			Nonce: uint64(epoch),
+		}
+		fakeRct := &types.MessageReceipt{
+			ExitCode: 0,
+			Return:   nil,
+			GasUsed:  0,
+		}
+
+		if err := cb(fakeMsg.Cid(), fakeMsg, &vm.ApplyRet{
+			MessageReceipt: *fakeRct,
+			ActorErr:       nil,
+			ExecutionTrace: types.ExecutionTrace{
+				Msg:        fakeMsg,
+				MsgRct:     fakeRct,
+				Error:      "",
+				Duration:   0,
+				GasCharges: nil,
+				Subcalls:   subcalls,
+			},
+			Duration: 0,
+			GasCosts: vm.ZeroGasOutputs(),
+		}); err != nil {
+			return cid.Undef, xerrors.Errorf("recording transfers: %w", err)
+		}
+	}
+
 	return tree.Flush(ctx)
 }
 
@@ -514,12 +544,12 @@ func UpgradeIgnition(ctx context.Context, sm *StateManager, cb ExecCallback, roo
 		return cid.Undef, xerrors.Errorf("resetting genesis msig start epochs: %w", err)
 	}
 
-	err = splitGenesisMultisig(ctx, cb, split1, store, tree, 50)
+	err = splitGenesisMultisig(ctx, cb, split1, store, tree, 50, epoch)
 	if err != nil {
 		return cid.Undef, xerrors.Errorf("splitting first msig: %w", err)
 	}
 
-	err = splitGenesisMultisig(ctx, cb, split2, store, tree, 50)
+	err = splitGenesisMultisig(ctx, cb, split2, store, tree, 50, epoch)
 	if err != nil {
 		return cid.Undef, xerrors.Errorf("splitting second msig: %w", err)
 	}
@@ -645,7 +675,7 @@ func setNetworkName(ctx context.Context, store adt.Store, tree *state.StateTree,
 	return nil
 }
 
-func splitGenesisMultisig(ctx context.Context, cb ExecCallback, addr address.Address, store adt0.Store, tree *state.StateTree, portions uint64) error {
+func splitGenesisMultisig(ctx context.Context, cb ExecCallback, addr address.Address, store adt0.Store, tree *state.StateTree, portions uint64, epoch abi.ChainEpoch) error {
 	if portions < 1 {
 		return xerrors.Errorf("cannot split into 0 portions")
 	}
@@ -714,6 +744,11 @@ func splitGenesisMultisig(ctx context.Context, cb ExecCallback, addr address.Add
 	}
 
 	i := uint64(0)
+	subcalls := make([]types.ExecutionTrace, 0, portions)
+	transferCb := func(trace types.ExecutionTrace) {
+		subcalls = append(subcalls, trace)
+	}
+
 	for i < portions {
 		keyAddr, err := makeKeyAddr(addr, i)
 		if err != nil {
@@ -730,13 +765,46 @@ func splitGenesisMultisig(ctx context.Context, cb ExecCallback, addr address.Add
 			return xerrors.Errorf("setting new msig actor state: %w", err)
 		}
 
-		if err := doTransfer(cb, tree, addr, idAddr, newIbal); err != nil {
+		if err := doTransfer(tree, addr, idAddr, newIbal, transferCb); err != nil {
 			return xerrors.Errorf("transferring split msig balance: %w", err)
 		}
 
 		i++
 	}
 
+	if cb != nil {
+		// record the transfer in execution traces
+
+		fakeMsg := &types.Message{
+			From:  builtin.SystemActorAddr,
+			To:    addr,
+			Value: big.Zero(),
+			Nonce: uint64(epoch),
+		}
+		fakeRct := &types.MessageReceipt{
+			ExitCode: 0,
+			Return:   nil,
+			GasUsed:  0,
+		}
+
+		if err := cb(fakeMsg.Cid(), fakeMsg, &vm.ApplyRet{
+			MessageReceipt: *fakeRct,
+			ActorErr:       nil,
+			ExecutionTrace: types.ExecutionTrace{
+				Msg:        fakeMsg,
+				MsgRct:     fakeRct,
+				Error:      "",
+				Duration:   0,
+				GasCharges: nil,
+				Subcalls:   subcalls,
+			},
+			Duration: 0,
+			GasCosts: vm.ZeroGasOutputs(),
+		}); err != nil {
+			return xerrors.Errorf("recording transfers: %w", err)
+		}
+	}
+
 	return nil
 }
 

chain/stmgr/stmgr.go

@@ -247,17 +247,12 @@ func (sm *StateManager) ApplyBlocks(ctx context.Context, parentEpoch abi.ChainEp
 		return cid.Undef, cid.Undef, xerrors.Errorf("making vm: %w", err)
 	}
 
-	runCron := func() error {
+	runCron := func(epoch abi.ChainEpoch) error {
-		// TODO: this nonce-getting is a tiny bit ugly
-		ca, err := vmi.StateTree().GetActor(builtin0.SystemActorAddr)
-		if err != nil {
-			return err
-		}
 
 		cronMsg := &types.Message{
 			To:         builtin0.CronActorAddr,
 			From:       builtin0.SystemActorAddr,
-			Nonce:      ca.Nonce,
+			Nonce:      uint64(epoch),
 			Value:      types.NewInt(0),
 			GasFeeCap:  types.NewInt(0),
 			GasPremium: types.NewInt(0),
@@ -284,7 +279,7 @@ func (sm *StateManager) ApplyBlocks(ctx context.Context, parentEpoch abi.ChainEp
 	for i := parentEpoch; i < epoch; i++ {
 		if i > parentEpoch {
 			// run cron for null rounds if any
-			if err := runCron(); err != nil {
+			if err := runCron(i); err != nil {
 				return cid.Undef, cid.Undef, err
 			}
 
@@ -350,15 +345,10 @@ func (sm *StateManager) ApplyBlocks(ctx context.Context, parentEpoch abi.ChainEp
 			return cid.Undef, cid.Undef, xerrors.Errorf("failed to serialize award params: %w", err)
 		}
 
-		sysAct, actErr := vmi.StateTree().GetActor(builtin0.SystemActorAddr)
-		if actErr != nil {
-			return cid.Undef, cid.Undef, xerrors.Errorf("failed to get system actor: %w", actErr)
-		}
-
 		rwMsg := &types.Message{
 			From:       builtin0.SystemActorAddr,
 			To:         reward.Address,
-			Nonce:      sysAct.Nonce,
+			Nonce:      uint64(epoch),
 			Value:      types.NewInt(0),
 			GasFeeCap:  types.NewInt(0),
 			GasPremium: types.NewInt(0),
@@ -381,7 +371,7 @@ func (sm *StateManager) ApplyBlocks(ctx context.Context, parentEpoch abi.ChainEp
 		}
 	}
 
-	if err := runCron(); err != nil {
+	if err := runCron(epoch); err != nil {
 		return cid.Cid{}, cid.Cid{}, err
 	}