Fix rolling average gas price

blockchain/blockchain.go

@@ -44,11 +44,15 @@ type Blockchain struct {
 
 	stream *eventStream // Event subscriptions
 
-	// Average gas price (rolling average)
-	averageGasPrice      *big.Int // The average gas price that gets queried
-	averageGasPriceCount *big.Int // Param used in the avg. gas price calculation
+	gpAverage *gasPriceAverage // A reference to the average gas price
+}
+
+// gasPriceAverage keeps track of the average gas price (rolling average)
+type gasPriceAverage struct {
+	sync.RWMutex
 
-	agpMux sync.Mutex // Mutex for the averageGasPrice calculation
+	price *big.Int // The average gas price that gets queried
+	count *big.Int // Param used in the avg. gas price calculation
 }
 
 type Verifier interface {
@@ -67,23 +71,82 @@ type BlockResult struct {
 	TotalGas uint64
 }
 
-// UpdateGasPriceAvg Updates the rolling average value of the gas price
-func (b *Blockchain) UpdateGasPriceAvg(newValue *big.Int) {
-	b.agpMux.Lock()
+// updateGasPriceAvg updates the rolling average value of the gas price
+func (b *Blockchain) updateGasPriceAvg(newValues []*big.Int) {
+	b.gpAverage.Lock()
+	defer b.gpAverage.Unlock()
+
+	//	Sum the values for quick reference
+	sum := big.NewInt(0)
+	for _, val := range newValues {
+		sum = sum.Add(sum, val)
+	}
+
+	// There is no previous average data,
+	// so this new value set will instantiate it
+	if b.gpAverage.count.Uint64() == 0 {
+		b.calcArithmeticAverage(newValues, sum)
+
+		return
+	}
+
+	// There is existing average data,
+	// use it to generate a new average
+	b.calcRollingAverage(newValues, sum)
+}
+
+// calcArithmeticAverage calculates and sets the arithmetic average
+// of the passed in data set
+func (b *Blockchain) calcArithmeticAverage(newValues []*big.Int, sum *big.Int) {
+	newAverageCount := big.NewInt(int64(len(newValues)))
+	newAverage := sum.Div(sum, newAverageCount)
+
+	b.gpAverage.price = newAverage
+	b.gpAverage.count = newAverageCount
+}
+
+// calcRollingAverage calculates the new average based on the
+// moving average formula:
+// new average = old average * (n-len(M))/n + (sum of values in M)/n)
+// where n is the old average data count, and M is the new data set
+func (b *Blockchain) calcRollingAverage(newValues []*big.Int, sum *big.Int) {
+	var (
+		// Save references to old counts
+		oldCount   = b.gpAverage.count
+		oldAverage = b.gpAverage.price
 
-	b.averageGasPriceCount.Add(b.averageGasPriceCount, big.NewInt(1))
+		inputSetCount = big.NewInt(0).SetInt64(int64(len(newValues)))
+	)
 
-	differential := big.NewInt(0)
-	differential.Div(newValue.Sub(newValue, b.averageGasPrice), b.averageGasPriceCount)
+	// old average * (n-len(M))/n
+	newAverage := big.NewInt(0).Div(
+		big.NewInt(0).Mul(
+			oldAverage,
+			big.NewInt(0).Sub(oldCount, inputSetCount),
+		),
+		oldCount,
+	)
 
-	b.averageGasPrice.Add(b.averageGasPrice, differential)
+	// + (sum of values in M)/n
+	newAverage.Add(
+		newAverage,
+		big.NewInt(0).Div(
+			sum,
+			oldCount,
+		),
+	)
 
-	b.agpMux.Unlock()
+	// Update the references
+	b.gpAverage.price = newAverage
+	b.gpAverage.count = inputSetCount.Add(inputSetCount, b.gpAverage.count)
 }
 
-// GetAvgGasPrice returns the average gas price
+// GetAvgGasPrice returns the average gas price for the chain
 func (b *Blockchain) GetAvgGasPrice() *big.Int {
-	return b.averageGasPrice
+	b.gpAverage.RLock()
+	defer b.gpAverage.RUnlock()
+
+	return b.gpAverage.price
 }
 
 // NewBlockchain creates a new blockchain object
@@ -100,6 +163,10 @@ func NewBlockchain(
 		consensus: consensus,
 		executor:  executor,
 		stream:    &eventStream{},
+		gpAverage: &gasPriceAverage{
+			price: big.NewInt(0),
+			count: big.NewInt(0),
+		},
 	}
 
 	var (
@@ -128,10 +195,6 @@ func NewBlockchain(
 	// Push the initial event to the stream
 	b.stream.push(&Event{})
 
-	// Initialize the average gas price
-	b.averageGasPrice = big.NewInt(0)
-	b.averageGasPriceCount = big.NewInt(0)
-
 	return b, nil
 }
 
@@ -311,9 +374,6 @@ func (b *Blockchain) writeGenesisImpl(header *types.Header) error {
 		return err
 	}
 
-	// Update the average gas price to take into account the genesis block
-	b.UpdateGasPriceAvg(new(big.Int).SetUint64(header.GasUsed))
-
 	// Advance the head
 	if _, err := b.advanceHead(header); err != nil {
 		return err
@@ -643,7 +703,7 @@ func (b *Blockchain) WriteBlock(block *types.Block) error {
 	b.dispatchEvent(evnt)
 
 	// Update the average gas price
-	b.UpdateGasPriceAvg(new(big.Int).SetUint64(header.GasUsed))
+	b.updateGasPriceAvgWithBlock(block)
 
 	logArgs := []interface{}{
 		"number", header.Number,
@@ -661,6 +721,23 @@ func (b *Blockchain) WriteBlock(block *types.Block) error {
 	return nil
 }
 
+// updateGasPriceAvgWithBlock extracts the gas price information from the
+// block, and updates the average gas price for the chain accordingly
+func (b *Blockchain) updateGasPriceAvgWithBlock(block *types.Block) {
+	if len(block.Transactions) < 1 {
+		// No transactions in the block,
+		// so no gas price average to update
+		return
+	}
+
+	gasPrices := make([]*big.Int, len(block.Transactions))
+	for i, transaction := range block.Transactions {
+		gasPrices[i] = transaction.GasPrice
+	}
+
+	b.updateGasPriceAvg(gasPrices)
+}
+
 // writeBody writes the block body to the DB.
 // Additionally, it also updates the txn lookup, for txnHash -> block lookups
 func (b *Blockchain) writeBody(block *types.Block) error {

blockchain/blockchain_test.go

@@ -620,3 +620,64 @@ func TestCalculateGasLimit(t *testing.T) {
 		})
 	}
 }
+
+// TestGasPriceAverage tests the average gas price of the
+// blockchain
+func TestGasPriceAverage(t *testing.T) {
+	testTable := []struct {
+		name               string
+		previousAverage    *big.Int
+		previousCount      *big.Int
+		newValues          []*big.Int
+		expectedNewAverage *big.Int
+	}{
+		{
+			"no previous average data",
+			big.NewInt(0),
+			big.NewInt(0),
+			[]*big.Int{
+				big.NewInt(1),
+				big.NewInt(2),
+				big.NewInt(3),
+				big.NewInt(4),
+				big.NewInt(5),
+			},
+			big.NewInt(3),
+		},
+		{
+			"previous average data",
+			// For example (5 + 5 + 5 + 5 + 5) / 5
+			big.NewInt(5),
+			big.NewInt(5),
+			[]*big.Int{
+				big.NewInt(1),
+				big.NewInt(2),
+				big.NewInt(3),
+			},
+			// (5 * 5 + 1 + 2 + 3) / 8
+			big.NewInt(3),
+		},
+	}
+
+	for _, testCase := range testTable {
+		t.Run(testCase.name, func(t *testing.T) {
+			// Setup the mock data
+			blockchain := NewTestBlockchain(t, nil)
+			blockchain.gpAverage.price = testCase.previousAverage
+			blockchain.gpAverage.count = testCase.previousCount
+
+			// Update the average gas price
+			blockchain.updateGasPriceAvg(testCase.newValues)
+
+			// Make sure the average gas price count is correct
+			assert.Equal(
+				t,
+				int64(len(testCase.newValues))+testCase.previousCount.Int64(),
+				blockchain.gpAverage.count.Int64(),
+			)
+
+			// Make sure the average gas price is correct
+			assert.Equal(t, testCase.expectedNewAverage.String(), blockchain.gpAverage.price.String())
+		})
+	}
+}