From 7fbd0a5e8e44acfddd628f052f5251d84794bb55 Mon Sep 17 00:00:00 2001 From: whyrusleeping Date: Tue, 15 Oct 2019 14:19:37 +0900 Subject: [PATCH] Dont use aerrors.Escalate unless you really mean it --- chain/actors/actor_miner.go | 36 ++++++++++++++--------------- chain/actors/actor_storagemarket.go | 25 ++++++++++---------- chain/actors/aerrors/wrap.go | 27 +++++++++++++++++++++- 3 files changed, 56 insertions(+), 32 deletions(-) diff --git a/chain/actors/actor_miner.go b/chain/actors/actor_miner.go index 0578c2716..9674a29bc 100644 --- a/chain/actors/actor_miner.go +++ b/chain/actors/actor_miner.go @@ -185,7 +185,7 @@ func (sma StorageMinerActor) StorageMinerConstructor(act *types.Actor, vmctx typ sectors := amt.NewAMT(types.WrapStorage(vmctx.Storage())) scid, serr := sectors.Flush() if serr != nil { - return nil, aerrors.Escalate(serr, "initializing AMT") + return nil, aerrors.HandleExternalError(serr, "initializing AMT") } self.Sectors = scid @@ -270,7 +270,7 @@ func (sma StorageMinerActor) CommitSector(act *types.Actor, vmctx types.VMContex // time to prove than large sectors do. Sector size is selected when pledging. pss, lerr := amt.LoadAMT(types.WrapStorage(vmctx.Storage()), self.ProvingSet) if lerr != nil { - return nil, aerrors.Escalate(lerr, "could not load proving set node") + return nil, aerrors.HandleExternalError(lerr, "could not load proving set node") } if pss.Count == 0 { @@ -366,7 +366,7 @@ func (sma StorageMinerActor) SubmitPoSt(act *types.Actor, vmctx types.VMContext, pss, lerr := amt.LoadAMT(types.WrapStorage(vmctx.Storage()), self.ProvingSet) if lerr != nil { - return nil, aerrors.Escalate(lerr, "could not load proving set node") + return nil, aerrors.HandleExternalError(lerr, "could not load proving set node") } var sectorInfos []sectorbuilder.SectorInfo @@ -409,19 +409,19 @@ func (sma StorageMinerActor) SubmitPoSt(act *types.Actor, vmctx types.VMContext, ss, lerr := amt.LoadAMT(types.WrapStorage(vmctx.Storage()), self.ProvingSet) if lerr != nil { - return nil, aerrors.Escalate(lerr, "could not load proving set node") + return nil, aerrors.HandleExternalError(lerr, "could not load proving set node") } if err := ss.BatchDelete(params.DoneSet.All()); err != nil { // TODO: this could fail for system reasons (block not found) or for // bad user input reasons (e.g. bad doneset). The latter should be a // non-fatal error - return nil, aerrors.Escalate(err, "failed to delete sectors in done set") + return nil, aerrors.HandleExternalError(err, "failed to delete sectors in done set") } self.ProvingSet, lerr = ss.Flush() if lerr != nil { - return nil, aerrors.Escalate(lerr, "could not flush AMT") + return nil, aerrors.HandleExternalError(lerr, "could not flush AMT") } oldPower := self.Power @@ -474,16 +474,16 @@ func SectorIsUnique(ctx context.Context, s types.Storage, sroot cid.Cid, sid uin func AddToSectorSet(ctx context.Context, s types.Storage, ss cid.Cid, sectorID uint64, commR, commD []byte) (cid.Cid, ActorError) { ssr, err := amt.LoadAMT(types.WrapStorage(s), ss) if err != nil { - return cid.Undef, aerrors.Escalate(err, "could not load sector set node") + return cid.Undef, aerrors.HandleExternalError(err, "could not load sector set node") } if err := ssr.Set(sectorID, [][]byte{commR, commD}); err != nil { - return cid.Undef, aerrors.Escalate(err, "failed to set commitment in sector set") + return cid.Undef, aerrors.HandleExternalError(err, "failed to set commitment in sector set") } ncid, err := ssr.Flush() if err != nil { - return cid.Undef, aerrors.Escalate(err, "failed to flush sector set") + return cid.Undef, aerrors.HandleExternalError(err, "failed to flush sector set") } return ncid, nil @@ -492,7 +492,7 @@ func AddToSectorSet(ctx context.Context, s types.Storage, ss cid.Cid, sectorID u func GetFromSectorSet(ctx context.Context, s types.Storage, ss cid.Cid, sectorID uint64) (bool, []byte, []byte, ActorError) { ssr, err := amt.LoadAMT(types.WrapStorage(s), ss) if err != nil { - return false, nil, nil, aerrors.Escalate(err, "could not load sector set node") + return false, nil, nil, aerrors.HandleExternalError(err, "could not load sector set node") } var comms [][]byte @@ -501,11 +501,11 @@ func GetFromSectorSet(ctx context.Context, s types.Storage, ss cid.Cid, sectorID if _, ok := err.(*amt.ErrNotFound); ok { return false, nil, nil, nil } - return false, nil, nil, aerrors.Escalate(err, "failed to find sector in sector set") + return false, nil, nil, aerrors.HandleExternalError(err, "failed to find sector in sector set") } if len(comms) != 2 { - return false, nil, nil, aerrors.Escalate(xerrors.New("sector set entry should only have 2 elements"), "") + return false, nil, nil, aerrors.Newf(20, "sector set entry should only have 2 elements") } return true, comms[0], comms[1], nil @@ -514,7 +514,7 @@ func GetFromSectorSet(ctx context.Context, s types.Storage, ss cid.Cid, sectorID func ValidatePoRep(maddr address.Address, ssize types.BigInt, params *CommitSectorParams) (bool, ActorError) { ok, err := sectorbuilder.VerifySeal(ssize.Uint64(), params.CommR, params.CommD, params.CommRStar, maddr, params.SectorID, params.Proof) if err != nil { - return false, aerrors.Escalate(err, "verify seal failed") + return false, aerrors.Absorb(err, 25, "verify seal failed") } return ok, nil @@ -659,11 +659,11 @@ func (sma StorageMinerActor) PaymentVerifyInclusion(act *types.Actor, vmctx type var voucherData PieceInclVoucherData if err := cbor.DecodeInto(params.Extra, &voucherData); err != nil { - return nil, aerrors.Escalate(err, "failed to decode storage voucher data for verification") + return nil, aerrors.Absorb(err, 2, "failed to decode storage voucher data for verification") } var proof InclusionProof if err := cbor.DecodeInto(params.Proof, &proof); err != nil { - return nil, aerrors.Escalate(err, "failed to decode storage payment proof") + return nil, aerrors.Absorb(err, 3, "failed to decode storage payment proof") } ok, _, commD, aerr := GetFromSectorSet(context.TODO(), vmctx.Storage(), self.Sectors, proof.Sector) @@ -671,15 +671,15 @@ func (sma StorageMinerActor) PaymentVerifyInclusion(act *types.Actor, vmctx type return nil, aerr } if !ok { - return nil, aerrors.New(1, "miner does not have required sector") + return nil, aerrors.New(4, "miner does not have required sector") } ok, err := sectorbuilder.VerifyPieceInclusionProof(mi.SectorSize.Uint64(), voucherData.PieceSize.Uint64(), voucherData.CommP, commD, proof.Proof) if err != nil { - return nil, aerrors.Escalate(err, "verify piece inclusion proof failed") + return nil, aerrors.Absorb(err, 5, "verify piece inclusion proof failed") } if !ok { - return nil, aerrors.New(2, "piece inclusion proof was invalid") + return nil, aerrors.New(6, "piece inclusion proof was invalid") } return nil, nil diff --git a/chain/actors/actor_storagemarket.go b/chain/actors/actor_storagemarket.go index e595be949..eb16d35d9 100644 --- a/chain/actors/actor_storagemarket.go +++ b/chain/actors/actor_storagemarket.go @@ -2,7 +2,6 @@ package actors import ( "context" - "fmt" "github.com/filecoin-project/go-lotus/build" "github.com/filecoin-project/go-lotus/chain/actors/aerrors" @@ -436,7 +435,7 @@ func pledgeCollateralForSize(vmctx types.VMContext, size, totalStorage types.Big func MinerSetHas(ctx context.Context, vmctx types.VMContext, rcid cid.Cid, maddr address.Address) (bool, aerrors.ActorError) { nd, err := hamt.LoadNode(ctx, vmctx.Ipld(), rcid) if err != nil { - return false, aerrors.Escalate(err, "failed to load miner set") + return false, aerrors.HandleExternalError(err, "failed to load miner set") } err = nd.Find(ctx, string(maddr.Bytes()), nil) @@ -446,7 +445,7 @@ func MinerSetHas(ctx context.Context, vmctx types.VMContext, rcid cid.Cid, maddr case nil: return true, nil default: - return false, aerrors.Escalate(err, "failed to do set lookup") + return false, aerrors.HandleExternalError(err, "failed to do set lookup") } } @@ -475,30 +474,30 @@ func MinerSetList(ctx context.Context, cst *hamt.CborIpldStore, rcid cid.Cid) ([ func MinerSetAdd(ctx context.Context, vmctx types.VMContext, rcid cid.Cid, maddr address.Address) (cid.Cid, aerrors.ActorError) { nd, err := hamt.LoadNode(ctx, vmctx.Ipld(), rcid) if err != nil { - return cid.Undef, aerrors.Escalate(err, "failed to load miner set") + return cid.Undef, aerrors.HandleExternalError(err, "failed to load miner set") } mkey := string(maddr.Bytes()) err = nd.Find(ctx, mkey, nil) if err == nil { - return cid.Undef, aerrors.Escalate(fmt.Errorf("miner already found"), "miner set add failed") + return cid.Undef, aerrors.New(20, "miner already in set") } if !xerrors.Is(err, hamt.ErrNotFound) { - return cid.Undef, aerrors.Escalate(err, "failed to do miner set check") + return cid.Undef, aerrors.HandleExternalError(err, "failed to do miner set check") } if err := nd.Set(ctx, mkey, uint64(1)); err != nil { - return cid.Undef, aerrors.Escalate(err, "adding miner address to set failed") + return cid.Undef, aerrors.HandleExternalError(err, "adding miner address to set failed") } if err := nd.Flush(ctx); err != nil { - return cid.Undef, aerrors.Escalate(err, "failed to flush miner set") + return cid.Undef, aerrors.HandleExternalError(err, "failed to flush miner set") } c, err := vmctx.Ipld().Put(ctx, nd) if err != nil { - return cid.Undef, aerrors.Escalate(err, "failed to persist miner set to storage") + return cid.Undef, aerrors.HandleExternalError(err, "failed to persist miner set to storage") } return c, nil @@ -507,7 +506,7 @@ func MinerSetAdd(ctx context.Context, vmctx types.VMContext, rcid cid.Cid, maddr func MinerSetRemove(ctx context.Context, vmctx types.VMContext, rcid cid.Cid, maddr address.Address) (cid.Cid, aerrors.ActorError) { nd, err := hamt.LoadNode(ctx, vmctx.Ipld(), rcid) if err != nil { - return cid.Undef, aerrors.Escalate(err, "failed to load miner set") + return cid.Undef, aerrors.HandleExternalError(err, "failed to load miner set") } mkey := string(maddr.Bytes()) @@ -515,17 +514,17 @@ func MinerSetRemove(ctx context.Context, vmctx types.VMContext, rcid cid.Cid, ma case hamt.ErrNotFound: return cid.Undef, aerrors.New(1, "miner not found in set on delete") default: - return cid.Undef, aerrors.Escalate(err, "failed to delete miner from set") + return cid.Undef, aerrors.HandleExternalError(err, "failed to delete miner from set") case nil: } if err := nd.Flush(ctx); err != nil { - return cid.Undef, aerrors.Escalate(err, "failed to flush miner set") + return cid.Undef, aerrors.HandleExternalError(err, "failed to flush miner set") } c, err := vmctx.Ipld().Put(ctx, nd) if err != nil { - return cid.Undef, aerrors.Escalate(err, "failed to persist miner set to storage") + return cid.Undef, aerrors.HandleExternalError(err, "failed to persist miner set to storage") } return c, nil diff --git a/chain/actors/aerrors/wrap.go b/chain/actors/aerrors/wrap.go index 29a833484..8f2db99c1 100644 --- a/chain/actors/aerrors/wrap.go +++ b/chain/actors/aerrors/wrap.go @@ -103,7 +103,7 @@ func Absorb(err error, retCode uint8, msg string) ActorError { fatal: true, retCode: 0, - msg: "tried absorbing an error that is alreay an fatal error", + msg: "tried absorbing an error that is already a fatal error", frame: xerrors.Caller(1), err: err, } @@ -142,3 +142,28 @@ func Escalate(err error, msg string) ActorError { err: err, } } + +func HandleExternalError(err error, msg string) ActorError { + if err == nil { + return nil + } + + if aerr, ok := err.(ActorError); ok { + return &actorError{ + fatal: IsFatal(aerr), + retCode: RetCode(aerr), + + msg: msg, + frame: xerrors.Caller(1), + err: aerr, + } + } + + return &actorError{ + fatal: true, + + msg: msg, + frame: xerrors.Caller(1), + err: err, + } +}