-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Initial Whisper implementation #6009
Conversation
I'm OK with adding a scheme for giving out topic bloom filters to peers, but I'd like the "darkness" to be configurable. The rough idea is to take the list of each bloom relevant to a local subscription and give each of N peers An optional darkness parameter when creating a subscription which controls how much information we leak about that subscription would be a further goal. |
(I'll ice this to post 1.7.0) |
|
||
fn generate(self) -> Result<KeyPair, Self::Error> { | ||
let (sec, publ) = SECP256K1.generate_keypair(self) | ||
.expect("context always created with full capabilities; qed"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is invalid capabilities the only possible source of failure here? Why not just pass the error as it was before the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's the only source of failure, yeah. @debris and I discussed making this function infallible before, and I figured this was a good time for it.
|
||
/// A topic of a message. | ||
#[derive(Debug, Default, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)] | ||
pub struct Topic(pub [u8; 4]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use H32
? It supports RLP serialization and bloom filters already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bloom algorithm is slightly different, I think. It's not too much overhead to have a separate type, and it conveys the usage better IMO
use rand::{Rng, SeedableRng, XorShiftRng}; | ||
|
||
let mut rng = { | ||
let mut thread_rng = ::rand::thread_rng(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it not require a crypto-secure rng?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's only used to find a nonce which brings the PoW below the threshold. Speed here would be the critical factor. From an outside observer's perspective it doesn't matter whether the method used to choose the nonce is high or low entropy. It could even be consecutive
whisper/src/message.rs
Outdated
if envelope.expiry <= envelope.ttl { return Err(Error::LivesTooLong) } | ||
if envelope.ttl == 0 { return Err(Error::ZeroTTL) } | ||
|
||
let issue_time_adjusted = Duration::from_secs(envelope.expiry - envelope.ttl - LEEWAY_SECONDS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this underflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least the subtraction of leeway can. I'll make that saturating.
whisper/src/net.rs
Outdated
|
||
// whether a message is known or within the bounds of PoW. | ||
fn may_accept(&self, message: &Message) -> bool { | ||
!self.known.contains(message.hash()) && { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment above says that a known message should return true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, should be is not known (I had it as is_known
before but forgot to update when I generalized it)
// when full, will accept messages above the minimum stored. | ||
struct Messages { | ||
slab: ::slab::Slab<Message>, | ||
sorted: Vec<SortedEntry>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps std::collections::BinaryHeap
would fit better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a binary heap will be optimized for the use case when we tend to pop off the first element. we tend to remove elements by two metrics: when they expire (~random access) and also by pushing out low PoW elements (sorted access). My intuition was that vector pops (removing low work entries) are very cheap O(n)
worst case, and a vector retain
periodically to prune expired messages is also relatively cheap O(n)
.
Doing an arbitrary number of pops from a binary heap to remove low work entries is O(nlogn)
, and the retainment logic for live messages would have to reconstruct the heap each time from the iterator, again O(nlogn)
let mut peer = peer.lock(); | ||
|
||
if !peer.can_send_messages() { | ||
return Err(Error::UnexpectedMessage); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can connection recover after this? Maybe it would be better to disconnect on any such error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the error will lead to a punishment in the caller of this function
Closes #4685
Summary of changes:
whisper::net
module contains the network handler, which maintains a pool of messages up to a given size. Messages with a higher PoW will push out those with lower.--whisper
flag, "shh" RPC API, and choose target memory (MB) with--whisper-pool-size
. This could be made dynamic in the future.