Summary

This issue brings light to a discussion around simplifying the API of prepare proposal. What is proposed are cosmetic changes that are believed to not impact the application in any meaningful way but may make implementation simpler and more intuitive.

Problem Definition

The new ResponsePrepareProposal object returns an array of TxRecord which are the txs (a.k.a the raw bytes) accompanied with an enum, Action, that can be either ADDED, UNMODIFIED, REMOVED. UNMODIFIED signals that a tx has been directly returned from the request object, ADDED means it's new and REMOVED tells Tendermint to remove whatever tx from the mempool if it's there.

message TxRecord {
  TxAction action = 1;
  bytes    tx     = 2;

  // TxAction contains App-provided information on what to do with a transaction that is part of a raw proposal
  enum TxAction {
    UNKNOWN    = 0;  // Unknown action
    UNMODIFIED = 1;  // The Application did not modify this transaction.
    ADDED      = 2;  // The Application added this transaction.
    REMOVED    = 3;  // The Application wants this transaction removed from the proposal and the mempool.
  }
}

Previously, it was decided that ADDED would immediately add the tx to the mempool before being propagated in the block. Later it was decided this wasn't necessary (as the tx is already in the block). Thus, currently, ADDED and UNMODIFIED are semantically the same.

This is additional complexity as it requires the application to know if a tx was proposed in the Request object and then to mark them accordingly. In the case of the SDK which is looking to use an app-side mempool, the application wants to simply return the transactions they already have queued up.

The REMOVED enum adds functionality in that the proposer can quickly garbage transactions that have become invalid from this round of selected transactions. However, there's no guarantee that this will be the block committed - thus the proposer may remove a transaction that may still be valuable for future rounds. More important, this is only for the proposer. The rest of the nodes will still have and gossip the removed transactions. Applications will still need to rely on ReCheckTx for global level garbage collection.

Proposal

There are currently three proposals:

1. Leave everything as it is

As we're relatively close to release we can choose to leave it as is and address it in a later release. The consequence of this means applications will have to do work twice to comply with the APIs

2. Merge the enum

One proposal is to merge these two actions into a single INCLUDED enum. This simply tells Tendermint to include these transactions in the proposed block:

enum TxAction {
    UNKNOWN    = 0;  // Unknown action
    INCLUDED   = 1;  // The Application includes the tx in the proposed block.
    REMOVED    = 2;  // The Application wants this transaction removed from the proposal and the mempool.
}

3. Return the raw transactions
This is the simplest API. The application returns directly the bytes it wants to be proposed in the block. There is no enum. Transactions can't be removed from the proposers mempool but must wait until after the block has been committed with ReCheckTx.

message ResponsePrepareProposal {
  repeated bytes txs = 1;
}