Skip to content
This repository has been archived by the owner on Jun 6, 2023. It is now read-only.

Commit

Permalink
BatchBalancer fee charged on precommit aggregate (#1497)
Browse files Browse the repository at this point in the history 
* Refactor fee methods and tests
* Precommit batch pays aggregate fee
* Tests pass
* Balance tests
* Happy path balance test
* Review Response

Co-authored-by: ZenGround0 <[email protected]>
  • Loading branch information
@ZenGround0
ZenGround0 and ZenGround0 authored Oct 1, 2021
1 parent ea6fa6b commit a2369c5
Show file tree
Hide file tree
Showing 7 changed files with 296 additions and 50 deletions.
10 changes: 9 additions & 1 deletion actors/builtin/miner/miner_actor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,14 @@ func (a Actor) PreCommitSectorBatch(rt Runtime, params *PreCommitSectorBatchPara
feeToBurn := abi.NewTokenAmount(0)
var needsCron bool
rt.StateTransaction(&st, func() {
// Aggregate fee applies only when batching.
if len(params.Sectors) > 1 {
aggregateFee := AggregatePreCommitNetworkFee(len(params.Sectors), rt.BaseFee())
// AggregateFee applied to fee debt to consolidate burn with outstanding debts
err := st.ApplyPenalty(aggregateFee)
builtin.RequireNoErr(rt, err, exitcode.ErrIllegalState, "failed to apply penalty")
}

// available balance already accounts for fee debt so it is correct to call
// this before RepayDebts. We would have to
// subtract fee debt explicitly if we called this after.
Expand Down Expand Up @@ -964,7 +972,7 @@ func (a Actor) ProveCommitAggregate(rt Runtime, params *ProveCommitAggregatePara
// Compute and burn the aggregate network fee. We need to re-load the state as
// confirmSectorProofsValid can change it.
rt.StateReadonly(&st)
aggregateFee := AggregateNetworkFee(len(precommitsToConfirm), rt.BaseFee())
aggregateFee := AggregateProveCommitNetworkFee(len(precommitsToConfirm), rt.BaseFee())
unlockedBalance, err := st.GetUnlockedBalance(rt.CurrentBalance())
builtin.RequireNoErr(rt, err, exitcode.ErrIllegalState, "failed to determine unlocked balance")
if unlockedBalance.LessThan(aggregateFee) {
Expand Down
208 changes: 197 additions & 11 deletions actors/builtin/miner/miner_commitment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,21 +440,25 @@ func TestPreCommitBatch(t *testing.T) {
name string
batchSize int
balanceSurplus abi.TokenAmount
baseFee abi.TokenAmount
deals []dealSpec
exit exitcode.ExitCode
error string
}{{
name: "one sector",
batchSize: 1,
balanceSurplus: big.Zero(),
baseFee: big.Zero(),
}, {
name: "max sectors",
batchSize: 32,
balanceSurplus: big.Zero(),
baseFee: big.Zero(),
}, {
name: "one deal",
batchSize: 3,
balanceSurplus: big.Zero(),
baseFee: big.Zero(),
deals: []dealSpec{{
size: 32 << 30,
verifiedSize: 0,
Expand All @@ -464,6 +468,7 @@ func TestPreCommitBatch(t *testing.T) {
name: "many deals",
batchSize: 3,
balanceSurplus: big.Zero(),
baseFee: big.Zero(),
deals: []dealSpec{{
size: 32 << 30,
verifiedSize: 0,
Expand All @@ -481,17 +486,20 @@ func TestPreCommitBatch(t *testing.T) {
name: "empty batch",
batchSize: 0,
balanceSurplus: big.Zero(),
baseFee: big.Zero(),
exit: exitcode.ErrIllegalArgument,
error: "batch empty",
}, {
name: "too many sectors",
batchSize: 257,
balanceSurplus: big.Zero(),
baseFee: big.Zero(),
exit: exitcode.ErrIllegalArgument,
error: "batch of 257 too large",
}, {
name: "insufficient balance",
batchSize: 10,
baseFee: big.Zero(),
balanceSurplus: abi.NewTokenAmount(1).Neg(),
exit: exitcode.ErrInsufficientFunds,
error: "insufficient funds",
Expand Down Expand Up @@ -522,6 +530,7 @@ func TestPreCommitBatch(t *testing.T) {
firstForMiner: true,
}
deposits := make([]big.Int, batchSize)

for i := 0; i < batchSize; i++ {
deals := dealSpec{}
if len(test.deals) > i {
Expand All @@ -539,13 +548,16 @@ func TestPreCommitBatch(t *testing.T) {
}
pwrEstimate := miner.QAPowerForWeight(actor.sectorSize, sectors[i].Expiration-precommitEpoch, dealWeight, verifiedDealWeight)
deposits[i] = miner.PreCommitDepositForPower(actor.epochRewardSmooth, actor.epochQAPowerSmooth, pwrEstimate)

}
netFee := miner.AggregatePreCommitNetworkFee(batchSize, test.baseFee)
totalDeposit := big.Sum(deposits...)
rt.SetBalance(big.Add(totalDeposit, test.balanceSurplus))
totalBalance := big.Add(netFee, totalDeposit)
rt.SetBalance(big.Add(totalBalance, test.balanceSurplus))

if test.exit != exitcode.Ok {
rt.ExpectAbortContainsMessage(test.exit, test.error, func() {
actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: sectors}, conf)
actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: sectors}, conf, test.baseFee)

// State untouched.
st := getState(rt)
Expand All @@ -555,7 +567,7 @@ func TestPreCommitBatch(t *testing.T) {
})
return
}
precommits := actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: sectors}, conf)
precommits := actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: sectors}, conf, test.baseFee)

// Check precommits
st := getState(rt)
Expand Down Expand Up @@ -607,7 +619,7 @@ func TestPreCommitBatch(t *testing.T) {
}

rt.ExpectAbortContainsMessage(exitcode.ErrIllegalArgument, "sector expiration", func() {
actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: sectors}, preCommitBatchConf{firstForMiner: true})
actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: sectors}, preCommitBatchConf{firstForMiner: true}, big.Zero())
})
})

Expand All @@ -628,7 +640,7 @@ func TestPreCommitBatch(t *testing.T) {
*actor.makePreCommit(100, precommitEpoch-1, sectorExpiration, nil),
}
rt.ExpectAbortContainsMessage(exitcode.ErrIllegalArgument, "duplicate sector number 100", func() {
actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: sectors}, preCommitBatchConf{firstForMiner: true})
actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: sectors}, preCommitBatchConf{firstForMiner: true}, big.Zero())
})
})
}
Expand Down Expand Up @@ -789,7 +801,7 @@ func TestProveCommit(t *testing.T) {
firstForMiner: true,
}

precommits := actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: sectors}, conf)
precommits := actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: sectors}, conf, big.Zero())

rt.SetEpoch(proveCommitEpoch)
noDealPower := miner.QAPowerForWeight(actor.sectorSize, sectorExpiration-proveCommitEpoch, big.Zero(), big.Zero())
Expand Down Expand Up @@ -1210,10 +1222,10 @@ func TestAggregateProveCommit(t *testing.T) {
})
}

func TestProveCommitAggregateFailures(t *testing.T) {
func TestBatchMethodNetworkFees(t *testing.T) {
periodOffset := abi.ChainEpoch(100)

t.Run("insufficient funds for network fee", func(t *testing.T) {
t.Run("insufficient funds for aggregated prove commit network fee", func(t *testing.T) {
actor := newHarness(t, periodOffset)
rt := builderForHarness(actor).
WithBalance(bigBalance, big.Zero()).
Expand All @@ -1224,7 +1236,6 @@ func TestProveCommitAggregateFailures(t *testing.T) {
dlInfo := actor.deadline(rt)

// Make a good commitment for the proof to target.

proveCommitEpoch := precommitEpoch + miner.PreCommitChallengeDelay + 1
expiration := dlInfo.PeriodEnd() + defaultSectorExpiration*miner.WPoStProvingPeriod // something on deadline boundary but > 180 days

Expand All @@ -1240,16 +1251,191 @@ func TestProveCommitAggregateFailures(t *testing.T) {
sectorNosBf, err := sectorNosBf.Copy() //flush map to run to match partition state
require.NoError(t, err)

// set base fee extremely high so AggregateNetworkFee is > 1000 FIL. Set balance to 1000 FIL to easily cover IP but not cover network fee
// set base fee extremely high so AggregateProveCommitNetworkFee is > 1000 FIL. Set balance to 1000 FIL to easily cover IP but not cover network fee
rt.SetEpoch(proveCommitEpoch)
balance := big.Mul(big.NewInt(1000), big.NewInt(1e18))
rt.SetBalance(balance)
baseFee := big.NewInt(1e16)
rt.SetBaseFee(baseFee)
require.True(t, miner.AggregateNetworkFee(len(precommits), baseFee).GreaterThan(balance))
require.True(t, miner.AggregateProveCommitNetworkFee(len(precommits), baseFee).GreaterThan(balance))

rt.ExpectAbort(exitcode.ErrInsufficientFunds, func() {
actor.proveCommitAggregateSector(rt, proveCommitConf{}, precommits, makeProveCommitAggregate(sectorNosBf), baseFee)
})
})

t.Run("insufficient funds for batch precommit network fee", func(t *testing.T) {
actor := newHarness(t, periodOffset)
rt := builderForHarness(actor).
WithBalance(bigBalance, big.Zero()).
Build(t)
precommitEpoch := periodOffset + 1
rt.SetEpoch(precommitEpoch)
actor.constructAndVerify(rt)
dlInfo := actor.deadline(rt)
expiration := dlInfo.PeriodEnd() + defaultSectorExpiration*miner.WPoStProvingPeriod // something on deadline boundary but > 180 days

var precommits []miner0.SectorPreCommitInfo
sectorNosBf := bitfield.New()
for i := 0; i < 4; i++ {
sectorNo := abi.SectorNumber(i)
sectorNosBf.Set(uint64(i))
precommit := actor.makePreCommit(sectorNo, precommitEpoch-1, expiration, nil)
precommits = append(precommits, *precommit)
}

// set base fee extremely high so AggregateProveCommitNetworkFee is > 1000 FIL. Set balance to 1000 FIL to easily cover PCD but not network fee
balance := big.Mul(big.NewInt(1000), big.NewInt(1e18))
rt.SetBalance(balance)
baseFee := big.NewInt(1e16)
rt.SetBaseFee(baseFee)
require.True(t, miner.AggregatePreCommitNetworkFee(len(precommits), baseFee).GreaterThan(balance))

rt.ExpectAbortContainsMessage(exitcode.ErrInsufficientFunds, "unlocked balance can not repay fee debt", func() {
actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: precommits}, preCommitBatchConf{firstForMiner: true}, baseFee)

// State untouched.
st := getState(rt)
assert.True(t, st.PreCommitDeposits.IsZero())
expirations := actor.collectPrecommitExpirations(rt, st)
assert.Equal(t, map[abi.ChainEpoch][]uint64{}, expirations)
})
})

t.Run("insufficient funds for batch precommit in combination of fee debt and network fee", func(t *testing.T) {
actor := newHarness(t, periodOffset)
rt := builderForHarness(actor).
WithBalance(bigBalance, big.Zero()).
Build(t)
precommitEpoch := periodOffset + 1
rt.SetEpoch(precommitEpoch)
actor.constructAndVerify(rt)
dlInfo := actor.deadline(rt)
expiration := dlInfo.PeriodEnd() + defaultSectorExpiration*miner.WPoStProvingPeriod // something on deadline boundary but > 180 days

var precommits []miner0.SectorPreCommitInfo
sectorNosBf := bitfield.New()
for i := 0; i < 4; i++ {
sectorNo := abi.SectorNumber(i)
sectorNosBf.Set(uint64(i))
precommit := actor.makePreCommit(sectorNo, precommitEpoch-1, expiration, nil)
precommits = append(precommits, *precommit)
}

// set base fee and fee debt high enough so that either could be repaid on its own, but together repayment fails
baseFee := big.NewInt(1e16)
rt.SetBaseFee(baseFee)
netFee := miner.AggregatePreCommitNetworkFee(len(precommits), baseFee)
// setup miner to have fee debt equal to net fee
st := getState(rt)
st.FeeDebt = netFee
rt.ReplaceState(st)

// Give miner almost enough balance to pay both
balance := big.Mul(big.NewInt(2), netFee)
balance = big.Sub(balance, big.NewInt(1))
rt.SetBalance(balance)

rt.ExpectAbortContainsMessage(exitcode.ErrInsufficientFunds, "unlocked balance can not repay fee debt", func() {
actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: precommits}, preCommitBatchConf{firstForMiner: true}, baseFee)

// State untouched.
st := getState(rt)
assert.True(t, st.PreCommitDeposits.IsZero())
expirations := actor.collectPrecommitExpirations(rt, st)
assert.Equal(t, map[abi.ChainEpoch][]uint64{}, expirations)
})
})

t.Run("enough funds for fee debt and network fee but not for PCD", func(t *testing.T) {
actor := newHarness(t, periodOffset)
rt := builderForHarness(actor).
WithBalance(bigBalance, big.Zero()).
Build(t)
precommitEpoch := periodOffset + 1
rt.SetEpoch(precommitEpoch)
actor.constructAndVerify(rt)
dlInfo := actor.deadline(rt)
expiration := dlInfo.PeriodEnd() + defaultSectorExpiration*miner.WPoStProvingPeriod // something on deadline boundary but > 180 days

var precommits []miner0.SectorPreCommitInfo
sectorNosBf := bitfield.New()
for i := 0; i < 4; i++ {
sectorNo := abi.SectorNumber(i)
sectorNosBf.Set(uint64(i))
precommit := actor.makePreCommit(sectorNo, precommitEpoch-1, expiration, nil)
precommits = append(precommits, *precommit)
}

// set base fee and fee debt high
baseFee := big.NewInt(1e16)
rt.SetBaseFee(baseFee)
netFee := miner.AggregatePreCommitNetworkFee(len(precommits), baseFee)
// setup miner to have fee debt equal to net fee
st := getState(rt)
st.FeeDebt = netFee
rt.ReplaceState(st)

// Give miner enough balance to pay both but not any extra for pcd
balance := big.Mul(big.NewInt(2), netFee)
rt.SetBalance(balance)

rt.ExpectAbortContainsMessage(exitcode.ErrInsufficientFunds, "insufficient funds 0 for pre-commit deposit:", func() {
actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: precommits}, preCommitBatchConf{firstForMiner: true}, baseFee)

// State untouched.
st := getState(rt)
assert.True(t, st.PreCommitDeposits.IsZero())
expirations := actor.collectPrecommitExpirations(rt, st)
assert.Equal(t, map[abi.ChainEpoch][]uint64{}, expirations)
})
})

t.Run("enough funds for everything", func(t *testing.T) {
actor := newHarness(t, periodOffset)
rt := builderForHarness(actor).
WithBalance(bigBalance, big.Zero()).
Build(t)
precommitEpoch := periodOffset + 1
rt.SetEpoch(precommitEpoch)
actor.constructAndVerify(rt)
dlInfo := actor.deadline(rt)
expiration := dlInfo.PeriodEnd() + defaultSectorExpiration*miner.WPoStProvingPeriod // something on deadline boundary but > 180 days

var precommits []miner0.SectorPreCommitInfo
sectorNosBf := bitfield.New()
for i := 0; i < 4; i++ {
sectorNo := abi.SectorNumber(i)
sectorNosBf.Set(uint64(i))
precommit := actor.makePreCommit(sectorNo, precommitEpoch-1, expiration, nil)
precommits = append(precommits, *precommit)
}

// set base fee and fee debt high
baseFee := big.NewInt(1e16)
rt.SetBaseFee(baseFee)
netFee := miner.AggregatePreCommitNetworkFee(len(precommits), baseFee)
// setup miner to have fee debt equal to net fee
st := getState(rt)
st.FeeDebt = netFee
rt.ReplaceState(st)

// Give miner enough balance to pay both and pcd
balance := big.Mul(big.NewInt(2), netFee)
oneSectorPowerEstimate := miner.QAPowerForWeight(actor.sectorSize, expiration-precommitEpoch, big.Zero(), big.Zero())
expectedDeposit := miner.PreCommitDepositForPower(actor.epochRewardSmooth, actor.epochQAPowerSmooth, big.Mul(big.NewInt(4), oneSectorPowerEstimate))
balance = big.Add(balance, expectedDeposit)
rt.SetBalance(balance)

actor.preCommitSectorBatch(rt, &miner.PreCommitSectorBatchParams{Sectors: precommits}, preCommitBatchConf{firstForMiner: true}, baseFee)
// State updated
st = getState(rt)
assert.Equal(t, expectedDeposit, st.PreCommitDeposits)
expirations := actor.collectPrecommitExpirations(rt, st)
assert.Equal(t, 1, len(expirations)) // one expiration epoch
for _, exp := range expirations {
assert.Equal(t, 4, len(exp)) // 4 precommits expiring
}
})

}
11 changes: 7 additions & 4 deletions actors/builtin/miner/miner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4892,7 +4892,7 @@ type preCommitBatchConf struct {
firstForMiner bool
}

func (h *actorHarness) preCommitSectorBatch(rt *mock.Runtime, params *miner.PreCommitSectorBatchParams, conf preCommitBatchConf) []*miner.SectorPreCommitOnChainInfo {
func (h *actorHarness) preCommitSectorBatch(rt *mock.Runtime, params *miner.PreCommitSectorBatchParams, conf preCommitBatchConf, baseFee abi.TokenAmount) []*miner.SectorPreCommitOnChainInfo {
rt.SetCaller(h.worker, builtin.AccountActorCodeID)
rt.ExpectValidateCallerAddr(append(h.controlAddrs, h.owner, h.worker)...)
{
Expand Down Expand Up @@ -4933,8 +4933,11 @@ func (h *actorHarness) preCommitSectorBatch(rt *mock.Runtime, params *miner.PreC
rt.ExpectSend(builtin.StorageMarketActorAddr, builtin.MethodsMarket.VerifyDealsForActivation, &vdParams, big.Zero(), &vdReturn, exitcode.Ok)
}
st := getState(rt)
if st.FeeDebt.GreaterThan(big.Zero()) {
rt.ExpectSend(builtin.BurntFundsActorAddr, builtin.MethodSend, nil, st.FeeDebt, nil, exitcode.Ok)
// burn networkFee
if st.FeeDebt.GreaterThan(big.Zero()) || len(params.Sectors) > 1 {
expectedNetworkFee := miner.AggregatePreCommitNetworkFee(len(params.Sectors), baseFee)
expectedBurn := big.Add(expectedNetworkFee, st.FeeDebt)
rt.ExpectSend(builtin.BurntFundsActorAddr, builtin.MethodSend, nil, expectedBurn, nil, exitcode.Ok)
}

if conf.firstForMiner {
Expand Down Expand Up @@ -5080,7 +5083,7 @@ func (h *actorHarness) proveCommitAggregateSector(rt *mock.Runtime, conf proveCo

// burn networkFee
{
expectedFee := miner.AggregateNetworkFee(len(precommits), baseFee)
expectedFee := miner.AggregateProveCommitNetworkFee(len(precommits), baseFee)
rt.ExpectSend(builtin.BurntFundsActorAddr, builtin.MethodSend, nil, expectedFee, nil, exitcode.Ok)
}

Expand Down
Loading

0 comments on commit a2369c5

Please sign in to comment.