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

Use Global Params For Fee Estimator Congestion and Priority Params #1259

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 4 additions & 36 deletions lib/blockchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -5010,15 +5010,7 @@ func (bc *Blockchain) CreateMaxSpend(
if bc.params.IsPoSBlockHeight(uint64(bc.BlockTip().Height)) {
maxBlockSizeBytes = utxoView.GetSoftMaxBlockSizeBytesPoS()
}
feeAmountNanos, err = mempool.EstimateFee(
txn,
minFeeRateNanosPerKB,
// TODO: Make these flags or GlobalParams
bc.params.DefaultMempoolCongestionFactorBasisPoints,
bc.params.DefaultMempoolPriorityPercentileBasisPoints,
bc.params.DefaultMempoolPastBlocksCongestionFactorBasisPoints,
bc.params.DefaultMempoolPastBlocksPriorityPercentileBasisPoints,
maxBlockSizeBytes)
feeAmountNanos, err = mempool.EstimateFee(txn, minFeeRateNanosPerKB, maxBlockSizeBytes)
if err != nil {
return nil, 0, 0, 0, errors.Wrapf(err, "CreateMaxSpend: Problem estimating fee: ")
}
Expand Down Expand Up @@ -5153,15 +5145,7 @@ func (bc *Blockchain) AddInputsAndChangeToTransactionWithSubsidy(
if bc.params.IsPoSBlockHeight(uint64(bc.BlockTip().Height)) {
maxBlockSizeBytes = utxoView.GetSoftMaxBlockSizeBytesPoS()
}
newTxFee, err := mempool.EstimateFee(
txArg,
minFeeRateNanosPerKB,
// TODO: Make these flags or GlobalParams
bc.params.DefaultMempoolCongestionFactorBasisPoints,
bc.params.DefaultMempoolPriorityPercentileBasisPoints,
bc.params.DefaultMempoolPastBlocksCongestionFactorBasisPoints,
bc.params.DefaultMempoolPastBlocksPriorityPercentileBasisPoints,
maxBlockSizeBytes)
newTxFee, err := mempool.EstimateFee(txArg, minFeeRateNanosPerKB, maxBlockSizeBytes)
UpdateTxnFee(txArg, newTxFee)
if err != nil {
return 0, 0, 0, 0, errors.Wrapf(err,
Expand Down Expand Up @@ -5875,15 +5859,7 @@ func (bc *Blockchain) CreateAtomicTxnsWrapper(
}
txn.ExtraData[NextAtomicTxnPreHash] = dummyAtomicHashBytes
txn.ExtraData[PreviousAtomicTxnPreHash] = dummyAtomicHashBytes
newFeeEstimate, err := mempool.EstimateFee(
txn,
minFeeRateNanosPerKB,
// TODO: Make these flags or GlobalParams
bc.params.DefaultMempoolCongestionFactorBasisPoints,
bc.params.DefaultMempoolPriorityPercentileBasisPoints,
bc.params.DefaultMempoolPastBlocksCongestionFactorBasisPoints,
bc.params.DefaultMempoolPastBlocksPriorityPercentileBasisPoints,
maxBlockSizeBytes)
newFeeEstimate, err := mempool.EstimateFee(txn, minFeeRateNanosPerKB, maxBlockSizeBytes)
if err != nil {
return nil, 0, errors.Wrapf(err, "CreateAtomicTxnsWrapper: failed to recompute fee estimate")
}
Expand Down Expand Up @@ -5968,15 +5944,7 @@ func (bc *Blockchain) CreateAtomicTxnsWrapper(

// Use EstimateFee to set the fee INCLUDING the wrapper. Note that this fee should generally be a bit
// higher than the totalFee computed above because the atomic wrapper adds overhead.
newFeeEstimate, err := mempool.EstimateFee(
atomicTxn,
0,
// TODO: Make these flags or GlobalParams
bc.params.DefaultMempoolCongestionFactorBasisPoints,
bc.params.DefaultMempoolPriorityPercentileBasisPoints,
bc.params.DefaultMempoolPastBlocksCongestionFactorBasisPoints,
bc.params.DefaultMempoolPastBlocksPriorityPercentileBasisPoints,
maxBlockSizeBytes)
newFeeEstimate, err := mempool.EstimateFee(atomicTxn, 0, maxBlockSizeBytes)
Copy link
Member

Choose a reason for hiding this comment

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

Should we pass in MinFeeRateNanosPerKB here instead of 0? I think I missed this call site in my earlier PR.

if err != nil {
return nil, 0, errors.Wrapf(err, "CreateAtomicTxnsWrapper: failed to compute "+
"fee on full txn")
Expand Down
13 changes: 3 additions & 10 deletions lib/legacy_mempool.go
Original file line number Diff line number Diff line change
Expand Up @@ -2511,19 +2511,12 @@ func EstimateMaxTxnFeeV1(txn *MsgDeSoTxn, minFeeRateNanosPerKB uint64) uint64 {
return feeAmountNanos
}

func (mp *DeSoMempool) EstimateFee(txn *MsgDeSoTxn, minFeeRateNanosPerKB uint64,
_ uint64, _ uint64, _ uint64, _ uint64, _ uint64) (uint64, error) {
feeRate := mp.EstimateFeeRate(minFeeRateNanosPerKB, 0, 0, 0, 0, 0)
func (mp *DeSoMempool) EstimateFee(txn *MsgDeSoTxn, minFeeRateNanosPerKB uint64, _ uint64) (uint64, error) {
feeRate := mp.EstimateFeeRate(minFeeRateNanosPerKB, 0)
return EstimateMaxTxnFeeV1(txn, feeRate), nil
}

func (mp *DeSoMempool) EstimateFeeRate(
minFeeRateNanosPerKB uint64,
_ uint64,
_ uint64,
_ uint64,
_ uint64,
_ uint64) uint64 {
func (mp *DeSoMempool) EstimateFeeRate(minFeeRateNanosPerKB uint64, _ uint64) uint64 {
if minFeeRateNanosPerKB < mp.readOnlyUtxoView.GetCurrentGlobalParamsEntry().MinimumNetworkFeeNanosPerKB {
return mp.readOnlyUtxoView.GetCurrentGlobalParamsEntry().MinimumNetworkFeeNanosPerKB
}
Expand Down
23 changes: 11 additions & 12 deletions lib/pos_fee_estimator.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,17 +290,15 @@ func (posFeeEstimator *PoSFeeEstimator) pruneBlocksToMaxNumPastBlocks(blocks []*
// EstimateFeeRateNanosPerKB estimates the fee rate in nanos per KB for the current mempool
// and past blocks using the congestionFactorBasisPoints, priorityPercentileBasisPoints, and
// maxBlockSize params.
func (posFeeEstimator *PoSFeeEstimator) EstimateFeeRateNanosPerKB(
minFeeRateNanosPerKB uint64,
mempoolCongestionFactorBasisPoints uint64,
mempoolPriorityPercentileBasisPoints uint64,
pastBlocksCongestionFactorBasisPoints uint64,
pastBlocksPriorityPercentileBasisPoints uint64,
maxBlockSize uint64,
) uint64 {
func (posFeeEstimator *PoSFeeEstimator) EstimateFeeRateNanosPerKB(minFeeRateNanosPerKB uint64, maxBlockSize uint64) uint64 {
posFeeEstimator.rwLock.RLock()
defer posFeeEstimator.rwLock.RUnlock()

mempoolCongestionFactorBasisPoints := posFeeEstimator.globalParams.MempoolCongestionFactorBasisPoints
mempoolPriorityPercentileBasisPoints := posFeeEstimator.globalParams.MempoolPriorityPercentileBasisPoints
pastBlocksCongestionFactorBasisPoints := posFeeEstimator.globalParams.MempoolPastBlocksCongestionFactorBasisPoints
pastBlocksPriorityPercentileBasisPoints := posFeeEstimator.globalParams.MempoolPastBlocksPriorityPercentileBasisPoints

pastBlockFeeRate := posFeeEstimator.estimateFeeRateNanosPerKBGivenTransactionRegister(
posFeeEstimator.pastBlocksTransactionRegister,
pastBlocksCongestionFactorBasisPoints,
Expand Down Expand Up @@ -333,15 +331,16 @@ func (posFeeEstimator *PoSFeeEstimator) EstimateFeeRateNanosPerKB(
func (posFeeEstimator *PoSFeeEstimator) EstimateFee(
txn *MsgDeSoTxn,
minFeeRateNanosPerKB uint64,
mempoolCongestionFactorBasisPoints uint64,
mempoolPriorityPercentileBasisPoints uint64,
pastBlocksCongestionFactorBasisPoints uint64,
pastBlocksPriorityPercentileBasisPoints uint64,
maxBlockSize uint64,
) (uint64, error) {
posFeeEstimator.rwLock.RLock()
defer posFeeEstimator.rwLock.RUnlock()

mempoolCongestionFactorBasisPoints := posFeeEstimator.globalParams.MempoolCongestionFactorBasisPoints
mempoolPriorityPercentileBasisPoints := posFeeEstimator.globalParams.MempoolPriorityPercentileBasisPoints
pastBlocksCongestionFactorBasisPoints := posFeeEstimator.globalParams.MempoolPastBlocksCongestionFactorBasisPoints
pastBlocksPriorityPercentileBasisPoints := posFeeEstimator.globalParams.MempoolPastBlocksPriorityPercentileBasisPoints

mempoolFeeEstimate, err := posFeeEstimator.mempoolFeeEstimate(
txn,
mempoolCongestionFactorBasisPoints,
Expand Down
50 changes: 37 additions & 13 deletions lib/pos_fee_estimator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestFeeEstimator(t *testing.T) {
require.NoError(t, err)
validateTxnFee(t, txn, computedFee, baseFeeRate)
// Hybrid estimator will also return the base fee rate * number of bytes.
computedFee, err = posFeeEstimator.EstimateFee(txn, 0, 10000, 10000, 1000, 10000, 1000)
computedFee, err = posFeeEstimator.EstimateFee(txn, 0, 1000)
require.NoError(t, err)
validateTxnFee(t, txn, computedFee, baseFeeRate)

Expand Down Expand Up @@ -161,10 +161,16 @@ func TestFeeEstimator(t *testing.T) {
require.Equal(t, estimatedMempoolFee, estimatedPastBlocksFee)
require.Equal(t, estimatedMempoolFeeRate, estimatedPastBlocksFeeRate)

// Update the global params
globalParams := _testGetDefaultGlobalParams()
globalParams.MempoolCongestionFactorBasisPoints = congestionFactor
globalParams.MempoolPastBlocksCongestionFactorBasisPoints = congestionFactor
globalParams.MempoolPriorityPercentileBasisPoints = priorityPercentileBasisPoints
globalParams.MempoolPastBlocksPriorityPercentileBasisPoints = priorityPercentileBasisPoints
posFeeEstimator.UpdateGlobalParams(globalParams)

// And the hybrid estimator is just the max, but for completeness, we check it.
estimatedHybridFee, err = posFeeEstimator.EstimateFee(
txn, 0, congestionFactor, priorityPercentileBasisPoints, congestionFactor, priorityPercentileBasisPoints,
maxBlockSizeHybrid)
estimatedHybridFee, err = posFeeEstimator.EstimateFee(txn, 0, maxBlockSizeHybrid)
require.NoError(t, err)
require.Equal(t, estimatedMempoolFee, estimatedHybridFee)
require.Equal(t, estimatedPastBlocksFee, estimatedHybridFee)
Expand Down Expand Up @@ -206,10 +212,16 @@ func TestFeeEstimator(t *testing.T) {
require.Equal(t, estimatedMempoolFee, estimatedPastBlocksFee)
require.Equal(t, estimatedMempoolFeeRate, estimatedPastBlocksFeeRate)

// Update the global params
globalParams := _testGetDefaultGlobalParams()
globalParams.MempoolCongestionFactorBasisPoints = congestionFactor
globalParams.MempoolPastBlocksCongestionFactorBasisPoints = congestionFactor
globalParams.MempoolPriorityPercentileBasisPoints = priorityPercentileBasisPoints
globalParams.MempoolPastBlocksPriorityPercentileBasisPoints = priorityPercentileBasisPoints
posFeeEstimator.UpdateGlobalParams(globalParams)

// And the hybrid estimator is just the max, but for completeness, we check it.
estimatedHybridFee, err = posFeeEstimator.EstimateFee(
txn, 0, congestionFactor, priorityPercentileBasisPoints, congestionFactor, priorityPercentileBasisPoints,
maxBlockSizeHybrid)
estimatedHybridFee, err = posFeeEstimator.EstimateFee(txn, 0, maxBlockSizeHybrid)
require.NoError(t, err)
require.Equal(t, estimatedMempoolFee, estimatedHybridFee)
require.Equal(t, estimatedPastBlocksFee, estimatedHybridFee)
Expand Down Expand Up @@ -251,10 +263,16 @@ func TestFeeEstimator(t *testing.T) {
require.Equal(t, estimatedMempoolFee, estimatedPastBlocksFee)
require.Equal(t, estimatedMempoolFeeRate, estimatedPastBlocksFeeRate)

// Update the global params
globalParams := _testGetDefaultGlobalParams()
globalParams.MempoolCongestionFactorBasisPoints = congestionFactor
globalParams.MempoolPastBlocksCongestionFactorBasisPoints = congestionFactor
globalParams.MempoolPriorityPercentileBasisPoints = priorityPercentileBasisPoints
globalParams.MempoolPastBlocksPriorityPercentileBasisPoints = priorityPercentileBasisPoints
posFeeEstimator.UpdateGlobalParams(globalParams)

// And the hybrid estimator is just the max, but for completeness, we check it.
estimatedHybridFee, err = posFeeEstimator.EstimateFee(
txn, 0, congestionFactor, priorityPercentileBasisPoints, congestionFactor, priorityPercentileBasisPoints,
maxBlockSizeHybrid)
estimatedHybridFee, err = posFeeEstimator.EstimateFee(txn, 0, maxBlockSizeHybrid)
require.NoError(t, err)
require.Equal(t, estimatedMempoolFee, estimatedHybridFee)
require.Equal(t, estimatedPastBlocksFee, estimatedHybridFee)
Expand Down Expand Up @@ -296,10 +314,16 @@ func TestFeeEstimator(t *testing.T) {
require.Equal(t, estimatedMempoolFee, estimatedPastBlocksFee)
require.Equal(t, estimatedMempoolFeeRate, estimatedPastBlocksFeeRate)

// Update the global params
globalParams := _testGetDefaultGlobalParams()
globalParams.MempoolCongestionFactorBasisPoints = congestionFactor
globalParams.MempoolPastBlocksCongestionFactorBasisPoints = congestionFactor
globalParams.MempoolPriorityPercentileBasisPoints = priorityPercentileBasisPoints
globalParams.MempoolPastBlocksPriorityPercentileBasisPoints = priorityPercentileBasisPoints
posFeeEstimator.UpdateGlobalParams(globalParams)

// And the hybrid estimator is just the max, but for completeness, we check it.
estimatedHybridFee, err = posFeeEstimator.EstimateFee(
txn, 0, congestionFactor, priorityPercentileBasisPoints, congestionFactor, priorityPercentileBasisPoints,
maxBlockSizeHybrid)
estimatedHybridFee, err = posFeeEstimator.EstimateFee(txn, 0, maxBlockSizeHybrid)
require.NoError(t, err)
require.Equal(t, estimatedMempoolFee, estimatedHybridFee)
require.Equal(t, estimatedPastBlocksFee, estimatedHybridFee)
Expand Down
45 changes: 6 additions & 39 deletions lib/pos_mempool.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,8 @@ type Mempool interface {
GetMempoolTipBlockHeight() uint64
GetMempoolTx(txHash *BlockHash) *MempoolTx
GetMempoolSummaryStats() map[string]*SummaryStats
EstimateFee(
txn *MsgDeSoTxn,
minFeeRateNanosPerKB uint64,
mempoolCongestionFactorBasisPoints uint64,
mempoolPriorityPercentileBasisPoints uint64,
pastBlocksCongestionFactorBasisPoints uint64,
pastBlocksPriorityPercentileBasisPoints uint64,
maxBlockSize uint64,
) (uint64, error)
EstimateFeeRate(
minFeeRateNanosPerKB uint64,
mempoolCongestionFactorBasisPoints uint64,
mempoolPriorityPercentileBasisPoints uint64,
pastBlocksCongestionFactorBasisPoints uint64,
pastBlocksPriorityPercentileBasisPoints uint64,
maxBlockSize uint64,
) uint64
EstimateFee(txn *MsgDeSoTxn, minFeeRateNanosPerKB uint64, maxBlockSize uint64) (uint64, error)
EstimateFeeRate(minFeeRateNanosPerKB uint64, maxBlockSize uint64) uint64
}

// GetAugmentedUniversalViewWithAdditionalTransactions is meant as a helper function
Expand Down Expand Up @@ -1133,28 +1118,10 @@ func (mp *PosMempool) GetMempoolSummaryStats() map[string]*SummaryStats {
return convertMempoolTxsToSummaryStats(mp.txnRegister.GetFeeTimeTransactions())
}

func (mp *PosMempool) EstimateFee(
txn *MsgDeSoTxn,
minFeeRateNanosPerKB uint64,
mempoolCongestionFactorBasisPoints uint64,
mempoolPriorityPercentileBasisPoints uint64,
pastBlocksCongestionFactorBasisPoints uint64,
pastBlocksPriorityPercentileBasisPoints uint64,
maxBlockSize uint64,
) (uint64, error) {
return mp.feeEstimator.EstimateFee(
txn, minFeeRateNanosPerKB, mempoolCongestionFactorBasisPoints, mempoolPriorityPercentileBasisPoints,
pastBlocksCongestionFactorBasisPoints, pastBlocksPriorityPercentileBasisPoints, maxBlockSize)
func (mp *PosMempool) EstimateFee(txn *MsgDeSoTxn, minFeeRateNanosPerKB uint64, maxBlockSize uint64) (uint64, error) {
return mp.feeEstimator.EstimateFee(txn, minFeeRateNanosPerKB, maxBlockSize)
}

func (mp *PosMempool) EstimateFeeRate(
minFeeRateNanosPerKB uint64,
mempoolCongestionFactorBasisPoints uint64,
mempoolPriorityPercentileBasisPoints uint64,
pastBlocksCongestionFactorBasisPoints uint64,
pastBlocksPriorityPercentileBasisPoints uint64,
maxBlockSize uint64) uint64 {
return mp.feeEstimator.EstimateFeeRateNanosPerKB(
minFeeRateNanosPerKB, mempoolCongestionFactorBasisPoints, mempoolPriorityPercentileBasisPoints,
pastBlocksCongestionFactorBasisPoints, pastBlocksPriorityPercentileBasisPoints, maxBlockSize)
func (mp *PosMempool) EstimateFeeRate(minFeeRateNanosPerKB uint64, maxBlockSize uint64) uint64 {
return mp.feeEstimator.EstimateFeeRateNanosPerKB(minFeeRateNanosPerKB, maxBlockSize)
}
6 changes: 6 additions & 0 deletions lib/pos_transaction_register_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,12 @@ func _testGetDefaultGlobalParams() *GlobalParamsEntry {
globalParams.MempoolFeeEstimatorNumMempoolBlocks = 1
globalParams.MempoolFeeEstimatorNumPastBlocks = 1

globalParams.MempoolCongestionFactorBasisPoints = 10000
globalParams.MempoolPastBlocksCongestionFactorBasisPoints = 10000

globalParams.MempoolPriorityPercentileBasisPoints = 10000
globalParams.MempoolPastBlocksPriorityPercentileBasisPoints = 10000

return &globalParams
}

Expand Down
Loading