Proof-of-Concept: Add alt options to PrepareProposal

[ttl33]

Idea/Problem:

• Original PR comment
• Currently, once the app mempool returns a list of txs to PrepareProposal via Select, the app does not know for sure if all txs will be included in the ResponsePrepareProposal.
• This matters to apps because the txs returned in Select were crafted in a specific way (i.e. the txs ordering or types of txs included). Ignoring or only including a subset of the returned txs would be undesirable from the app's perspective.

Options:

1. PrepareProposal assumes that app-side mempool will return valid set of txs. If it doesn't return a valid set then, it's considered a bug, so panic.
2. PrepareProposal asks app-side mempool: "hey, these are the final txs that will be included in ResponsePrepareProposal , do these txs look okay to you still?"

I think there are better options out there, but drafting this PR to communicate the high-level thought

[kocubinski]

• Currently, once the app mempool returns a list of txs to PrepareProposal via Select, the app does not know for sure if all txs will be included in the ResponsePrepareProposal.

Select returns a stateful iterator, not a list of txs, for this kind of reason. Given a partially ordered mempool (totally ordered shouldn't be an issue), if validity checks in PrepareProposal fail, the tx is removed from the mempool, and the stateful iterator handles it gracefully (it should), I don't see a problem.

But in case I am missing something 2 of the solutions outlined in #13810 could work too. Either an API providing a validity handler (called in both Prepare/Process; this could have tight coupling to the mempool), or just allowing apps to define their own PrepareProposal outright.

[kocubinski]

I don't agree that any invalid tx in the mempool should cause a panic in PrepareProposal.

[ttl33]

yah, this didn't quite feel right, but I thought it was sufficient to show case the idea

[kocubinski]

We opted for an iterator to accommodate highly customized cases like you're describing. I'm leaning towards just letting apps define PrepareProposal.

[ttl33]

💯 I agree with this idea!

[ttl33]

Currently, once the app mempool returns a list of txs to PrepareProposal via Select, the app does not know for sure if all txs will be included in the ResponsePrepareProposal.

Select returns a stateful iterator, not a list of txs, for this kind of reason. Given a partially ordered mempool (totally ordered shouldn't be an issue), if validity checks in PrepareProposal fail, the tx is removed from the mempool, and the stateful iterator handles it gracefully (it should), I don't see a problem.

There are two main arguments against this:

1. The current PrepareProposal is too restrictive and gives the app's mempool the illusion that what is returned by Select will be included in the proposal. The filtering logic (ie if tx does not pass CheckTx or if the num tx bytes exceed max bytes, then ignore/throw away those txs) is too restrictive given that the main idea behind PrepareProposal is to give the app the full control of how the proposal will look like. At the end of the day, what's returned by Select is not guaranteed to end up in the final result of PrepareProposal; therefore gives the mempool the illusion.
2. I would argue that removing tx from the app side mempool inside PrepareProposal as a side effect of running through the iterator is unexpected from the app mempool's perspective. For example, app side mempool could think: "I returned these txs to Select, but whoever used the Select iterator has now removed some txs from my mempool" 🤔

---

Having said that, what you have proposed in #13831 does give the app the full flexibility/control to manage PrepareProposal results. This should address my main concern given that the app can fully define its own PrepareProposalHandler and there's no filtering logic that wraps this.

Thank you all for entertaining this idea and I appreciate you guys being very responsive + receptive of feedback 💪

[ttl33]

Given that #13831 addresses the main concern for this PR, I will close this PR