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

Allow negative performing chains and reduce block packing when the basefee is sky high #3550

Closed
wants to merge 5 commits into from
Closed

Allow negative performing chains and reduce block packing when the basefee is sky high #3550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2ab6099
remove unused ts argument from Trim
vyzo Sep 4, 2020
14f4f9b
allow negative gas perf and decrease block packing when the baseFee i…
vyzo Sep 4, 2020
9647ba0
add selection test for sky high base fee conditions
vyzo Sep 4, 2020
40ca582
optimal selection partitioning should make smaller blocks when in sky…
vyzo Sep 4, 2020
c068bc9
Adjust numbers
Sep 4, 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
5 changes: 4 additions & 1 deletion chain/messagepool/messagepool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,16 @@ type testMpoolAPI struct {
tipsets []*types.TipSet

published int

baseFee types.BigInt
}

func newTestMpoolAPI() *testMpoolAPI {
tma := &testMpoolAPI{
bmsgs: make(map[cid.Cid][]*types.SignedMessage),
statenonce: make(map[address.Address]uint64),
balance: make(map[address.Address]types.BigInt),
baseFee: types.NewInt(100),
}
genesis := mock.MkBlock(nil, 1, 1)
tma.tipsets = append(tma.tipsets, mock.TipSet(genesis))
Expand Down Expand Up @@ -182,7 +185,7 @@ func (tma *testMpoolAPI) LoadTipSet(tsk types.TipSetKey) (*types.TipSet, error)
}

func (tma *testMpoolAPI) ChainComputeBaseFee(ctx context.Context, ts *types.TipSet) (types.BigInt, error) {
return types.NewInt(100), nil
return tma.baseFee, nil
}

func assertNonce(t *testing.T, mp *MessagePool, addr address.Address, val uint64) {
Expand Down
11 changes: 9 additions & 2 deletions chain/messagepool/repub.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ func (mp *MessagePool) republishPendingMessages() error {

gasLimit := int64(build.BlockGasLimit)
minGas := int64(gasguess.MinGas)

allowNegative := false
if skyHighBaseFeeThreshold.LessThan(baseFee) {
allowNegative = true
gasLimit = int64(float64(gasLimit) * skyHighBaseFeeGasLimitRatio)
}

var msgs []*types.SignedMessage
for i := 0; i < len(chains); {
chain := chains[i]
Expand All @@ -91,7 +98,7 @@ func (mp *MessagePool) republishPendingMessages() error {

// we don't republish negative performing chains, as they won't be included in
// a block anyway
if chain.gasPerf < 0 {
if !allowNegative && chain.gasPerf < 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

As a note, we should republish slightly negative chains. Base fee can fall quite quickly.

Copy link
Contributor

@Kubuxu Kubuxu Sep 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I didn't even think about this effect. In essence, miners who updated to 0.5.10 won't re-pub messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about that, we might as well wait for the next republish interval.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But yeah, the effect is real -- and I didn't even think about it either.

break
}

Expand All @@ -111,7 +118,7 @@ func (mp *MessagePool) republishPendingMessages() error {

// we can't fit the current chain but there is gas to spare
// trim it and push it down
chain.Trim(gasLimit, mp, baseFee, ts)
chain.Trim(gasLimit, mp, baseFee, allowNegative)
for j := i; j < len(chains)-1; j++ {
if chains[j].Before(chains[j+1]) {
break
Expand Down
63 changes: 45 additions & 18 deletions chain/messagepool/selection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ import (
abig "github.com/filecoin-project/specs-actors/actors/abi/big"
)

var bigBlockGasLimit = big.NewInt(build.BlockGasLimit)
var (
bigBlockGasLimit = big.NewInt(build.BlockGasLimit)

skyHighBaseFeeThreshold = abig.NewInt(111902962585)
skyHighBaseFeeGasLimitRatio = 0.4
)

const MaxBlocks = 15

Expand Down Expand Up @@ -59,6 +64,11 @@ func (mp *MessagePool) selectMessagesOptimal(curTs, ts *types.TipSet, tq float64
return nil, xerrors.Errorf("computing basefee: %w", err)
}

allowNegative := false
if skyHighBaseFeeThreshold.LessThan(baseFee) {
allowNegative = true
}

// 0. Load messages from the target tipset; if it is the same as the current tipset in
// the mpool, then this is just the pending messages
pending, err := mp.getPendingMessages(curTs, ts)
Expand Down Expand Up @@ -100,8 +110,8 @@ func (mp *MessagePool) selectMessagesOptimal(curTs, ts *types.TipSet, tq float64
return chains[i].Before(chains[j])
})

if len(chains) != 0 && chains[0].gasPerf < 0 {
log.Warnw("all messages in mpool have non-positive gas performance", "bestGasPerf", chains[0].gasPerf)
if len(chains) != 0 && !allowNegative && chains[0].gasPerf < 0 {
log.Warnw("all messages in mpool have negative gas performance", "bestGasPerf", chains[0].gasPerf)
return nil, nil
}

Expand All @@ -112,6 +122,12 @@ func (mp *MessagePool) selectMessagesOptimal(curTs, ts *types.TipSet, tq float64
partitions := make([][]*msgChain, MaxBlocks)
for i := 0; i < MaxBlocks && nextChain < len(chains); i++ {
gasLimit := int64(build.BlockGasLimit)

// if base fee is sky high and we allow negatives, then the blocks will be smaller
if allowNegative {
gasLimit = int64(float64(gasLimit) * skyHighBaseFeeGasLimitRatio)
}

for nextChain < len(chains) {
chain := chains[nextChain]
nextChain++
Expand Down Expand Up @@ -153,7 +169,7 @@ func (mp *MessagePool) selectMessagesOptimal(curTs, ts *types.TipSet, tq float64
last := len(chains)
for i, chain := range chains {
// did we run out of performing chains?
if chain.gasPerf < 0 {
if !allowNegative && chain.gasPerf < 0 {
break
}

Expand Down Expand Up @@ -217,7 +233,7 @@ tailLoop:
for gasLimit >= minGas && last < len(chains) {
// trim if necessary
if chains[last].gasLimit > gasLimit {
chains[last].Trim(gasLimit, mp, baseFee, ts)
chains[last].Trim(gasLimit, mp, baseFee, allowNegative)
}

// push down if it hasn't been invalidated
Expand All @@ -243,7 +259,7 @@ tailLoop:
}

// if gasPerf < 0 we have no more profitable chains
if chain.gasPerf < 0 {
if !allowNegative && chain.gasPerf < 0 {
break tailLoop
}

Expand Down Expand Up @@ -284,7 +300,7 @@ tailLoop:
}

// dependencies fit, just trim it
chain.Trim(gasLimit-depGasLimit, mp, baseFee, ts)
chain.Trim(gasLimit-depGasLimit, mp, baseFee, allowNegative)
last += i
continue tailLoop
}
Expand All @@ -308,6 +324,11 @@ func (mp *MessagePool) selectMessagesGreedy(curTs, ts *types.TipSet) ([]*types.S
return nil, xerrors.Errorf("computing basefee: %w", err)
}

allowNegative := false
if skyHighBaseFeeThreshold.LessThan(baseFee) {
allowNegative = true
}

// 0. Load messages for the target tipset; if it is the same as the current tipset in the mpool
// then this is just the pending messages
pending, err := mp.getPendingMessages(curTs, ts)
Expand Down Expand Up @@ -349,8 +370,8 @@ func (mp *MessagePool) selectMessagesGreedy(curTs, ts *types.TipSet) ([]*types.S
return chains[i].Before(chains[j])
})

if len(chains) != 0 && chains[0].gasPerf < 0 {
log.Warnw("all messages in mpool have non-positive gas performance", "bestGasPerf", chains[0].gasPerf)
if len(chains) != 0 && !allowNegative && chains[0].gasPerf < 0 {
log.Warnw("all messages in mpool have negative gas performance", "bestGasPerf", chains[0].gasPerf)
return nil, nil
}

Expand All @@ -360,7 +381,7 @@ func (mp *MessagePool) selectMessagesGreedy(curTs, ts *types.TipSet) ([]*types.S
last := len(chains)
for i, chain := range chains {
// did we run out of performing chains?
if chain.gasPerf < 0 {
if !allowNegative && chain.gasPerf < 0 {
break
}

Expand Down Expand Up @@ -389,7 +410,7 @@ func (mp *MessagePool) selectMessagesGreedy(curTs, ts *types.TipSet) ([]*types.S
tailLoop:
for gasLimit >= minGas && last < len(chains) {
// trim
chains[last].Trim(gasLimit, mp, baseFee, ts)
chains[last].Trim(gasLimit, mp, baseFee, allowNegative)

// push down if it hasn't been invalidated
if chains[last].valid {
Expand All @@ -409,7 +430,7 @@ tailLoop:
}

// if gasPerf < 0 we have no more profitable chains
if chain.gasPerf < 0 {
if !allowNegative && chain.gasPerf < 0 {
break tailLoop
}

Expand Down Expand Up @@ -448,6 +469,12 @@ func (mp *MessagePool) selectPriorityMessages(pending map[address.Address]map[ui
gasLimit := int64(build.BlockGasLimit)
minGas := int64(gasguess.MinGas)

allowNegative := false
if skyHighBaseFeeThreshold.LessThan(baseFee) {
allowNegative = true
gasLimit = int64(float64(gasLimit) * skyHighBaseFeeGasLimitRatio)
}

// 1. Get priority actor chains
var chains []*msgChain
priority := mp.cfg.PriorityAddrs
Expand All @@ -471,15 +498,15 @@ func (mp *MessagePool) selectPriorityMessages(pending map[address.Address]map[ui
return chains[i].Before(chains[j])
})

if len(chains) != 0 && chains[0].gasPerf < 0 {
if len(chains) != 0 && !allowNegative && chains[0].gasPerf < 0 {
log.Warnw("all priority messages in mpool have negative gas performance", "bestGasPerf", chains[0].gasPerf)
return nil, gasLimit
}

// 3. Merge chains until the block limit, as long as they have non-negative gas performance
last := len(chains)
for i, chain := range chains {
if chain.gasPerf < 0 {
if !allowNegative && chain.gasPerf < 0 {
break
}

Expand All @@ -497,7 +524,7 @@ func (mp *MessagePool) selectPriorityMessages(pending map[address.Address]map[ui
tailLoop:
for gasLimit >= minGas && last < len(chains) {
// trim, discarding negative performing messages
chains[last].Trim(gasLimit, mp, baseFee, ts)
chains[last].Trim(gasLimit, mp, baseFee, allowNegative)

// push down if it hasn't been invalidated
if chains[last].valid {
Expand All @@ -517,7 +544,7 @@ tailLoop:
}

// if gasPerf < 0 we have no more profitable chains
if chain.gasPerf < 0 {
if !allowNegative && chain.gasPerf < 0 {
break tailLoop
}

Expand Down Expand Up @@ -775,9 +802,9 @@ func (mc *msgChain) Before(other *msgChain) bool {
(mc.gasPerf == other.gasPerf && mc.gasReward.Cmp(other.gasReward) > 0)
}

func (mc *msgChain) Trim(gasLimit int64, mp *MessagePool, baseFee types.BigInt, ts *types.TipSet) {
func (mc *msgChain) Trim(gasLimit int64, mp *MessagePool, baseFee types.BigInt, allowNegative bool) {
i := len(mc.msgs) - 1
for i >= 0 && (mc.gasLimit > gasLimit || mc.gasPerf < 0) {
for i >= 0 && (mc.gasLimit > gasLimit || (!allowNegative && mc.gasPerf < 0)) {
gasReward := mp.getGasReward(mc.msgs[i], baseFee)
mc.gasReward = new(big.Int).Sub(mc.gasReward, gasReward)
mc.gasLimit -= mc.msgs[i].Message.GasLimit
Expand Down
110 changes: 110 additions & 0 deletions chain/messagepool/selection_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,116 @@ func TestPriorityMessageSelection2(t *testing.T) {
}
}

func TestSkyHighBaseFeeMessageSelection(t *testing.T) {
mp, tma := makeTestMpool()

w1, err := wallet.NewWallet(wallet.NewMemKeyStore())
if err != nil {
t.Fatal(err)
}

a1, err := w1.GenerateKey(crypto.SigTypeSecp256k1)
if err != nil {
t.Fatal(err)
}

w2, err := wallet.NewWallet(wallet.NewMemKeyStore())
if err != nil {
t.Fatal(err)
}

a2, err := w2.GenerateKey(crypto.SigTypeSecp256k1)
if err != nil {
t.Fatal(err)
}

block := tma.nextBlock()
ts := mock.TipSet(block)
tma.applyBlock(t, block)

gasLimit := gasguess.Costs[gasguess.CostKey{Code: builtin.StorageMarketActorCodeID, M: 2}]

tma.setBalance(a1, 1) // in FIL
tma.setBalance(a2, 1) // in FIL

// we create 1 block worth of messages; the selection algorithms should only fill the block
// up to the sky high ratio
tma.baseFee = types.BigAdd(skyHighBaseFeeThreshold, types.NewInt(1_000_000))
skyHighGasLimit := int64(float64(build.BlockGasLimit) * skyHighBaseFeeGasLimitRatio)

nMessages := int((build.BlockGasLimit / gasLimit) + 1)
for i := 0; i < nMessages; i++ {
bias := (nMessages - i) / 3
m := makeTestMessage(w1, a1, a2, uint64(i), gasLimit, uint64(1+i%3+bias))
mustAdd(t, mp, m)
}

// test greedy selection
msgs, err := mp.SelectMessages(ts, 1.0)
if err != nil {
t.Fatal(err)
}

mGasLimit := int64(0)
for _, m := range msgs {
mGasLimit += m.Message.GasLimit
}

if mGasLimit > skyHighGasLimit {
t.Fatalf("expected block gas limit to be less than sky high gas limit; got %d, limit is %d",
mGasLimit, skyHighGasLimit)
}

// test optimal selection
msgs, err = mp.SelectMessages(ts, 0.1)
if err != nil {
t.Fatal(err)
}

mGasLimit = 0
for _, m := range msgs {
mGasLimit += m.Message.GasLimit
}

if mGasLimit > skyHighGasLimit {
t.Fatalf("expected block gas limit to be less than sky high gas limit; got %d, limit is %d",
mGasLimit, skyHighGasLimit)
}

// test priority selection
mp.cfg.PriorityAddrs = []address.Address{a1}

msgs, err = mp.SelectMessages(ts, 1.0)
if err != nil {
t.Fatal(err)
}

mGasLimit = int64(0)
for _, m := range msgs {
mGasLimit += m.Message.GasLimit
}

if mGasLimit > skyHighGasLimit {
t.Fatalf("expected block gas limit to be less than sky high gas limit; got %d, limit is %d",
mGasLimit, skyHighGasLimit)
}

msgs, err = mp.SelectMessages(ts, 0.1)
if err != nil {
t.Fatal(err)
}

mGasLimit = 0
for _, m := range msgs {
mGasLimit += m.Message.GasLimit
}

if mGasLimit > skyHighGasLimit {
t.Fatalf("expected block gas limit to be less than sky high gas limit; got %d, limit is %d",
mGasLimit, skyHighGasLimit)
}
}

func TestOptimalMessageSelection1(t *testing.T) {
// this test uses just a single actor sending messages with a low tq
// the chain depenent merging algorithm should pick messages from the actor
Expand Down