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.

Sign up for GitHub

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

Jump to bottom

Set lower feecap on PoSt messages with low balance #4217

Merged
merged 5 commits into from
Nov 20, 2020
Merged

Set lower feecap on PoSt messages with low balance #4217

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
16dbbad
Set lower feecap on PoSt messages with low balance
magik6k Oct 7, 2020
9f807f4
storageminer: Simplify AddressFor
magik6k Oct 26, 2020
b7a282d
wdpost: fix build
magik6k Nov 19, 2020
b4fa099
wdpost: fix TestWDPostDoPost
magik6k Nov 19, 2020
1999156
wdpost: always pick least-bad address
magik6k Nov 19, 2020
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
2 changes: 1 addition & 1 deletion cmd/lotus-storage-miner/actor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
93 changes: 50 additions & 43 deletions storage/addresses.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -28,67 +28,74 @@ 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
}

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

if b.GreaterThanEqual(minFunds) {
k, err := a.StateAccountKey(ctx, addr, types.EmptyTSK)
if err != nil {
log.Errorw("getting account key", "error", err)
continue
}
leastBad := mi.Worker
bestAvail := minFunds
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this method could be written more cleanly as something like:

for _,a := range append(controlAddrs, owner, worker) {
  ok, err := maybeUseAddress(a, &leastBad, &bestAvail)
  if ok {
   return leastBad, bestAvail
  }
}


have, err := a.WalletHas(ctx, k)
if err != nil {
return address.Undef, xerrors.Errorf("failed to check control address: %w", err)
}

if !have {
log.Errorw("don't have key", "key", k)
continue
}

return addr, nil
for _, addr := range append(mi.ControlAddresses, mi.Owner, mi.Worker) {
if maybeUseAddress(ctx, a, addr, goodFunds, &leastBad, &bestAvail) {
return leastBad, bestAvail, nil
}
}

log.Warnw("control address didn't have enough funds for window post message", "address", addr, "required", types.FIL(minFunds), "balance", types.FIL(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))

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

b, err := a.WalletBalance(ctx, mi.Owner)
workerBalance, err := a.WalletBalance(ctx, mi.Worker)
if err != nil {
return address.Undef, xerrors.Errorf("checking owner balance: %w", err)
return address.Undef, big.Zero(), xerrors.Errorf("checking owner balance: %w", err)
}

if !b.GreaterThanEqual(minFunds) {
return mi.Worker, nil
}
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
}

k, err := a.StateAccountKey(ctx, mi.Owner, types.EmptyTSK)
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 {
log.Errorw("getting owner account key", "error", err)
return mi.Worker, nil
log.Errorw("checking control address balance", "addr", addr, "error", err)
return false
}

have, err := a.WalletHas(ctx, k)
if err != nil {
return address.Undef, xerrors.Errorf("failed to check owner address: %w", err)
if b.GreaterThanEqual(goodFunds) {
k, err := a.StateAccountKey(ctx, addr, types.EmptyTSK)
if err != nil {
log.Errorw("getting account key", "error", err)
return false
}

have, err := a.WalletHas(ctx, k)
if err != nil {
log.Errorw("failed to check control address", "addr", addr, "error", err)
return false
}

if !have {
log.Errorw("don't have key", "key", k)
return false
}

*leastBad = addr
*bestAvail = b
return true
}

if !have {
return mi.Worker, nil
if b.GreaterThan(*bestAvail) {
*leastBad = addr
*bestAvail = b
}

return mi.Owner, 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
}
2 changes: 2 additions & 0 deletions storage/miner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
29 changes: 27 additions & 2 deletions storage/wdpost_run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -781,14 +782,38 @@ 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) {
mff := func() (abi.TokenAmount, error) {
return msg.RequiredFunds(), nil
}

messagepool.CapGasFee(mff, msg, big.Min(big.Sub(avail, msg.Value), msg.RequiredFunds()))
}
return nil
}
12 changes: 10 additions & 2 deletions storage/wdpost_run_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -94,8 +98,12 @@ 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
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 {
Expand Down