[Bug]: Priority Nonce Mempool Nil Pointer Dereference (Next)

[davidterpay]

What happened?

At Skip, we're utilizing the priority nonce mempool for some custom ABCI++ proposal building. We came across an issue where there was a nil pointer dereference in the Next function on the Priority Nonce Mempool.

cosmos-sdk/types/mempool/priority_nonce.go

Lines 312 to 324 in dded2e9

	// We've reached a transaction with a priority lower than the next highest
	// priority in the pool.
	if i.mempool.cfg.TxPriority.Compare(key.priority, i.nextPriority) < 0 {
	return i.iteratePriority()
	} else if i.mempool.cfg.TxPriority.Compare(key.priority, i.nextPriority) == 0 {
	// Weight is incorporated into the priority index key only (not sender index)
	// so we must fetch it here from the scores map.
	weight := i.mempool.scores[txMeta[C]{nonce: key.nonce, sender: key.sender}].weight
	if i.mempool.cfg.TxPriority.Compare(weight, i.priorityNode.Next().Key().(txMeta[C]).weight) < 0 {
	return i.iteratePriority()
	}
	}

Cosmos SDK Version

main

How to reproduce?

This issue can be replicated in the following way,

• have the key.priority be the minimum value defined on the TxConfig
• This will make the else if statement true (as it looks like i.nextPriority defaults to the same min val defined on the TxConfig but im not fully sure on this)
• when it attempts to fetch i.priorityNode.Next().Key().(txMeta[C]).weight) it will attempt to dereference a nil pointer.

Simplest fix is likely just wrapping that code in an if i.priorityNode.Next() != nil....

[kocubinski]

Thanks for finding this @davidterpay, especially the steps to reproduce. It may be a lot to ask, but do you have a failing test which could be added to priority_nonce_test.go? If not, I will get to it eventually, but this could speed things up depending on your urgency.