I'm not thrilled about this change... it makes a perfectly correct outcome, yet if there are a ton of transactions in mempool below the minfeerate then it will slow down template creation a bit.

Alternatively we can redefine blockMinFeeRate to be based on virtualsize feerate (i.e., just change packageSize -> packageVirtualSize here), however that would change behaviour since we would now suddenly stop mining some 1 sat/byte transactions by default, just because they had a few too many sigops. If we were to go that route then we should set the default -blockmintxfee to 0 (see D4902).

It's a weird python feature, one that I love dearly :D

https://docs.python.org/3/tutorial/controlflow.html#break-and-continue-statements-and-else-clauses-on-loops

Yes, I believe that's right.

If we left it as-is, suppose blockmintxfee = 10, and there are these packages, in order of vbytes feerate:

A: 12 sat/byte, 12 sat/vbyte
B: 9 sat/byte, 9 sat/vbyte
C: 100 sat/byte, 8 sat/vbyte
D: 8 sat/byte, 7.8 sat/vbyte

Packages up to (and including) A would be included in block, but transaction C would not be included, and that's because of B. (if B were not present, then C would get included but not D).

For default settings with blockmintxfee = minrelaytxfee = 1 sat/byte, there would be no difference in normal operation, UNLESS it's after a reorg and there is a single 0.99 sat/byte low-sigops transaction in mempool, then all high sigops <0.99 sat/vbyte transactions would never get cleared out of mempool until that transaction goes away, which would mean the mempool could explode in size.

As mentioned earlier, an acceptable alternative would be to redefine this check to use packageVirtualSize instead of packageSize. But we should then also do D4902 (we don't want to stop mining below 1 sat/vbyte as do we accept such transactions).

Hmm, thinking about it a bit more, that latter scenario does suck. Imagine there is a reorg including a single 0.99 sat/byte transaction, then all 0.98 sat/vbyte transactions might not get mined for a long time.

0.98 sat/vbyte isn't abusive, and does occur in normal usage. For example a 1 sat/byte transaction that fans out 1 P2PKH input to 11 P2PKH outputs has 11 sigops in ~532 bytes, i.e., 0.97 sat/vbyte.

yes, though I need the height here anyway