Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: many incorrect pointer equality comparisons #1018

Merged
merged 2 commits into from
Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 29 additions & 32 deletions tasks/actorstate/miner/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,51 +21,48 @@ func NewMinerStateExtractionContext(ctx context.Context, a actorstate.ActorInfo,
return nil, fmt.Errorf("loading current miner state: %w", err)
}

prevTipset := a.Current
prevState := curState
if a.Current.Height() != 1 {
prevTipset = a.Executed

prevActor, err := node.Actor(ctx, a.Address, a.Executed.Key())
if err != nil {
// if the actor exists in the current state and not in the parent state then the
// actor was created in the current state.
if err == types.ErrActorNotFound {
return &MinerStateExtractionContext{
PrevState: prevState,
PrevTs: prevTipset,
CurrActor: &a.Actor,
CurrState: curState,
CurrTs: a.Current,
}, nil
}
return nil, fmt.Errorf("loading previous miner %s at tipset %s epoch %d: %w", a.Address, a.Executed.Key(), a.Current.Height(), err)
prevActor, err := node.Actor(ctx, a.Address, a.Executed.Key())
if err != nil {
// actor doesn't exist yet, may have just been created.
if err == types.ErrActorNotFound {
return &MinerStateExtractionContext{
PrevTs: a.Executed,
CurrActor: &a.Actor,
CurrState: curState,
CurrTs: a.Current,
PrevState: nil,
PreviousStatePresent: false,
}, nil
}
return nil, fmt.Errorf("loading previous miner %s at tipset %s epoch %d: %w", a.Address, a.Executed.Key(), a.Current.Height(), err)
}

prevState, err = node.MinerLoad(node.Store(), prevActor)
if err != nil {
return nil, fmt.Errorf("loading previous miner actor state: %w", err)
}
// actor exists in previous state, load it.
prevState, err := node.MinerLoad(node.Store(), prevActor)
if err != nil {
return nil, fmt.Errorf("loading previous miner actor state: %w", err)
}

return &MinerStateExtractionContext{
PrevState: prevState,
PrevTs: prevTipset,
CurrActor: &a.Actor,
CurrState: curState,
CurrTs: a.Current,
PrevState: prevState,
PrevTs: a.Executed,
CurrActor: &a.Actor,
CurrState: curState,
CurrTs: a.Current,
PreviousStatePresent: true,
}, nil
}

type MinerStateExtractionContext struct {
PrevState miner.State
PrevTs *types.TipSet

CurrActor *types.Actor
CurrState miner.State
CurrTs *types.TipSet
CurrActor *types.Actor
CurrState miner.State
CurrTs *types.TipSet
PreviousStatePresent bool
}

func (m *MinerStateExtractionContext) HasPreviousState() bool {
return !(m.CurrTs.Height() == 1 || m.PrevState == m.CurrState)
return m.PreviousStatePresent
}
8 changes: 7 additions & 1 deletion tasks/actorstate/miner/deadline_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,17 @@ func (DeadlineInfoExtractor) Extract(ctx context.Context, a actorstate.ActorInfo
if err != nil {
return nil, err
}
if prevDeadlineInfo == currDeadlineInfo {
// TODO implement equality function
// dereference pointers to check equality
// if these are different then return a model in the bottom of function
if prevDeadlineInfo != nil &&
currDeadlineInfo != nil &&
*prevDeadlineInfo == *currDeadlineInfo {
return nil, nil
}
}

// if there is no previous state and the deadlines have changed, return a model
return &minermodel.MinerCurrentDeadlineInfo{
Height: int64(ec.CurrTs.Height()),
MinerID: a.Address.String(),
Expand Down
2 changes: 1 addition & 1 deletion tasks/actorstate/miner/fee_debt.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (FeeDebtExtractor) Extract(ctx context.Context, a actorstate.ActorInfo, nod
if err != nil {
return nil, fmt.Errorf("loading previous miner fee debt: %w", err)
}
if prevDebt == currDebt {
if prevDebt.Equals(currDebt) {
return nil, nil
}
}
Expand Down
6 changes: 5 additions & 1 deletion tasks/actorstate/miner/locked_funds.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ func (LockedFundsExtractor) Extract(ctx context.Context, a actorstate.ActorInfo,
if err != nil {
return nil, fmt.Errorf("loading previous miner locked funds: %w", err)
}
if prevLocked == currLocked {

// if all values are equal there is no change.
if prevLocked.VestingFunds.Equals(currLocked.VestingFunds) &&
prevLocked.PreCommitDeposits.Equals(currLocked.PreCommitDeposits) &&
prevLocked.InitialPledgeRequirement.Equals(currLocked.InitialPledgeRequirement) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Protect against nil-deref panic here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the LockedFunds type isn't a pointer, nor are its fields so we can safely use them here.

return nil, nil
}
}
Expand Down
54 changes: 27 additions & 27 deletions tasks/actorstate/multisig/multisig.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,12 @@ type MsigExtractionContext struct {
CurrState multisig.State
CurrTs *types.TipSet

Store adt.Store
Store adt.Store
PreviousStatePresent bool
}

func (m *MsigExtractionContext) HasPreviousState() bool {
return !(m.CurrTs.Height() == 1 || m.CurrState == m.PrevState)
return m.PreviousStatePresent
}

func NewMultiSigExtractionContext(ctx context.Context, a actorstate.ActorInfo, node actorstate.ActorStateAPI) (*MsigExtractionContext, error) {
Expand All @@ -132,35 +133,34 @@ func NewMultiSigExtractionContext(ctx context.Context, a actorstate.ActorInfo, n
return nil, fmt.Errorf("loading current multisig state at head %s: %w", a.Actor.Head, err)
}

prevState := curState
if a.Current.Height() != 1 {
prevActor, err := node.Actor(ctx, a.Address, a.Executed.Key())
if err != nil {
// if the actor exists in the current state and not in the parent state then the
// actor was created in the current state.
if err == types.ErrActorNotFound {
return &MsigExtractionContext{
PrevState: prevState,
CurrActor: &a.Actor,
CurrState: curState,
CurrTs: a.Current,
Store: node.Store(),
}, nil
}
return nil, fmt.Errorf("loading previous multisig %s at tipset %s epoch %d: %w", a.Address, a.Executed.Key(), a.Current.Height(), err)
prevActor, err := node.Actor(ctx, a.Address, a.Executed.Key())
if err != nil {
// actor doesn't exist yet, may have just been created.
if err == types.ErrActorNotFound {
return &MsigExtractionContext{
CurrActor: &a.Actor,
CurrState: curState,
CurrTs: a.Current,
Store: node.Store(),
PrevState: nil,
PreviousStatePresent: false,
}, nil
}
return nil, fmt.Errorf("loading previous multisig %s from parent tipset %s current epoch %d: %w", a.Address, a.Executed.Key(), a.Current.Height(), err)
}

prevState, err = multisig.Load(node.Store(), prevActor)
if err != nil {
return nil, fmt.Errorf("loading previous multisig actor state: %w", err)
}
// actor exists in previous state, load it.
prevState, err := multisig.Load(node.Store(), prevActor)
if err != nil {
return nil, fmt.Errorf("loading previous multisig actor state: %w", err)
}

return &MsigExtractionContext{
PrevState: prevState,
CurrActor: &a.Actor,
CurrState: curState,
CurrTs: a.Current,
Store: node.Store(),
PrevState: prevState,
CurrActor: &a.Actor,
CurrState: curState,
CurrTs: a.Current,
Store: node.Store(),
PreviousStatePresent: true,
}, nil
}
50 changes: 25 additions & 25 deletions tasks/actorstate/power/power.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,33 +25,32 @@ func NewPowerStateExtractionContext(ctx context.Context, a actorstate.ActorInfo,
return nil, fmt.Errorf("loading current power state: %w", err)
}

prevState := curState
if a.Current.Height() != 1 {
prevActor, err := node.Actor(ctx, a.Address, a.Executed.Key())
if err != nil {
// if the actor exists in the current state and not in the parent state then the
// actor was created in the current state.
if err == types.ErrActorNotFound {
return &PowerStateExtractionContext{
PrevState: prevState,
CurrState: curState,
CurrTs: a.Current,
Store: node.Store(),
}, nil
}
return nil, fmt.Errorf("loading previous power actor at tipset %s epoch %d: %w", a.Executed.Key(), a.Current.Height(), err)
prevActor, err := node.Actor(ctx, a.Address, a.Executed.Key())
if err != nil {
// actor doesn't exist yet, may have just been created.
if err == types.ErrActorNotFound {
return &PowerStateExtractionContext{
CurrState: curState,
CurrTs: a.Current,
Store: node.Store(),
PrevState: nil,
PreviousStatePresent: false,
}, nil
}
return nil, fmt.Errorf("loading previous power actor from parent tipset %s current epoch %d: %w", a.Executed.Key(), a.Current.Height(), err)
}

prevState, err = power.Load(node.Store(), prevActor)
if err != nil {
return nil, fmt.Errorf("loading previous power actor state: %w", err)
}
// actor exists in previous state, load it.
prevState, err := power.Load(node.Store(), prevActor)
if err != nil {
return nil, fmt.Errorf("loading previous power actor state: %w", err)
}
return &PowerStateExtractionContext{
PrevState: prevState,
CurrState: curState,
CurrTs: a.Current,
Store: node.Store(),
PrevState: prevState,
CurrState: curState,
CurrTs: a.Current,
Store: node.Store(),
PreviousStatePresent: true,
}, nil
}

Expand All @@ -60,11 +59,12 @@ type PowerStateExtractionContext struct {
CurrState power.State
CurrTs *types.TipSet

Store adt.Store
Store adt.Store
PreviousStatePresent bool
}

func (p *PowerStateExtractionContext) HasPreviousState() bool {
return !(p.CurrTs.Height() == 1 || p.PrevState == p.CurrState)
return p.PreviousStatePresent
}

func (StoragePowerExtractor) Extract(ctx context.Context, a actorstate.ActorInfo, node actorstate.ActorStateAPI) (model.Persistable, error) {
Expand Down
54 changes: 27 additions & 27 deletions tasks/actorstate/verifreg/verifreg.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ type VerifiedRegistryExtractionContext struct {
PrevState, CurrState verifreg.State
PrevTs, CurrTs *types.TipSet

Store adt.Store
Store adt.Store
PreviousStatePresent bool
}

func (v *VerifiedRegistryExtractionContext) HasPreviousState() bool {
return !(v.CurrTs.Height() == 1 || v.PrevState == v.CurrState)
return v.PreviousStatePresent
}

func NewVerifiedRegistryExtractorContext(ctx context.Context, a actorstate.ActorInfo, node actorstate.ActorStateAPI) (*VerifiedRegistryExtractionContext, error) {
Expand All @@ -33,35 +34,34 @@ func NewVerifiedRegistryExtractorContext(ctx context.Context, a actorstate.Actor
return nil, fmt.Errorf("loading current verified registry state: %w", err)
}

prevState := curState
if a.Current.Height() != 0 {
prevActor, err := node.Actor(ctx, a.Address, a.Executed.Key())
if err != nil {
// if the actor exists in the current state and not in the parent state then the
// actor was created in the current state.
if err == types.ErrActorNotFound {
return &VerifiedRegistryExtractionContext{
PrevState: prevState,
CurrState: curState,
PrevTs: a.Executed,
CurrTs: a.Current,
Store: node.Store(),
}, nil
}
return nil, fmt.Errorf("loading previous verified registry actor at tipset %s epoch %d: %w", a.Executed.Key(), a.Current.Height(), err)
prevActor, err := node.Actor(ctx, a.Address, a.Executed.Key())
if err != nil {
// actor doesn't exist yet, may have just been created.
if err == types.ErrActorNotFound {
return &VerifiedRegistryExtractionContext{
CurrState: curState,
PrevTs: a.Executed,
CurrTs: a.Current,
Store: node.Store(),
PrevState: nil,
PreviousStatePresent: false,
}, nil
}
return nil, fmt.Errorf("loading previous verified registry actor from parent tipset %s current height epoch %d: %w", a.Executed.Key(), a.Current.Height(), err)
}

prevState, err = verifreg.Load(node.Store(), prevActor)
if err != nil {
return nil, fmt.Errorf("loading previous verified registry state: %w", err)
}
// actor exists in previous state, load it.
prevState, err := verifreg.Load(node.Store(), prevActor)
if err != nil {
return nil, fmt.Errorf("loading previous verified registry state: %w", err)
}
return &VerifiedRegistryExtractionContext{
PrevState: prevState,
CurrState: curState,
PrevTs: a.Executed,
CurrTs: a.Current,
Store: node.Store(),
PrevState: prevState,
CurrState: curState,
PrevTs: a.Executed,
CurrTs: a.Current,
Store: node.Store(),
PreviousStatePresent: true,
}, nil
}

Expand Down