Tendermint 0.26 + Fix panic on too many/large transactions

[tomtau]

Fixes #20

Tested with Tendermint 0.26.3

[zramsay]

thanks! We'll get to this shortly :)

[melekes]

🎄 🎅 🦌 🌟

[melekes]

Looks good to me!

@JacksonCoder @davebryson I'd appreciate if you guys find some time for a quick review.

[melekes]

@tomtau can we get this rebased against latest master? thanks

[davebryson]

This will work for sure. It'd be nice to add more tests since NetStream has been added.

While we're fiddling with this, I wonder though if it would be better to do a bit of refactoring and just allocate (create) BytesMut based on the actual size of the response - versus preallocating it with BUFFER_SIZE and then doing the additional checks and the potential call to reserve?

[tomtau]

@melekes rebased.

@davebryson I agree, but it may be better to open separate smaller issues and PRs for that. Besides more tests, here are a few points I'd like to fix:

1. The current fix only helps with responses, but it's still possible to get a panic here:

   rust-abci/src/server.rs

   Line 99 in c947920

   let _ = stream.read(bytes.as_mut());

   
   e.g. if one sends a very large QueryRequest. But this is a separate issue and I'm not yet sure what the best way to pre-allocate is for requests from streams -- perhaps bound it to the max block bytes consensus parameter? Or make use of that TM convention (https://tendermint.com/docs/spec/abci/client-server.html#server-implementations) on the receiving side?

Messages are serialized using Protobuf3 and length-prefixed with a signed Varint

2. Add support for unix sockets to NetStream.

3. Have a way to distinguish between connections (info, mempool, consensus) and have a relaxed trait that wouldn't need to mutate the app on requests from info and mempool, so that there's no need for that mutex.

4. async processing for DeliverTx and CheckTx requests

5. Have additional sanity checks on the ABCI protocol execution, so that client apps don't need to worry about them -- e.g. use https://docs.rs/session_types/0.2.0/session_types/

6. instead of that nested if and unwrapping, just directly pattern match on request.value

7. trait (e.g. ResponseTx) for ResponseCheckTx and ResponseDeliverTx, since they contain the same fields

[melekes]

Merged. Thanks everyone 👍

@tomtau feel free to open a new issue (or many small ones).