Skip to content

Commit a45e48c

Browse files
committed
test(sync): failing tests for peer 'started' status flipping without the peer's handshake
CoreSyncState marks a peer's per-core status 'started' when core.update({ wait: true }) resolves. That call answers a core-global question - "am I up to date with the swarm?" - not "has this peer completed its length handshake", and the two diverge three ways: 1. update() resolves immediately for writable cores, so peers on our own writer cores are marked 'started' before they have sent anything. 2. All concurrent update() calls share one upgrade request, resolved as soon as any peer advances the core's length - so on an actively transferring core a newly-connected peer is marked 'started' the moment the next block batch lands, from someone else. 3. The upgrade check samples at most MAX_PEERS_UPGRADE (3) peers, so with 4+ connected peers and any ambient traffic the request resolves without ever consulting a new peer's handshake. A prematurely-'started' peer reads as "has nothing, wants everything"; where we hold no blocks that is indistinguishable from "nothing left to sync", so isSynced()/waitForSync()/autostop can act on completion before hearing from the peer. Field deployments routinely run 4+ concurrent peers with transfers in flight, so modes 2 and 3 are everyday occurrences - and none of them were covered by tests before. The new tests hold a peer in the "channel open, Synchronize not yet processed" state via a holdSynchronize() helper and assert the peer must still be 'starting'; one test per mode. All three fail on the current implementation. (src/types.ts additions are type-only, needed for the tests to typecheck.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CzqT9t19AV5voYFM2vYV8h
1 parent 7a361b5 commit a45e48c

2 files changed

Lines changed: 276 additions & 0 deletions

File tree

src/types.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,25 @@ export type HypercorePeer = {
166166
remotePublicKey: Buffer
167167
remoteBitfield: HypercoreRemoteBitfield
168168
remoteContiguousLength: number
169+
/**
170+
* Set (synchronously) when the peer's first Synchronize wire message is
171+
* processed, i.e. once we know the peer's length and fork.
172+
*/
173+
remoteSynced: boolean
174+
/** Set when the peer's channel has closed and it was removed from the replicator. */
175+
removed: boolean
169176
onbitfield: (options: { start: number; bitfield: Buffer }) => void
170177
onrange: (options: { drop: boolean; start: number; length: number }) => void
178+
onsync: (options: {
179+
fork: number
180+
length: number
181+
remoteLength: number
182+
canUpgrade: boolean
183+
uploading: boolean
184+
downloading: boolean
185+
hasManifest: boolean
186+
allowPush: boolean
187+
}) => Promise<void>
171188
}
172189

173190
type ProtocolStream = Omit<NoiseStream, 'userData'> & {

test/sync/core-sync-state.js

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,265 @@ test('CoreReplicationState', async (t) => {
463463
}
464464
})
465465

466+
test('peer status stays "starting" until that peer\'s first Synchronize (writable core)', async (t) => {
467+
// `core.update({ wait: true })` resolves immediately for writable cores
468+
// (hypercore short-circuits: a writer is always "up to date"), so it can
469+
// never be a signal that a *peer* has completed its length handshake.
470+
const localCore = await createCore(t)
471+
await localCore.append(['a', 'b', 'c'])
472+
473+
const emitter = new EventEmitter()
474+
const crs = new CoreSyncState({
475+
onUpdate: () => emitter.emit('update'),
476+
peerSyncControllers: new Map(),
477+
namespace: 'auth',
478+
deviceId: '',
479+
hasDownloadFilter: () => false,
480+
})
481+
crs.attachCore(localCore)
482+
483+
const hold = holdSynchronize(localCore)
484+
const remoteCore = await createCore(t, localCore.key)
485+
const kp2 = NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(1))
486+
const peerId = kp2.publicKey.toString('hex')
487+
const destroy = replicate(localCore, remoteCore, { kp2 })
488+
t.after(destroy)
489+
490+
await once(localCore, 'peer-add')
491+
// Allow any (incorrect) async continuations to settle
492+
await new Promise((res) => setTimeout(res, 200))
493+
494+
assert.equal(
495+
localCore.peers[0].remoteSynced,
496+
false,
497+
'sanity: the peer has not sent its first Synchronize yet'
498+
)
499+
assert.equal(
500+
crs.getState().remoteStates[peerId].status,
501+
'starting',
502+
'peer is still "starting" before its first Synchronize'
503+
)
504+
505+
hold.release()
506+
await waitForStatus(crs, emitter, peerId, 'started')
507+
assert.equal(
508+
localCore.peers[0].remoteSynced,
509+
true,
510+
'sanity: Synchronize has now been processed'
511+
)
512+
})
513+
514+
test('peer status stays "starting" while another peer supplies an upgrade', async (t) => {
515+
// `core.update({ wait: true })` resolves for *all* waiters as soon as any
516+
// peer advances the core's length, so on an actively-transferring core a
517+
// newly-connected peer would be marked "started" before it has said
518+
// anything at all.
519+
const writerCore = await createCore(t)
520+
await writerCore.append(['a', 'b', 'c'])
521+
const localCore = await createCore(t, writerCore.key)
522+
523+
const emitter = new EventEmitter()
524+
const crs = new CoreSyncState({
525+
onUpdate: () => emitter.emit('update'),
526+
peerSyncControllers: new Map(),
527+
namespace: 'auth',
528+
deviceId: '',
529+
hasDownloadFilter: () => false,
530+
})
531+
crs.attachCore(localCore)
532+
533+
// Sync fully with the writer first
534+
const kpWriter = NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(1))
535+
const destroyWriterConn = replicate(localCore, writerCore, {
536+
kp2: kpWriter,
537+
})
538+
t.after(destroyWriterConn)
539+
localCore.download({ start: 0, end: -1 })
540+
await once(localCore, 'peer-add')
541+
await waitFor(() => localCore.contiguousLength === 3)
542+
543+
// Now a new peer connects, but its first Synchronize is withheld
544+
const hold = holdSynchronize(localCore)
545+
const newPeerCore = await createCore(t, writerCore.key)
546+
const kpNew = NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(2))
547+
const newPeerId = kpNew.publicKey.toString('hex')
548+
const destroyNewConn = replicate(localCore, newPeerCore, { kp2: kpNew })
549+
t.after(destroyNewConn)
550+
await once(localCore, 'peer-add')
551+
552+
// The writer appends and we download it: the core's length advances, which
553+
// resolves every pending `core.update()` — but tells us nothing about the
554+
// new peer
555+
await writerCore.append('d')
556+
await waitFor(() => localCore.contiguousLength === 4)
557+
await new Promise((res) => setTimeout(res, 200))
558+
559+
const newPeer = localCore.peers.find((p) =>
560+
p.remotePublicKey.equals(kpNew.publicKey)
561+
)
562+
assert(newPeer, 'sanity: new peer is connected')
563+
assert.equal(
564+
newPeer.remoteSynced,
565+
false,
566+
'sanity: the new peer has not sent its first Synchronize yet'
567+
)
568+
assert.equal(
569+
crs.getState().remoteStates[newPeerId].status,
570+
'starting',
571+
'new peer is still "starting" before its first Synchronize'
572+
)
573+
574+
hold.release()
575+
await waitForStatus(crs, emitter, newPeerId, 'started')
576+
})
577+
578+
test('peer status stays "starting" for a 4th concurrent peer (hypercore upgrade quorum cap)', async (t) => {
579+
// `core.update({ wait: true })` samples at most MAX_PEERS_UPGRADE (3)
580+
// peers, so with 4+ connected peers it can resolve without ever
581+
// consulting the 4th peer's handshake — it must not be treated as a
582+
// per-peer "handshake complete" signal.
583+
const keySource = await createCore(t) // never replicated; provides a key
584+
const localCore = await createCore(t, keySource.key)
585+
586+
const emitter = new EventEmitter()
587+
const crs = new CoreSyncState({
588+
onUpdate: () => emitter.emit('update'),
589+
peerSyncControllers: new Map(),
590+
namespace: 'auth',
591+
deviceId: '',
592+
hasDownloadFilter: () => false,
593+
})
594+
crs.attachCore(localCore)
595+
596+
// Three peers, fully synced
597+
for (let i = 1; i <= 3; i++) {
598+
const remoteCore = await createCore(t, keySource.key)
599+
const kp2 = NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(i))
600+
const destroy = replicate(localCore, remoteCore, { kp2 })
601+
t.after(destroy)
602+
}
603+
await waitFor(
604+
() =>
605+
localCore.peers.length === 3 &&
606+
localCore.peers.every((p) => p.remoteSynced)
607+
)
608+
609+
// A 4th peer connects, but its first Synchronize is withheld
610+
const hold = holdSynchronize(localCore, { maxPeers: 1 })
611+
const fourthCore = await createCore(t, keySource.key)
612+
const kp4 = NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(4))
613+
const fourthPeerId = kp4.publicKey.toString('hex')
614+
const destroy = replicate(localCore, fourthCore, { kp2: kp4 })
615+
t.after(destroy)
616+
await once(localCore, 'peer-add')
617+
618+
// A 5th peer connects and syncs normally: its handshake re-evaluates
619+
// hypercore's shared upgrade request, which samples only the first
620+
// MAX_PEERS_UPGRADE peers — never the still-silent 4th
621+
const fifthCore = await createCore(t, keySource.key)
622+
const kp5 = NoiseSecretStream.keyPair(Buffer.allocUnsafe(32).fill(5))
623+
const destroy5 = replicate(localCore, fifthCore, { kp2: kp5 })
624+
t.after(destroy5)
625+
await waitFor(() =>
626+
localCore.peers.some(
627+
(p) => p.remotePublicKey.equals(kp5.publicKey) && p.remoteSynced
628+
)
629+
)
630+
// Allow any (incorrect) async continuations to settle
631+
await new Promise((res) => setTimeout(res, 300))
632+
633+
assert.equal(
634+
crs.getState().remoteStates[fourthPeerId].status,
635+
'starting',
636+
'4th peer is still "starting" before its first Synchronize'
637+
)
638+
639+
hold.release()
640+
await waitForStatus(crs, emitter, fourthPeerId, 'started')
641+
})
642+
643+
/**
644+
* Intercept and queue the first Synchronize message(s) from peers that are
645+
* added to `core` after this is called, so tests can hold a peer in the
646+
* "channel open, length handshake not yet processed" state. Uses the same
647+
* interception point as production code: hypercore dispatches wire messages
648+
* by property lookup on the peer object, so shadowing `onsync` on the
649+
* instance sees every message.
650+
*
651+
* @param {import('hypercore')} core
652+
* @param {object} [opts]
653+
* @param {number} [opts.maxPeers] only hold the first `maxPeers` peers added
654+
* after this call (later peers' messages flow normally)
655+
*/
656+
function holdSynchronize(core, { maxPeers = Infinity } = {}) {
657+
/** @type {Array<() => unknown>} */
658+
const queued = []
659+
/** @type {Array<() => void>} */
660+
const restores = []
661+
let held = true
662+
let heldPeerCount = 0
663+
/** @param {import('../../src/types.js').HypercorePeer} peer */
664+
const onPeerAdd = (peer) => {
665+
if (heldPeerCount >= maxPeers) return
666+
heldPeerCount++
667+
const originalOnSync = peer.onsync
668+
peer.onsync = (...args) => {
669+
if (!held) return originalOnSync.apply(peer, args)
670+
queued.push(() => originalOnSync.apply(peer, args))
671+
return Promise.resolve()
672+
}
673+
restores.push(() => {
674+
peer.onsync = originalOnSync
675+
})
676+
}
677+
core.on('peer-add', onPeerAdd)
678+
return {
679+
release() {
680+
held = false
681+
core.off('peer-add', onPeerAdd)
682+
for (const apply of queued) apply()
683+
for (const restore of restores) restore()
684+
},
685+
}
686+
}
687+
688+
/**
689+
* @param {() => boolean} condition
690+
* @param {number} [timeoutMs]
691+
*/
692+
async function waitFor(condition, timeoutMs = 1000) {
693+
const start = Date.now()
694+
while (!condition()) {
695+
if (Date.now() - start > timeoutMs) {
696+
throw new Error('Timed out waiting for condition')
697+
}
698+
await new Promise((res) => setTimeout(res, 10))
699+
}
700+
}
701+
702+
/**
703+
* Wait until the peer's status matches, driven by state update events.
704+
*
705+
* @param {CoreSyncState} crs
706+
* @param {EventEmitter} emitter
707+
* @param {string} peerId
708+
* @param {import('../../src/sync/core-sync-state.js').PeerNamespaceState['status']} status
709+
* @param {number} [timeoutMs]
710+
*/
711+
async function waitForStatus(crs, emitter, peerId, status, timeoutMs = 1000) {
712+
await pTimeout(
713+
(async () => {
714+
while (crs.getState().remoteStates[peerId]?.status !== status) {
715+
await once(emitter, 'update')
716+
}
717+
})(),
718+
{
719+
milliseconds: timeoutMs,
720+
message: `Timed out waiting for status ${status}`,
721+
}
722+
)
723+
}
724+
466725
test('bitCount32', () => {
467726
const testCases = new Set([0, 2 ** 32 - 1])
468727
for (let i = 0; i < 32; i++) {

0 commit comments

Comments
 (0)