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.

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

eth/gasprice: improve stability of estimated price #22722

Merged
merged 13 commits into from
Apr 28, 2021
49 changes: 48 additions & 1 deletion eth/gasprice/gasprice.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ import (
"github.com/ethereum/go-ethereum/rpc"
)

const sampleNumber = 3 // Number of transactions sampled in a block
// Number of transactions sampled in a block
// Roughly the last 5 minutes
GregTheGreek marked this conversation as resolved.
Show resolved Hide resolved
const sampleNumber = 20

var DefaultMaxPrice = big.NewInt(500 * params.GWei)

Expand Down Expand Up @@ -207,12 +209,57 @@ func (gpo *Oracle) getBlockPrices(ctx context.Context, signer types.Signer, bloc
}
}
}
if len(prices) > 0 {
prices = removeOutliers(prices)
}
select {
case result <- getBlockPricesResult{prices, nil}:
case <-quit:
}
}

// removeOutliers calculates the interquartile range of gas prices that are
// significant outliers with the goal of removing edge cases where MEV skews
// the gas prices
func removeOutliers(prices []*big.Int) []*big.Int {
GregTheGreek marked this conversation as resolved.
Show resolved Hide resolved
var (
mean *big.Int
sd *big.Int // Standard deviation
variance = big.NewInt(0)
sum = big.NewInt(0)
length = big.NewInt(int64(len(prices)))
deviations = big.NewInt(3) // The max acceptable std, anything outsid will be removed from the set
)
for _, price := range prices {
// Calculate sum
sum.Add(sum, price)
}
mean = big.NewInt(0).Div(sum, length)
// Calculate variance (sum(x - mean)^2 )/ length
Copy link

Choose a reason for hiding this comment

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

// Calculate variance (sum((x - mean)^2)/ length) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woops this is old.

for _, price := range prices {
// Calculate the summation
x := big.NewInt(0).Sub(price, mean)
square := big.NewInt(0).Mul(x, x)
variance.Add(variance, square)
}
variance.Div(variance, length)
// Calculate standard deviation from the variance
sd = big.NewInt(0).Sqrt(variance)

filtered := []*big.Int{}
deviation := big.NewInt(0).Mul(deviations, sd)
lowerBound := big.NewInt(0).Sub(mean, deviation)
upperBound := big.NewInt(0).Add(mean, deviation)

for _, price := range prices {
// Remove items that are not within the upper and lower bounds of the deviation
if price.Cmp(lowerBound) == 1 && price.Cmp(upperBound) == -1 {
filtered = append(filtered, price)
}
}
return filtered
}

type bigIntArray []*big.Int

func (s bigIntArray) Len() int { return len(s) }
Expand Down
84 changes: 83 additions & 1 deletion eth/gasprice/gasprice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import (
"context"
"math"
"math/big"
"math/rand"
"testing"
"time"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/consensus/ethash"
Expand Down Expand Up @@ -111,8 +113,88 @@ func TestSuggestPrice(t *testing.T) {
if err != nil {
t.Fatalf("Failed to retrieve recommended gas price: %v", err)
}
expect := big.NewInt(params.GWei * int64(30))
expect := big.NewInt(params.GWei)
if got.Cmp(expect) != 0 {
t.Fatalf("Gas price mismatch, want %d, got %d", expect, got)
}
}

func generateFakeGasPrices(min, max int) []*big.Int {
rand.Seed(time.Now().UnixNano())
gasPrices := make([]*big.Int, 0)
for i := 0; i < 300; i++ {
randGasPrice := rand.Intn(max-min+1) + min
gasPrices = append(gasPrices, big.NewInt(int64(randGasPrice)))
}
return gasPrices
}

func TestRemoveLowOutliers(t *testing.T) {
gasPrices := generateFakeGasPrices(180, 220)
cpy := make([]*big.Int, len(gasPrices))
copy(cpy, gasPrices)
// add low gas prices
cpy = append(cpy, big.NewInt(5))
cpy = append(cpy, big.NewInt(10))
cpy = append(cpy, big.NewInt(10))
cpy = append(cpy, big.NewInt(10))
cpy = append(cpy, big.NewInt(15))
cpy = append(cpy, big.NewInt(15))
cpy = append(cpy, big.NewInt(15))
cpy = append(cpy, big.NewInt(15))

res := removeOutliers(cpy)
// It should remove all the lower gasPrices in the extreme case
if len(gasPrices) != len(res) {
t.Fatalf("Low gas prices not removed, want length less than %d, got %d", len(gasPrices), len(res))
}
}

func TestRemoveHighOutliars(t *testing.T) {
gasPrices := generateFakeGasPrices(180, 220)
cpy := make([]*big.Int, len(gasPrices))
copy(cpy, gasPrices)
// add low gas prices
GregTheGreek marked this conversation as resolved.
Show resolved Hide resolved
cpy = append(cpy, big.NewInt(300))
cpy = append(cpy, big.NewInt(310))
cpy = append(cpy, big.NewInt(350))
cpy = append(cpy, big.NewInt(250))
cpy = append(cpy, big.NewInt(251))
cpy = append(cpy, big.NewInt(245))
cpy = append(cpy, big.NewInt(255))
cpy = append(cpy, big.NewInt(256))

res := removeOutliers(cpy)
// It should remove most of the higher values
if len(cpy) < len(res) {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't len(cpy) < len(res) always false?

t.Fatalf("Low gas prices not removed, want length less than %d, got %d", len(gasPrices), len(res))
}
}

func TestRemoveHighAndLowOutliars(t *testing.T) {
gasPrices := generateFakeGasPrices(180, 220)
cpy := make([]*big.Int, len(gasPrices))
copy(cpy, gasPrices)
// add low gas prices
cpy = append(cpy, big.NewInt(300))
cpy = append(cpy, big.NewInt(310))
cpy = append(cpy, big.NewInt(350))
cpy = append(cpy, big.NewInt(250))
cpy = append(cpy, big.NewInt(251))
cpy = append(cpy, big.NewInt(245))
cpy = append(cpy, big.NewInt(255))
cpy = append(cpy, big.NewInt(256))
cpy = append(cpy, big.NewInt(50))
cpy = append(cpy, big.NewInt(100))
cpy = append(cpy, big.NewInt(70))
cpy = append(cpy, big.NewInt(75))
cpy = append(cpy, big.NewInt(5))
cpy = append(cpy, big.NewInt(0))
cpy = append(cpy, big.NewInt(4))
cpy = append(cpy, big.NewInt(2))
res := removeOutliers(cpy)
// It should remove most of the higher values
if len(cpy) < len(res) {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't len(cpy) < len(res) always false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

answering for both comments.

In theory yes, assuming the function operates correctly, I could switch to deterministic values instead ?

Copy link
Member

Choose a reason for hiding this comment

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

I mean even the outliers are not filtered out, the len(res) should be 0. And len(cpy) < len(res) can't be true in any case.

Copy link

Choose a reason for hiding this comment

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

All of these tests should use len(gasPrices) != len(res) for the error check. Just like TestRemoveLowOutliers

t.Fatalf("Low gas prices not removed, want length less than %d, got %d", len(gasPrices), len(res))
}
}