search

stellar / slingshot

A new blockchain architecture under active development, with a strong focus on scalability, privacy and safety
Apache License 2.0
415 stars 61 forks source link

zkvm: refactor encoding/decoding #424

Closed oleganza closed 4 years ago

oleganza commented 4 years ago

We currently have multiple uses for encoding throughout zkvm, blockchain and p2p crates:

  1. Writing various objects (TxHeader, TxLog, Tx, BlockTx) into merlin::Transcript.
  2. Encoding and decoding contracts within VM from byte strings.
  3. Reading and writing BlockTx to the network sockets.

Downsides of the current approach:

  1. Writing to the Transcript is usually done with an intermediate encoding of the entire object into a buffer (see zkvm::Contract::id() and blockchain::BlockTx::witness_hash()).
  2. When we write to the wire, we want to use Tokio Codecs with their buffers (bytes::BytesMut/BufMut), but keep the rest of the system independent from Tokio or any I/O framework.

Requirements

We need a pair of traits Reader/Writer akin to Buf/BufMut, but with several modifications:

  1. Never panics.
  2. Reads and writes return Result, with associated Error type, defined by the impl.
  3. Read fn checks the remaining length.
  4. Write fn accepts a label of type &'static str that can be used with the Transcript (or JSON ;-)) Binary buffers will simply ignore the label.
  5. We only need u8/u16/u32/u64 (LE) and &[u8] types supported for now. (Keep _le explicit in the naming, though.)

Organization

  1. Separate readerwriter crate.
  2. Optional dependency on bytes (for interop with Bytes/Buf), with corresponding module bytes_impls.rs enabled on #[cfg(feature="bytes")].
  3. Optional dependency on merlin (for interop with Transcript), with corresponding module merlin_impls.rs enabled on #[cfg(feature="merlin")].
  4. Crate zkvm depends on readerwriter, SliceReader is removed.
  5. In crates zkvm and blockchain: encoding/decoding methods are changed to use Reader/Writer API. Hashing is redefined in the spec to write data to transcript field-by-field.
  6. Crate blockchain depends on readerwriter, but not on p2p.
  7. p2p uses same readerwriter API for its encoding needs.
oleganza commented 4 years ago

Reader/Writer traits: #427, #429

p0lunin commented 4 years ago
  1. Crate blockchain depends on readerwriter, but not on p2p.

But CustomMessage trait placed in p2p, so p2p will be in blockchain dependencies. Moving this trait into another place seems illogical to me.

oleganza commented 4 years ago

But CustomMessage trait placed in p2p, so p2p will be in blockchain dependencies. Moving this trait into another place seems illogical to me.

How about this:

  1. blockchain implements encoding via Reader/Writer traits.
  2. The node crate (e.g. demo) that provides concrete storage and I/O, implements p2p::CustomMessage for blockchain::protocol::Message (via a newtype wrapper) and wires all things together.
p0lunin commented 4 years ago

Final results:

  1. Add traits Encodable/Decodable in readerwriter crate. Implement trait Codable for T where T both implement Encodable + Decodable.
  2. Use in p2p trait Codable instead of CustomMessage.
  3. Implement Encodable/Decodable to blockchain::Message.
oleganza commented 4 years ago

430 seems to be the final nail in this coffin