Private tx enabled flag added into STATUS packet#9999
Conversation
grbIzl
added
A0-pleasereview 🤓
|
is it compatible with geth nodes who aren't aware of this packet field? |
How should it be communicated? There are only basic fields of status packet described in https://github.com/ethereum/wiki/wiki/Ethereum-Wire-Protocol If we add something to the end of the packet (like we did for warp), what is the safe strategy for handling it? |
| packet.append(&manifest_hash); | ||
| packet.append(&block_number); | ||
| } | ||
| packet.append(&self.private_tx_handler.is_some()); |
There was a problem hiding this comment.
This should be a warp_protocol extension
|
All custom fields should only be added in the parity protocol. |
grbIzl
added
A4-gotissues 💥
| let protocol = if warp_protocol { warp_protocol_version } else { ETH_PROTOCOL_VERSION_63.0 }; | ||
| trace!(target: "sync", "Sending status to {}, protocol version {}", peer, protocol); | ||
| let mut packet = RlpStream::new_list(if warp_protocol { 7 } else { 5 }); | ||
| let rlp_len = if warp_protocol { |
There was a problem hiding this comment.
Would be best to just use begin_unbounded_list and get rid of rlp_len
grbIzl
added
A5-grumble 🔥
grbIzl
force-pushed
the
PrivateTXPeers
branch
from
3dcce90 to
1854c8e
Compare
grbIzl
added
A0-pleasereview 🤓
|
could you look at this again @niklasad1 ? |
| if lucky_peers.is_empty() { | ||
| error!(target: "privatetx", "Cannot propagate the packet, no peers with private tx enabled connected"); | ||
| } else { | ||
| trace!(target: "sync", "Sending private transaction packet to {:?}", lucky_peers); |
There was a problem hiding this comment.
I'm a little confused on the log target would be nice to be consistent and use privatetx here? Or am I missing something?
There was a problem hiding this comment.
Yeah, it looks weird indeed, I agree. I'll change it to privatetx for both cases
niklasad1
added
the
A6-mustntgrumble 💦
grbIzl
removed
the
A6-mustntgrumble 💦
|
Thanks. Could you resolve the conflicts? |
niklasad1
added
A7-looksgoodcantmerge 🙄
6 participants
This PR is aimed to fix the following situation:
As a result, on big (public) networks node might not have peers with private tx enabled among connected peers and private tx functionality doesn't work (because packets are not delivered among nodes).
The requirement to have nodes in reserved peers doesn't solve the issue, because we randomly select subset of connected peers for packets propagation and packet can be still lost.
In terms of this PR private tx enabled flag added into STATUS packet and node can explicitly select for propagation only nodes with private tx enabled.