From 5669ca0c5a3faad8684a635b46bd14ecc6d615b2 Mon Sep 17 00:00:00 2001 From: aarshkshah1992 Date: Fri, 19 Apr 2024 19:18:20 +0530 Subject: [PATCH 1/6] fix eth call --- chain/stmgr/call.go | 69 ++++++++++++++++++++++++------------------- node/impl/full/eth.go | 14 ++++----- 2 files changed, 45 insertions(+), 38 deletions(-) diff --git a/chain/stmgr/call.go b/chain/stmgr/call.go index 61056528f11..8cadca39dee 100644 --- a/chain/stmgr/call.go +++ b/chain/stmgr/call.go @@ -48,12 +48,17 @@ 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, false, false) +} + +// 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, false, true) } // 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) + return sm.callInternal(ctx, msg, priorMsgs, ts, cid.Undef, sm.GetNetworkVersion, true, applyTsMessages, false) } // CallAtStateAndVersion allows you to specify a message to execute on the given stateCid and network version. @@ -65,13 +70,13 @@ func (sm *StateManager) CallAtStateAndVersion(ctx context.Context, msg *types.Me return v } - return sm.callInternal(ctx, msg, nil, nil, stateCid, nvGetter, true, false) + return sm.callInternal(ctx, msg, nil, nil, stateCid, nvGetter, true, false, false) } // - 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, applyTsMessages bool, noReExec bool) (*api.InvocResult, error) { ctx, span := trace.StartSpan(ctx, "statemanager.callInternal") defer span.End() @@ -117,24 +122,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 +156,38 @@ 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) + + if !noReExec { + 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) } - } - // 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) + 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...) + } + 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) + } } stTree, err := state.LoadStateTree(cbor.NewCborStore(buffStore), stateCid) diff --git a/node/impl/full/eth.go b/node/impl/full/eth.go index e7aeafa9085..51b2c29384c 100644 --- a/node/impl/full/eth.go +++ b/node/impl/full/eth.go @@ -980,14 +980,14 @@ 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) + 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 != stmgr.ErrExpensiveFork { break } @@ -997,7 +997,7 @@ func (a *EthModule) applyMessage(ctx context.Context, msg *types.Message, tsk ty } } 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) From 4799463790cf8d63fd61d56e8578cfe63207280b Mon Sep 17 00:00:00 2001 From: aarshkshah1992 Date: Fri, 19 Apr 2024 21:10:52 +0530 Subject: [PATCH 2/6] tests --- chain/stmgr/call.go | 1 - 1 file changed, 1 deletion(-) diff --git a/chain/stmgr/call.go b/chain/stmgr/call.go index 8cadca39dee..9158016481c 100644 --- a/chain/stmgr/call.go +++ b/chain/stmgr/call.go @@ -69,7 +69,6 @@ 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, false) } From 187e837ab8bf8261bc704daeb42de7348c294274 Mon Sep 17 00:00:00 2001 From: aarshkshah1992 Date: Tue, 14 May 2024 14:40:38 +0530 Subject: [PATCH 3/6] changes as per review --- chain/stmgr/call.go | 36 +++++++++++++++++++++++++++--------- go.sum | 3 ++- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/chain/stmgr/call.go b/chain/stmgr/call.go index 9158016481c..a9d77d2b3c5 100644 --- 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,17 +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, 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, false, true) + 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, false) + 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. @@ -69,13 +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, 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, noReExec 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() @@ -156,15 +172,17 @@ func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, pr return nil, xerrors.Errorf("failed to set up vm: %w", err) } - if !noReExec { + switch strategy { + case execNoMessages: + // Do nothing + case execAllMessages, execSameSenderMessages: 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 { + if strategy == execAllMessages { priorMsgs = append(tsMsgs, priorMsgs...) - } else { + } 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 diff --git a/go.sum b/go.sum index 4e6ce0dec0f..cf067f71b9c 100644 --- a/go.sum +++ b/go.sum @@ -354,7 +354,6 @@ github.com/filecoin-project/go-state-types v0.0.0-20201102161440-c8033295a1fc/go github.com/filecoin-project/go-state-types v0.1.0/go.mod h1:ezYnPf0bNkTsDibL/psSz5dy4B5awOJ/E7P2Saeep8g= github.com/filecoin-project/go-state-types v0.1.6/go.mod h1:UwGVoMsULoCK+bWjEdd/xLCvLAQFBC7EDT477SKml+Q= github.com/filecoin-project/go-state-types v0.1.10/go.mod h1:UwGVoMsULoCK+bWjEdd/xLCvLAQFBC7EDT477SKml+Q= -github.com/filecoin-project/go-state-types v0.11.2-0.20230712101859-8f37624fa540/go.mod h1:SyNPwTsU7I22gL2r0OAPcImvLoTVfgRwdK/Y5rR1zz8= github.com/filecoin-project/go-state-types v0.13.1 h1:4CivvlcHAIoAtFFVVlZtokynaMQu5XLXGoTKhQkfG1I= github.com/filecoin-project/go-state-types v0.13.1/go.mod h1:cHpOPup9H1g2T29dKHAjC2sc7/Ef5ypjuW9A3I+e9yY= github.com/filecoin-project/go-statemachine v0.0.0-20200925024713-05bd7c71fbfe/go.mod h1:FGwQgZAt2Gh5mjlwJUlVB62JeYdo+if0xWxSEfBD9ig= @@ -953,6 +952,7 @@ github.com/klauspost/compress v1.17.6/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6K github.com/klauspost/cpuid/v2 v2.0.4/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/klauspost/cpuid/v2 v2.0.6/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= +github.com/klauspost/cpuid/v2 v2.2.3/go.mod h1:RVVoqg1df56z8g3pUjL/3lE5UfnlrJX8tyFgg4nqhuY= github.com/klauspost/cpuid/v2 v2.2.7 h1:ZWSB3igEs+d0qvnxR/ZBzXVmxkgt8DdzP6m9pfuVLDM= github.com/klauspost/cpuid/v2 v2.2.7/go.mod h1:Lcz8mBdAVJIBVzewtcLocK12l3Y+JytZYpaMropDUws= github.com/koalacxr/quantile v0.0.1 h1:wAW+SQ286Erny9wOjVww96t8ws+x5Zj6AKHDULUK+o0= @@ -2065,6 +2065,7 @@ golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220310020820-b874c991c1a5/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220708085239-5a0f0661e09d/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= From 7bb9fdba3aa149518842b4ee9ba0293b5bb2c756 Mon Sep 17 00:00:00 2001 From: aarshkshah1992 Date: Tue, 14 May 2024 17:05:12 +0530 Subject: [PATCH 4/6] changes as per review --- chain/stmgr/call.go | 4 ++-- chain/stmgr/forks.go | 2 +- node/impl/full/eth.go | 33 ++++++++++++++++++++------------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/chain/stmgr/call.go b/chain/stmgr/call.go index a9d77d2b3c5..7f2a57a6112 100644 --- a/chain/stmgr/call.go +++ b/chain/stmgr/call.go @@ -115,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 } @@ -126,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 } } diff --git a/chain/stmgr/forks.go b/chain/stmgr/forks.go index 9a236196187..c6c513e3a26 100644 --- 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 index 51b2c29384c..491ced9c5e8 100644 --- a/node/impl/full/eth.go +++ b/node/impl/full/eth.go @@ -980,25 +980,32 @@ func (a *EthModule) applyMessage(ctx context.Context, msg *types.Message, tsk ty return nil, xerrors.Errorf("cannot get tipset: %w", err) } - // Try calling until we find a height with no migration. - for { - st, _, err := a.StateManager.TipSetState(ctx, ts) - if err != nil { - return nil, xerrors.Errorf("cannot get tipset state: %w", err) + // Walk back until we find a tipset where no state migration is needed + if ts.Height() > 0 { + for { + pts, err := a.Chain.GetTipSetFromKey(ctx, ts.Parents()) + if err != nil { + return nil, xerrors.Errorf("failed to find a non-forking epoch: %w", err) + } + if !a.StateManager.HasExpensiveForkBetween(pts.Height(), ts.Height()+1) { + break + } + ts, err = a.Chain.GetTipSetFromKey(ctx, ts.Parents()) + if err != nil { + return nil, xerrors.Errorf("getting parent tipset: %w", err) + } } + } - res, err = a.StateManager.ApplyOnStateWithGas(ctx, st, msg, ts) - if err != stmgr.ErrExpensiveFork { - break - } - ts, err = a.Chain.GetTipSetFromKey(ctx, ts.Parents()) - if err != nil { - return nil, xerrors.Errorf("getting parent tipset: %w", err) - } + 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("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) From 01ba062f5acd83c362b2a36c79fa797326e1874f Mon Sep 17 00:00:00 2001 From: Aarsh Shah Date: Fri, 31 May 2024 18:35:51 +0400 Subject: [PATCH 5/6] Update node/impl/full/eth.go Co-authored-by: Rod Vagg --- node/impl/full/eth.go | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/node/impl/full/eth.go b/node/impl/full/eth.go index 491ced9c5e8..d043e2637ac 100644 --- a/node/impl/full/eth.go +++ b/node/impl/full/eth.go @@ -982,18 +982,13 @@ func (a *EthModule) applyMessage(ctx context.Context, msg *types.Message, tsk ty // Walk back until we find a tipset where no state migration is needed if ts.Height() > 0 { - for { - pts, err := a.Chain.GetTipSetFromKey(ctx, ts.Parents()) - if err != nil { - return nil, xerrors.Errorf("failed to find a non-forking epoch: %w", err) - } - if !a.StateManager.HasExpensiveForkBetween(pts.Height(), ts.Height()+1) { - break - } - ts, err = a.Chain.GetTipSetFromKey(ctx, ts.Parents()) - if err != nil { - return nil, xerrors.Errorf("getting parent tipset: %w", err) - } + pts, err := a.Chain.GetTipSetFromKey(ctx, ts.Parents()) + if err != nil { + 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 } } From 9ef95fc0b9f6972d95483c0013e3575e87733cbf Mon Sep 17 00:00:00 2001 From: aarshkshah1992 Date: Fri, 31 May 2024 20:15:30 +0530 Subject: [PATCH 6/6] fix as per review --- node/impl/full/eth.go | 1 - 1 file changed, 1 deletion(-) diff --git a/node/impl/full/eth.go b/node/impl/full/eth.go index d043e2637ac..6dfa2f1e45d 100644 --- a/node/impl/full/eth.go +++ b/node/impl/full/eth.go @@ -980,7 +980,6 @@ func (a *EthModule) applyMessage(ctx context.Context, msg *types.Message, tsk ty return nil, xerrors.Errorf("cannot get tipset: %w", err) } - // Walk back until we find a tipset where no state migration is needed if ts.Height() > 0 { pts, err := a.Chain.GetTipSetFromKey(ctx, ts.Parents()) if err != nil {