tx_pool: full tx revalidation on fork boundaries #7169
Conversation
moneromooo-monero
force-pushed
the
txpv
branch
3 times, most recently
from
032444a to
bbbcc51
Compare
There was a problem hiding this comment.
LGTM
One question: it looks like all tx's from the pool are loaded into memory -- would this be a concern? Thinking of a scenario where someone malicious tries to cause issues at hard fork time by spamming. The default cap on the pool is ~618 MB and it can be adjusted lower via a param for lower end hardware, so probably not I would think (which is why I approved)
| // get all txids | ||
| std::vector<tx_entry_t> txes; | ||
| m_blockchain.for_all_txpool_txes([this, &txes](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata_ref*) { | ||
| if (!meta.pruned) // skip pruned txes |
There was a problem hiding this comment.
Also, my reading of this is that at the fork, pruned nodes won't have any validation logic a number of their tx's meet the fork's new rules (even on tx weight). So at fork time, pruned nodes would be relying on non-pruned nodes to avoid any issues, which seems alright? Am I reading that right?
There was a problem hiding this comment.
In normal "cruise" usage, transactions get pruned after 5500 blocks.
The only way a pruned tx can get into the txpool is when it is an old tx that was requested by a syncing node. In that case, this only gets requested if the node has a known block hash to compare it with, and this test will not matter.
|
That is a good point, they don't seem to need to be loaded at once. I'll fix that. |
|
Well, loaded fully parsed in memory at least. |
avoids mining txes after a fork that are invalid by this fork's rules, but were valid by the previous fork rules at the time they were verified and added to the txpool.
moneromooo-monero
force-pushed
the
txpv
branch
from
bbbcc51 to
bbe3b27
Compare
|
Done, though tbh I don't remember the original test case. Simple enough though. |
There was a problem hiding this comment.
Update looks good
I also used the view tag stuff I've been working on to test behavior. I repro'd the scenario without this code, then added this code to make sure invalid tx's that were valid prior to fork height would get ejected from the pool at fork height. It worked as expected
avoids mining txes after a fork that are invalid by this fork's
rules, but were valid by the previous fork rules at the time
they were verified and added to the txpool.