Skip to content
This repository has been archived by the owner on Jun 25, 2021. It is now read-only.

fix/send: fix acks, test with prng, rename some events #1044

Merged
merged 6 commits into from Jun 11, 2016

Conversation

afck
Copy link
Contributor

@afck afck commented Jun 11, 2016

This change is Reviewable

Viv Rajkumar and others added 4 commits June 11, 2016 07:56
If we are the src and the dest handle the message that we would have sent to other peers instead of the original message.
…k-crust tests

This changes the use of the CSRNG in mock-crust tests to a seeded PRNG.  Failing tests cause the
seed to be printed as part of the error message.

Conflicts:
	src/mock_crust/support.rs
This records the cumulative size of all messages sent via Crust.
@maidsafe-highfive
Copy link

@afck: no appropriate reviewer found, use r? to override

@afck
Copy link
Contributor Author

afck commented Jun 11, 2016

r? @Viv-Rajkumar

@Viv-Rajkumar
Copy link
Contributor

Reviewed 1 of 6 files at r3, 2 of 2 files at r4.
Review status: 3 of 13 files reviewed at latest revision, 1 unresolved discussion.


src/core.rs, line 2359 [r4] (raw file):

                           self,
                           GROUP_SIZE - 1);
                    let _ = self.event_sender.send(Event::RestartRequired);

maybe worth checking here if we are the first node and instead raise a Terminate event so the user running the vault can take corresponding action and not run their vault with the -f flag next time. Just restarting is gonna get vault(with routing) to start a new network.


Comments from Reviewable

@Viv-Rajkumar
Copy link
Contributor

Reviewed 1 of 2 files at r1, 4 of 6 files at r2, 5 of 6 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@afck
Copy link
Contributor Author

afck commented Jun 11, 2016

Review status: 12 of 14 files reviewed at latest revision, 1 unresolved discussion.


src/core.rs, line 2359 [r4] (raw file):

Previously, Viv-Rajkumar (Viv Rajkumar) wrote…

maybe worth checking here if we are the first node and instead raise a Terminate event so the user running the vault can take corresponding action and not run their vault with the -f flag next time. Just restarting is gonna get vault(with routing) to start a new network.

Done.

Comments from Reviewable

@Viv-Rajkumar
Copy link
Contributor

Reviewed 2 of 2 files at r5.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


src/core.rs, line 2374 [r5] (raw file):

    }

    fn rebootstrap(&mut self) {

we still don't call crust_service.disconnect() to the currently connected peer(one we remove from peer_mgr) ?


Comments from Reviewable

@Viv-Rajkumar
Copy link
Contributor

Reviewed 1 of 1 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

Add the proxy to the blacklist on connect, so that the `SocketAddr`
doesn't need to be saved in the `State`, and remove the proxy from the
`PeerManager` in `rebootstrap`.
@Viv-Rajkumar
Copy link
Contributor

Reviewed 1 of 1 files at r7, 1 of 1 files at r8.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@Viv-Rajkumar Viv-Rajkumar merged commit 8316a59 into maidsafe:async Jun 11, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants