This is mainly to formalise situations where we can and cannot use unwraps (or things that cause panic) in production (along with alternatives if any).

The cases where we tend to have unwraps

1. locking a mutex
2. spawning a thread
3. joining a thread
4. A logic like this -> You put something in a Some() right at construction and later do take().unwrap() because we know it will be a logical fallacy to have ​_None_​ there. A use case for this is: there are situations where we don't want to clone (or cannot clone/copy) an object and want to hand it to other. So you wrap it in an Option::Some and do a take() to move it out. Search for option dance idiom in rust.

The test is currently here https://github.com/inetic/crust/tree/utp-network , I'll soon create a PR to add it into master (with #[ignored] for now), because CIs currently seem to fail for other reasons.

The test creates N nodes and exchanges K messages between them, if TCP sockets are used (test_network_tcp) then everything is OK, but when uTP is used, some connections are created and some messages exchanged, but then the test seems to freeze with CPU being used on 100%.

My theory is that there is a bug in the congestion control algorithm in rust-utp, so I'll now be working on creating a similar test against rust-utp itself to prove or disprove this theory.

as crust introduced a proper error-type I will now expose that in RoutingError::Crust(::crust::error::Error) however ::crust::error::Error does not implement core::fmt::Display

Compiling routing v0.3.8 (file:///home/bbollen/SAFE/rust/routing)
src/error.rs:270:45: 270:62 error: the trait `core::fmt::Display` is not implemented for the type `crust::error::Error` [E0277]
src/error.rs:270             RoutingError::Crust(ref err) => fmt::Display::fmt(err, f),
                                                             ^~~~~~~~~~~~~~~~~
src/error.rs:270:45: 270:62 help: run `rustc --explain E0277` to see a detailed explanation
src/error.rs:270:45: 270:62 note: `crust::error::Error` cannot be formatted with the default formatter; try using `:?` instead if you are using a format string
src/error.rs:270             RoutingError::Crust(ref err) => fmt::Display::fmt(err, f),
                                                             ^~~~~~~~~~~~~~~~~
src/error.rs:270:45: 270:62 note: required by `core::fmt::Display::fmt`
src/error.rs:270             RoutingError::Crust(ref err) => fmt::Display::fmt(err, f),

Would crust be okay with implementing that for its error type? Routing needs it when Crust would pass an error up on to routing; eg soon on ::crust::ConnectionManager::new()

I'll need to keep adding debug printing until I figure out what's going wrong here. So far it looks like somewhere I'm failing to set a port number, and therefore bootstrapping is done to the wrong location.

The test does not pass as the number of messages received is half the one expected. I would be more than happy to investigate it. also the number of NewConnection events are half the number it should be.

Fedora release 20 (Heisenbug) / rustc 1.4.0-nightly (7e13faee1 2015-08-19) 2 cores and 2 gb memory

Tests hanging with error

test beacon::test::test_avoid_beacon ... thread 'test_avoid_beacon seek_peers 2' panicked at 'index out of bounds: the len is 0 but the index is 0', ../src/libcollections/vec.rs:1047

Whoever picks this one up I have created a Fedora CI to replicate and test this on - access on request.

Compilation error on ARMv7 - rustc 1.6.0-dev (8ed8679b2 2015-11-18) - Ubuntu 14.04

Running `rustc src/lib.rs --crate-name crust --crate-type lib -g --out-dir /home/odroid/rust/crust/target/debug --emit=dep-info,link -L dependency=/home/odroid/rust/crust/target/debug -L dependency=/home/odroid/rust/crust/target/debug/deps --extern libc=/home/odroid/rust/crust/target/debug/deps/liblibc-6bc3b8388045ae15.rlib --extern term=/home/odroid/rust/crust/target/debug/deps/libterm-b378416c15baaed0.rlib --extern rand=/home/odroid/rust/crust/target/debug/deps/librand-c45b9b7482b3db07.rlib --extern net2=/home/odroid/rust/crust/target/debug/deps/libnet2-b20381e786fc70c7.rlib --extern itertools=/home/odroid/rust/crust/target/debug/deps/libitertools-db2bb4e433a35a08.rlib --extern utp=/home/odroid/rust/crust/target/debug/deps/libutp-c27fe215c6387c0b.rlib --extern crossbeam=/home/odroid/rust/crust/target/debug/deps/libcrossbeam-3ec287e8a309877a.rlib --extern log=/home/odroid/rust/crust/target/debug/deps/liblog-22662799efb6c4d2.rlib --extern cbor=/home/odroid/rust/crust/target/debug/deps/libcbor-48752123a6e4a0aa.rlib --extern time=/home/odroid/rust/crust/target/debug/deps/libtime-05012e9817ae890c.rlib --extern igd=/home/odroid/rust/crust/target/debug/deps/libigd-5db43e84dc4a2df2.rlib --extern rustc_serialize=/home/odroid/rust/crust/target/debug/deps/librustc_serialize-e13845f83f5be3dc.rlib --extern docopt=/home/odroid/rust/crust/target/debug/deps/libdocopt-a3f0762386caca4d.rlib`
src/getifaddrs.rs:124:48: 124:63 error: mismatched types:
 expected `*const u8`,
    found `*mut i8`
(expected u8,
    found i8) [E0308]
src/getifaddrs.rs:124             let name = unsafe { CStr::from_ptr(ifaddr.ifa_name) }.to_bytes();
                                                                     ^~~~~~~~~~~~~~~
src/getifaddrs.rs:124:48: 124:63 help: run `rustc --explain E0308` to see a detailed explanation
error: aborting due to previous error
Could not compile `crust`.

Caused by:
  Process didn't exit successfully: `rustc src/lib.rs --crate-name crust --crate-type lib -g --out-dir /home/odroid/rust/crust/target/debug --emit=dep-info,link -L dependency=/home/odroid/rust/crust/target/debug -L dependency=/home/odroid/rust/crust/target/debug/deps --extern libc=/home/odroid/rust/crust/target/debug/deps/liblibc-6bc3b8388045ae15.rlib --extern term=/home/odroid/rust/crust/target/debug/deps/libterm-b378416c15baaed0.rlib --extern rand=/home/odroid/rust/crust/target/debug/deps/librand-c45b9b7482b3db07.rlib --extern net2=/home/odroid/rust/crust/target/debug/deps/libnet2-b20381e786fc70c7.rlib --extern itertools=/home/odroid/rust/crust/target/debug/deps/libitertools-db2bb4e433a35a08.rlib --extern utp=/home/odroid/rust/crust/target/debug/deps/libutp-c27fe215c6387c0b.rlib --extern crossbeam=/home/odroid/rust/crust/target/debug/deps/libcrossbeam-3ec287e8a309877a.rlib --extern log=/home/odroid/rust/crust/target/debug/deps/liblog-22662799efb6c4d2.rlib --extern cbor=/home/odroid/rust/crust/target/debug/deps/libcbor-48752123a6e4a0aa.rlib --extern time=/home/odroid/rust/crust/target/debug/deps/libtime-05012e9817ae890c.rlib --extern igd=/home/odroid/rust/crust/target/debug/deps/libigd-5db43e84dc4a2df2.rlib --extern rustc_serialize=/home/odroid/rust/crust/target/debug/deps/librustc_serialize-e13845f83f5be3dc.rlib --extern docopt=/home/odroid/rust/crust/target/debug/deps/libdocopt-a3f0762386caca4d.rlib` (exit code: 101)
odroid@CI-ODROID-4:~/rust/crust$ rustc --version
rustc 1.6.0-dev (8ed8679b2 2015-11-18)

An example where it fails: https://travis-ci.org/maidsafe/crust/jobs/81865528#L217

It succeed in the same environment/CI-machine in other situations. So it's probably a race.

@inetic's big picture explanation of the test:

It starts a number of crust::Services, starts listening on each one, then they interconnect such that each one is connected to the other once. Once they are connected, they exchange few messages. The Stats object counts how many messages is exchanged. Once the exact number of messages has been sent, threads will exit and stats merged to see if the numbers correspond.

Start to improving the test by introducing timeouts for each task, then when the timeout fires we know which task is failing. Something like this might help. If so, put it in util.rs.

Currently Crust does 2 things we don't like:

1. bootstrap cache has no limit. That can cause the system to behave unpredictibly: consume too much memory in RAM and/or disk, act slow where it's not expected, like loading the cache, etc.
2. Service::bootstrap() will attempt all peers in parallel, even if there are thousands of cached peers. The problem with this is that many systems won't allow you to use more than ~1000 sockets in parallel.

So

1. introduce configurable bootstrap cache limit with the reasonable defaults (say 10000 which roughly results in 40 kB cache). When bootstrap cache exceeds the limit, remove the contacts with smallest trust level - see bellow for description.
2. Introduce a counter for bootstrap cache entries that logs how many times a peer was used to successfully bootstrap off. Then this counter will be used as a trust metric - the more we've bootstrap from the peer the more we trust it's availability.
3. Limit parallel connections during bootstrap to prevent from exhausting system sockets.

A certain behavior was observed on osx:

When the wifi is turned off, you dont get a disconnect (LostPeer) raised on the osx machine nor the peer it’s connecting to even if thats a different platform.

For communication purposes (within this thread only), let client be the peer who turns off the wifi. server will be the other end.

TCP is reliable and connection-oriented and could be able to recover these connections if client re-enable its wifi soon enough. For server, the only option to get the LostPeer event will be to wait until timeout is reached. For client, the network interfaces could be watched and notification could be made immediately. In crust, we want client to be notified of connection loss immediately (this means watching network interfaces and ungracefully closing the TCP connection to avoid connection being restored by the OS).

Related issue: https://github.com/maidsafe/get_if_addrs/issues/17

I ran a Client example with a bootstrap file containing the following at the start:

{
  "preferred_port": {
    "variant": "Tcp",
    "fields": [
      5483
    ]
  },
  "hard_coded_contacts": [
    {
      "endpoint": "45.55.207.180:5483"
    },
    {
      "endpoint": "178.62.7.96:5483"
    },
    {
      "endpoint": "128.199.199.210:5483"
    },
    {
      "endpoint": "37.59.98.1:5483"
    },
    {
      "endpoint": "45.79.93.11:5483"
    },
    {
      "endpoint": "45.79.2.52:5483"
    },
  ],
  "contacts": [
    {
      "endpoint": "45.55.207.180:5483"
    },
    {
      "endpoint": "178.62.7.96:5483"
    },
    {
      "endpoint": "128.199.199.210:5483"
    },
    {
      "endpoint": "37.59.98.1:5483"
    },
    {
      "endpoint": "45.79.93.11:5483"
    },
    {
      "endpoint": "45.79.2.52:5483"
    },
  ]
}

After the example ran, it contained:

{
  "preferred_port": {
    "variant": "Tcp",
    "fields": [
      0
    ]
  },
  "hard_coded_contacts": [],
  "contacts": [
    {
      "endpoint": "127.0.0.1:48660"
    },
    {
      "endpoint": "192.168.1.65:48660"
    },
    {
      "endpoint": "[::1]:48660"
    }
  ]
}

I note that crust depends on mio directly and doesn't use futures, which is a solid choice for a project that started many years ago (when futures in its present form didn't exist, and there were multiple competing async frameworks that were eventually abandoned).

Now that async/await is hitting stable (in 1.39), I was wondering if there were any plans to use it. Benefits would include better integration with other runtimes, leveraging executors with work-stealing threadpools like tokio, and easier to write state machines with async/await.

What do you think?

On stable 1.37 using a trait object without a dyn is a warning

That along with forbidding warnings in the example causes the example to currently fail.

A quick fix locally is to either remove that line, or run cargo fix --all.

This is an easy fix, run cargo fix

Hello,

it would be very useful to provide some minimal usage example to the users of the crate that does not make use of internal structs like PeerId and at least mark examples that are not examples of the API functionality.

Cheers

The docs currently say: "Crust implements several NAT traversal techniques such as hole punching and use of IGD."

Since understanding the nat traversal techniques used is critical to understanding whether or not this package will work in a particular use case, please document all techniques used, ideally also summarizing the cases that are and aren't covered by those techniques. That kind of documentation is critical for understanding the expected behavior of this code.

pub fn print_connected_nodes(&self, service: &Service) {
    println!("Node count: {}", self.nodes.len());
    for (id, node) in &self.nodes {
        let ip = service.get_peer_ip_addr(node).unwrap();
        let status = if service.is_connected(node) {
            "Connected   "
        } else {
            "Disconnected"
        };

        println!("[{} - {}] {} {:?}", id, ip, status, node);
    }

    println!();
}

On the examples I've added the following line to obtain a peer IP let ip = sesion.get_peer_ip_addr(node).unwrap();

This method gives the IP but I can't find how to obtain the port. Is there any method that returns the peer port?

Thanks.

0. At any given time Crust will try and limit the max active connections/sessions to 100 (peers)
1. It will sever a connection if it needs to make another once there are 100 active ones in the pool.
2. Upper layer is not informed about any of these (details only internal to Crust)
3. Connects will happen either on calling send() API or indirectly by accepting a connection from peer
4. As usual, we should eventually have only one active connection to a peer if we are deciding to maintain/initiate a connection to it.
5. Currently the state of the socket in sock-coll crate has associated data. When a socket is destroyed (because say the pool wanted a new connection established and an existing connection is severed) the associated data would be lost by both ends when they realise the connection is dropped. This logic should change. a. Tx side should keep all the full or partial remaining data around to be delivered later. We will simplify this in (c) below b. Rx should keep all the partial data yet to be reconstructed and given to upper layer around and wait for Tx side to send more stuff in future by connecting to it. It should have a timer to not keep it around for always though ofc. This is a bit complex so we'll try avoiding this via the next point (c) below c. Since session resumption is going to be a bit complex for TCP we will simplify it to sever a connection for the purposes of dynamic connection preserving only full messages which are not yet written to the socket. Note the ones written are not guaranteed rxd by the peer but we are accepting the message loss at this point. Things could be made better by having Crust ACKs and duplicate detection but we are avoiding complications just now. Now the Rx either buffers data until it can reconstruct it at Crust level or discard it if the connection was dropped before it could re-construct a single Crust message unit. d. Again, to keep things simple once we reach the limit for max connection and we want to send to a new peer we will simply displace the least recently used peer. It might happen that this peer was in the process of sending us something in which case it might attempt an immediate reconnection and displace someone else from our active pool. We will let this logic be like this for simplicity. This is going to be a problem where more than the active connections limit number of peers had something to send us and we keep severing as we get newer incoming connection (at the same time maybe wanting ourselves to send something) and the ones disconnected immediately attempt to connect back disconnecting yet someone else who attempts to connect and so on slowing things down greatly. Assuming this won't happen we will let the logic be simply to displace someone from the pool if there's a fresh send or an incoming connection. e. At this point there might be a few displaced connections which had something to send. Each of these will wait for a global tick of 10secs before displacing someone from the active pool. These pending ones should be in a FIFO queue.