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

add handshake to connection establishment #330

Merged
merged 1 commit into from
Sep 23, 2015

Conversation

vinipsmaker
Copy link
Contributor

Solves https://maidsafe.atlassian.net/browse/MAID-1353

Currently an "empty" message is sent as the handshake, but the plan is
to change the message to exchange public endpoints which will help with
hole punching.

Review on Reviewable

@maidsafe-highfive
Copy link

r? @inetic

(maidsafe_highfive has picked a reviewer for you, use r? to override)

@vinipsmaker
Copy link
Contributor Author

  • service::test::bootstrap test is failing because Event::BootstrapFinished is being emitted before Event::OnConnect and Event::OnAccept. Needs to delay new node confirmation until the handshake is finished.
  • I'm still investigating why service::test::connection_manager_start is hanging. Hopefully is the same issue as above, then I can tackle the two problems with a single patch.

@vinipsmaker
Copy link
Contributor Author

service::test::connection_manager_start was hanging because a race in the test code itself. I'll fix the bootstrap and see if all tests which depend upon bootstrap are also fixed.

@vinipsmaker vinipsmaker force-pushed the MAID-1353 branch 10 times, most recently from 6d721a6 to a27964c Compare September 23, 2015 03:06
@vinipsmaker vinipsmaker changed the title **WIP DO NOT MERGE IT**: add handshake to connection establishment add handshake to connection establishment Sep 23, 2015
@vinipsmaker
Copy link
Contributor Author

hey @inetic, PR is ready for review (just waiting for CI tests).

@vinipsmaker
Copy link
Contributor Author

All tests are passing and I'm seeing a green light from the CI machines.

///
/// Only after the handshake is exchanged, crust should generate new
/// connection events.
Handshake,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think Handshake should not be part of the Message enum as we'll never send UserBlob nor Contacts while handshaking and we'll never send Handshake while receiving UserBlob or Contacts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solved. Just waiting for the CI tests.

Solves https://maidsafe.atlassian.net/browse/MAID-1353

Currently an "empty" message is sent as the handshake, but the plan is
to change the message to exchange public endpoints which will help with
hole punching.
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling d80c8a6 on vinipsmaker:MAID-1353 into ** on maidsafe:master**.

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

4 participants