From 83fe302954cce8da13d4d495407c9fe15d81ea75 Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Sun, 7 Jan 2018 18:52:24 +0000 Subject: [PATCH 1/9] eth: Set default GPO percentile to 35%, and count blocks instead of transactions --- eth/config.go | 4 ++-- eth/gasprice/gasprice.go | 25 +++++++++++++------------ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/eth/config.go b/eth/config.go index 383cd6783c12..f8aee8c65a28 100644 --- a/eth/config.go +++ b/eth/config.go @@ -49,8 +49,8 @@ var DefaultConfig = Config{ TxPool: core.DefaultTxPoolConfig, GPO: gasprice.Config{ - Blocks: 10, - Percentile: 50, + Blocks: 100, + Percentile: 35, }, } diff --git a/eth/gasprice/gasprice.go b/eth/gasprice/gasprice.go index c662348e165b..310584949046 100644 --- a/eth/gasprice/gasprice.go +++ b/eth/gasprice/gasprice.go @@ -101,7 +101,7 @@ func (gpo *Oracle) SuggestPrice(ctx context.Context) (*big.Int, error) { ch := make(chan getBlockPricesResult, gpo.checkBlocks) sent := 0 exp := 0 - var txPrices []*big.Int + var blockPrices []*big.Int for sent < gpo.checkBlocks && blockNum > 0 { go gpo.getBlockPrices(ctx, blockNum, ch) sent++ @@ -115,9 +115,8 @@ func (gpo *Oracle) SuggestPrice(ctx context.Context) (*big.Int, error) { return lastPrice, res.err } exp-- - if len(res.prices) > 0 { - txPrices = append(txPrices, res.prices...) - continue + if res.price != nil { + blockPrices = append(blockPrices, res.price) } if maxEmpty > 0 { maxEmpty-- @@ -131,9 +130,9 @@ func (gpo *Oracle) SuggestPrice(ctx context.Context) (*big.Int, error) { } } price := lastPrice - if len(txPrices) > 0 { - sort.Sort(bigIntArray(txPrices)) - price = txPrices[(len(txPrices)-1)*gpo.percentile/100] + if len(blockPrices) > 0 { + sort.Sort(bigIntArray(blockPrices)) + price = blockPrices[(len(blockPrices)-1)*gpo.percentile/100] } if price.Cmp(maxPrice) > 0 { price = new(big.Int).Set(maxPrice) @@ -147,7 +146,7 @@ func (gpo *Oracle) SuggestPrice(ctx context.Context) (*big.Int, error) { } type getBlockPricesResult struct { - prices []*big.Int + price *big.Int err error } @@ -160,11 +159,13 @@ func (gpo *Oracle) getBlockPrices(ctx context.Context, blockNum uint64, ch chan return } txs := block.Transactions() - prices := make([]*big.Int, len(txs)) - for i, tx := range txs { - prices[i] = tx.GasPrice() + var minPrice *big.Int + for _, tx := range txs { + if minPrice == nil || tx.GasPrice().Cmp(minPrice) < 0 { + minPrice = tx.GasPrice() + } } - ch <- getBlockPricesResult{prices, nil} + ch <- getBlockPricesResult{minPrice, nil} } type bigIntArray []*big.Int From 3d1d61e8e5c4eee15669ba8f93e7b28055b18f9b Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Sun, 7 Jan 2018 19:27:20 +0000 Subject: [PATCH 2/9] eth/gasprice: Don't include transactions made by the miner themselves --- eth/gasprice/gasprice.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/eth/gasprice/gasprice.go b/eth/gasprice/gasprice.go index 310584949046..4093dd6cd0b3 100644 --- a/eth/gasprice/gasprice.go +++ b/eth/gasprice/gasprice.go @@ -23,6 +23,7 @@ import ( "sync" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" @@ -103,7 +104,7 @@ func (gpo *Oracle) SuggestPrice(ctx context.Context) (*big.Int, error) { exp := 0 var blockPrices []*big.Int for sent < gpo.checkBlocks && blockNum > 0 { - go gpo.getBlockPrices(ctx, blockNum, ch) + go gpo.getBlockPrices(ctx, types.MakeSigner(gpo.backend.ChainConfig(), big.NewInt(int64(blockNum))), blockNum, ch) sent++ exp++ blockNum-- @@ -123,7 +124,7 @@ func (gpo *Oracle) SuggestPrice(ctx context.Context) (*big.Int, error) { continue } if blockNum > 0 && sent < gpo.maxBlocks { - go gpo.getBlockPrices(ctx, blockNum, ch) + go gpo.getBlockPrices(ctx, types.MakeSigner(gpo.backend.ChainConfig(), big.NewInt(int64(blockNum))), blockNum, ch) sent++ exp++ blockNum-- @@ -152,7 +153,7 @@ type getBlockPricesResult struct { // getLowestPrice calculates the lowest transaction gas price in a given block // and sends it to the result channel. If the block is empty, price is nil. -func (gpo *Oracle) getBlockPrices(ctx context.Context, blockNum uint64, ch chan getBlockPricesResult) { +func (gpo *Oracle) getBlockPrices(ctx context.Context, signer types.Signer, blockNum uint64, ch chan getBlockPricesResult) { block, err := gpo.backend.BlockByNumber(ctx, rpc.BlockNumber(blockNum)) if block == nil { ch <- getBlockPricesResult{nil, err} @@ -161,6 +162,10 @@ func (gpo *Oracle) getBlockPrices(ctx context.Context, blockNum uint64, ch chan txs := block.Transactions() var minPrice *big.Int for _, tx := range txs { + sender, err := types.Sender(signer, tx) + if err != nil || sender == block.Coinbase() { + continue + } if minPrice == nil || tx.GasPrice().Cmp(minPrice) < 0 { minPrice = tx.GasPrice() } From 8c2e33f727b22d998974295f2739d7df7e7ef5ae Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Sun, 7 Jan 2018 19:34:15 +0000 Subject: [PATCH 3/9] eth: Use 60% for GPO, not 35% --- eth/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth/config.go b/eth/config.go index f8aee8c65a28..68a02e78c4ef 100644 --- a/eth/config.go +++ b/eth/config.go @@ -50,7 +50,7 @@ var DefaultConfig = Config{ TxPool: core.DefaultTxPoolConfig, GPO: gasprice.Config{ Blocks: 100, - Percentile: 35, + Percentile: 60, }, } From 85cc91662d08b644b2bf88b248576fbf7d1e772f Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Sun, 7 Jan 2018 20:49:06 +0000 Subject: [PATCH 4/9] eth: Reduce the GPO lookback to 20 blocks from 100, for speed --- eth/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth/config.go b/eth/config.go index 68a02e78c4ef..4399560fa391 100644 --- a/eth/config.go +++ b/eth/config.go @@ -49,7 +49,7 @@ var DefaultConfig = Config{ TxPool: core.DefaultTxPoolConfig, GPO: gasprice.Config{ - Blocks: 100, + Blocks: 20, Percentile: 60, }, } From ef4d9fb75b06e240e7e470de1d449cbcece56c9d Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Sun, 7 Jan 2018 22:40:22 +0000 Subject: [PATCH 5/9] eth/gasprice: Add mistakenly deleted 'continue' --- eth/gasprice/gasprice.go | 1 + 1 file changed, 1 insertion(+) diff --git a/eth/gasprice/gasprice.go b/eth/gasprice/gasprice.go index 4093dd6cd0b3..a10e4d15a3d5 100644 --- a/eth/gasprice/gasprice.go +++ b/eth/gasprice/gasprice.go @@ -118,6 +118,7 @@ func (gpo *Oracle) SuggestPrice(ctx context.Context) (*big.Int, error) { exp-- if res.price != nil { blockPrices = append(blockPrices, res.price) + continue } if maxEmpty > 0 { maxEmpty-- From 67d16811374df01300e2b4fc91195f504119c99c Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Mon, 8 Jan 2018 14:37:57 +0000 Subject: [PATCH 6/9] eth/gasprice: gofmt --- eth/gasprice/gasprice.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth/gasprice/gasprice.go b/eth/gasprice/gasprice.go index a10e4d15a3d5..96fbbe945341 100644 --- a/eth/gasprice/gasprice.go +++ b/eth/gasprice/gasprice.go @@ -149,7 +149,7 @@ func (gpo *Oracle) SuggestPrice(ctx context.Context) (*big.Int, error) { type getBlockPricesResult struct { price *big.Int - err error + err error } // getLowestPrice calculates the lowest transaction gas price in a given block From 38e8ac09c9d2e3ab3f512903a25a7bbeee153c4c Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Tue, 9 Jan 2018 11:27:21 +0000 Subject: [PATCH 7/9] eth/gasprice: Sort transactions by price first, to minimise use of ecrecover --- eth/gasprice/gasprice.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/eth/gasprice/gasprice.go b/eth/gasprice/gasprice.go index 96fbbe945341..edd04d02974f 100644 --- a/eth/gasprice/gasprice.go +++ b/eth/gasprice/gasprice.go @@ -152,7 +152,13 @@ type getBlockPricesResult struct { err error } -// getLowestPrice calculates the lowest transaction gas price in a given block +type transactionsByGasPrice []*types.Transaction + +func (t transactionsByGasPrice) Len() int { return len(t) } +func (t transactionsByGasPrice) Swap(i, j int) { t[i], t[j] = t[j], t[i] } +func (t transactionsByGasPrice) Less(i, j int) bool { return t[i].GasPrice().Cmp(t[j].GasPrice()) < 0 } + +// getBlockPrices calculates the lowest transaction gas price in a given block // and sends it to the result channel. If the block is empty, price is nil. func (gpo *Oracle) getBlockPrices(ctx context.Context, signer types.Signer, blockNum uint64, ch chan getBlockPricesResult) { block, err := gpo.backend.BlockByNumber(ctx, rpc.BlockNumber(blockNum)) @@ -160,18 +166,18 @@ func (gpo *Oracle) getBlockPrices(ctx context.Context, signer types.Signer, bloc ch <- getBlockPricesResult{nil, err} return } + txs := block.Transactions() - var minPrice *big.Int + sort.Sort(transactionsByGasPrice(txs)) + for _, tx := range txs { sender, err := types.Sender(signer, tx) - if err != nil || sender == block.Coinbase() { - continue - } - if minPrice == nil || tx.GasPrice().Cmp(minPrice) < 0 { - minPrice = tx.GasPrice() + if err == nil && sender != block.Coinbase() { + ch <- getBlockPricesResult{tx.GasPrice(), nil} + return } } - ch <- getBlockPricesResult{minPrice, nil} + ch <- getBlockPricesResult{nil, nil} } type bigIntArray []*big.Int From cd85458aa2e28ce1ff64333dab4d9b8915f8ec25 Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Tue, 9 Jan 2018 12:35:00 +0000 Subject: [PATCH 8/9] eth/gasprice: gofmt --- eth/gasprice/gasprice.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eth/gasprice/gasprice.go b/eth/gasprice/gasprice.go index edd04d02974f..2b7870e05ba5 100644 --- a/eth/gasprice/gasprice.go +++ b/eth/gasprice/gasprice.go @@ -154,8 +154,8 @@ type getBlockPricesResult struct { type transactionsByGasPrice []*types.Transaction -func (t transactionsByGasPrice) Len() int { return len(t) } -func (t transactionsByGasPrice) Swap(i, j int) { t[i], t[j] = t[j], t[i] } +func (t transactionsByGasPrice) Len() int { return len(t) } +func (t transactionsByGasPrice) Swap(i, j int) { t[i], t[j] = t[j], t[i] } func (t transactionsByGasPrice) Less(i, j int) bool { return t[i].GasPrice().Cmp(t[j].GasPrice()) < 0 } // getBlockPrices calculates the lowest transaction gas price in a given block From 26f303f3ed39fbb89bb1c87159a2b08f47fa4ea5 Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Tue, 9 Jan 2018 13:15:50 +0000 Subject: [PATCH 9/9] eth/gasprice: Make a copy of transaction pointers before sorting --- eth/gasprice/gasprice.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/eth/gasprice/gasprice.go b/eth/gasprice/gasprice.go index 2b7870e05ba5..54325692c6ef 100644 --- a/eth/gasprice/gasprice.go +++ b/eth/gasprice/gasprice.go @@ -167,7 +167,9 @@ func (gpo *Oracle) getBlockPrices(ctx context.Context, signer types.Signer, bloc return } - txs := block.Transactions() + blockTxs := block.Transactions() + txs := make([]*types.Transaction, len(blockTxs)) + copy(txs, blockTxs) sort.Sort(transactionsByGasPrice(txs)) for _, tx := range txs {