Implement the PreprocessTxs ABCI method using WirePayForMessage

[evan-forbes]

After defining WirePayForMessage and connecting it the rest of the application, the draft implementation for PreprocessTxs needs to:

1. cleanly separate share messages from other transactions
2. perform basic validation on WirePayForMessages
3. pick block size (fixed for now)
4. remove unused share commitments

There are still some design decisions that need to be made. Notably:

1. How are we handling errors in PreprocessTxs? The ResponsePreprocessTxs only expects txs and messages.
2. Are we burning fees the same way most ethereum projects do? Send to the zero address?
3. Should the fees actually get burned in PreprocessTxs or in DeliverTxs?
4. The spec has a set of enums for the transaction types, so I think that means that each transaction goes through the same endpoint, not unlike ethereum, but the sdk expects typed rpc calls. Thoughts?
5. I'm guessing that block size and fees should be static for the very first draft, but in the future, is that going to be handled by core or the app?
6. Same goes for namespace size. Constant? If not, who decides, the app or core?
7. Should we be using big.Int instead of uint64?

[liamsi]

How are we handling errors in PreprocessTxs? The ResponsePreprocessTxs only expects txs and messages.

What kind of errors can occur when preprocessing? If a Tx is causing an error it would be dropped (not included in the proposed block). I don't think we need to return an error code to the consensus engine as there isn't much it can do about them.

Are we burning fees the same way most ethereum projects do? Send to the zero address?

Copy & pasting @adlerjohn's reply from discord here for reference:

2. No, SignedTransactionDataBurn will actually properly burn coins if you want manual burn. Burnt fees are similarly actually burnt, but automatically for each transaction. The spec accounts for burnt transaction fees by not allocating the entire fee to the block proposer (see totalCost https://github.com/lazyledger/lazyledger-specs/blob/1a71da17e8faf7ca3c85d4624c6cbe29205bf1df/specs/consensus.md#blockavailabledatatransactiondata).

Should the fees actually get burned in PreprocessTxs or in DeliverTxs?

In the future (with immediate execution), on Preprocess, for a first version (mvp), deliverTx (or even skip the fee burning logic completely and only deduct the fees the sdk deducts for "regular" Tx).

I'm guessing that block size and fees should be static for the very first draft, but in the future, is that going to be handled by core or the app?

We have not settled on this but the app would make the most sense.

Same goes for namespace size. Constant? If not, who decides, the app or core?

Namespace size is constant and set to 8 bytes: https://github.com/lazyledger/lazyledger-core/blob/585566317e519bbb6d35d149b7e856c4c1e8657c/types/consts.go#L16

Should we be using big.Int instead of uint64?

It depends where exactly but as a rule of thumb: if that is what the SDK still does (check the latest version/upcoming changes) I would say yes (even if that diverts from the spec), otherwise use unsigned ints where the value shouldn't be negative.

TODO (@liamsi ):

• answer question 4.

[evan-forbes]

What kind of errors can occur when preprocessing? If a Tx is causing an error it would be dropped (not included in the proposed block). I don't think we need to return an error code to the consensus engine as there isn't much it can do about them.

Yeah, that makes sense, will do. There shouldn't be any errors, now that I think about it more.

[liamsi]

The spec has a set of enums for the transaction types, so I think that means that each transaction goes through the same endpoint, not unlike ethereum, but the sdk expects typed rpc calls. Thoughts?

IMO, all tx types not specific to LL (staking/vanilla transfers etc) should follow the existing SDK pattern and go through their (already existing) module's endpoint. Only the LL-specific tx should show up in the lazyledger module (for the first version, only payformessage).
Unless, it would be simpler to only have one endpoint and route all the tx from the LL module to the other modules - if that is simpler to implement, that would still be preferable (as far as I understand it would be more work though, is that right?).

[evan-forbes]

It would be a small amount of work to change, and might look a little odd to someone if they are familiar with the sdk. Nothing too crazy though. Last I checked with @adlerjohn, he wasn't 100% sure if anything bad would happen, so I'll plan on removing them for now. If we need to add them back in for any reason, it really won't be that much work.