Fix underpriced check for multi currency (#2230)

contracts/currency/currency.go

@@ -153,6 +153,11 @@ func NewManager(vmRunner vm.EVMRunner) *CurrencyManager {
 	return newManager(GetExchangeRate, vmRunner)
 }
 
+// NewCacheOnlyManager creates a cache-full currency manager. TESTING PURPOSES ONLY
+func NewCacheOnlyManager(currencyCache map[common.Address]*Currency) *CurrencyManager {
+	return &CurrencyManager{currencyCache: currencyCache}
+}
+
 func newManager(_getExchangeRate func(vm.EVMRunner, *common.Address) (*ExchangeRate, error), vmRunner vm.EVMRunner) *CurrencyManager {
 	return &CurrencyManager{
 		vmRunner:         vmRunner,

core/tx_list.go

@@ -696,18 +696,29 @@ func (l *txPricedList) Underpriced(tx *types.Transaction) bool {
 }
 
 func (l *txPricedList) underpricedForMulti(h *multiCurrencyPriceHeap, tx *types.Transaction) bool {
-	underpriced := l.underpricedFor(h.nativeCurrencyHeap, tx)
+	// wellpriced returns if, after pruning, tx is above minimum price
+	// compared to the top of the heap (minimum priced tx in heap)
+	wellpriced := func(heap *priceHeap) bool {
+		l.prune(heap)
+		return heap.Len() > 0 && !h.IsCheaper(tx, heap.list[0])
+	}
+
+	// If tx is wellpriced for at least one heap, then it is not underpriced
+	if wellpriced(h.nativeCurrencyHeap) {
+		return false
+	}
 	for _, sh := range h.currencyHeaps {
-		if l.underpricedFor(sh, tx) {
-			underpriced = true
+		if wellpriced(sh) {
+			return false
 		}
 	}
-	return underpriced
+	// While this impl. is returning true when all heaps are empty (h.Len() == 0), that border
+	// case is managed on invokation of this fn.
+	return true
 }
 
-// underpricedFor checks whether a transaction is cheaper than (or as cheap as) the
-// lowest priced (remote) transaction in the given heap.
-func (l *txPricedList) underpricedFor(h *priceHeap, tx *types.Transaction) bool {
+// prune discards from the top of the heap txs that are no longer in the pool
+func (l *txPricedList) prune(h *priceHeap) {
 	// Discard stale price points if found at the heap start
 	for len(h.list) > 0 {
 		head := h.list[0]
@@ -718,13 +729,6 @@ func (l *txPricedList) underpricedFor(h *priceHeap, tx *types.Transaction) bool
 		}
 		break
 	}
-	// Check if the transaction is underpriced or not
-	if len(h.list) == 0 {
-		return false // There is no remote transaction at all.
-	}
-	// If the remote transaction is even cheaper than the
-	// cheapest one tracked locally, reject it.
-	return h.cmp(h.list[0], tx) >= 0
 }
 
 // Discard finds a number of most underpriced transactions, removes them from the

core/tx_multicurrency_priceheap.go

@@ -10,9 +10,37 @@ import (
 
 type CurrencyCmpFn func(*big.Int, *common.Address, *big.Int, *common.Address) int
 
-// IsCheaper returns true if tx1 is cheaper than tx2 (GasPrice with currency comparison)
-func (cc CurrencyCmpFn) IsCheaper(tx1, tx2 *types.Transaction) bool {
-	return cc(tx1.GasPrice(), tx1.FeeCurrency(), tx2.GasPrice(), tx2.FeeCurrency()) < 0
+func (cc CurrencyCmpFn) GasTipCapCmp(tx, other *types.Transaction) int {
+	return cc(tx.GasTipCap(), tx.FeeCurrency(), other.GasTipCap(), other.FeeCurrency())
+}
+
+// EffectiveGasTipCmp returns the same comparison result as Transaction.EffectiveGasTipCmp
+// but taking into account the exchange rate comparison of the CurrencyCmpFn.
+// Each baseFee is expressed in each tx's currency.
+func (cc CurrencyCmpFn) EffectiveGasTipCmp(tx, other *types.Transaction, baseFeeA, baseFeeB *big.Int) int {
+	return cc(tx.EffectiveGasTipValue(baseFeeA), tx.FeeCurrency(), other.EffectiveGasTipValue(baseFeeB), other.FeeCurrency())
+}
+
+func (cc CurrencyCmpFn) GasFeeCapCmp(a, b *types.Transaction) int {
+	return cc(a.GasFeeCap(), a.FeeCurrency(), b.GasFeeCap(), b.FeeCurrency())
+}
+
+// Cmp returns the same comparison as the priceHeap comparison but taking into account
+// the exchange rate comparison of the CurrencyCmpFn.
+// Each baseFee is expressed in each tx's currency.
+func (cc CurrencyCmpFn) Cmp(a, b *types.Transaction, baseFeeA, baseFeeB *big.Int) int {
+	if baseFeeA != nil && baseFeeB != nil {
+		// Compare effective tips if baseFee is specified
+		if c := cc.EffectiveGasTipCmp(a, b, baseFeeA, baseFeeB); c != 0 {
+			return c
+		}
+	}
+	// Compare fee caps if baseFee is not specified or effective tips are equal
+	if c := cc.GasFeeCapCmp(a, b); c != 0 {
+		return c
+	}
+	// Compare tips if effective tips and fee caps are equal
+	return cc.GasTipCapCmp(a, b)
 }
 
 // multiCurrencyPriceHeap is a heap.Interface implementation over transactions
@@ -34,8 +62,10 @@ func newMultiCurrencyPriceHeap(currencyCmp CurrencyCmpFn, gpm GasPriceMinimums)
 
 		// inner state
 
-		nativeCurrencyHeap: &priceHeap{},
-		currencyHeaps:      make(map[common.Address]*priceHeap),
+		nativeCurrencyHeap: &priceHeap{}, // Not initializing the basefee
+		// since it gets updated as soon as the node starts, and
+		// tx pool tests (upstream) assume baseFee == nil
+		currencyHeaps: make(map[common.Address]*priceHeap),
 	}
 }
 
@@ -79,11 +109,18 @@ func (h *multiCurrencyPriceHeap) cheapestTxs() []*types.Transaction {
 	return txs
 }
 
+// IsCheaper returs true iff tx1 effective gas price <= tx2's
+func (h *multiCurrencyPriceHeap) IsCheaper(tx1, tx2 *types.Transaction) bool {
+	baseFee1 := h.getHeapFor(tx1).baseFee
+	baseFee2 := h.getHeapFor(tx2).baseFee
+	return h.currencyCmp.Cmp(tx1, tx2, baseFee1, baseFee2) <= 0
+}
+
 func (h *multiCurrencyPriceHeap) cheapestTx() *types.Transaction {
 	txs := h.cheapestTxs()
 	var cheapestTx *types.Transaction
 	for _, tx := range txs {
-		if cheapestTx == nil || h.currencyCmp.IsCheaper(tx, cheapestTx) {
+		if cheapestTx == nil || h.IsCheaper(tx, cheapestTx) {
 			cheapestTx = tx
 		}
 	}