rpc: Remove ABCI-related types

[thanethomson]

Version(s) of tendermint-rs: master

Description

Follows from #1022, which introduced domain types into the tendermint crate for ABCI. The RPC crate used to have ABCI-specific data structures to use in its responses, but these have been superseded by the ABCI domain types in the tendermint crate.

These old ABCI RPC data structures must be removed.

Definition of "done"

When all of the old ABCI RPC data structures have been removed.

[mzabaluev]

Some questions while working on #1204:

• In the response struct for /broadcast_tx_commit, do we care to change the check_tx and deliver_tx fields to the corresponding domain types from the tendermint crate? Serde for TxResult reflects some quirks that may be only relevant for this endpoint, and also features the mempool_error field which is consciously omitted from the abci::response::CheckTx domain type.

• The Code enum is more useful in idiomatic Rust than an integer. It explains itself in match contexts, it enforces the convention, and even has the same size as an u32. Would it make sense to promote it into the idiomatic types in tendermint?

[thanethomson]

In the response struct for /broadcast_tx_commit, do we care to change the check_tx and deliver_tx fields to the corresponding domain types from the tendermint crate? Serde for TxResult reflects some quirks that may be only relevant for this endpoint, and also features the mempool_error field which is consciously omitted from the abci::response::CheckTx domain type.

The mempool_error field actually should be in abci::response::CheckTx. As per the comment in the file, it shouldn't be set by the ABCI application. But what about when deserializing it via the RPC? Does it get returned via the RPC at any point?

I'd say we should use the domain types as far as practically possible.

The Code enum is more useful in idiomatic Rust than an integer. It explains itself in match contexts, it enforces the convention, and even has the same size as an u32. Would it make sense to promote it into the idiomatic types in tendermint?

Are you referring to the Code type defined in tendermint-rpc? I seem to remember @hdevalence preferring native Rust types. But for Code specifically it makes sense to me to promote it to the tendermint crate. Would this then be added to the abci module and used throughout that module?

[mzabaluev]

The mempool_error field actually should be in abci::response::CheckTx. As per the comment in the file, it shouldn't be set by the ABCI application. But what about when deserializing it via the RPC? Does it get returned via the RPC at any point?

It probably does in the response for /broadcast_tx_commit. If we consider the domain types as unified representation for both protobuf and RPC data, I should change the fields' type from the custom TxResult struct to CheckTx/DomainTx, add the optional field to CheckTx, and deal with the possible JSON quirks.

Are you referring to the Code type defined in tendermint-rpc? I seem to remember @hdevalence preferring native Rust types. But for Code specifically it makes sense to me to promote it to the tendermint crate. Would this then be added to the abci module and used throughout that module?

Yes, I did that as part of #1204.