Feat: add heuristics to miner for nakamoto

[kantai]

This add two heuristics for mining in nakamoto:

• For the first block in a tenure, just mine an empty block
• Estimate the time it takes to eval a tx, and see if it will interfere with block deadline. If it will, skip the transaction.

Marking as draft until tests can be fleshed out and likely broken integration tests fixed.

[jcnelson]

I think the idea is sound, but testing this in a reproducible way is gonna be interesting.

[obycode]

LGTM! I like this addition! Just noticed one rogue comment.

[obycode]

👍

[jcnelson]

The changes look fine to me, but I'm curious if the mempool changes are sufficient to stave off a network stall in fuzzing. The code will only detect that a transaction would exceed its designated processing time after it has been run once. But if the fuzzer is bombarding the network with lots and lots of expensive transactions, which are only meant to ever be evaluated once, then this wouldn't materially change anything.

Do we know for sure that the cause of block production slowness in fuzzing is due to re-considering the same transaction over and over?

Also, would this change mean that some transactions are never mined, despite being valid, simply because they were considered once and effectively blocked from re-consideration due to taking too long to run? When it comes time to rolling this out into production, we'll want to be sure that this doesn't lead to some contracts being effectively blocked from executing.

[kantai]

The changes look fine to me, but I'm curious if the mempool changes are sufficient to stave off a network stall in fuzzing. The code will only detect that a transaction would exceed its designated processing time after it has been run once. But if the fuzzer is bombarding the network with lots and lots of expensive transactions, which are only meant to ever be evaluated once, then this wouldn't materially change anything.

That's mostly true, but its also an argument against incremental improvement. Does this change address all possible long running transactions in the mempool? No, but it does at least prevent the same long running transaction from repeatedly preventing the miner from making progress.

Do we know for sure that the cause of block production slowness in fuzzing is due to re-considering the same transaction over and over?

In the particular case that this addresses, yes.

Also, would this change mean that some transactions are never mined, despite being valid, simply because they were considered once and effectively blocked from re-consideration due to taking too long to run? When it comes time to rolling this out into production, we'll want to be sure that this doesn't lead to some contracts being effectively blocked from executing.

That would be ultimately up to the miner and signer sets. Having an estimate for how long a transaction will take to run and comparing against the miners self-configured execution deadline doesn't seem like a policy change vis-a-vis long-running transactions: rather, it just makes the miner somewhat smarter about hitting its deadling.

[jcnelson]

That's mostly true, but its also an argument against incremental improvement. Does this change address all possible long running transactions in the mempool? No, but it does at least prevent the same long running transaction from repeatedly preventing the miner from making progress.

Apologies if I sounded like I was against incremental improvement. I was only trying to confirm my own understanding.

[kantai]

New tests and assertions added for the behaviors, and all existing tests pass. Ready for review!

[obycode]

Lgtm!

[blockstack-devops]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.