Zilliqa / zq2

Zilliqa 2.0 code base
Apache License 2.0
9 stars 0 forks source link

Attempting to send a transaction via foundry halts the network #1076

Closed rrw-zilliqa closed 3 months ago

rrw-zilliqa commented 3 months ago

To reproduce, start a network with z2, then fire the txns from zilliqa-experimental/richard/hello-forge at it. You'll get a serialisation error and the nodes will panic.

They do this because p2p_node.rs:299 fails:

 let message = serde_json::from_slice::<ExternalMessage>(&data).unwrap();

and this in turn fails because JSON-encoded transactions encode their tx data as an array of bytes (rather than rlp) and the visitor for rlp_encoded txns in transaction.rs:96 doesn't implement visit_seq<>, presumably because you can't easily transmit a byte sequence in JSON.

Two things might help here:

  1. Fix our serde setup so that we send RLP-encoded tx data as (eg.) a string in JSON and continue to use JSON
  2. Abolish JSON/utterly and use CBOR for gossip'd transaction messages.
JamesHinshelwood commented 3 months ago

I think both of your suggestions are sensible.

We should fix the Deserialization impl to cope with arrays - https://github.com/Zilliqa/zq2/pull/1077

And we should remove JSON anyway. It makes no sense to use different formats for broadcasts and p2p messages - https://github.com/Zilliqa/zq2/pull/1078