Skip to content

Framing support for realtime network messages#1972

Merged
SergiySW merged 3 commits intonanocurrency:masterfrom
SergiySW:tcp/live_messages_framing
May 14, 2019
Merged

Framing support for realtime network messages#1972
SergiySW merged 3 commits intonanocurrency:masterfrom
SergiySW:tcp/live_messages_framing

Conversation

@SergiySW
Copy link
Copy Markdown
Contributor

  • 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

- 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 00:25
@SergiySW SergiySW self-assigned this May 10, 2019
@SergiySW
Copy link
Copy Markdown
Contributor Author

Breaking up #1962

@cryptocode
Copy link
Copy Markdown
Contributor

cryptocode 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.

Copy link
Copy Markdown
Contributor

@clemahieu clemahieu left a comment

Choose a reason for hiding this comment

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

Looks good after some small modifications.

@SergiySW SergiySW merged commit 2728912 into nanocurrency:master May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants