fix: ETH RPC API: ETH Call should use the parent state root of the su…

…bsequent tipset (filecoin-project#11905) * fix eth call * tests * changes as per review * changes as per review * Update node/impl/full/eth.go Co-authored-by: Rod Vagg <[email protected]> * fix as per review --------- Co-authored-by: Rod Vagg <[email protected]>
ribasushi · Jun 15, 2024 · d0a9e6d · d0a9e6d
1 parent 462eda8
commit d0a9e6d
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 49 deletions.
diff --git a/chain/stmgr/call.go b/chain/stmgr/call.go
@@ -25,6 +25,14 @@ import (
 	"github.com/filecoin-project/lotus/chain/vm"
 )
 
+type execMessageStrategy int
+
+const (
+	execNoMessages         execMessageStrategy = iota // apply no prior or current tipset messages
+	execAllMessages                                   // apply all prior and current tipset messages
+	execSameSenderMessages                            // apply all prior messages and any current tipset messages from the same sender
+)
+
 var ErrExpensiveFork = errors.New("refusing explicit call due to state fork at epoch")
 
 // Call applies the given message to the given tipset's parent state, at the epoch following the
@@ -48,12 +56,24 @@ func (sm *StateManager) Call(ctx context.Context, msg *types.Message, ts *types.
 		msg.Value = types.NewInt(0)
 	}
 
-	return sm.callInternal(ctx, msg, nil, ts, cid.Undef, sm.GetNetworkVersion, false, false)
+	return sm.callInternal(ctx, msg, nil, ts, cid.Undef, sm.GetNetworkVersion, false, execSameSenderMessages)
+}
+
+// ApplyOnStateWithGas applies the given message on top of the given state root with gas tracing enabled
+func (sm *StateManager) ApplyOnStateWithGas(ctx context.Context, stateCid cid.Cid, msg *types.Message, ts *types.TipSet) (*api.InvocResult, error) {
+	return sm.callInternal(ctx, msg, nil, ts, stateCid, sm.GetNetworkVersion, true, execNoMessages)
 }
 
 // CallWithGas calculates the state for a given tipset, and then applies the given message on top of that state.
 func (sm *StateManager) CallWithGas(ctx context.Context, msg *types.Message, priorMsgs []types.ChainMsg, ts *types.TipSet, applyTsMessages bool) (*api.InvocResult, error) {
-	return sm.callInternal(ctx, msg, priorMsgs, ts, cid.Undef, sm.GetNetworkVersion, true, applyTsMessages)
+	var strategy execMessageStrategy
+	if applyTsMessages {
+		strategy = execAllMessages
+	} else {
+		strategy = execSameSenderMessages
+	}
+
+	return sm.callInternal(ctx, msg, priorMsgs, ts, cid.Undef, sm.GetNetworkVersion, true, strategy)
 }
 
 // CallAtStateAndVersion allows you to specify a message to execute on the given stateCid and network version.
@@ -64,14 +84,14 @@ func (sm *StateManager) CallAtStateAndVersion(ctx context.Context, msg *types.Me
 	nvGetter := func(context.Context, abi.ChainEpoch) network.Version {
 		return v
 	}
-
-	return sm.callInternal(ctx, msg, nil, nil, stateCid, nvGetter, true, false)
+	return sm.callInternal(ctx, msg, nil, nil, stateCid, nvGetter, true, execSameSenderMessages)
 }
 
 //   - If no tipset is specified, the first tipset without an expensive migration or one in its parent is used.
 //   - If executing a message at a given tipset or its parent would trigger an expensive migration, the call will
 //     fail with ErrExpensiveFork.
-func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, priorMsgs []types.ChainMsg, ts *types.TipSet, stateCid cid.Cid, nvGetter rand.NetworkVersionGetter, checkGas, applyTsMessages bool) (*api.InvocResult, error) {
+func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, priorMsgs []types.ChainMsg, ts *types.TipSet, stateCid cid.Cid,
+	nvGetter rand.NetworkVersionGetter, checkGas bool, strategy execMessageStrategy) (*api.InvocResult, error) {
 	ctx, span := trace.StartSpan(ctx, "statemanager.callInternal")
 	defer span.End()
 
@@ -95,7 +115,7 @@ func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, pr
 				return nil, xerrors.Errorf("failed to find a non-forking epoch: %w", err)
 			}
 			// Checks for expensive forks from the parents to the tipset, including nil tipsets
-			if !sm.hasExpensiveForkBetween(pts.Height(), ts.Height()+1) {
+			if !sm.HasExpensiveForkBetween(pts.Height(), ts.Height()+1) {
 				break
 			}
 
@@ -106,7 +126,7 @@ func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, pr
 		if err != nil {
 			return nil, xerrors.Errorf("failed to find a non-forking epoch: %w", err)
 		}
-		if sm.hasExpensiveForkBetween(pts.Height(), ts.Height()+1) {
+		if sm.HasExpensiveForkBetween(pts.Height(), ts.Height()+1) {
 			return nil, ErrExpensiveFork
 		}
 	}
@@ -117,24 +137,6 @@ func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, pr
 	if stateCid == cid.Undef {
 		stateCid = ts.ParentState()
 	}
-	tsMsgs, err := sm.cs.MessagesForTipset(ctx, ts)
-	if err != nil {
-		return nil, xerrors.Errorf("failed to lookup messages for parent tipset: %w", err)
-	}
-
-	if applyTsMessages {
-		priorMsgs = append(tsMsgs, priorMsgs...)
-	} else {
-		var filteredTsMsgs []types.ChainMsg
-		for _, tsMsg := range tsMsgs {
-			//TODO we should technically be normalizing the filecoin address of from when we compare here
-			if tsMsg.VMMessage().From == msg.VMMessage().From {
-				filteredTsMsgs = append(filteredTsMsgs, tsMsg)
-			}
-		}
-		priorMsgs = append(filteredTsMsgs, priorMsgs...)
-	}
-
 	// Technically, the tipset we're passing in here should be ts+1, but that may not exist.
 	stateCid, err = sm.HandleStateForks(ctx, stateCid, ts.Height(), nil, ts)
 	if err != nil {
@@ -169,18 +171,40 @@ func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, pr
 	if err != nil {
 		return nil, xerrors.Errorf("failed to set up vm: %w", err)
 	}
-	for i, m := range priorMsgs {
-		_, err = vmi.ApplyMessage(ctx, m)
+
+	switch strategy {
+	case execNoMessages:
+		// Do nothing
+	case execAllMessages, execSameSenderMessages:
+		tsMsgs, err := sm.cs.MessagesForTipset(ctx, ts)
 		if err != nil {
-			return nil, xerrors.Errorf("applying prior message (%d, %s): %w", i, m.Cid(), err)
+			return nil, xerrors.Errorf("failed to lookup messages for parent tipset: %w", err)
+		}
+		if strategy == execAllMessages {
+			priorMsgs = append(tsMsgs, priorMsgs...)
+		} else if strategy == execSameSenderMessages {
+			var filteredTsMsgs []types.ChainMsg
+			for _, tsMsg := range tsMsgs {
+				//TODO we should technically be normalizing the filecoin address of from when we compare here
+				if tsMsg.VMMessage().From == msg.VMMessage().From {
+					filteredTsMsgs = append(filteredTsMsgs, tsMsg)
+				}
+			}
+			priorMsgs = append(filteredTsMsgs, priorMsgs...)
+		}
+		for i, m := range priorMsgs {
+			_, err = vmi.ApplyMessage(ctx, m)
+			if err != nil {
+				return nil, xerrors.Errorf("applying prior message (%d, %s): %w", i, m.Cid(), err)
+			}
 		}
-	}
 
-	// We flush to get the VM's view of the state tree after applying the above messages
-	// This is needed to get the correct nonce from the actor state to match the VM
-	stateCid, err = vmi.Flush(ctx)
-	if err != nil {
-		return nil, xerrors.Errorf("flushing vm: %w", err)
+		// We flush to get the VM's view of the state tree after applying the above messages
+		// This is needed to get the correct nonce from the actor state to match the VM
+		stateCid, err = vmi.Flush(ctx)
+		if err != nil {
+			return nil, xerrors.Errorf("flushing vm: %w", err)
+		}
 	}
 
 	stTree, err := state.LoadStateTree(cbor.NewCborStore(buffStore), stateCid)

diff --git a/chain/stmgr/forks.go b/chain/stmgr/forks.go
@@ -227,7 +227,7 @@ func (sm *StateManager) HandleStateForks(ctx context.Context, root cid.Cid, heig
 // Returns true executing tipsets between the specified heights would trigger an expensive
 // migration. NOTE: migrations occurring _at_ the target height are not included, as they're
 // executed _after_ the target height.
-func (sm *StateManager) hasExpensiveForkBetween(parent, height abi.ChainEpoch) bool {
+func (sm *StateManager) HasExpensiveForkBetween(parent, height abi.ChainEpoch) bool {
 	for h := parent; h < height; h++ {
 		if _, ok := sm.expensiveUpgrades[h]; ok {
 			return true

diff --git a/node/impl/full/eth.go b/node/impl/full/eth.go
@@ -1028,25 +1028,26 @@ func (a *EthModule) applyMessage(ctx context.Context, msg *types.Message, tsk ty
 		return nil, xerrors.Errorf("cannot get tipset: %w", err)
 	}
 
-	applyTsMessages := true
-	if os.Getenv("LOTUS_SKIP_APPLY_TS_MESSAGE_CALL_WITH_GAS") == "1" {
-		applyTsMessages = false
-	}
-
-	// Try calling until we find a height with no migration.
-	for {
-		res, err = a.StateManager.CallWithGas(ctx, msg, []types.ChainMsg{}, ts, applyTsMessages)
-		if err != stmgr.ErrExpensiveFork {
-			break
-		}
-		ts, err = a.Chain.GetTipSetFromKey(ctx, ts.Parents())
+	if ts.Height() > 0 {
+		pts, err := a.Chain.GetTipSetFromKey(ctx, ts.Parents())
 		if err != nil {
-			return nil, xerrors.Errorf("getting parent tipset: %w", err)
+			return nil, xerrors.Errorf("failed to find a non-forking epoch: %w", err)
+		}
+		// Check for expensive forks from the parents to the tipset, including nil tipsets
+		if a.StateManager.HasExpensiveForkBetween(pts.Height(), ts.Height()+1) {
+			return nil, stmgr.ErrExpensiveFork
 		}
 	}
+
+	st, _, err := a.StateManager.TipSetState(ctx, ts)
+	if err != nil {
+		return nil, xerrors.Errorf("cannot get tipset state: %w", err)
+	}
+	res, err = a.StateManager.ApplyOnStateWithGas(ctx, st, msg, ts)
 	if err != nil {
-		return nil, xerrors.Errorf("CallWithGas failed: %w", err)
+		return nil, xerrors.Errorf("ApplyWithGasOnState failed: %w", err)
 	}
+
 	if res.MsgRct.ExitCode.IsError() {
 		reason := parseEthRevert(res.MsgRct.Return)
 		return nil, xerrors.Errorf("message execution failed: exit %s, revert reason: %s, vm error: %s", res.MsgRct.ExitCode, reason, res.Error)