Skip to content

EIP-1559: miner changes #22896

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.

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

Merged
merged 9 commits into from
May 21, 2021
2 changes: 1 addition & 1 deletion core/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func genTxRing(naccounts int) func(int, *BlockGen) {
from := 0
return func(i int, gen *BlockGen) {
block := gen.PrevBlock(i - 1)
gas := CalcGasLimit(block, block.GasLimit(), block.GasLimit())
gas := block.GasLimit()
for {
gas -= params.TxGas
if gas < params.TxGas {
Expand Down
36 changes: 30 additions & 6 deletions core/block_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,12 @@ func (v *BlockValidator) ValidateState(block *types.Block, statedb *state.StateD
// to keep the baseline gas above the provided floor, and increase it towards the
// ceil if the blocks are full. If the ceil is exceeded, it will always decrease
// the gas allowance.
func CalcGasLimit(parent *types.Block, gasFloor, gasCeil uint64) uint64 {
func CalcGasLimit(parentGasUsed, parentGasLimit, gasFloor, gasCeil uint64) uint64 {
// contrib = (parentGasUsed * 3 / 2) / 1024
contrib := (parent.GasUsed() + parent.GasUsed()/2) / params.GasLimitBoundDivisor
contrib := (parentGasUsed + parentGasUsed/2) / params.GasLimitBoundDivisor

// decay = parentGasLimit / 1024 -1
decay := parent.GasLimit()/params.GasLimitBoundDivisor - 1
decay := parentGasLimit/params.GasLimitBoundDivisor - 1

/*
strategy: gasLimit of block-to-mine is set based on parent's
Expand All @@ -120,21 +120,45 @@ func CalcGasLimit(parent *types.Block, gasFloor, gasCeil uint64) uint64 {
at that usage) the amount increased/decreased depends on how far away
from parentGasLimit * (2/3) parentGasUsed is.
*/
limit := parent.GasLimit() - decay + contrib
limit := parentGasLimit - decay + contrib
if limit < params.MinGasLimit {
limit = params.MinGasLimit
}
// If we're outside our allowed gas range, we try to hone towards them
if limit < gasFloor {
limit = parent.GasLimit() + decay
limit = parentGasLimit + decay
if limit > gasFloor {
limit = gasFloor
}
} else if limit > gasCeil {
limit = parent.GasLimit() - decay
limit = parentGasLimit - decay
if limit < gasCeil {
limit = gasCeil
}
}
return limit
}

// CalcGasLimit1559 calculates the next block gas limit under 1559 rules.
func CalcGasLimit1559(parentGasLimit, desiredLimit uint64) uint64 {
delta := parentGasLimit/params.GasLimitBoundDivisor - 1
limit := parentGasLimit
if desiredLimit < params.MinGasLimit {
desiredLimit = params.MinGasLimit
}
// If we're outside our allowed gas range, we try to hone towards them
if limit < desiredLimit {
limit = parentGasLimit + delta
if limit > desiredLimit {
limit = desiredLimit
}
return limit
}
if limit > desiredLimit {
limit = parentGasLimit - delta
if limit < desiredLimit {
limit = desiredLimit
}
}
return limit
}
33 changes: 33 additions & 0 deletions core/block_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,36 @@ func testHeaderConcurrentAbortion(t *testing.T, threads int) {
t.Errorf("verification count too large: have %d, want below %d", verified, 2*threads)
}
}

func TestCalcGasLimit1559(t *testing.T) {

for i, tc := range []struct {
pGasLimit uint64
max uint64
min uint64
}{
{20000000, 20019530, 19980470},
{40000000, 40039061, 39960939},
} {
// Increase
if have, want := CalcGasLimit1559(tc.pGasLimit, 2*tc.pGasLimit), tc.max; have != want {
t.Errorf("test %d: have %d want <%d", i, have, want)
}
// Decrease
if have, want := CalcGasLimit1559(tc.pGasLimit, 0), tc.min; have != want {
t.Errorf("test %d: have %d want >%d", i, have, want)
}
// Small decrease
if have, want := CalcGasLimit1559(tc.pGasLimit, tc.pGasLimit-1), tc.pGasLimit-1; have != want {
t.Errorf("test %d: have %d want %d", i, have, want)
}
// Small increase
if have, want := CalcGasLimit1559(tc.pGasLimit, tc.pGasLimit+1), tc.pGasLimit+1; have != want {
t.Errorf("test %d: have %d want %d", i, have, want)
}
// No change
if have, want := CalcGasLimit1559(tc.pGasLimit, tc.pGasLimit), tc.pGasLimit; have != want {
t.Errorf("test %d: have %d want %d", i, have, want)
}
}
}
2 changes: 1 addition & 1 deletion core/chain_makers.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ func makeHeader(chain consensus.ChainReader, parent *types.Block, state *state.S
Difficulty: parent.Difficulty(),
UncleHash: parent.UncleHash(),
}),
GasLimit: CalcGasLimit(parent, parent.GasLimit(), parent.GasLimit()),
GasLimit: parent.GasLimit(),
Number: new(big.Int).Add(parent.Number(), common.Big1),
Time: time,
}
Expand Down
2 changes: 1 addition & 1 deletion core/state_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func GenerateBadBlock(parent *types.Block, engine consensus.Engine, txs types.Tr
Difficulty: parent.Difficulty(),
UncleHash: parent.UncleHash(),
}),
GasLimit: CalcGasLimit(parent, parent.GasLimit(), parent.GasLimit()),
GasLimit: parent.GasLimit(),
Number: new(big.Int).Add(parent.Number(), common.Big1),
Time: parent.Time() + 10,
UncleHash: types.EmptyUncleHash,
Expand Down
80 changes: 60 additions & 20 deletions core/types/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var (
ErrUnexpectedProtection = errors.New("transaction type does not supported EIP-155 protected signatures")
ErrInvalidTxType = errors.New("transaction type not valid in this context")
ErrTxTypeNotSupported = errors.New("transaction type not supported")
ErrFeeCapTooLow = errors.New("fee cap less than base fee")
errEmptyTypedTx = errors.New("empty typed transaction bytes")
)

Expand Down Expand Up @@ -299,6 +300,19 @@ func (tx *Transaction) Cost() *big.Int {
return total
}

// EffectiveTip returns the effective miner tip for the given base fee.
// Returns error in case of a negative effective miner tip.
func (tx *Transaction) EffectiveTip(baseFee *big.Int) (*big.Int, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is correct but I also added this function with slightly different semantics in my tx pool PR: my version does not return an error, it returns the negative value which the caller can handle as it wishes. For me this value is useful even if negative (I described the reasons in my gist). Do you think we should have two versions? Or use my version and check at the caller side when necessary? I guess you should check it for the minimum effective fee threshold anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think maybe let's start with this. We could have this method maybe wrap your method, so the 'normal' case doesn't need to handle negative tips

if baseFee == nil {
return tx.Tip(), nil
}
feeCap := tx.FeeCap()
if feeCap.Cmp(baseFee) == -1 {
return nil, ErrFeeCapTooLow
}
return math.BigMin(tx.Tip(), feeCap.Sub(feeCap, baseFee)), nil
}

// RawSignatureValues returns the V, R, S signature values of the transaction.
// The return values should not be modified by the caller.
func (tx *Transaction) RawSignatureValues() (v, r, s *big.Int) {
Expand Down Expand Up @@ -400,24 +414,44 @@ func (s TxByNonce) Len() int { return len(s) }
func (s TxByNonce) Less(i, j int) bool { return s[i].Nonce() < s[j].Nonce() }
func (s TxByNonce) Swap(i, j int) { s[i], s[j] = s[j], s[i] }

// TxWithMinerFee wraps a transaction with its gas price or effective miner tip
type TxWithMinerFee struct {
tx *Transaction
minerFee *big.Int
}

// NewTxWithMinerFee creates a wrapped transaction, calculating the effective
// miner tip if a base fee is provided.
// Returns error in case of a negative effective miner tip.
func NewTxWithMinerFee(tx *Transaction, baseFee *big.Int) (*TxWithMinerFee, error) {
minerFee, err := tx.EffectiveTip(baseFee)
if err != nil {
return nil, err
}
return &TxWithMinerFee{
tx: tx,
minerFee: minerFee,
}, nil
}

// TxByPriceAndTime implements both the sort and the heap interface, making it useful
// for all at once sorting as well as individually adding and removing elements.
type TxByPriceAndTime Transactions
type TxByPriceAndTime []*TxWithMinerFee

func (s TxByPriceAndTime) Len() int { return len(s) }
func (s TxByPriceAndTime) Less(i, j int) bool {
// If the prices are equal, use the time the transaction was first seen for
Copy link
Member

Choose a reason for hiding this comment

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

Comment is outdated

// deterministic sorting
cmp := s[i].GasPrice().Cmp(s[j].GasPrice())
cmp := s[i].minerFee.Cmp(s[j].minerFee)
if cmp == 0 {
return s[i].time.Before(s[j].time)
return s[i].tx.time.Before(s[j].tx.time)
}
return cmp > 0
}
func (s TxByPriceAndTime) Swap(i, j int) { s[i], s[j] = s[j], s[i] }

func (s *TxByPriceAndTime) Push(x interface{}) {
*s = append(*s, x.(*Transaction))
*s = append(*s, x.(*TxWithMinerFee))
}

func (s *TxByPriceAndTime) Pop() interface{} {
Expand All @@ -432,35 +466,39 @@ func (s *TxByPriceAndTime) Pop() interface{} {
// transactions in a profit-maximizing sorted order, while supporting removing
// entire batches of transactions for non-executable accounts.
type TransactionsByPriceAndNonce struct {
txs map[common.Address]Transactions // Per account nonce-sorted list of transactions
heads TxByPriceAndTime // Next transaction for each unique account (price heap)
signer Signer // Signer for the set of transactions
txs map[common.Address]Transactions // Per account nonce-sorted list of transactions
heads TxByPriceAndTime // Next transaction for each unique account (price heap)
signer Signer // Signer for the set of transactions
baseFee *big.Int // Current base fee
}

// NewTransactionsByPriceAndNonce creates a transaction set that can retrieve
// price sorted transactions in a nonce-honouring way.
//
// Note, the input map is reowned so the caller should not interact any more with
// if after providing it to the constructor.
func NewTransactionsByPriceAndNonce(signer Signer, txs map[common.Address]Transactions) *TransactionsByPriceAndNonce {
func NewTransactionsByPriceAndNonce(signer Signer, txs map[common.Address]Transactions, baseFee *big.Int) *TransactionsByPriceAndNonce {
// Initialize a price and received time based heap with the head transactions
heads := make(TxByPriceAndTime, 0, len(txs))
for from, accTxs := range txs {
// Ensure the sender address is from the signer
if acc, _ := Sender(signer, accTxs[0]); acc != from {
acc, _ := Sender(signer, accTxs[0])
wrapped, err := NewTxWithMinerFee(accTxs[0], baseFee)
// Remove transaction if sender doesn't match from, or if wrapping fails.
if acc != from || err != nil {
delete(txs, from)
continue
}
heads = append(heads, accTxs[0])
heads = append(heads, wrapped)
txs[from] = accTxs[1:]
}
heap.Init(&heads)

// Assemble and return the transaction set
return &TransactionsByPriceAndNonce{
txs: txs,
heads: heads,
signer: signer,
txs: txs,
heads: heads,
signer: signer,
baseFee: baseFee,
}
}

Expand All @@ -469,18 +507,20 @@ func (t *TransactionsByPriceAndNonce) Peek() *Transaction {
if len(t.heads) == 0 {
return nil
}
return t.heads[0]
return t.heads[0].tx
}

// Shift replaces the current best head with the next one from the same account.
func (t *TransactionsByPriceAndNonce) Shift() {
acc, _ := Sender(t.signer, t.heads[0])
acc, _ := Sender(t.signer, t.heads[0].tx)
if txs, ok := t.txs[acc]; ok && len(txs) > 0 {
t.heads[0], t.txs[acc] = txs[0], txs[1:]
heap.Fix(&t.heads, 0)
} else {
heap.Pop(&t.heads)
if wrapped, err := NewTxWithMinerFee(txs[0], t.baseFee); err == nil {
t.heads[0], t.txs[acc] = wrapped, txs[1:]
heap.Fix(&t.heads, 0)
return
}
}
heap.Pop(&t.heads)
}

// Pop removes the best transaction, *not* replacing it with the next one from
Expand Down
63 changes: 55 additions & 8 deletions core/types/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"encoding/json"
"fmt"
"math/big"
"math/rand"
"reflect"
"testing"
"time"
Expand Down Expand Up @@ -258,36 +259,77 @@ func TestRecipientNormal(t *testing.T) {
}
}

func TestTransactionPriceNonceSortLegacy(t *testing.T) {
testTransactionPriceNonceSort(t, nil)
}

func TestTransactionPriceNonceSort1559(t *testing.T) {
testTransactionPriceNonceSort(t, big.NewInt(0))
testTransactionPriceNonceSort(t, big.NewInt(5))
testTransactionPriceNonceSort(t, big.NewInt(50))
}

// Tests that transactions can be correctly sorted according to their price in
// decreasing order, but at the same time with increasing nonces when issued by
// the same account.
func TestTransactionPriceNonceSort(t *testing.T) {
func testTransactionPriceNonceSort(t *testing.T, baseFee *big.Int) {
// Generate a batch of accounts to start with
keys := make([]*ecdsa.PrivateKey, 25)
for i := 0; i < len(keys); i++ {
keys[i], _ = crypto.GenerateKey()
}
signer := HomesteadSigner{}
signer := LatestSignerForChainID(common.Big1)

// Generate a batch of transactions with overlapping values, but shifted nonces
groups := map[common.Address]Transactions{}
expectedCount := 0
for start, key := range keys {
addr := crypto.PubkeyToAddress(key.PublicKey)
count := 25
for i := 0; i < 25; i++ {
tx, _ := SignTx(NewTransaction(uint64(start+i), common.Address{}, big.NewInt(100), 100, big.NewInt(int64(start+i)), nil), signer, key)
var tx *Transaction
feeCap := rand.Intn(50)
if baseFee == nil {
tx = NewTx(&LegacyTx{
Nonce: uint64(start + i),
To: &common.Address{},
Value: big.NewInt(100),
Gas: 100,
GasPrice: big.NewInt(int64(feeCap)),
Data: nil,
})
} else {
tx = NewTx(&DynamicFeeTx{
Nonce: uint64(start + i),
To: &common.Address{},
Value: big.NewInt(100),
Gas: 100,
FeeCap: big.NewInt(int64(feeCap)),
Tip: big.NewInt(int64(rand.Intn(feeCap + 1))),
Data: nil,
})
if count == 25 && int64(feeCap) < baseFee.Int64() {
count = i
}
}
tx, err := SignTx(tx, signer, key)
if err != nil {
t.Fatalf("failed to sign tx: %s", err)
}
groups[addr] = append(groups[addr], tx)
}
expectedCount += count
}
// Sort the transactions and cross check the nonce ordering
txset := NewTransactionsByPriceAndNonce(signer, groups)
txset := NewTransactionsByPriceAndNonce(signer, groups, baseFee)

txs := Transactions{}
for tx := txset.Peek(); tx != nil; tx = txset.Peek() {
txs = append(txs, tx)
txset.Shift()
}
if len(txs) != 25*25 {
t.Errorf("expected %d transactions, found %d", 25*25, len(txs))
if len(txs) != expectedCount {
t.Errorf("expected %d transactions, found %d", expectedCount, len(txs))
}
for i, txi := range txs {
fromi, _ := Sender(signer, txi)
Expand All @@ -303,7 +345,12 @@ func TestTransactionPriceNonceSort(t *testing.T) {
if i+1 < len(txs) {
next := txs[i+1]
fromNext, _ := Sender(signer, next)
if fromi != fromNext && txi.GasPrice().Cmp(next.GasPrice()) < 0 {
tip, err := txi.EffectiveTip(baseFee)
nextTip, nextErr := next.EffectiveTip(baseFee)
if err != nil || nextErr != nil {
t.Errorf("error calculating effective tip")
}
if fromi != fromNext && tip.Cmp(nextTip) < 0 {
t.Errorf("invalid gasprice ordering: tx #%d (A=%x P=%v) < tx #%d (A=%x P=%v)", i, fromi[:4], txi.GasPrice(), i+1, fromNext[:4], next.GasPrice())
}
}
Expand Down Expand Up @@ -331,7 +378,7 @@ func TestTransactionTimeSort(t *testing.T) {
groups[addr] = append(groups[addr], tx)
}
// Sort the transactions and cross check the nonce ordering
txset := NewTransactionsByPriceAndNonce(signer, groups)
txset := NewTransactionsByPriceAndNonce(signer, groups, nil)

txs := Transactions{}
for tx := txset.Peek(); tx != nil; tx = txset.Peek() {
Expand Down
Loading