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

Upgrade node-datachannel to 0.8.0 #4941

Merged
merged 4 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ironfish/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"leveldown": "6.1.1",
"levelup": "4.4.0",
"lodash": "4.17.21",
"node-datachannel": "0.5.1",
"node-datachannel": "0.8.0",
"node-forge": "1.3.1",
"parse-json": "5.2.0",
"sqlite": "4.0.23",
Expand Down
6 changes: 0 additions & 6 deletions ironfish/src/network/peerNetwork.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,10 @@ describe('PeerNetwork', () => {

describe('when peers connect', () => {
it('changes isReady', async () => {
const dc = await import('node-datachannel')

const peerNetwork = new PeerNetwork({
identity: mockPrivateIdentity('local'),
agent: 'sdk/1/cli',
webSocket: ws,
nodeDataChannel: dc,
node: mockNode(),
chain: mockChain(),
minPeers: 1,
Expand Down Expand Up @@ -138,13 +135,10 @@ describe('PeerNetwork', () => {
describe('when at max peers', () => {
it('rejects websocket connections', async () => {
const wsActual = jest.requireActual<typeof WSWebSocket>('ws')
const dc = await import('node-datachannel')

const peerNetwork = new PeerNetwork({
identity: mockPrivateIdentity('local'),
agent: 'sdk/1/cli',
webSocket: wsActual,
nodeDataChannel: dc,
node: mockNode(),
chain: mockChain(),
listen: true,
Expand Down
4 changes: 1 addition & 3 deletions ironfish/src/network/peerNetwork.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ import { BAN_SCORE, KnownBlockHashesValue, Peer } from './peers/peer'
import { PeerConnectionManager } from './peers/peerConnectionManager'
import { PeerManager } from './peers/peerManager'
import { TransactionFetcher } from './transactionFetcher'
import { IsomorphicWebSocketConstructor, NodeDataChannelType } from './types'
import { IsomorphicWebSocketConstructor } from './types'
import { getBlockSize } from './utils/serializers'
import { parseUrl, WebSocketAddress } from './utils/url'
import {
Expand Down Expand Up @@ -159,7 +159,6 @@ export class PeerNetwork {
identity: PrivateIdentity
agent?: string
webSocket: IsomorphicWebSocketConstructor
nodeDataChannel: NodeDataChannelType
listen?: boolean
port?: number
bootstrapNodes?: string[]
Expand Down Expand Up @@ -195,7 +194,6 @@ export class PeerNetwork {
VERSION_PROTOCOL,
options.chain,
options.webSocket,
options.nodeDataChannel,
options.networkId,
this.enableSyncing,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ import { WebRtcConnection } from './webRtcConnection'
describe('WebRtcConnection', () => {
describe('send', () => {
describe('with no datachannel', () => {
it('returns false', async () => {
const nodeDataChannel = await import('node-datachannel')

const connection = new WebRtcConnection(nodeDataChannel, false, createRootLogger())
it('returns false', () => {
const connection = new WebRtcConnection(false, createRootLogger())
const message = new IdentifyMessage({
agent: '',
head: Buffer.alloc(32, 0),
Expand All @@ -31,10 +29,8 @@ describe('WebRtcConnection', () => {
})

describe('with a valid message', () => {
it('serializes and sends the message on the datachannel', async () => {
const nodeDataChannel = await import('node-datachannel')

const connection = new WebRtcConnection(nodeDataChannel, true, createRootLogger())
it('serializes and sends the message on the datachannel', () => {
const connection = new WebRtcConnection(true, createRootLogger())
const sendSpy = jest.spyOn(connection, '_send')

const message = new IdentifyMessage({
Expand Down
9 changes: 3 additions & 6 deletions ironfish/src/network/peers/connections/webRtcConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */

import type { Logger } from '../../../logger'
// @ts-expect-error Allow type-only import https://github.com/microsoft/TypeScript/issues/49721
import type { DataChannel, DescriptionType, PeerConnection } from 'node-datachannel'
import colors from 'colors/safe'
import { DataChannel, DescriptionType, PeerConnection } from 'node-datachannel'
import { Assert } from '../../../assert'
import { Event } from '../../../event'
import { MetricsMonitor } from '../../../metrics'
import { ErrorUtils } from '../../../utils'
import { parseNetworkMessage } from '../../messageRegistry'
import { NodeDataChannelType } from '../../types'
import { MAX_MESSAGE_SIZE } from '../../version'
import { Connection, ConnectionDirection, ConnectionType } from './connection'
import { NetworkError } from './errors'
Expand Down Expand Up @@ -57,7 +55,6 @@ export class WebRtcConnection extends Connection {
onSignal = new Event<[SignalData]>()

constructor(
nodeDataChannel: NodeDataChannelType,
initiator: boolean,
logger: Logger,
metrics?: MetricsMonitor,
Expand All @@ -70,7 +67,7 @@ export class WebRtcConnection extends Connection {
metrics,
)

this.peer = new nodeDataChannel.PeerConnection('peer', {
this.peer = new PeerConnection('peer', {
iceServers: options.stunServers ?? [],
maxMessageSize: MAX_MESSAGE_SIZE,
})
Expand Down Expand Up @@ -222,7 +219,7 @@ export class WebRtcConnection extends Connection {
this.datachannel?.close()

try {
this.peer.destroy()
this.peer.close()
} catch (e) {
// peer.destroy() may throw "It seems peer-connection is closed" if the
// peer connection has been disposed already
Expand Down
6 changes: 1 addition & 5 deletions ironfish/src/network/peers/localPeer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Assert } from '../../assert'
import { Blockchain } from '../../blockchain'
import { Identity, PrivateIdentity, privateIdentityToIdentity } from '../identity'
import { IdentifyMessage } from '../messages/identify'
import { IsomorphicWebSocketConstructor, NodeDataChannelType } from '../types'
import { IsomorphicWebSocketConstructor } from '../types'

/**
* Wraps configuration needed for establishing connections with other peers
Expand All @@ -24,8 +24,6 @@ export class LocalPeer {
readonly version: number
// constructor for either a Node WebSocket or a browser WebSocket
readonly webSocket: IsomorphicWebSocketConstructor
// asynchronously imported WebRTC datachannel library
readonly nodeDataChannel: NodeDataChannelType
// the unique ID number of the network
readonly networkId: number
// true if the peer supports syncing and gossip messages
Expand All @@ -42,7 +40,6 @@ export class LocalPeer {
version: number,
chain: Blockchain,
webSocket: IsomorphicWebSocketConstructor,
nodeDataChannel: NodeDataChannelType,
networkId: number,
enableSyncing: boolean,
) {
Expand All @@ -55,7 +52,6 @@ export class LocalPeer {
this.enableSyncing = enableSyncing

this.webSocket = webSocket
this.nodeDataChannel = nodeDataChannel
this.port = null
this.name = null
}
Expand Down
42 changes: 14 additions & 28 deletions ironfish/src/network/peers/peer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,10 @@ describe('setWebSocketConnection', () => {
})

describe('setWebRtcConnection', () => {
it('Changes to CONNECTING when in DISCONNECTED', async () => {
const nodeDataChannel = await import('node-datachannel')

it('Changes to CONNECTING when in DISCONNECTED', () => {
const identity = mockIdentity('peer')
const peer = new Peer(identity)
const connection = new WebRtcConnection(nodeDataChannel, false, createRootLogger())
const connection = new WebRtcConnection(false, createRootLogger())

peer.setWebRtcConnection(connection)
expect(peer.state).toEqual({
Expand All @@ -67,10 +65,8 @@ describe('setWebRtcConnection', () => {
})
})

it('Times out WebRTC handshake', async () => {
const nodeDataChannel = await import('node-datachannel')

const connection = new WebRtcConnection(nodeDataChannel, false, createRootLogger())
it('Times out WebRTC handshake', () => {
const connection = new WebRtcConnection(false, createRootLogger())
expect(connection.state.type).toEqual('CONNECTING')

const peer = new Peer(null)
Expand Down Expand Up @@ -114,10 +110,8 @@ it('Times out WebRTC handshake', async () => {
})

describe('Handles message send failure', () => {
it('Disconnects peer on error in _send', async () => {
const nodeDataChannel = await import('node-datachannel')

const connection = new WebRtcConnection(nodeDataChannel, true, createRootLogger())
it('Disconnects peer on error in _send', () => {
const connection = new WebRtcConnection(true, createRootLogger())
expect(connection.state.type).toEqual('CONNECTING')

const peer = new Peer(null)
Expand Down Expand Up @@ -155,10 +149,8 @@ describe('Handles message send failure', () => {
expect(peer.state.type).toEqual('CONNECTED')
})

it('Falls back to WebSockets if available and WebRTC send fails', async () => {
const nodeDataChannel = await import('node-datachannel')

const wrtcConnection = new WebRtcConnection(nodeDataChannel, true, createRootLogger())
it('Falls back to WebSockets if available and WebRTC send fails', () => {
const wrtcConnection = new WebRtcConnection(true, createRootLogger())
const wsConnection = new WebSocketConnection(
new ws(''),
ConnectionDirection.Outbound,
Expand Down Expand Up @@ -255,9 +247,7 @@ it('Transitions to CONNECTED when adding a connection with state CONNECTED', ()
})
})

it('Stays in CONNECTED when adding an additional connection', async () => {
const nodeDataChannel = await import('node-datachannel')

it('Stays in CONNECTED when adding an additional connection', () => {
const identity = mockIdentity('peer')
const peer = new Peer(null)
const connection = new WebSocketConnection(
Expand All @@ -275,7 +265,7 @@ it('Stays in CONNECTED when adding an additional connection', async () => {
connection.setState({ type: 'CONNECTED', identity })

// Add in an additional connection
const wrtcConnection = new WebRtcConnection(nodeDataChannel, true, createRootLogger())
const wrtcConnection = new WebRtcConnection(true, createRootLogger())
peer.setWebRtcConnection(wrtcConnection)
expect(wrtcConnection.state.type).not.toBe('CONNECTED')

Expand All @@ -287,9 +277,7 @@ it('Stays in CONNECTED when adding an additional connection', async () => {
})

describe('Stays in CONNECTED when one connection disconnects', () => {
it('WebSocket disconnects', async () => {
const nodeDataChannel = await import('node-datachannel')

it('WebSocket disconnects', () => {
const identity = mockIdentity('peer')
const peer = new Peer(null)

Expand All @@ -303,7 +291,7 @@ describe('Stays in CONNECTED when one connection disconnects', () => {
connection.setState({ type: 'CONNECTED', identity })

// Add a CONNECTED WebRTC connection
const wrtcConnection = new WebRtcConnection(nodeDataChannel, true, createRootLogger())
const wrtcConnection = new WebRtcConnection(true, createRootLogger())
peer.setWebRtcConnection(wrtcConnection)
wrtcConnection.setState({ type: 'CONNECTED', identity })

Expand All @@ -318,9 +306,7 @@ describe('Stays in CONNECTED when one connection disconnects', () => {
})
})

it('WebRTC disconnects', async () => {
const nodeDataChannel = await import('node-datachannel')

it('WebRTC disconnects', () => {
const identity = mockIdentity('peer')
const peer = new Peer(null)

Expand All @@ -334,7 +320,7 @@ describe('Stays in CONNECTED when one connection disconnects', () => {
connection.setState({ type: 'CONNECTED', identity })

// Add a CONNECTED WebRTC connection
const wrtcConnection = new WebRtcConnection(nodeDataChannel, true, createRootLogger())
const wrtcConnection = new WebRtcConnection(true, createRootLogger())
peer.setWebRtcConnection(wrtcConnection)
wrtcConnection.setState({ type: 'CONNECTED', identity })

Expand Down
2 changes: 1 addition & 1 deletion ironfish/src/network/peers/peerManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1159,7 +1159,7 @@ describe('PeerManager', () => {

peer.onMessage.emit(message, connection)
expect(initWebRtcConnectionMock).toHaveBeenCalledTimes(1)
expect(initWebRtcConnectionMock).toHaveBeenCalledWith(peer, expect.anything(), true)
expect(initWebRtcConnectionMock).toHaveBeenCalledWith(peer, true)
expect(pm['getBrokeringPeers'](peer)[0]).toEqual(peer)
})

Expand Down
32 changes: 9 additions & 23 deletions ironfish/src/network/peers/peerManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { PeerListMessage } from '../messages/peerList'
import { PeerListRequestMessage } from '../messages/peerListRequest'
import { SignalMessage } from '../messages/signal'
import { SignalRequestMessage } from '../messages/signalRequest'
import { IsomorphicWebSocket, NodeDataChannelType } from '../types'
import { IsomorphicWebSocket } from '../types'
import { formatWebSocketAddress, WebSocketAddress } from '../utils'
import { VERSION_PROTOCOL_MIN } from '../version'
import { ConnectionRetry } from './connectionRetry'
Expand Down Expand Up @@ -275,7 +275,7 @@ export class PeerManager {
}

if (canInitiateWebRTC(this.localPeer.publicIdentity, peer.state.identity)) {
this.initWebRtcConnection(peer, this.localPeer.nodeDataChannel, true)
this.initWebRtcConnection(peer, true)
return true
}

Expand All @@ -284,7 +284,7 @@ export class PeerManager {
destinationIdentity: peer.state.identity,
})

const connection = this.initWebRtcConnection(peer, this.localPeer.nodeDataChannel, false)
const connection = this.initWebRtcConnection(peer, false)
connection.setState({ type: 'REQUEST_SIGNALING' })

const brokeringPeers = this.getBrokeringPeers(peer)
Expand Down Expand Up @@ -338,20 +338,10 @@ export class PeerManager {
* @param peer The peer to establish a connection with
* @param initiator Set to true if we are initiating a connection with `peer`
*/
private initWebRtcConnection(
peer: Peer,
nodeDataChannel: NodeDataChannelType,
initiator: boolean,
): WebRtcConnection {
const connection = new WebRtcConnection(
nodeDataChannel,
initiator,
this.logger,
this.metrics,
{
stunServers: this.stunServers,
},
)
private initWebRtcConnection(peer: Peer, initiator: boolean): WebRtcConnection {
const connection = new WebRtcConnection(initiator, this.logger, this.metrics, {
stunServers: this.stunServers,
})

connection.onSignal.on((data) => {
let errorMessage
Expand Down Expand Up @@ -1271,7 +1261,7 @@ export class PeerManager {
return
}

this.initWebRtcConnection(targetPeer, this.localPeer.nodeDataChannel, true)
this.initWebRtcConnection(targetPeer, true)
}

/**
Expand Down Expand Up @@ -1376,11 +1366,7 @@ export class PeerManager {
return
}

signalingConnection = this.initWebRtcConnection(
signalingPeer,
this.localPeer.nodeDataChannel,
false,
)
signalingConnection = this.initWebRtcConnection(signalingPeer, false)
} else {
signalingConnection = signalingPeer.state.connections.webRtc
}
Expand Down
Loading
Loading