From 16dbbadddf4c671e9581e570dd7f0146b8740f8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Wed, 7 Oct 2020 19:41:07 +0200 Subject: [PATCH 1/5] Set lower feecap on PoSt messages with low balance --- cmd/lotus-storage-miner/actor.go | 2 +- storage/addresses.go | 78 +++++++++++++++++++++++--------- storage/miner.go | 2 + storage/wdpost_run.go | 25 +++++++++- storage/wdpost_run_test.go | 8 ++-- 5 files changed, 86 insertions(+), 29 deletions(-) diff --git a/cmd/lotus-storage-miner/actor.go b/cmd/lotus-storage-miner/actor.go index 69486eaf5b9..dcf3d35c0b1 100644 --- a/cmd/lotus-storage-miner/actor.go +++ b/cmd/lotus-storage-miner/actor.go @@ -414,7 +414,7 @@ var actorControlList = &cli.Command{ tablewriter.Col("balance"), ) - postAddr, err := storage.AddressFor(ctx, api, mi, storage.PoStAddr, types.FromFil(1)) + postAddr, _, err := storage.AddressFor(ctx, api, mi, storage.PoStAddr, types.FromFil(1), types.FromFil(1)) if err != nil { return xerrors.Errorf("getting address for post: %w", err) } diff --git a/storage/addresses.go b/storage/addresses.go index f5640794ef3..fcc8467edbb 100644 --- a/storage/addresses.go +++ b/storage/addresses.go @@ -3,13 +3,13 @@ package storage import ( "context" - "github.com/filecoin-project/lotus/chain/actors/builtin/miner" - "golang.org/x/xerrors" "github.com/filecoin-project/go-address" "github.com/filecoin-project/go-state-types/abi" + "github.com/filecoin-project/go-state-types/big" + "github.com/filecoin-project/lotus/chain/actors/builtin/miner" "github.com/filecoin-project/lotus/chain/types" ) @@ -28,20 +28,23 @@ type addrSelectApi interface { StateAccountKey(context.Context, address.Address, types.TipSetKey) (address.Address, error) } -func AddressFor(ctx context.Context, a addrSelectApi, mi miner.MinerInfo, use AddrUse, minFunds abi.TokenAmount) (address.Address, error) { +func AddressFor(ctx context.Context, a addrSelectApi, mi miner.MinerInfo, use AddrUse, goodFunds, minFunds abi.TokenAmount) (address.Address, abi.TokenAmount, error) { switch use { case PreCommitAddr, CommitAddr: // always use worker, at least for now - return mi.Worker, nil + return mi.Worker, big.Zero(), nil } + leastBad := address.Undef + bestAvail := minFunds + for _, addr := range mi.ControlAddresses { b, err := a.WalletBalance(ctx, addr) if err != nil { - return address.Undef, xerrors.Errorf("checking control address balance: %w", err) + return address.Undef, big.Zero(), xerrors.Errorf("checking control address balance: %w", err) } - if b.GreaterThanEqual(minFunds) { + if b.GreaterThanEqual(goodFunds) { k, err := a.StateAccountKey(ctx, addr, types.EmptyTSK) if err != nil { log.Errorw("getting account key", "error", err) @@ -50,7 +53,7 @@ func AddressFor(ctx context.Context, a addrSelectApi, mi miner.MinerInfo, use Ad have, err := a.WalletHas(ctx, k) if err != nil { - return address.Undef, xerrors.Errorf("failed to check control address: %w", err) + return address.Undef, big.Zero(), xerrors.Errorf("failed to check control address: %w", err) } if !have { @@ -58,37 +61,68 @@ func AddressFor(ctx context.Context, a addrSelectApi, mi miner.MinerInfo, use Ad continue } - return addr, nil + return addr, b, nil + } + + if b.GreaterThan(bestAvail) { + leastBad = addr + bestAvail = b } - log.Warnw("control address didn't have enough funds for window post message", "address", addr, "required", types.FIL(minFunds), "balance", types.FIL(b)) + log.Warnw("control address didn't have enough funds for window post message", "address", addr, "required", types.FIL(goodFunds), "balance", types.FIL(b)) } // Try to use the owner account if we can, fallback to worker if we can't - b, err := a.WalletBalance(ctx, mi.Owner) + k, err := a.StateAccountKey(ctx, mi.Owner, types.EmptyTSK) if err != nil { - return address.Undef, xerrors.Errorf("checking owner balance: %w", err) + log.Errorw("getting owner account key", "error", err) + return mi.Worker, big.Zero(), nil } - if !b.GreaterThanEqual(minFunds) { - return mi.Worker, nil + haveOwner, err := a.WalletHas(ctx, k) + if err != nil { + return address.Undef, big.Zero(), xerrors.Errorf("failed to check owner address: %w", err) } - k, err := a.StateAccountKey(ctx, mi.Owner, types.EmptyTSK) - if err != nil { - log.Errorw("getting owner account key", "error", err) - return mi.Worker, nil + if haveOwner { + ownerBalance, err := a.WalletBalance(ctx, mi.Owner) + if err != nil { + return address.Undef, big.Zero(), xerrors.Errorf("checking owner balance: %w", err) + } + + if ownerBalance.GreaterThanEqual(goodFunds) { + return mi.Owner, goodFunds, nil + } + + if ownerBalance.GreaterThan(bestAvail) { + leastBad = mi.Owner + bestAvail = ownerBalance + } } - have, err := a.WalletHas(ctx, k) + workerBalance, err := a.WalletBalance(ctx, mi.Worker) if err != nil { - return address.Undef, xerrors.Errorf("failed to check owner address: %w", err) + return address.Undef, big.Zero(), xerrors.Errorf("checking owner balance: %w", err) } - if !have { - return mi.Worker, nil + if workerBalance.GreaterThanEqual(goodFunds) { + return mi.Worker, goodFunds, nil } - return mi.Owner, nil + if workerBalance.GreaterThan(bestAvail) { + leastBad = mi.Worker + bestAvail = workerBalance + } + + if bestAvail.GreaterThan(minFunds) { + log.Warnw("No address had enough funds to for full PoSt message Fee, selecting least bad address", "address", leastBad, "balance", types.FIL(bestAvail), "optimalFunds", types.FIL(goodFunds), "minFunds", types.FIL(minFunds)) + + return leastBad, bestAvail, nil + } + + // This most likely won't work, but can't hurt to try + + log.Warnw("No address had enough funds to for minimum PoSt message Fee, selecting worker address as a fallback", "address", mi.Worker, "balance", types.FIL(workerBalance), "optimalFunds", types.FIL(goodFunds), "minFunds", types.FIL(minFunds)) + return mi.Worker, workerBalance, nil } diff --git a/storage/miner.go b/storage/miner.go index daeb0ef207b..dcec234fd4e 100644 --- a/storage/miner.go +++ b/storage/miner.go @@ -95,6 +95,8 @@ type storageMinerApi interface { MpoolPushMessage(context.Context, *types.Message, *api.MessageSendSpec) (*types.SignedMessage, error) GasEstimateMessageGas(context.Context, *types.Message, *api.MessageSendSpec, types.TipSetKey) (*types.Message, error) + GasEstimateFeeCap(context.Context, *types.Message, int64, types.TipSetKey) (types.BigInt, error) + GasEstimateGasPremium(_ context.Context, nblocksincl uint64, sender address.Address, gaslimit int64, tsk types.TipSetKey) (types.BigInt, error) ChainHead(context.Context) (*types.TipSet, error) ChainNotify(context.Context) (<-chan []*api.HeadChange, error) diff --git a/storage/wdpost_run.go b/storage/wdpost_run.go index 8bf2cc6add1..daaa0af93ef 100644 --- a/storage/wdpost_run.go +++ b/storage/wdpost_run.go @@ -26,6 +26,7 @@ import ( "github.com/filecoin-project/lotus/chain/actors" "github.com/filecoin-project/lotus/chain/actors/builtin/miner" "github.com/filecoin-project/lotus/chain/actors/policy" + "github.com/filecoin-project/lotus/chain/messagepool" "github.com/filecoin-project/lotus/chain/types" ) @@ -781,14 +782,34 @@ func (s *WindowPoStScheduler) setSender(ctx context.Context, msg *types.Message, } *msg = *gm - minFunds := big.Add(msg.RequiredFunds(), msg.Value) + // estimate + minGasFeeMsg := *msg - pa, err := AddressFor(ctx, s.api, mi, PoStAddr, minFunds) + minGasFeeMsg.GasPremium, err = s.api.GasEstimateGasPremium(ctx, 5, msg.From, msg.GasLimit, types.TipSetKey{}) + if err != nil { + log.Errorf("failed to estimate minimum gas premium: %+v", err) + minGasFeeMsg.GasPremium = msg.GasPremium + } + + minGasFeeMsg.GasFeeCap, err = s.api.GasEstimateFeeCap(ctx, &minGasFeeMsg, 4, types.EmptyTSK) + if err != nil { + log.Errorf("failed to estimate minimum gas fee cap: %+v", err) + minGasFeeMsg.GasFeeCap = msg.GasFeeCap + } + + goodFunds := big.Add(msg.RequiredFunds(), msg.Value) + minFunds := big.Min(big.Add(minGasFeeMsg.RequiredFunds(), minGasFeeMsg.Value), goodFunds) + + pa, avail, err := AddressFor(ctx, s.api, mi, PoStAddr, goodFunds, minFunds) if err != nil { log.Errorw("error selecting address for window post", "error", err) return nil } msg.From = pa + bestReq := big.Add(msg.RequiredFunds(), msg.Value) + if avail.LessThan(bestReq) { + messagepool.CapGasFee(msg, big.Min(big.Sub(avail, msg.Value), msg.RequiredFunds())) + } return nil } diff --git a/storage/wdpost_run_test.go b/storage/wdpost_run_test.go index 436141295b4..9506abe2199 100644 --- a/storage/wdpost_run_test.go +++ b/storage/wdpost_run_test.go @@ -49,6 +49,10 @@ func (m *mockStorageMinerAPI) StateMinerInfo(ctx context.Context, a address.Addr }, nil } +func (m *mockStorageMinerAPI) StateNetworkVersion(ctx context.Context, key types.TipSetKey) (network.Version, error) { + return build.NewestNetworkVersion, nil +} + func (m *mockStorageMinerAPI) ChainGetRandomnessFromTickets(ctx context.Context, tsk types.TipSetKey, personalization crypto.DomainSeparationTag, randEpoch abi.ChainEpoch, entropy []byte) (abi.Randomness, error) { return abi.Randomness("ticket rand"), nil } @@ -94,10 +98,6 @@ func (m *mockStorageMinerAPI) StateWaitMsg(ctx context.Context, cid cid.Cid, con }, nil } -func (m *mockStorageMinerAPI) StateNetworkVersion(context.Context, types.TipSetKey) (network.Version, error) { - return build.NewestNetworkVersion, nil -} - type mockProver struct { } From 9f807f4aeb62111ab7feaf8acdf7a693903aed4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Mon, 26 Oct 2020 16:28:39 +0100 Subject: [PATCH 2/5] storageminer: Simplify AddressFor --- storage/addresses.go | 103 ++++++++++++++++--------------------------- 1 file changed, 38 insertions(+), 65 deletions(-) diff --git a/storage/addresses.go b/storage/addresses.go index fcc8467edbb..8d535b589f9 100644 --- a/storage/addresses.go +++ b/storage/addresses.go @@ -35,94 +35,67 @@ func AddressFor(ctx context.Context, a addrSelectApi, mi miner.MinerInfo, use Ad return mi.Worker, big.Zero(), nil } - leastBad := address.Undef + leastBad := mi.Worker bestAvail := minFunds - for _, addr := range mi.ControlAddresses { - b, err := a.WalletBalance(ctx, addr) - if err != nil { - return address.Undef, big.Zero(), xerrors.Errorf("checking control address balance: %w", err) - } - - if b.GreaterThanEqual(goodFunds) { - k, err := a.StateAccountKey(ctx, addr, types.EmptyTSK) - if err != nil { - log.Errorw("getting account key", "error", err) - continue - } - - have, err := a.WalletHas(ctx, k) - if err != nil { - return address.Undef, big.Zero(), xerrors.Errorf("failed to check control address: %w", err) - } - - if !have { - log.Errorw("don't have key", "key", k) - continue - } - - return addr, b, nil + for _, addr := range append(mi.ControlAddresses, mi.Owner, mi.Worker) { + if maybeUseAddress(ctx, a, addr, goodFunds, &leastBad, &bestAvail) { + return leastBad, bestAvail, nil } + } - if b.GreaterThan(bestAvail) { - leastBad = addr - bestAvail = b - } + if bestAvail.GreaterThan(minFunds) { + log.Warnw("No address had enough funds to for full PoSt message Fee, selecting least bad address", "address", leastBad, "balance", types.FIL(bestAvail), "optimalFunds", types.FIL(goodFunds), "minFunds", types.FIL(minFunds)) - log.Warnw("control address didn't have enough funds for window post message", "address", addr, "required", types.FIL(goodFunds), "balance", types.FIL(b)) + return leastBad, bestAvail, nil } - // Try to use the owner account if we can, fallback to worker if we can't + // This most likely won't work, but can't hurt to try - k, err := a.StateAccountKey(ctx, mi.Owner, types.EmptyTSK) + workerBalance, err := a.WalletBalance(ctx, mi.Worker) if err != nil { - log.Errorw("getting owner account key", "error", err) - return mi.Worker, big.Zero(), nil + return address.Undef, big.Zero(), xerrors.Errorf("checking owner balance: %w", err) } - haveOwner, err := a.WalletHas(ctx, k) + log.Warnw("No address had enough funds to for minimum PoSt message Fee, selecting worker address as a fallback", "address", mi.Worker, "balance", types.FIL(workerBalance), "optimalFunds", types.FIL(goodFunds), "minFunds", types.FIL(minFunds)) + return mi.Worker, workerBalance, nil +} + +func maybeUseAddress(ctx context.Context, a addrSelectApi, addr address.Address, goodFunds abi.TokenAmount, leastBad *address.Address, bestAvail *abi.TokenAmount) bool { + b, err := a.WalletBalance(ctx, addr) if err != nil { - return address.Undef, big.Zero(), xerrors.Errorf("failed to check owner address: %w", err) + log.Errorw("checking control address balance", "addr", addr, "error", err) + return false } - if haveOwner { - ownerBalance, err := a.WalletBalance(ctx, mi.Owner) + if b.GreaterThanEqual(goodFunds) { + k, err := a.StateAccountKey(ctx, addr, types.EmptyTSK) if err != nil { - return address.Undef, big.Zero(), xerrors.Errorf("checking owner balance: %w", err) + log.Errorw("getting account key", "error", err) + return false } - if ownerBalance.GreaterThanEqual(goodFunds) { - return mi.Owner, goodFunds, nil + have, err := a.WalletHas(ctx, k) + if err != nil { + log.Errorw("failed to check control address", "addr", addr, "error", err) + return false } - if ownerBalance.GreaterThan(bestAvail) { - leastBad = mi.Owner - bestAvail = ownerBalance + if !have { + log.Errorw("don't have key", "key", k) + return false } - } - - workerBalance, err := a.WalletBalance(ctx, mi.Worker) - if err != nil { - return address.Undef, big.Zero(), xerrors.Errorf("checking owner balance: %w", err) - } - - if workerBalance.GreaterThanEqual(goodFunds) { - return mi.Worker, goodFunds, nil - } - if workerBalance.GreaterThan(bestAvail) { - leastBad = mi.Worker - bestAvail = workerBalance + *leastBad = addr + *bestAvail = b + return true } - if bestAvail.GreaterThan(minFunds) { - log.Warnw("No address had enough funds to for full PoSt message Fee, selecting least bad address", "address", leastBad, "balance", types.FIL(bestAvail), "optimalFunds", types.FIL(goodFunds), "minFunds", types.FIL(minFunds)) - - return leastBad, bestAvail, nil + if b.GreaterThan(*bestAvail) { + *leastBad = addr + *bestAvail = b } - // This most likely won't work, but can't hurt to try - - log.Warnw("No address had enough funds to for minimum PoSt message Fee, selecting worker address as a fallback", "address", mi.Worker, "balance", types.FIL(workerBalance), "optimalFunds", types.FIL(goodFunds), "minFunds", types.FIL(minFunds)) - return mi.Worker, workerBalance, nil + log.Warnw("address didn't have enough funds for window post message", "address", addr, "required", types.FIL(goodFunds), "balance", types.FIL(b)) + return false } From b7a282deccfb489db1446b0ed1c47b91072c7e73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Thu, 19 Nov 2020 18:30:53 +0100 Subject: [PATCH 3/5] wdpost: fix build --- storage/wdpost_run.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/storage/wdpost_run.go b/storage/wdpost_run.go index daaa0af93ef..7cb656f307c 100644 --- a/storage/wdpost_run.go +++ b/storage/wdpost_run.go @@ -809,7 +809,11 @@ func (s *WindowPoStScheduler) setSender(ctx context.Context, msg *types.Message, msg.From = pa bestReq := big.Add(msg.RequiredFunds(), msg.Value) if avail.LessThan(bestReq) { - messagepool.CapGasFee(msg, big.Min(big.Sub(avail, msg.Value), msg.RequiredFunds())) + mff := func() (abi.TokenAmount, error) { + return msg.RequiredFunds(), nil + } + + messagepool.CapGasFee(mff, msg, big.Min(big.Sub(avail, msg.Value), msg.RequiredFunds())) } return nil } From b4fa099257325360436a46c84bf1dd7047df500d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Thu, 19 Nov 2020 18:57:43 +0100 Subject: [PATCH 4/5] wdpost: fix TestWDPostDoPost --- storage/wdpost_run_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/storage/wdpost_run_test.go b/storage/wdpost_run_test.go index 9506abe2199..d426f5f31cd 100644 --- a/storage/wdpost_run_test.go +++ b/storage/wdpost_run_test.go @@ -98,6 +98,14 @@ func (m *mockStorageMinerAPI) StateWaitMsg(ctx context.Context, cid cid.Cid, con }, nil } +func (m *mockStorageMinerAPI) GasEstimateGasPremium(_ context.Context, nblocksincl uint64, sender address.Address, gaslimit int64, tsk types.TipSetKey) (types.BigInt, error) { + return big.Zero(), nil +} + +func (m *mockStorageMinerAPI) GasEstimateFeeCap(context.Context, *types.Message, int64, types.TipSetKey) (types.BigInt, error) { + return big.Zero(), nil +} + type mockProver struct { } From 1999156d06412511d3e5bfec73fc6aff11b81048 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Thu, 19 Nov 2020 20:37:00 +0100 Subject: [PATCH 5/5] wdpost: always pick least-bad address --- storage/addresses.go | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/storage/addresses.go b/storage/addresses.go index 8d535b589f9..f34625a5629 100644 --- a/storage/addresses.go +++ b/storage/addresses.go @@ -3,8 +3,6 @@ package storage import ( "context" - "golang.org/x/xerrors" - "github.com/filecoin-project/go-address" "github.com/filecoin-project/go-state-types/abi" "github.com/filecoin-project/go-state-types/big" @@ -44,21 +42,9 @@ func AddressFor(ctx context.Context, a addrSelectApi, mi miner.MinerInfo, use Ad } } - if bestAvail.GreaterThan(minFunds) { - log.Warnw("No address had enough funds to for full PoSt message Fee, selecting least bad address", "address", leastBad, "balance", types.FIL(bestAvail), "optimalFunds", types.FIL(goodFunds), "minFunds", types.FIL(minFunds)) - - return leastBad, bestAvail, nil - } - - // This most likely won't work, but can't hurt to try - - workerBalance, err := a.WalletBalance(ctx, mi.Worker) - if err != nil { - return address.Undef, big.Zero(), xerrors.Errorf("checking owner balance: %w", err) - } + log.Warnw("No address had enough funds to for full PoSt message Fee, selecting least bad address", "address", leastBad, "balance", types.FIL(bestAvail), "optimalFunds", types.FIL(goodFunds), "minFunds", types.FIL(minFunds)) - log.Warnw("No address had enough funds to for minimum PoSt message Fee, selecting worker address as a fallback", "address", mi.Worker, "balance", types.FIL(workerBalance), "optimalFunds", types.FIL(goodFunds), "minFunds", types.FIL(minFunds)) - return mi.Worker, workerBalance, nil + return leastBad, bestAvail, nil } func maybeUseAddress(ctx context.Context, a addrSelectApi, addr address.Address, goodFunds abi.TokenAmount, leastBad *address.Address, bestAvail *abi.TokenAmount) bool {