Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Initial Whisper implementation #6009

Merged
merged 39 commits into from
Jul 14, 2017
Merged

Initial Whisper implementation #6009

merged 39 commits into from
Jul 14, 2017

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Jul 6, 2017

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.
  • We respect other peers' topic bloom filters, but do not issue them ourselves (TODO)
  • The RPC layer expects a common payload format, as well as three kinds of encryption:
    1. Asymmetric: ECIES
    2. Symmetric AES-GCM with multi-use key, random nonce appended to ciphertext
    3. Broadcast: Symmetric AES-GCM with single use key and global nonce, with key XOR'd into topic hashes. "salted topics" prepended to ciphertext.
  • The payload format consists of the message body, arbitrary padding, and an optional signature.
  • There are RPCs for creating ephemeral identities, either asymmetric (Secp256k1) or symmetric (AES-256-GCM).
  • Enable Whisper with the --whisper flag, "shh" RPC API, and choose target memory (MB) with --whisper-pool-size. This could be made dynamic in the future.
  • Subscribe or poll filters for messages with certain topics, signatures, and encryption/broadcast mode

@rphmeier rphmeier added the M4-core ⛓ Core client code / Rust. label Jul 6, 2017
@rphmeier
Copy link
Contributor Author

rphmeier commented Jul 6, 2017

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 1 / N of them xor'd together, with a few random bits set. The hard part is making sure we always have coverage of all the subscriptions we care about even in the face of a changing or unreliable peer set.

An optional darkness parameter when creating a subscription which controls how much information we leak about that subscription would be a further goal.

@rphmeier rphmeier mentioned this pull request Jul 6, 2017
@rphmeier rphmeier added A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 6, 2017
@rphmeier
Copy link
Contributor Author

rphmeier commented Jul 6, 2017

(I'll ice this to post 1.7.0)

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. and removed A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. labels Jul 13, 2017

fn generate(self) -> Result<KeyPair, Self::Error> {
let (sec, publ) = SECP256K1.generate_keypair(self)
.expect("context always created with full capabilities; qed");
Copy link
Collaborator

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?

Copy link
Contributor Author

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]);
Copy link
Collaborator

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.

Copy link
Contributor Author

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();
Copy link
Collaborator

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?

Copy link
Contributor Author

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

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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this underflow?

Copy link
Contributor Author

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.


// whether a message is known or within the bounds of PoW.
fn may_accept(&self, message: &Message) -> bool {
!self.known.contains(message.hash()) && {
Copy link
Collaborator

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

Copy link
Contributor Author

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>,
Copy link
Collaborator

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?

Copy link
Contributor Author

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);
Copy link
Collaborator

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?

Copy link
Contributor Author

@rphmeier rphmeier Jul 14, 2017

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

@arkpar arkpar added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 14, 2017
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jul 14, 2017
@gavofyork gavofyork merged commit 99075ad into master Jul 14, 2017
@gavofyork gavofyork deleted the whisper branch July 14, 2017 18:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants