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

Length prefixed protobuf messages #54

Merged
merged 5 commits into from
Nov 2, 2022
Merged

Conversation

ckousik
Copy link
Collaborator

@ckousik ckousik commented Oct 31, 2022

Add length prefix to protobuf messages.

src/stream.ts Outdated Show resolved Hide resolved
src/stream.ts Outdated Show resolved Hide resolved
src/stream.ts Outdated Show resolved Hide resolved
src/stream.ts Outdated Show resolved Hide resolved
src/stream.ts Show resolved Hide resolved
const msgbuf = pb.Message.toBinary({message: buf.subarray()});
const sendbuf = lp.encode.single(msgbuf)
log.trace(`[stream:${this.id}][${this.stat.direction}] sending message: length: ${res.length} ${res}, encoded through pb as ${msgbuf}`);
this.channel.send(sendbuf.subarray())
Copy link

Choose a reason for hiding this comment

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

More of a question, but why do you have to call subarray all the time? Is it to switch type somehow?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Uint8ArrayList can have a single message spread across multiple different Uint8Array instances. Calling subarray makes them readable as a single Uint8Array which is much easier to work with.

src/stream.ts Show resolved Hide resolved
src/stream.ts Outdated Show resolved Hide resolved
src/stream.ts Show resolved Hide resolved
src/stream.ts Outdated Show resolved Hide resolved
src/stream.ts Show resolved Hide resolved
src/transport.ts Show resolved Hide resolved
src/stream.ts Outdated Show resolved Hide resolved
@mxinden
Copy link
Member

mxinden commented Nov 1, 2022

@ckousik does this resolve the compatibility issues you were seeing with libp2p/rust-libp2p#2622?

@ckousik
Copy link
Collaborator Author

ckousik commented Nov 2, 2022

@ckousik does this resolve the compatibility issues you were seeing with libp2p/rust-libp2p#2622?

yes

@ckousik ckousik merged commit 2b7b2c1 into develop Nov 2, 2022
@github-actions
Copy link

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants