Skip to content

Conversation

@LucienHH
Copy link
Contributor

No description provided.

@LucienHH
Copy link
Contributor Author

Tests are running ok locally, will investigate later why not ok in the actions env.

@LucienHH
Copy link
Contributor Author

LucienHH commented Aug 7, 2025

@extremeheat Ready for review

@extremeheat
Copy link
Member

@LucienHH what do you think about porting the serializer / framing code to protodef? It'd align with rest of projects. We don't have to do it in this PR (can be done in another), but curious if there's any blocker there like a missing primitive

@LucienHH
Copy link
Contributor Author

@extremeheat Done. Let me know what you think of the implementation. NetherNet does have versioning, currently v4 but not sure if it's worth/possible maintaining all versions?

@LucienHH LucienHH requested a review from extremeheat August 25, 2025 10:50
/**
* Encapsulated data with length prefix
*/
Read.encapsulated = ['parametrizable', (compiler, { lengthType, type }) => {
Copy link
Member

Choose a reason for hiding this comment

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

This should be defined inside of protodef, but we dont have to block on it

const { SignalType, SignalStructure } = require('./signalling')

const { getRandomUint64, createPacketData, prepareSecurePacket, processSecurePacket } = require('./util')
const { PeerConnection } = require('node-datachannel')
Copy link
Member

Choose a reason for hiding this comment

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

Need to figure out how to not requre native deps at some point. It'd be great if we could get something working with WASM if needed

@extremeheat
Copy link
Member

currently v4 but not sure if it's worth/possible maintaining all versions?

Yes, you can simply create a new protocol.json for version changes. Speaking of which, if it's part of minecraft bedrock protocol any reason to not put this into minecraft-data as a bedrock/1.21.100/nethernetProtocol.json? Assuming each minecraft version will be hard coded to use one version of the transport protocol this should be possible. The specific protocol version can be put as a header field inside the protocol.json.

@extremeheat extremeheat requested a review from Copilot August 25, 2025 17:43

This comment was marked as outdated.

@LucienHH
Copy link
Contributor Author

@extremeheat OK, agree on moving this to minecraft-data, assume we can integrate that in a separate PR? Went ahead and made the other recommended amendments

Co-authored-by: Copilot <[email protected]>
@extremeheat extremeheat requested a review from Copilot August 25, 2025 23:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Initial implementation of the NetherNet protocol for Node.js, adding client/server functionality with WebRTC-based connections, cryptographic packet processing, and UDP discovery mechanism.

  • Complete client/server implementation with RTC-based connections and UDP discovery
  • Cryptographic packet encryption/decryption and protocol serialization system
  • Test suite covering ping, connection, and kick scenarios

Reviewed Changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/basic.test.js Comprehensive test suite with ping, connection, and kick tests
src/util.js Utility functions for packet creation and secure packet processing
src/signalling.js WebRTC signalling structure and message types
src/server.js Server implementation with UDP discovery and RTC connection handling
src/serializer.js Protocol serialization/deserialization with packet type definitions
src/protocol.json JSON protocol definition for discovery packets
src/crypto.js AES encryption/decryption and HMAC checksum functions
src/connection.js Connection management with message fragmentation
src/compilerTypes.js Custom protocol compiler types for encapsulated data
src/client.js Client implementation with discovery and RTC connection
package.json Updated dependencies and package name
index.js Main module exports
example.js Basic usage example
README.md Updated documentation with usage examples

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@socket-security
Copy link

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addednode-datachannel@​0.26.0921001009170
Addedprotodef@​1.19.0971008682100

View full report

@extremeheat extremeheat merged commit 622ad0e into PrismarineJS:master Aug 29, 2025
3 checks passed
@extremeheat
Copy link
Member

Let's merge, I created #4 #5 #6 to track some of comments

Will have claude take a look at them sometime next week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants