From c979c6ca5c4b4a2aa6e6390b956d5bb8953b15bb Mon Sep 17 00:00:00 2001 From: Ocnrb Date: Wed, 30 Sep 2026 08:19:03 +0100 Subject: [PATCH 1/2] Leave a moderator's removal to the owner's rotation, with no debt on their device - Only the owner announces epochs, so only the owner takes on a rotation debt - A debt this account cannot pay is dropped on send and on connect, freeing devices already stuck - The moderator is told the key rotates when the owner next opens the channel Co-authored-by: Claude Opus 5.5 --- src/js/channels.js | 4 +- src/js/channels/RotationRetry.js | 23 +++++--- src/js/ui/ChannelSettingsUI.js | 4 +- .../ChannelSettingsUI.removeMember.test.js | 58 +++++++++++++++++++ tests/unit/channels.extended.test.js | 19 +++++- tests/unit/rotationRetry.test.js | 42 +++++++++++++- 6 files changed, 136 insertions(+), 14 deletions(-) create mode 100644 tests/unit/ChannelSettingsUI.removeMember.test.js diff --git a/src/js/channels.js b/src/js/channels.js index 22dc1b1..0edb585 100644 --- a/src/js/channels.js +++ b/src/js/channels.js @@ -1411,10 +1411,10 @@ class ChannelManager { banMemberLevels(messageStreamId, address, levels) { return this.membership.banMemberLevels(messageStreamId, address, levels); } /** A ban or removal whose key rotation has not gone out yet. */ isRotationOwed(messageStreamId) { return this.rotationRetry.isOwed(messageStreamId); } - /** Owed rotations of the gated channels this account owns, taken up once the client connects. */ + /** Owed rotations of the gated channels, taken up once the client connects; the ones this account cannot pay are dropped. */ resumeOwedRotations() { this.rotationRetry.resume([...this.channels.values()] - .filter(ch => ch.gate?.address && epochKeyManager.isOwnAdmin(ch)) + .filter(ch => ch.gate?.address) .map(ch => ch.messageStreamId)); } unbanMemberLevels(messageStreamId, address) { return this.membership.unbanMemberLevels(messageStreamId, address); } diff --git a/src/js/channels/RotationRetry.js b/src/js/channels/RotationRetry.js index f4bfbbd..940f7b5 100644 --- a/src/js/channels/RotationRetry.js +++ b/src/js/channels/RotationRetry.js @@ -65,20 +65,23 @@ export class RotationRetry { } /** - * Rotate for the addresses now; on failure keep them owed and retry. + * Rotate for the addresses now; on failure keep them owed and retry. Only + * the owner announces epochs: anyone else's cut is left to the owner's + * next open, and a debt they took on would hold back their own sends. * @returns {Promise} true when the rotation went out now */ async rotateFor(messageStreamId, addresses) { + if (!this.host.stillOwned(messageStreamId)) return false; this._update(messageStreamId, (owed) => [...owed, ...addresses.map(a => a.toLowerCase())]); if (await this._attempt(messageStreamId)) return true; this._ensureLoop(messageStreamId); return false; } - /** Take up what an earlier session left owed on these channels. */ + /** Take up what an earlier session left owed on these channels, and drop what this account cannot pay. */ resume(messageStreamIds) { for (const messageStreamId of messageStreamIds) { - if (!this.isOwed(messageStreamId)) continue; + if (!this.isOwed(messageStreamId) || this._dropUnpayable(messageStreamId)) continue; this._attempt(messageStreamId).then((done) => { if (!done) this._ensureLoop(messageStreamId); }); @@ -87,10 +90,17 @@ export class RotationRetry { /** Before the admin publishes: an owed rotation goes first, or the publish does not go. */ async settle(messageStreamId) { - if (!this.isOwed(messageStreamId)) return; + if (!this.isOwed(messageStreamId) || this._dropUnpayable(messageStreamId)) return; if (!await this._attempt(messageStreamId)) throw new Error(OWED_ROTATION_MESSAGE); } + /** A debt on a channel this account does not own can never be paid. */ + _dropUnpayable(messageStreamId) { + if (this.host.stillOwned(messageStreamId)) return false; + this._update(messageStreamId, () => []); + return true; + } + _attempt(messageStreamId) { const previous = this.attempts.get(messageStreamId) || Promise.resolve(); const next = previous.catch(() => {}).then(() => this._rotateOnce(messageStreamId)); @@ -127,10 +137,7 @@ export class RotationRetry { try { for (let round = 0; this.isOwed(messageStreamId); round++) { await this.sleep(this.delaysMs[Math.min(round, this.delaysMs.length - 1)]); - if (!this.host.stillOwned(messageStreamId)) { - this._update(messageStreamId, () => []); - return; - } + if (this._dropUnpayable(messageStreamId)) return; await this._attempt(messageStreamId); } } finally { diff --git a/src/js/ui/ChannelSettingsUI.js b/src/js/ui/ChannelSettingsUI.js index b0b621a..d9a80c2 100644 --- a/src/js/ui/ChannelSettingsUI.js +++ b/src/js/ui/ChannelSettingsUI.js @@ -2001,7 +2001,9 @@ class ChannelSettingsUI { try { showLoading('Removing member (on-chain transaction)...'); await channelManager.removeMember(currentChannel.streamId, address); - if (channelManager.isRotationOwed(currentChannel.streamId)) { + if (!channelManager.isChannelOwner(currentChannel.streamId)) { + showNotification('Member removed. The key rotates when the owner next opens the channel.', 'info', 5000); + } else if (channelManager.isRotationOwed(currentChannel.streamId)) { showNotification('Member removed. The channel key rotates the next time the app connects.', 'warning', 5000); } else { showNotification('Member removed successfully!', 'success'); diff --git a/tests/unit/ChannelSettingsUI.removeMember.test.js b/tests/unit/ChannelSettingsUI.removeMember.test.js new file mode 100644 index 0000000..7f1ac75 --- /dev/null +++ b/tests/unit/ChannelSettingsUI.removeMember.test.js @@ -0,0 +1,58 @@ +/** + * What removing a member says about the channel key. Only the owner rotates + * it: their own device does it now or owes it, and a moderator's removal is + * left to the owner's next open. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +vi.mock('../../src/js/logger.js', () => ({ + Logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() } +})); +vi.mock('../../src/js/ui/ModalManager.js', () => ({ modalManager: { show: vi.fn(), hide: vi.fn() } })); +vi.mock('../../src/js/relayManager.js', () => ({ relayManager: {} })); +vi.mock('../../src/js/graph.js', () => ({ graphAPI: {} })); +vi.mock('../../src/js/identity.js', () => ({ identityManager: { getCachedENS: vi.fn(() => null) } })); +vi.mock('../../src/js/media.js', () => ({ mediaController: {} })); +vi.mock('../../src/js/channelImageManager.js', () => ({ channelImageManager: {} })); + +const { channelSettingsUI } = await import('../../src/js/ui/ChannelSettingsUI.js'); + +const CHANNEL = { streamId: '0xowner/room-1' }; +const MEMBER = '0x03e2b466754f187f571ab48c69e3ab592e76d819'; + +describe('removing a member', () => { + let channelManager; + let showNotification; + + beforeEach(() => { + channelManager = { + getCurrentChannel: () => CHANNEL, + removeMember: vi.fn().mockResolvedValue(true), + isChannelOwner: vi.fn().mockReturnValue(true), + isRotationOwed: vi.fn().mockReturnValue(false) + }; + showNotification = vi.fn(); + channelSettingsUI.setDependencies({ channelManager, showLoading: vi.fn(), hideLoading: vi.fn(), showNotification }); + vi.spyOn(channelSettingsUI, 'loadMembers').mockResolvedValue(undefined); + }); + + it('tells the owner it is done when the key rotated', async () => { + await channelSettingsUI.executeRemoveMember(MEMBER); + expect(showNotification).toHaveBeenCalledWith('Member removed successfully!', 'success'); + }); + + it('tells the owner the rotation is still owed', async () => { + channelManager.isRotationOwed.mockReturnValue(true); + await channelSettingsUI.executeRemoveMember(MEMBER); + expect(showNotification).toHaveBeenCalledWith( + 'Member removed. The channel key rotates the next time the app connects.', 'warning', 5000); + }); + + it('tells a moderator the owner rotates the key', async () => { + channelManager.isChannelOwner.mockReturnValue(false); + await channelSettingsUI.executeRemoveMember(MEMBER); + expect(showNotification).toHaveBeenCalledWith( + 'Member removed. The key rotates when the owner next opens the channel.', 'info', 5000); + }); +}); diff --git a/tests/unit/channels.extended.test.js b/tests/unit/channels.extended.test.js index 112d251..75bf19b 100644 --- a/tests/unit/channels.extended.test.js +++ b/tests/unit/channels.extended.test.js @@ -190,7 +190,8 @@ vi.mock('../../src/js/epochKeyManager.js', () => ({ handleKeysMessage: vi.fn(), forgetChannel: vi.fn().mockResolvedValue(undefined), ensureChannelKeys: vi.fn().mockResolvedValue(undefined), - getWaitingInfo: vi.fn().mockReturnValue({ waiting: false }) + getWaitingInfo: vi.fn().mockReturnValue({ waiting: false }), + isOwnAdmin: vi.fn().mockReturnValue(true) } })); @@ -330,6 +331,22 @@ describe('ChannelManager Extended', () => { expect(epochKeyManager.rotateEpoch).toHaveBeenCalled(); }); + it("leaves a moderator's removal to the owner, with no rotation owed on this device", async () => { + const { gateManager } = await import('../../src/js/gate.js'); + const { epochKeyManager } = await import('../../src/js/epochKeyManager.js'); + epochKeyManager.rotateEpoch.mockClear(); + epochKeyManager.isOwnAdmin.mockReturnValue(false); + try { + await channelManager.removeMember(streamId, '0xmember1'); + + expect(gateManager.revokeAllow).toHaveBeenCalledWith('0xgate', '0xmember1'); + expect(epochKeyManager.rotateEpoch).not.toHaveBeenCalled(); + expect(channelManager.isRotationOwed(streamId)).toBe(false); + } finally { + epochKeyManager.isOwnAdmin.mockReturnValue(true); + } + }); + it('removes member from local list', async () => { await channelManager.removeMember(streamId, '0xmember1'); diff --git a/tests/unit/rotationRetry.test.js b/tests/unit/rotationRetry.test.js index 600356c..a792537 100644 --- a/tests/unit/rotationRetry.test.js +++ b/tests/unit/rotationRetry.test.js @@ -99,9 +99,11 @@ describe('RotationRetry', () => { it('a channel this account no longer owns stops the retry', async () => { failures = Infinity; - owned = false; - + clock = parkedClock(); expect(await retry.rotateFor(CHANNEL, ['0xabc'])).toBe(false); + + owned = false; + clock.resolve(); await settleDown(); expect(waits).toEqual([5000]); @@ -109,6 +111,42 @@ describe('RotationRetry', () => { expect(covered).toEqual([]); }); + it("a moderator's cut owes nothing: the owner rotates on their next open", async () => { + owned = false; + + expect(await retry.rotateFor(CHANNEL, ['0xabc'])).toBe(false); + await settleDown(); + + expect(retry.isOwed(CHANNEL)).toBe(false); + expect(rotations).toBe(0); + expect(waits).toEqual([]); + }); + + it('a debt this account cannot pay is dropped, not held against its sends', async () => { + failures = Infinity; + clock = parkedClock(); + await retry.rotateFor(CHANNEL, ['0xabc']); + owned = false; + + await retry.settle(CHANNEL); + + expect(retry.isOwed(CHANNEL)).toBe(false); + }); + + it('a debt left on a channel this account does not own is dropped when the next session connects', async () => { + failures = Infinity; + clock = parkedClock(); + await retry.rotateFor(CHANNEL, ['0xabc']); + const next = newRetry(); + owned = false; + + next.resume([CHANNEL]); + await settleDown(); + + expect(next.isOwed(CHANNEL)).toBe(false); + await expect(next.settle(CHANNEL)).resolves.toBeUndefined(); + }); + it('one rotation covers every cut owed on the channel', async () => { failures = Infinity; clock = parkedClock(); From e6ab1310b894e60c347d1150a5cc17ab1091bf1f Mon Sep 17 00:00:00 2001 From: Ocnrb Date: Wed, 30 Sep 2026 08:19:03 +0100 Subject: [PATCH 2/2] Offer the gate's ban to the owner only, and let a moderator hide from the members list too Co-authored-by: Claude Opus 5.5 --- src/js/ui/ChannelModalsUI.js | 40 +++++++++++++++++--- src/js/ui/MessageContextMenuUI.js | 16 -------- tests/unit/ChannelModalsUI.ban.test.js | 51 ++++++++++++++++++++++++-- 3 files changed, 82 insertions(+), 25 deletions(-) diff --git a/src/js/ui/ChannelModalsUI.js b/src/js/ui/ChannelModalsUI.js index 9ce2357..60384ff 100644 --- a/src/js/ui/ChannelModalsUI.js +++ b/src/js/ui/ChannelModalsUI.js @@ -5,6 +5,7 @@ import { GasEstimator } from './GasEstimator.js'; import { authManager } from '../auth.js'; +import { confirmDialog } from './ConfirmDialogUI.js'; import { streamrController } from '../streamr.js'; import { CONFIG } from '../config.js'; import { snapRetentionDays, retentionLabel } from '../utils/retention.js'; @@ -750,9 +751,15 @@ class ChannelModalsUI { * @param {() => void} [options.onBanned] - runs once the ban went through */ showBanMemberModal(address, channel, { onBanned } = {}) { + const owner = !!channel && this.channelManager.isChannelOwner(channel.streamId); + if (!owner && this.channelManager.isCachedModerator?.(channel?.streamId)) { + this._hideAsModerator(address, channel, { onBanned }); + return; + } const gated = !!channel?.gate?.address; - const me = authManager.getAddress()?.toLowerCase(); - const canClientBan = !!me && me === channel?.createdBy?.toLowerCase(); + const canClientBan = owner; + // The gate's ban() is onlyOwner: offered to anyone else, it reverts. + const canProtocolBan = gated && owner; const label = document.getElementById('ban-member-label'); if (label) label.textContent = `${address.slice(0, 6)}…${address.slice(-4)}`; @@ -790,8 +797,8 @@ class ChannelModalsUI { } document.getElementById('ban-level-purge-row')?.classList.toggle('opacity-40', !canPurge); if (protocol) { - protocol.checked = gated; - protocol.disabled = !gated; + protocol.checked = canProtocolBan; + protocol.disabled = !canProtocolBan; } if (clientDetail && !canClientBan) { clientDetail.textContent = 'Only the channel creator can publish this.'; @@ -800,18 +807,20 @@ class ChannelModalsUI { } if (protocolDetail && !gated) { protocolDetail.textContent = 'Only gated channels have a gate to ban on.'; + } else if (protocolDetail && !owner) { + protocolDetail.textContent = 'Only the channel creator can cut access.'; } else if (protocolDetail) { protocolDetail.textContent = 'Cuts their access on the gate and rotates the channel key. One transaction.'; } document.getElementById('ban-level-client-row')?.classList.toggle('opacity-40', !canClientBan); - document.getElementById('ban-level-protocol-row')?.classList.toggle('opacity-40', !gated); + document.getElementById('ban-level-protocol-row')?.classList.toggle('opacity-40', !canProtocolBan); const confirmBtn = document.getElementById('confirm-ban-member-btn'); if (confirmBtn) { confirmBtn.onclick = async () => { const levels = { client: !!client?.checked && canClientBan, - protocol: !!protocol?.checked && gated + protocol: !!protocol?.checked && canProtocolBan }; const erase = !!purge?.checked && canPurge && levels.client; if (!levels.client && !levels.protocol) return; @@ -854,6 +863,25 @@ class ChannelModalsUI { this.deps.modalManager?.show('ban-member-modal'); } + /** A moderator's ban: a delta every client composes over the owner's state. It hides, with no on-chain half. */ + async _hideAsModerator(address, channel, { onBanned } = {}) { + if (!await confirmDialog({ + title: 'Hide their messages', + message: `Every message from ${address.slice(0, 10)}… is hidden from now on. Only the channel creator can cut their access.`, + confirmLabel: 'Hide' + })) return; + try { + const { epochKeyManager } = await import('../epochKeyManager.js'); + await this.channelManager.publishModAction( + channel.streamId, 'ban', address, epochKeyManager.currentEpoch(channel.streamId)); + this.showNotification('Member banned', 'success'); + } catch (error) { + this.showNotification(error?.message || 'Failed to ban member', 'error'); + return; + } + onBanned?.(); + } + /** * Entry screen for a gated channel the user cannot enter yet: * reads the gate mode on-chain and shows the requirement, the user's diff --git a/src/js/ui/MessageContextMenuUI.js b/src/js/ui/MessageContextMenuUI.js index 17f9e75..2238b18 100644 --- a/src/js/ui/MessageContextMenuUI.js +++ b/src/js/ui/MessageContextMenuUI.js @@ -537,22 +537,6 @@ class MessageContextMenuUI { case 'ban-user': { const ch = channelManager?.getCurrentChannel?.(); if (!ch) break; - // A moderator's ban is a delta and has no on-chain half, so - // the two-level modal (which spends gas) is the owner's. - if (channelManager.isCachedModerator?.(ch.streamId) - && !channelManager.getCachedDeletePermission?.(ch.streamId)?.canDelete) { - if (!await confirmDialog({ title: 'Hide their messages', message: `Every message from ${address.slice(0, 10)}… is hidden from now on.`, confirmLabel: 'Hide' })) break; - try { - const { epochKeyManager } = await import('../epochKeyManager.js'); - await channelManager.publishModAction( - ch.streamId, 'ban', address, - epochKeyManager.currentEpoch(ch.streamId)); - showNotification('Member banned', 'success'); - } catch (err) { - showNotification(err?.message || 'Failed to ban member', 'error'); - } - break; - } const { channelModalsUI } = await import('./ChannelModalsUI.js'); channelModalsUI.showBanMemberModal(address, ch); break; diff --git a/tests/unit/ChannelModalsUI.ban.test.js b/tests/unit/ChannelModalsUI.ban.test.js index bd96e54..feb996e 100644 --- a/tests/unit/ChannelModalsUI.ban.test.js +++ b/tests/unit/ChannelModalsUI.ban.test.js @@ -1,6 +1,7 @@ /** * The ban modal reports back once the ban went through, so the panel that - * opened it can show the member gone from the members list. + * opened it can show the member gone from the members list. The gate's ban is + * the owner's alone; a moderator hides by delta and never reaches the gate. */ import { describe, it, expect, beforeEach, vi } from 'vitest'; @@ -10,6 +11,8 @@ vi.mock('../../src/js/logger.js', () => ({ })); vi.mock('../../src/js/auth.js', () => ({ authManager: { getAddress: () => '0xowner' } })); vi.mock('../../src/js/streamr.js', () => ({ streamrController: {} })); +vi.mock('../../src/js/ui/ConfirmDialogUI.js', () => ({ confirmDialog: vi.fn(async () => true) })); +vi.mock('../../src/js/epochKeyManager.js', () => ({ epochKeyManager: { currentEpoch: () => 3 } })); const { ChannelModalsUI } = await import('../../src/js/ui/ChannelModalsUI.js'); @@ -19,28 +22,70 @@ const MEMBER = '0x03e2b466754f187f571ab48c69e3ab592e76d819'; describe('ban modal', () => { let ui; let channelManager; + let modalManager; beforeEach(() => { document.body.innerHTML = ` + `; channelManager = { banMemberLevels: vi.fn().mockResolvedValue(true), - isRotationOwed: vi.fn().mockReturnValue(false) + isRotationOwed: vi.fn().mockReturnValue(false), + isChannelOwner: vi.fn().mockReturnValue(true), + isCachedModerator: vi.fn().mockReturnValue(false), + publishModAction: vi.fn().mockResolvedValue(undefined) }; + modalManager = { show: vi.fn(), hide: vi.fn() }; ui = new ChannelModalsUI(); ui.setDependencies({ channelManager, - modalManager: { show: vi.fn(), hide: vi.fn() }, + modalManager, notificationUI: { showLoadingToast: vi.fn(), hideLoadingToast: vi.fn() }, showNotification: vi.fn() }); }); + it('offers the gate to the owner', () => { + ui.showBanMemberModal(MEMBER, CHANNEL); + + const protocol = document.getElementById('ban-level-protocol'); + expect(protocol.disabled).toBe(false); + expect(protocol.checked).toBe(true); + }); + + it('never offers the gate to anyone else, and says why', async () => { + channelManager.isChannelOwner.mockReturnValue(false); + ui.showBanMemberModal(MEMBER, CHANNEL); + + const protocol = document.getElementById('ban-level-protocol'); + expect(protocol.disabled).toBe(true); + expect(protocol.checked).toBe(false); + expect(document.getElementById('ban-level-protocol-detail').textContent) + .toBe('Only the channel creator can cut access.'); + + protocol.checked = true; + await document.getElementById('confirm-ban-member-btn').onclick(); + expect(channelManager.banMemberLevels).not.toHaveBeenCalled(); + }); + + it('lets a moderator hide by delta, with no transaction', async () => { + channelManager.isChannelOwner.mockReturnValue(false); + channelManager.isCachedModerator.mockReturnValue(true); + const onBanned = vi.fn(); + + ui.showBanMemberModal(MEMBER, CHANNEL, { onBanned }); + + await vi.waitFor(() => expect(onBanned).toHaveBeenCalledTimes(1)); + expect(channelManager.publishModAction).toHaveBeenCalledWith(CHANNEL.streamId, 'ban', MEMBER, 3); + expect(channelManager.banMemberLevels).not.toHaveBeenCalled(); + expect(modalManager.show).not.toHaveBeenCalled(); + }); + it('tells the opener once the ban went through', async () => { const onBanned = vi.fn(); ui.showBanMemberModal(MEMBER, CHANNEL, { onBanned });