Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Framing support for realtime network messages #1972

Merged
merged 3 commits into from May 14, 2019

Conversation

@SergiySW
Copy link
Collaborator

commented May 10, 2019

  • Sizes addes for each message types & subtypes
  • New count field added to message header extensions (4 bit, max size 15) for confirm_req/confirm_ack by hash
  • Node ID handshake header flags functions moved to nano::message_header instead from nano::node_id_handshake
  • Tests expanded to check count extensions field
Framing support for realtime network messages
- Sizes addes for each message types & subtypes
- New count field added to message header extensions (4 bit, max size 15) for confirm_req/confirm_ack by hash
- Node ID handshake header flags functions moved to nano::message_header instead from nano::node_id_handshake
- Tests expanded to check count extensions field

@SergiySW SergiySW added this to the V19.0 milestone May 10, 2019

@SergiySW SergiySW requested review from clemahieu and cryptocode May 10, 2019

@SergiySW SergiySW self-assigned this May 10, 2019

@SergiySW

This comment has been minimized.

Copy link
Collaborator Author

commented May 10, 2019

Breaking up #1962

@SergiySW SergiySW added this to During RC in V19 May 10, 2019

@SergiySW SergiySW added the networking label May 10, 2019

@cryptocode

This comment has been minimized.

Copy link
Collaborator

commented May 10, 2019

If we don't want to expend extension bits, we could maybe retrofit max version to be a count field, since it's not used and doesn't seem useful in the future either. We could then define a count of 0xff to mean "the rest of the count bits follows the header" if we need larger payloads in the future.

A new message type to mean "tcp envelope" was also discussed at some point (the payload would be the old header+message), but that probably means too many changes.

@clemahieu
Copy link
Collaborator

left a comment

Looks good after some small modifications.

Show resolved Hide resolved nano/node/bootstrap.cpp Outdated
Show resolved Hide resolved nano/node/bootstrap.cpp Outdated

SergiySW added some commits May 10, 2019

@SergiySW SergiySW merged commit 2728912 into nanocurrency:master May 14, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@zhyatt zhyatt moved this from During RC to RC 3 (TBD) in V19 May 20, 2019

argakiig added a commit to argakiig/raiblocks that referenced this pull request May 22, 2019

Framing support for realtime network messages (nanocurrency#1972)
- Sizes addes for each message types & subtypes
- New count field added to message header extensions (4 bit, max size 15) for confirm_req/confirm_ack by hash
- Node ID handshake header flags functions moved to nano::message_header instead from nano::node_id_handshake
- Tests expanded to check count extensions field
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.