diff --git a/chain/stmgr/call.go b/chain/stmgr/call.go index caa815132..0f045c5f4 100644 --- a/chain/stmgr/call.go +++ b/chain/stmgr/call.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/filecoin-project/go-address" + "github.com/filecoin-project/go-state-types/abi" "github.com/filecoin-project/go-state-types/crypto" "github.com/ipfs/go-cid" "go.opencensus.io/trace" @@ -20,41 +21,49 @@ import ( 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 +// tipset's parent. In the presence of null blocks, the height at which the message is invoked may +// be less than the specified tipset. +// +// - If no tipset is specified, the first tipset without an expensive migration is used. +// - If executing a message at a given tipset would trigger an expensive migration, the call will +// fail with ErrExpensiveFork. func (sm *StateManager) Call(ctx context.Context, msg *types.Message, ts *types.TipSet) (*api.InvocResult, error) { ctx, span := trace.StartSpan(ctx, "statemanager.Call") defer span.End() + var pheight abi.ChainEpoch = -1 + // If no tipset is provided, try to find one without a fork. if ts == nil { ts = sm.cs.GetHeaviestTipSet() - // Search back till we find a height with no fork, or we reach the beginning. - for ts.Height() > 0 && sm.hasExpensiveFork(ctx, ts.Height()-1) { - var err error - ts, err = sm.cs.GetTipSetFromKey(ts.Parents()) + for ts.Height() > 0 { + pts, err := sm.cs.GetTipSetFromKey(ts.Parents()) if err != nil { return nil, xerrors.Errorf("failed to find a non-forking epoch: %w", err) } + if !sm.hasExpensiveFork(pts.Height()) { + pheight = pts.Height() + break + } + ts = pts + } + } else { + pts, err := sm.cs.LoadTipSet(ts.Parents()) + if err != nil { + return nil, xerrors.Errorf("failed to load parent tipset: %w", err) + } + pheight = pts.Height() + if sm.hasExpensiveFork(pheight) { + return nil, ErrExpensiveFork } } bstate := ts.ParentState() - pts, err := sm.cs.LoadTipSet(ts.Parents()) - if err != nil { - return nil, xerrors.Errorf("failed to load parent tipset: %w", err) - } - pheight := pts.Height() - - // If we have to run an expensive migration, and we're not at genesis, - // return an error because the migration will take too long. - // - // We allow this at height 0 for at-genesis migrations (for testing). - if pheight > 0 && sm.hasExpensiveFork(ctx, pheight) { - return nil, ErrExpensiveFork - } // Run the (not expensive) migration. - bstate, err = sm.handleStateForks(ctx, bstate, pheight, nil, ts) + bstate, err := sm.handleStateForks(ctx, bstate, pheight, nil, ts) if err != nil { return nil, fmt.Errorf("failed to handle fork: %w", err) } @@ -140,18 +149,25 @@ func (sm *StateManager) CallWithGas(ctx context.Context, msg *types.Message, pri // run the fork logic in `sm.TipSetState`. We need the _current_ // height to have no fork, because we'll run it inside this // function before executing the given message. - for ts.Height() > 0 && (sm.hasExpensiveFork(ctx, ts.Height()) || sm.hasExpensiveFork(ctx, ts.Height()-1)) { - var err error - ts, err = sm.cs.GetTipSetFromKey(ts.Parents()) + for ts.Height() > 0 { + pts, err := sm.cs.GetTipSetFromKey(ts.Parents()) if err != nil { return nil, xerrors.Errorf("failed to find a non-forking epoch: %w", err) } - } - } + if !sm.hasExpensiveForkBetween(pts.Height(), ts.Height()+1) { + break + } - // When we're not at the genesis block, make sure we don't have an expensive migration. - if ts.Height() > 0 && (sm.hasExpensiveFork(ctx, ts.Height()) || sm.hasExpensiveFork(ctx, ts.Height()-1)) { - return nil, ErrExpensiveFork + ts = pts + } + } else if ts.Height() > 0 { + pts, err := sm.cs.GetTipSetFromKey(ts.Parents()) + if err != nil { + return nil, xerrors.Errorf("failed to find a non-forking epoch: %w", err) + } + if sm.hasExpensiveForkBetween(pts.Height(), ts.Height()+1) { + return nil, ErrExpensiveFork + } } state, _, err := sm.TipSetState(ctx, ts) @@ -159,6 +175,7 @@ func (sm *StateManager) CallWithGas(ctx context.Context, msg *types.Message, pri return nil, xerrors.Errorf("computing tipset state: %w", err) } + // Technically, the tipset we're passing in here should be ts+1, but that may not exist. state, err = sm.handleStateForks(ctx, state, ts.Height(), nil, ts) if err != nil { return nil, fmt.Errorf("failed to handle fork: %w", err) diff --git a/chain/stmgr/execute.go b/chain/stmgr/execute.go index 3191a45db..a8675e347 100644 --- a/chain/stmgr/execute.go +++ b/chain/stmgr/execute.go @@ -99,7 +99,6 @@ func (sm *StateManager) ApplyBlocks(ctx context.Context, parentEpoch abi.ChainEp } // handle state forks - // XXX: The state tree newState, err := sm.handleStateForks(ctx, pstate, i, em, ts) if err != nil { return cid.Undef, cid.Undef, xerrors.Errorf("error handling state forks: %w", err) diff --git a/chain/stmgr/forks.go b/chain/stmgr/forks.go index 212272a95..b6c6985e2 100644 --- a/chain/stmgr/forks.go +++ b/chain/stmgr/forks.go @@ -41,8 +41,11 @@ type MigrationCache interface { // - The oldState is the state produced by the upgrade epoch. // - The returned newState is the new state that will be used by the next epoch. // - The height is the upgrade epoch height (already executed). -// - The tipset is the tipset for the last non-null block before the upgrade. Do -// not assume that ts.Height() is the upgrade height. +// - The tipset is the first non-null tipset after the upgrade height (the tipset in +// which the upgrade is executed). Do not assume that ts.Height() is the upgrade height. +// +// NOTE: In StateCompute and CallWithGas, the passed tipset is actually the tipset _before_ the +// upgrade. The tipset should really only be used for referencing the "current chain". type MigrationFunc func( ctx context.Context, sm *StateManager, cache MigrationCache, @@ -208,7 +211,21 @@ func (sm *StateManager) handleStateForks(ctx context.Context, root cid.Cid, heig return retCid, nil } -func (sm *StateManager) hasExpensiveFork(ctx context.Context, height abi.ChainEpoch) bool { +// Returns true if executing the current tipset would trigger an expensive fork. +// +// - If the tipset is the genesis, this function always returns false. +// - If inclusive is true, this function will also return true if applying a message on-top-of the +// tipset would trigger a fork. +func (sm *StateManager) hasExpensiveForkBetween(parent, height abi.ChainEpoch) bool { + for h := parent; h < height; h++ { + if _, ok := sm.expensiveUpgrades[h]; ok { + return true + } + } + return false +} + +func (sm *StateManager) hasExpensiveFork(height abi.ChainEpoch) bool { _, ok := sm.expensiveUpgrades[height] return ok } diff --git a/chain/stmgr/forks_test.go b/chain/stmgr/forks_test.go index 0df6ce397..97ec8643b 100644 --- a/chain/stmgr/forks_test.go +++ b/chain/stmgr/forks_test.go @@ -242,6 +242,19 @@ func TestForkHeightTriggers(t *testing.T) { func TestForkRefuseCall(t *testing.T) { logging.SetAllLoggers(logging.LevelInfo) + for after := 0; after < 3; after++ { + for before := 0; before < 3; before++ { + // Makes the lints happy... + after := after + before := before + t.Run(fmt.Sprintf("after:%d,before:%d", after, before), func(t *testing.T) { + testForkRefuseCall(t, before, after) + }) + } + } + +} +func testForkRefuseCall(t *testing.T, nullsBefore, nullsAfter int) { ctx := context.TODO() cg, err := gen.NewGenerator() @@ -249,6 +262,7 @@ func TestForkRefuseCall(t *testing.T) { t.Fatal(err) } + var migrationCount int sm, err := NewStateManagerWithUpgradeSchedule( cg.ChainStore(), cg.StateManager().VMSys(), UpgradeSchedule{{ Network: network.Version1, @@ -256,6 +270,7 @@ func TestForkRefuseCall(t *testing.T) { Height: testForkHeight, Migration: func(ctx context.Context, sm *StateManager, cache MigrationCache, cb ExecMonitor, root cid.Cid, height abi.ChainEpoch, ts *types.TipSet) (cid.Cid, error) { + migrationCount++ return root, nil }}}) if err != nil { @@ -292,14 +307,20 @@ func TestForkRefuseCall(t *testing.T) { GasFeeCap: types.NewInt(0), } + nullStart := abi.ChainEpoch(testForkHeight - nullsBefore) + nullLength := abi.ChainEpoch(nullsBefore + nullsAfter) + for i := 0; i < 50; i++ { - ts, err := cg.NextTipSet() + pts := cg.CurTipset.TipSet() + skip := abi.ChainEpoch(0) + if pts.Height() == nullStart { + skip = nullLength + } + ts, err := cg.NextTipSetFromMiners(pts, cg.Miners, skip) if err != nil { t.Fatal(err) } - pts, err := cg.ChainStore().LoadTipSet(ts.TipSet.TipSet().Parents()) - require.NoError(t, err) parentHeight := pts.Height() currentHeight := ts.TipSet.TipSet().Height() @@ -321,7 +342,20 @@ func TestForkRefuseCall(t *testing.T) { require.NoError(t, err) require.True(t, ret.MsgRct.ExitCode.IsSuccess()) } + + // Calls without a tipset should walk back to the last non-fork tipset. + // We _verify_ that the migration wasn't run multiple times at the end of the + // test. + ret, err = sm.CallWithGas(ctx, m, nil, nil) + require.NoError(t, err) + require.True(t, ret.MsgRct.ExitCode.IsSuccess()) + + ret, err = sm.Call(ctx, m, nil) + require.NoError(t, err) + require.True(t, ret.MsgRct.ExitCode.IsSuccess()) } + // Make sure we didn't execute the migration multiple times. + require.Equal(t, migrationCount, 1) } func TestForkPreMigration(t *testing.T) { diff --git a/chain/stmgr/utils.go b/chain/stmgr/utils.go index a4d78f997..8776fbcd6 100644 --- a/chain/stmgr/utils.go +++ b/chain/stmgr/utils.go @@ -143,13 +143,14 @@ func ComputeState(ctx context.Context, sm *StateManager, height abi.ChainEpoch, } for i := ts.Height(); i < height; i++ { - // handle state forks + // Technically, the tipset we're passing in here should be ts+1, but that may not exist. base, err = sm.handleStateForks(ctx, base, i, &InvocationTracer{trace: &trace}, ts) if err != nil { return cid.Undef, nil, xerrors.Errorf("error handling state forks: %w", err) } - // TODO: should we also run cron here? + // We intentionally don't run cron here, as we may be trying to look into the + // future. It's not guaranteed to be accurate... but that's fine. } r := store.NewChainRand(sm.cs, ts.Cids())