ok, I did something kinda weird here: I implemented a txpool, but I did it inside the chain manager.
This requires some justification. The "normal" way to implement the txpool would be as a subscriber to the chain manager. This is how it's done in siad: modules.TransactionPool subscribes to modules.ConsensusSet. The pool receives transactions from the user (and from peers), and when a block comes in, it removes any confirmed transactions from the pool. Simple, right? This is the approach I initially used for core as well.
Once I had finished prototyping and started hooking everything up, though, I ran into some difficulties. I came up with workarounds, but something still felt off. The final straw came when, after implementing (what I thought was) the final workaround, I realized that I had introduced a deadlock. It was time to take a larger step back.
The source of the trouble was transaction validation. The txpool has to ensure that all of the transactions in the pool could potentially be included in the next block. So when a new block is mined, all of the transactions in the pool have to be revalidated against the new state. Transactions in the pool may also depend on each other, so invalidating one transaction could cascade to many more. Separately, validation also has to occur when new transactions are added to the pool. (These requirements are what led me to implementing #107) But here's the problem: validating transactions requires access to the consensus database. This creates a dangerous cycle: the consensus database is owned by the chain manager, so the txpool needs to call into the chain manager to validate new transactions; but the txpool is also subscribed to the chain manager, so the chain manager needs to call into the txpool to give it new blocks. The situation is ripe for a deadlock.
siad solves this problem by having the txpool call a special method on the chain manager: LockedTryTransactionSet. This is a function that takes a function that takes a transaction validation function and calls it while holding the chain manager mutex, so that the second function can call the validation function arbitrarily while protected by the lock. Simple, right? 🙃 (I was, um, not particularly happy with this solution, and expressed as much in the commit message and branch name.)
I think I must have repressed the memory of writing that code, because otherwise I wouldn't have plowed ahead with the same exact architecture in core. It was only when things failed to come together nicely that I remembered what had transpired years ago...
Anyway, when you have two components that are all mixed up in each other's biz, there's really only one solution: smash them together into one big component. Usually this feels wrong; my gut insists that the chain manager and txpool are "different things" that shouldn't live under one roof, and there's a way to cleanly separate them if I try hard enough. But hey, sometimes your gut is wrong. After all, the deadlock is gone now -- can't argue with that.
Interestingly, this problem entirely disappears in Utreexo. There's no more consensus database, so the txpool no longer needs to access it through the chain manager; instead, it can maintain its own copy of the full consensus state (updated via subscription), and validate all transactions against that -- nice! So once we migrate to Utreexo, it should be possible to split these two components apart again. But who knows, there may be other reasons to keep them together that aren't clear yet. Time will tell.
Oh, one more tidbit: the new txpool is lazy. Rather than revalidating the pool after every block, it only does so when necessary, e.g. when someone calls PoolTransactions. A similar principle is applied to the RecommendedFee and UnconfirmedParents methods. In steady-state, this probably doesn't make a big difference in performance (since the UI will poll more than once per 10 minutes), but when you're syncing thousands of blocks, it absolutely does!
