Skip to content

Commit

Permalink
Improvements to feeEstimator.
Browse files Browse the repository at this point in the history 
 * Rollback now rolls back a specific block rather than
   the most recent one registered.
 * Parameter minRegisteredBlocks added. An error is
   returned for fee estimation requests if fewer than
   minRegisteredBlocks have been registered.
 * result returned is in bitcoins rather than satoshis.
 * server.EstimateFee() removed.
 * Improve comments.
  • Loading branch information
@DanielKrawisz
DanielKrawisz committed Aug 13, 2016
1 parent b38f1b0 commit d7c798c
Show file tree
Hide file tree
Showing 5 changed files with 145 additions and 59 deletions.
4 changes: 1 addition & 3 deletions blockmanager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1255,10 +1255,8 @@ func (b *blockManager) handleNotifyMsg(notification *blockchain.Notification) {
}

// Rollback previous block recorded by the fee estimator.
// TODO instead of rolling back the previous block,
// check whether we are rolling back the correct one!
if b.server.feeEstimator != nil {
b.server.feeEstimator.Rollback()
b.server.feeEstimator.Rollback(block)
}

// Notify registered websocket clients.
Expand Down
138 changes: 103 additions & 35 deletions estimatefee.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,35 +40,51 @@ type observedTransaction struct {
mined int32 // The block in which this tx was mined.
}

// tt is safe for concurrent access.
// registeredBlock
type registeredBlock struct {
hash *chainhash.Hash
transactions []*observedTransaction
}

// feeEstimator is a class that manages the data necessary to create
// fee estimations. It is safe for concurrent access.
type feeEstimator struct {
maxRollback uint32
binSize int
maxRollback uint32
binSize int

// The maximum number of replacements that can be made in a single
// bin per block. Default is estimateFeeMaxReplacements
maxReplacements int

// The minimum number of blocks that can be registered with the fee
// estimator before it will provide answers.
minRegisteredBlocks uint32

// The last known height.
height int32
lastKnownHeight int32

sync.RWMutex
observed map[chainhash.Hash]observedTransaction
bin [estimateFeeBins][]*observedTransaction
observed map[chainhash.Hash]observedTransaction
bin [estimateFeeBins][]*observedTransaction
numBlocksRegistered uint32 // The number of blocks that have been registered.

// The cached estimates.
cached []float64

// Transactions that have been removed from the bins. This allows us to
// revert in case of an orphaned block.
dropped [][]*observedTransaction
dropped []registeredBlock
}

func NewFeeEstimator(maxRollback uint32) *feeEstimator {
func NewFeeEstimator(maxRollback, minRegisteredBlocks uint32) *feeEstimator {
return &feeEstimator{
maxRollback: maxRollback,
height: mempoolHeight,
binSize: estimateFeeBinSize,
maxReplacements: estimateFeeMaxReplacements,
observed: make(map[chainhash.Hash]observedTransaction),
dropped: make([][]*observedTransaction, 0, maxRollback),
maxRollback: maxRollback,
minRegisteredBlocks: minRegisteredBlocks,
lastKnownHeight: mempoolHeight,
binSize: estimateFeeBinSize,
maxReplacements: estimateFeeMaxReplacements,
observed: make(map[chainhash.Hash]observedTransaction),
dropped: make([]registeredBlock, 0, maxRollback),
}
}

Expand Down Expand Up @@ -102,12 +118,14 @@ func (ef *feeEstimator) RecordBlock(block *btcutil.Block) {
ef.cached = nil

height := block.Height()
if height != ef.height+1 && ef.height != mempoolHeight {
panic(fmt.Sprint("intermediate block not recorded; current height is ", ef.height,
if height != ef.lastKnownHeight+1 && ef.lastKnownHeight != mempoolHeight {
panic(fmt.Sprint("intermediate block not recorded; current height is ", ef.lastKnownHeight,
"; new height is ", height))
}

ef.height = height
// Update the last known height.
ef.lastKnownHeight = height
ef.numBlocksRegistered++

// Randomly order txs in block.
transactions := make(map[*btcutil.Tx]struct{})
Expand All @@ -120,7 +138,10 @@ func (ef *feeEstimator) RecordBlock(block *btcutil.Block) {
var replacementCounts [estimateFeeBins]int

// Keep track of which txs were dropped in case of an orphan block.
dropped := make([]*observedTransaction, 0, 100)
dropped := registeredBlock{
hash: block.Hash(),
transactions: make([]*observedTransaction, 0, 100),
}

// Go through the txs in the block.
for t, _ := range transactions {
Expand Down Expand Up @@ -150,7 +171,7 @@ func (ef *feeEstimator) RecordBlock(block *btcutil.Block) {
if len(bin) == int(ef.binSize) {
l := int(ef.binSize - replacementCounts[blocksToConfirm])
drop := rand.Intn(l)
dropped = append(dropped, bin[drop])
dropped.transactions = append(dropped.transactions, bin[drop])

bin[drop] = bin[l-1]
bin[l-1] = &o
Expand Down Expand Up @@ -178,13 +199,49 @@ func (ef *feeEstimator) RecordBlock(block *btcutil.Block) {
}
}

// Rollback reverses the effect of the last block on the fee estimator. This
// can be used in the case of an orphaned block. The maximum number of rollbacks
// allowed is given by maxRollbacks.
func (ef *feeEstimator) Rollback() error {
// Rollback unregisters a recently registered block from the fee estimator.
// This can be used to reverse the effect of an orphaned block on the fee
// estimator. The maximum number of rollbacks allowed is given by
// maxRollbacks.
//
// Note: not everything can be rolled back because some transactions are
// deleted if they have been observed too long ago. That means the result
// of Rollback won't always be exactly the same as if the last block had not
// happened, but it should be close enough.
func (ef *feeEstimator) Rollback(block *btcutil.Block) error {
ef.Lock()
defer ef.Unlock()

hash := block.Hash()

var n int = 1
// Find this block in the stack of recent registered blocks.
for n < len(ef.dropped) {
if ef.dropped[len(ef.dropped)-n].hash.IsEqual(hash) {
break
}

n++
}

if n == len(ef.dropped) {
return errors.New("No such block was recently registered.")
}

for i := 0; i < n; i++ {
err := ef.rollback()
if err != nil {
return err
}
}

return nil
}

// rollback rolls back the effect of the last block in the stack
// of registered blocks.
func (ef *feeEstimator) rollback() error {

// The previous sorted list is invalid, so delete it.
ef.cached = nil

Expand All @@ -195,14 +252,16 @@ func (ef *feeEstimator) Rollback() error {
return errors.New("Max rollbacks reached.")
}

ef.numBlocksRegistered--

dropped := ef.dropped[last]
ef.dropped = ef.dropped[0:last]

// where we are in each bin as we replace txs.
// where we are in each bin as we replace txs?
var replacementCounters [estimateFeeBins]int

// Go through the txs in the dropped box.
for _, o := range dropped {
for _, o := range dropped.transactions {
// Which bin was this tx in?
blocksToConfirm := o.mined - o.observed - 1

Expand All @@ -218,7 +277,7 @@ func (ef *feeEstimator) Rollback() error {

prev := bin[counter]

if prev.mined == ef.height {
if prev.mined == ef.lastKnownHeight {
prev.mined = mempoolHeight

bin[counter] = o
Expand All @@ -242,7 +301,7 @@ func (ef *feeEstimator) Rollback() error {

prev := ef.bin[i][j]

if prev.mined == ef.height {
if prev.mined == ef.lastKnownHeight {
prev.mined = mempoolHeight

ef.bin[i] = append(ef.bin[i][0:j], ef.bin[i][j+1:l]...)
Expand All @@ -254,7 +313,7 @@ func (ef *feeEstimator) Rollback() error {
}
}

ef.height--
ef.lastKnownHeight--

return nil
}
Expand Down Expand Up @@ -300,7 +359,7 @@ func (b *estimateFeeSet) EstimateFee(confirmations int) float64 {
return 0
}

return b.feeRate[(min+max-1)/2]
return b.feeRate[(min+max-1)/2] * 1E-8
}

// newEstimateFeeSet creates a temporary data structure that
Expand Down Expand Up @@ -345,21 +404,30 @@ func (ef *feeEstimator) estimates() []float64 {

// Estimate the fee per kb to have a tx confirmed a given number of blocks
// from now.
func (ef *feeEstimator) EstimateFee(confirmations uint32) float64 {
func (ef *feeEstimator) EstimateFee(numBlocks uint32) (float64, error) {
ef.Lock()
defer ef.Unlock()

if confirmations <= 0 {
return math.Inf(1)
// If the number of registered blocks is below the minimum, return
// an error.
if ef.numBlocksRegistered < ef.minRegisteredBlocks {
return -1, errors.New("Not enough blocks have been observed.")
}

if confirmations > estimateFeeBins {
return 0
if numBlocks == 0 {
return -1, errors.New("Cannot confirm transaction in zero blocks.")
}

if numBlocks > estimateFeeBins {
return -1, errors.New(fmt.Sprintf(
"Can only estimate fees for up to %s blocks from now.",
estimateFeeBinSize))
}

// If there are no cached results, generate them.
if ef.cached == nil {
ef.cached = ef.estimates()
}

return ef.cached[int(confirmations)-1]
return ef.cached[int(numBlocks)-1], nil
}
48 changes: 32 additions & 16 deletions estimatefee_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,17 @@ import (
"github.com/btcsuite/btcutil"
)

// NewTestFeeEstimator creates a feeEstimator with some different parameters
// for testing purposes.
func NewTestFeeEstimator(binSize, maxReplacements, maxRollback uint32) *feeEstimator {
return &feeEstimator{
maxRollback: maxRollback,
height: mempoolHeight,
binSize: int(binSize),
maxReplacements: int(maxReplacements),
observed: make(map[chainhash.Hash]observedTransaction),
dropped: make([][]*observedTransaction, 0, maxRollback),
maxRollback: maxRollback,
lastKnownHeight: mempoolHeight,
binSize: int(binSize),
minRegisteredBlocks: 0,
maxReplacements: int(maxReplacements),
observed: make(map[chainhash.Hash]observedTransaction),
dropped: make([]registeredBlock, 0, maxRollback),
}
}

Expand Down Expand Up @@ -49,7 +52,7 @@ func expectedFeePerKb(t *mempoolTxDesc) float64 {
size := uint32(t.TxDesc.Tx.MsgTx().SerializeSize())
fee := uint64(t.TxDesc.Fee)

return float64(1000*fee) / float64(size)
return float64(1000*fee) / float64(size) * 1E-8
}

func (eft *estimateFeeTester) testBlock(txs []*wire.MsgTx) *btcutil.Block {
Expand All @@ -70,7 +73,7 @@ func TestEstimateFee(t *testing.T) {
// Try with no txs and get zero for all queries.
expected := 0.0
for i := uint32(1); i <= estimateFeeBins; i++ {
estimated := ef.EstimateFee(i)
estimated, _ := ef.EstimateFee(i)

if estimated != expected {
t.Errorf("Estimate fee error: expected %s when estimator is empty; got %s", expected, estimated)
Expand All @@ -84,18 +87,30 @@ func TestEstimateFee(t *testing.T) {
// Expected should still be zero because this is still in the mempool.
expected = 0.0
for i := uint32(1); i <= estimateFeeBins; i++ {
estimated := ef.EstimateFee(i)
estimated, _ := ef.EstimateFee(i)

if estimated != expected {
t.Errorf("Estimate fee error: expected %s when estimator has one tx in mempool; got %s", expected, estimated)
}
}

// Change minRegisteredBlocks to make sure that works. Error return
// value expected.
ef.minRegisteredBlocks = 1
expected = -1.0
for i := uint32(1); i <= estimateFeeBins; i++ {
estimated, _ := ef.EstimateFee(i)

if estimated != expected {
t.Errorf("Estimate fee error: expected %s before any blocks have been registered; got %s", expected, estimated)
}
}

// Record a block.
ef.RecordBlock(eft.testBlock([]*wire.MsgTx{tx.Tx.MsgTx()}))
expected = expectedFeePerKb(tx)
for i := uint32(1); i <= estimateFeeBins; i++ {
estimated := ef.EstimateFee(i)
estimated, _ := ef.EstimateFee(i)

if estimated != expected {
t.Errorf("Estimate fee error: expected %f when one tx is binned; got %f", expected, estimated)
Expand All @@ -121,7 +136,7 @@ func TestEstimateFee(t *testing.T) {
// Now the estimated amount should depend on the value
// of the argument to estimate fee.
for i := uint32(1); i <= estimateFeeBins; i++ {
estimated := ef.EstimateFee(i)
estimated, _ := ef.EstimateFee(i)
if i > 8 {
expected = expectedFeePerKb(txA)
} else {
Expand All @@ -143,7 +158,7 @@ func TestEstimateFee(t *testing.T) {
// Now the estimated amount should depend on the value
// of the argument to estimate fee.
for i := uint32(1); i <= estimateFeeBins; i++ {
estimated := ef.EstimateFee(i)
estimated, _ := ef.EstimateFee(i)
if i <= 8 {
expected = expectedFeePerKb(txB)
} else if i <= 8+6 {
Expand All @@ -168,7 +183,7 @@ func TestEstimateFee(t *testing.T) {
// This should have no effect on the outcome because too
// many blocks have been mined for txC to be recorded.
for i := uint32(1); i <= estimateFeeBins; i++ {
estimated := ef.EstimateFee(i)
estimated, _ := ef.EstimateFee(i)
if i <= 8 {
expected = expectedFeePerKb(txB)
} else if i <= 8+6 {
Expand All @@ -188,7 +203,7 @@ func (eft *estimateFeeTester) estimates(ef *feeEstimator) [estimateFeeBins]float
// Generate estimates
var estimates [estimateFeeBins]float64
for i := 0; i < estimateFeeBins; i++ {
estimates[i] = ef.EstimateFee(1)
estimates[i], _ = ef.EstimateFee(1)
}

// Check that all estimated fee results go in descending order.
Expand Down Expand Up @@ -286,7 +301,7 @@ func TestEstimateFeeRollback(t *testing.T) {
txPerRound, txPerBlock, uint32(stepsBack))

for step := 0; step < stepsBack; step++ {
err := ef.Rollback()
err := ef.rollback()
if err != nil {
t.Fatal("Could not rollback: ", err)
}
Expand All @@ -297,7 +312,8 @@ func TestEstimateFeeRollback(t *testing.T) {
// Ensure that these are both the same.
for i := 0; i < estimateFeeBins; i++ {
if expected[i] != estimates[i] {
t.Error("Rollback value mismatch.")
t.Errorf("Rollback value mismatch. Expected %f, got %f. ",
expected[i], estimates[i])
}
}
}
Expand Down
Loading

0 comments on commit d7c798c

Please sign in to comment.