fix: correctly handle null blocks when detecting an expensive fork

Also improve/fix documentation to reflect the _actual_ tipset that's
passed into upgrades.

And update some comments.

fixes #7192
This commit is contained in:
Steven Allen 2021-08-26 17:23:04 -07:00
parent 714635c75c
commit 165735d01a
5 changed files with 103 additions and 35 deletions

View File

@ -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)

View File

@ -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)

View File

@ -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
}

View File

@ -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) {

View File

@ -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())