Can we merge CheckTx and DeliverTx response types

[robert-zaremba]

Protocol Change Proposal

Summary

In Cosmos SDK are re working the Ante Handlers concept. We rethink the middleware process and interfaces and thinking about merging return types of DeliverTx and CheckTx into single response type.
Ref: cosmos/cosmos-sdk#9920, cosmos/cosmos-sdk#9920 (comment)

The type definition for ABCI ResponseDeliverTx and

Proposal

We would like to have more opinions:

1. if the Tendermint Team is planning merging these types
2. if not, then if there is any risk if we will merge it for the middle-ware purpose.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[tac0turtle]

merging responses to reduce the interface makes sense. In 0.35 ResponseCheckTx gets a few new fields, but it shouldn't block the merging of the two. I could see app integrators being slightly confused with the response having extra fields but needing to ignore them in deliverTx.

I wonder if we can change things with how FinalizeBlock treats this type tho.

With ABCI++ upcoming, we should try to integrate this proposal.

[creachadair]

Whether it makes sense to merge two types depends not only on whether they're "currently similar", but whether they're likely to remain similar. If the contents of the two messages are (approximately) the same, and any change to one would need to be reflected in the other, merging probably makes sense. But if they're only superficially similar, I wouldn't advise merging them.

If we're talking about adding some fields to one of these types but not the other, I'd advise not merging unless there's a better reason than reducing the number of types in the interface. That's a good goal, but if the result is a hybrid type with disjoint usage patterns, I think the result would be net-negative for the API.

I don't know the right answer for this specific case, but that's the question I think we should index on.

Another point to consider is usage pattern: If the messages are used in very different contexts, then it may be worth keeping them separate even if they're identical. This goes to the footprint of ongoing maintenance.

[ValarDragon]

I don't think these should be treated the same at the ABCI response level, I view the following separation of concerns making a lot of sense.

1. CheckTx - You get data you need for mempool-relevant work
2. DeliverTx - You get execution results for (eventually...) merkelizing into tags for provable querying.

(Note, if (2) isn't intended to ever cause events for merkelization, than I see no usecase of it, other than just that Tendermint needs something to know when to send the next tx)

I think app-side having txs get a unified execution flow to get data for both response types makes sense. But then, these should be split apart right before going into ABCI. It will be very confusing for an execution of a tx to return a priority field imo.

[cmwaters]

I tend to agree that these responses are different in the way they are used and should remain distinct. It's worth pointing out that with the new ABCI++ method FinalizeBlock we might be changing the way the application returns the events/responses for a transaction anyway