Announce hosted rooms on the Statement Store - #119
Conversation
The Host signRaw wire format is not pinned by the SDK: HostSignPayloadResponse carries an untagged signature, and a Substrate host may sign a raw payload verbatim or inside the conventional <Bytes> envelope. Verification assumed one shape, so a wrong guess would have failed every Product key request with an error indistinguishable from a wrong signer. Accept a bounded set instead: the canonical message verbatim or <Bytes>-wrapped, and a bare 64-byte or MultiSignature-tagged 65-byte sr25519 signature. Every variant carries the identical domain-bound message, so this adds no replay, cross-app, cross-chain, or cross-track surface; a non-sr25519 tag still fails closed. Route schemas widen to 128 or 130 hex so the tag is checked by the verifier rather than rejected before it. Reject EVM-derived account ids for product-sr25519-v1. A 20-byte H160 padded with 0xee derives back to the H160 it contains, so accepting that shape let a caller name any paying EVM listener as the requester and rested the boundary on the curve check alone. A real Product account is a native AccountId32. A key that parses and derives to the requester but verifies under no variant now returns PRODUCT_SIGNATURE_REJECTED, kept distinct from SIGNATURE_INVALID so an envelope problem is separable from a wrong-account problem in logs. Pin @scure/sr25519 exactly, matching @noble/hashes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Wiring the Product key signatures changed the behaviour but three places still described the old one. The wallet modal told Product-host users that protected playback required an EVM signer, the runbook asked operators to confirm no key is released through the Product identity, and the architecture matrix said the shipped UI used EIP-191 or a session token - contradicted by its own prose two sections later. All three now describe what ships: a connected Product account requests protected keys through product-sr25519-v1, and paid access plus artist publishing remain on the EVM signer. This matters beyond tidiness - the stale runbook step would have had an operator sign off on a denial as correct behaviour, hiding a real signing failure. Record the signing envelope decision and the EVM-derived key rejection, and turn the runbook step into an evidence capture that names which envelope the live host actually produced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Track and split counts come from contract storage and the directory enumerates
runtimes Dotify does not control, so Array.from({ length: Number(count) })
allocated before anything could reject a malformed or hostile value. Both
adapters now validate counts first and throw rather than truncate, since a
silent cap would present a partial catalog as complete. The catalog loader
already isolates per-runtime failures, so one bad runtime degrades to a missing
artist.
Mark the two unverified spots in the CDM adapter that must be settled before it
can be selected: waitForTransaction returns immediately where the viem writer
awaits a receipt, and the payForAccess value-transfer shape is inferred rather
than confirmed against generated contract types.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Dotify has no CDM-registered packages and no `cdm install`, but it does not need them. Its Solidity contracts are deployed through Asset Hub's eth-rpc, which is a compatibility layer over pallet-revive - the same pallet the Product SDK contract helpers target - so the deployed H160 addresses are already reachable without PolkaVM recompilation or a registry entry. CdmJsonContract needs only version, address, and abi for getContract(), and `new ContractManager(...)` is documented as snapshot-only. The generator emits exactly that snapshot from the same Hardhat artifacts the viem bindings come from, so the two adapters cannot disagree about an ABI. Artist runtimes are deliberately absent from the manifest: a diamond is deployed per artist, so its address is known at call time, not build time, and a placeholder would misrepresent the deployment. Their merged facet ABI is emitted separately and bound to an address by createContract. The generator lives in web/ because it needs the SDK's codegen, which is a frontend dependency; adding the Product SDK tree to contracts/evm just to emit types would be a worse trade. Unnamed Solidity getter params are named positionally for codegen only - generateContractTypes interpolates the name into a tuple label and emits `args: [: HexString]`, which does not parse. The manifest ABI stays byte-faithful to the artifact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Implements the contract resolver the CDM adapter was missing. Fixed-address contracts resolve from the generated manifest through ContractManager; per-artist runtime diamonds bind the merged facet ABI to their call-time address through createContract. Two SDK constraints shape this. First, createChainClient routes exclusively through the Product host provider with no direct-WebSocket fallback, so Product mode cannot work in a standalone build - validateProductionEnvironment now rejects product-cdm unless the host mode is enabled. Second, the host decides which chain an environment resolves to, and Dotify's runtimes live on Polkadot Hub TestNet, reached through the `paseo` preset rather than `devnet`. verifyDeployment queries the directory before any catalog read so a wrong-chain connection fails with a named error instead of looking like artists with no releases. Selection is build-time rather than runtime, for two reasons. Switching the authority for access policy is a deployment decision made with evidence, not something a page should flip. And Vite inlines the value, so a viem build tree-shakes the whole Product graph away: 4.4 MB against 10 MB when opted in. The difference is @parity/product-sdk-descriptors, whose shared descriptors module references every chain's metadata - only one chunk is ever fetched, but all are published, and Bulletin storage is a finite quota. Both shipped builds stay at 4.4 MB. Reads only. Writes stay on the viem signer path in every mode, because routing a payment or a publication through a signer with no host transaction evidence is not a reasonable default. A failed Product setup rejects every read rather than falling back to viem: the adapter in use must never be ambiguous. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The blocking unknown for Product contract mode is no longer the manifest or generated types - both now exist and are wired. It is whether the Product host serves a chain that holds Dotify's runtimes, since the host controls that mapping and the contracts are on Polkadot Hub TestNet rather than Product DevNet Asset Hub. Also records the measured build-size trade-off, so an operator weighs it against the Bulletin quota before enabling product-cdm for a .dot deployment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous default was wrong in a way that would have produced an empty catalog. Product DevNet is not a separate network: it is a preset over the Paseo system parachains - Asset Hub (1000), People (1004), Bulletin (1010) - at EVM chain 420420417. That is exactly where Dotify is already deployed. Verified read-only against both endpoints for the ArtistDirectory at 0xcf1534c6e2b0e43b9436c1e86a076466dc0f2108: eth-rpc-testnet.polkadot.io and paseo-assethub-rpc.laissez-faire.trade both report chain id 0x190f1b41, blocks one apart, and byte-identical contract code. They are two providers for one chain, so no contract redeploy is needed to port Dotify to DevNet. The SDK's `paseo` preset is the trap: it targets Paseo Next (Asset Hub Next 1500 / People Next 1502), which the Product docs call a different network where "funds sent there will not appear on this Devnet". Dotify has no deployment there, so ProductChainEnvironment now admits only `devnet` - selecting a chain that cannot hold the catalog is a bug, not a configuration option. Also realigns the environment reference tables, clearing the markdownlint MD060 warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Porting Dotify to Product DevNet turned out to be a configuration question rather than a migration: DevNet is a preset over the Paseo system parachains (Asset Hub 1000, People 1004, Bulletin 1010) at EVM chain 420420417, which is where Dotify's contracts already are. This makes that claim checkable instead of asserted. `npm run smoke:devnet` reads web/.env.product-devnet and deployments.json and verifies, read-only, that the configured Asset Hub reports chain 420420417, is producing blocks past the 2026-07 halt, still serves bytecode for the ArtistDirectory and ArtistRuntimeFactory, and that the Bulletin RPC and IPFS gateway respond. It sends no transaction, reads no secret, and prints no credential. Network-dependent, so it stays out of the unit test path. The build profile needed no endpoint changes - the configured Bulletin and IPFS gateway were already the DevNet ones. What it needed was honest comments: the previous note framed the Asset Hub endpoint as a DevNet-compatible stopgap when it is in fact the DevNet chain. Adds the second DevNet IPFS gateway as a read fallback, and a warning against Asset Hub Next (1500) and People Next (1502), which are a different network holding none of Dotify's contracts. Verified: 6/6 checks pass against the live chain at head 11546553. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The known-limits bullet still said Product contract mode was blocked on CDM-installed packages and generated contract types. Both now exist, so the list overstated what is missing. Only pallet-revive account mapping and host-signed transaction evidence remain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fetchIpfsCid and fetchAssetRef walked 4-6 gateways serially with no per-gateway timeout, so one unresponsive gateway cost the listener the browser's full connection timeout before the next candidate was tried. Covers, manifests, and v1 audio all read through that path. Race the candidates instead, the way audioV2Gateway already does for byte ranges: bound time-to-headers, hedge onto the next gateway after a delay, cap concurrency at 3, and abort the losers once a winner answers. Every URL addresses the same immutable CID, so racing them cannot diverge. The winner's own controller is never aborted, so its body stays readable, and a caller abort stops the queue where it is rather than walking the rest of the list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
min_machines_running = 0 let the API scale to zero, and the first content key request after an idle period paid the cold start. Measured against the running deployment: 8.17s to /health cold (uptime 0), 0.11s warm. That request sits directly in front of first sound, so the saving was being taken out of the listening experience. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Socket.IO reports a CORS rejection, a stopped server, and a wrong URL identically, so every one of them surfaced as "Room service unavailable." That is the vague failure the product invariants rule out, and it hid a real deployment gap: the running signaling server allows only muzinga.netlify.app, so opening a room from the Polkadot Product host origin is refused with a 403 the browser will not explain. On connect_error, read the server's unauthenticated /health, compare the page origin against the allowlist it reports, and upgrade the message in place. The generic reason stands if health cannot be read or if the origin is allowed, so this only ever widens an error - room access stays decided by the server. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deploying with `flyctl deploy -c services/api/fly.toml` from the repository root fails at `COPY src ./src`: -c selects the config file, but the build context stays the shell's working directory, and the Dockerfile is written against services/api. It also uploads a ~1.3 GB context, because Docker reads .dockerignore from the context root and only the service directories have one. A cached `npm ci` layer from an earlier correct build hides the cause, so the error surfaces at the first uncached step rather than the first wrong one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`makeAttempt` detaches its parent-abort listener as soon as the fetch settles, which is when headers arrive - before the body has streamed. The winner was therefore returned already disconnected from the caller's signal, so aborting afterwards no longer stopped the download. That is a regression against the serial reader this replaced, which passed the caller's signal straight to `fetch`. It bites where it matters: `useCatalog` passes a signal to every audio and asset read, so a listener skipping tracks left the previous audio downloading to completion, unread. Re-link the winner to the caller's signal before returning it, and abort the winner on the late-abort path too - it has already been removed from `active`, so the cleanup block would not otherwise reach it. Covered by a regression test that failed before this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Inside the Product host the app is served from polkadot://app.dotify-test01.dot, not the DotNS web gateway. SIGNAL_ORIGINS already carried it; API_ORIGINS did not, so a container would have had working rooms and no content keys - catalog and key delivery both go to the API, so free and protected playback would fail CORS while the room layer looked healthy. Deliberately does not add a bare `null`. `polkadot:` is a non-special scheme, so its origin is opaque and a browser may send `Origin: null` instead of the literal value. Allowing that would admit every sandboxed iframe and file:// page on the web to the authenticated upload and content-key routes. If a host request is still refused, the actual Origin header from the Fly log is the evidence to act on. Two regression tests pin both halves: the custom-scheme origin is answered, a null origin is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Grounded in the Product docs, SDK reference, and Community Foundation repos rather than assumption. Three findings drive the proposal. Dotify is closer to the stack than the roadmap assumed. Its Solidity contracts already run on pallet-revive via Asset Hub's eth-rpc - the exact pallet the stack specifies - and app delivery, content addressing, and app-scoped identity are already on-stack. Its DAV2 encryption is not redundant with a Bulletin move but the precondition for one, since Bulletin gates storing, not reading. The genuine gaps are worth closing and mostly make the product better: the personhood precompile returns a per-app unlinkable alias and would finally make `human-free` real while retiring the dev registrar; CDM registration makes the catalog composable by other products; CASH is the asset users actually hold. One gap will not close. Statement Store writes require an Individuality allowance, official calls are 1:1 and mobile-only, and an SDP exceeds both the 512-byte statement and 1 KiB per-account ceilings. Moving rooms onto the official messaging layer would convert every listener into an attested person, which does not degrade the product - it deletes the gesture it exists to protect. The anonymous guest is therefore treated as a design constraint, not a legacy compromise. Proposes a three-ring architecture that shrinks the trusted core instead of denying it, and names content-key custody as a stated exception with per-artist custody as the strongest remedy - turning the most centralized component into an expression of artist sovereignty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Dotify's `human-free` access mode was gated by an admin registrar that the runtime initializer sets to the artist. An artist could therefore grant personhood to their own listeners, which meant the contract could not actually support the claim the product was making. This replaces that with the Individuality precompile. Verified live before writing any of it: the precompile at 0x000000000000000000000000000000000a010000 answers on EVM chain 420420417 - the chain Dotify is deployed on - returning a 64-byte PersonhoodInfo, where neighbouring and empty addresses return 0x. The interface is taken from paseo-network/runtimes, not inferred. The tiers already lined up: PersonhoodLevel None/DIM1/DIM2 maps by ordinal onto None/Lite/Full, so no enum or stored track policy changes. Three properties come out of this. Personhood can no longer be forged by anyone, including the artist - the setter reverts rather than writing to storage no access decision reads, because silently accepting it would leave an operator believing a listener was granted access they do not have. Personhood becomes a fact about a person rather than per-runtime state, so every runtime reads the same answer. And the precompile returns a per-context alias, so Dotify can recognise a distinct person without learning who they are in any other application - a privacy property the registrar could not offer at all. Reads use a low-level staticcall on purpose. The precompile declares HAS_CONTRACT_INFO = false, so its extcodesize can be zero and Solidity's high-level call would revert on its own extcodesize check. The staticcall also lets a chain without the precompile resolve to "not live" instead of reverting every access query, and `live` is surfaced separately so a misconfigured deployment is not mistaken for an empty user base. Absent precompile denies every gated track, per the fail-closed invariant. Tests install mock precompile code at the real address with hardhat_setCode, which is the only way to exercise both the granted and fail-closed paths on a chain with no Individuality pallet. Coverage includes tier-below-required, and personhood held in another application context not unlocking Dotify. Not deployed. Existing runtimes need a diamond Add cut to gain musicAccPersonhoodInfo; their access decisions already follow the precompile without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`cdm deploy` builds, deploys, publishes metadata, and registers in one pass, so it cannot be used here: Dotify's contracts are already deployed and already hold the live catalog. Running it would mint new addresses and orphan every existing artist runtime. Reading the registry contract showed the operation that is actually needed is already exposed. `publishLatest(name, address, metadata_uri)` binds a name to an arbitrary address, and the contract's own comment states the rule: the caller may publish only if the name is free or they already own it. So a free name is claimable directly, no deployment involved. `metadata_uri` is stored verbatim and never validated. Adds the `cdm:publish` task for that registration. Read-only by default - it prints the plan and exact calldata and stops. Registration is first-writer-owns and the registry exposes no release or transfer entry point, so a claimed name is permanent; execution therefore requires --confirm and an explicit key, and this commit claims nothing. Verified live against the devnet registry at 0x59b0245778917af55224e5f8fb55f7f8d452619f on chain 420420417: all three @dotify/* names are unclaimed, and both target addresses carry bytecode. Guards confirmed to fail closed for a wrong registry address and for --confirm without a key. The task also refuses a target with no bytecode, since publishing a name that points at nothing is worse than not publishing it. @dotify/smart-runtime is deliberately not registered: artist runtimes are per-artist diamonds with no single address, and publishing one artist's runtime under a shared name would misrepresent the catalog. Two findings recorded in the architecture doc. CDM's docs independently confirm the preset distinction this project already encodes - `paseo` is paseo-next (para 1500), `devnet` is the Paseo testnet Asset Hub (para 1000, chain 420420417). And CDM does support Solidity via a `@custom:cdm` NatSpec tag, which the architecture page's PolkaVM-only framing hides; Dotify's existing toolchain is not an obstacle to CDM participation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Redeploying was authorised, since Dotify's on-chain data is test data. That removes the objection this task was built around, so it was worth checking whether `cdm deploy` becomes viable. It does not, and the reason is a hard chain limit rather than a preference. CDM's Solidity path compiles with resolc to PolkaVM. resolc handles Dotify's contracts fine - all 24 files compile, including the diamond's delegatecall fallback and every one of its 17 inline assembly blocks, with only an informational extcodesize warning. Feasibility is not the blocker. Size is. The Asset Hub initcode limit is 49,152 bytes and resolc emits roughly 4-10x more bytecode than solc. MusicRegistryPallet, which holds the catalog, is 8,855 bytes as deployed EVM bytecode and 71,252 bytes as a PolkaVM blob - 45% over the limit. MusicRightsRegistry is 81% over. Clearing it would require splitting the registry into storage and logic contracts, and the practitioner report documenting that workaround also records that diamond-style generic mappings were ineffective at reducing size, which is exactly this architecture. Asset Hub's pallet-revive accepts both EVM bytecode via eth-rpc and PolkaVM blobs via resolc. For Dotify the EVM path is not a legacy compromise, it is the only one that currently fits, and every deployed contract sits comfortably inside the limit. publishLatest registration is therefore the correct mechanism rather than a way to avoid redeploying. Measured, not assumed: resolc 0.6.0 compile in a scratch project, and eth_getCode against all ten deployed addresses on chain 420420417. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The registry stores (name -> address) and (name -> metadata_uri). Registering only the first leaves the composability claim nominal: another product can resolve where Dotify's contracts are but not what they expose, and `cdm install` fails outright because it requires an ABI in the metadata blob. Shape is taken from CDM's own consumers rather than guessed. `install.ts` validates exactly one thing - the blob must be an object whose `abi` is a non-empty array - and contracts.dot.li additionally renders description, readme, homepage, repository, license, keywords, authors and published_at. Both are satisfied; the generated blobs pass that validation, with 11 and 18 ABI entries. Output is deterministic, which matters for a content-addressed artifact: the same contracts produce the same bytes and therefore the same CID, so what is published can always be checked against the repository. `published_at` is omitted by default for exactly that reason and is opt-in behind --published-at, since a wall-clock field would mint a new CID on every run. Verified by generating twice and diffing the CID index. CIDs are computed locally with the Bulletin SDK (raw codec, blake2b-256), so the value that goes on-chain is derived from the generated bytes. cdm:publish now reads that index per package instead of taking one hand-pasted URI for all, which removes the failure mode where a correct-looking CID is attached to the wrong contract. Uploading the blobs to Bulletin needs a storage authorization and stays a separate credentialed step, documented in the runbook. Nothing is published or registered by this commit. Also adds TextEncoder to the eslint script globals, which the existing node globals list was missing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Product SDK cannot do this upload. `CloudStorageClient.create()` resolves its chain connection through getChainAPI/createChainClient, both of which route exclusively through the Product host container with no direct-WebSocket fallback, so uploading from a terminal is impossible that way. The Bulletin chain itself accepts a plain signed TransactionStorage.store extrinsic over WebSocket, which is what this script uses - the same approach the existing deploy-bulletin.cjs already proved. Read-only by default. It recomputes every CID from the bytes on disk and refuses to upload when one drifts from cids.json, because a blob edited after generation would otherwise land under a CID the registry can never match, and the registry entry is immutable once published. It also reports the account's remaining Bulletin quota against what the upload needs, and names the two ways to grant one when it is short. Separately fixes a real bug in deploy-bulletin.cjs. Its CID builder tagged the multihash as 0x1e, which is blake3 - an algorithm the Bulletin Chain does not accept - while the digest was blake2b-256. The correct code is 0xb220, which is 45600 and needs three varint bytes: 0xa0 0xe4 0x02. So the printed CID, the gateway URL built from it, and any DotNS registration pointing at it resolved nowhere. Confirmed by decomposing a CID from @parity/bulletin-sdk: the prefix is 01 55 a0 e4 02 20, and the remaining digest equals getContentHash(bytes, 45600). Note that script cannot currently run at all - blakejs is not installed in web/node_modules - which is a separate pre-existing issue and is left alone, since it serves the legacy single-file Bulletin build. Nothing is uploaded or registered by this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Requiring --private-key was wrong on two counts. The repository already configures an account for polkadotTestnet from the encrypted PRIVATE_KEY hardhat var, and it is already set, so the flag was redundant. It was also worse: a key passed on the command line lands in shell history and process listings, while the var is encrypted at rest. registryUpgrade.ts already used hre.viem.getWalletClients() for exactly this reason. The task now resolves the publisher from the network config and keeps --private-key only as an explicit override for publishing from a different account than the deployer. Also makes the stakes visible. publish_latest records `caller` as the permanent owner of every name it creates, so the signing account owns @dotify/* from then on - it is not just paying fees. The dry run now prints the resolved publisher and says so, since that is the last chance to notice before an irreversible claim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rooms become discoverable without Dotify's signaling server. Additive only: a beacon never carries SDP, ICE, chat, or audio, and is never required to join, so a share link still works with no wallet, no account, and no chain. Joining deliberately cannot move here. A WebRTC offer is 1.5-4 KB against a 512-byte statement ceiling, and a guest would have to publish an answer to complete a handshake, which needs an identity and an allowance. That would turn every listener into a registered person and delete the gesture the product exists to protect. Only the host publishes, and the host is already identified. The byte budget drives the design. MAX_STATEMENT_SIZE is 512 and MAX_USER_TOTAL is 1024, so one account holds at most two full statements. Beacons are written to a per-room channel for last-write-wins, otherwise every heartbeat would be a new statement and the budget would be gone within a couple of beats. The account total is checked against what would be live after a write, so the overflowing beacon is the one refused - the chain enforces this by silently rejecting, which a user experiences as a room that never appears. Privacy is a deliberate choice, not a default. A beacon carries the room code, host name, and an aggregate listener count, never listener identities. Now-playing is opt-in per host, because a beacon is globally readable and outlives the room by up to the retention window, which is a different exposure than sharing a link with someone. Everything received is re-parsed as untrusted: statements come from arbitrary accounts, so a malformed beacon is dropped rather than allowed to break a discovery list, and remote values are clamped before display. Host mode signs through the product's allowance account via the RFC-10 sponsored path, so hosting does not require the listener to hold an Individuality allowance. The client runs only inside the Product host container, so the production guard rejects VITE_DOTIFY_ROOM_BEACONS=on without a host mode rather than shipping chain code that can never connect. Off by default. Enabling costs about 24 KB. A build with it off still carries a ~69 KB statement-store chunk that is never fetched, because Rollup emits a chunk for the nested dynamic import before it can prove the build-time guard makes it unreachable; that is documented rather than claimed away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
knzeng-e
left a comment
There was a problem hiding this comment.
Careful review completed against the PR diff, the exact installed @parity/product-sdk-statement-store@0.6.2 implementation, the existing room lifecycle, and the Product deployment path.
The payload boundary is well scoped: it keeps SDP/ICE/chat/audio out, preserves wallet-free guest joining, validates untrusted JSON, and treats Statement Store failure as additive. Automated evidence also reproduces locally: 269 unit tests pass, 49 signaling tests pass, lint has 0 errors and the 3 existing warnings, git diff --check is clean, and VITE_DOTIFY_ROOM_BEACONS=on npm run build:product-devnet passes.
I would not treat the draft as merge-ready yet because the normal Product build leaves the capability disabled and the reader retains expired rooms indefinitely. I also left an inline note on the account-wide quota claim, which the local map cannot enforce.
Documentation is still out of sync with the architectural change: docs/operations/deployment-configuration.md and docs/operations/product-devnet-deployment.md do not contain the flag, operator prerequisites, enabled-build command, live publish/subscribe smoke, or rollback; web/.env.product-devnet omits the value; and the roadmap/public architecture material still describes Statement Store presence as proposed/future. Per AGENTS.md, this PR changes deployment settings and architecture narrative, so those sources should be updated in the same PR. A focused hook lifecycle test would also close the remaining timer/cancellation coverage gap. Live host evidence can remain a clearly stated draft prerequisite, but the runbook should say exactly how to collect it.
| | ------------ | ------------ | | ||
| | **Type** | `on` or `off` | | ||
| | **Required** | No | | ||
| | **Default** | `off` | |
There was a problem hiding this comment.
[P1] The standard Product deployment never enables this feature. web/.env.product-devnet does not set VITE_DOTIFY_ROOM_BEACONS, and npm run deploy:product-devnet always rebuilds in product-devnet mode, so the documented deploy command produces the off build and no host ever announces a room. Either enable the flag in the tracked Product profile, or document an explicit opt-in build/deploy command and describe this PR as a dormant capability rather than an active outcome. The selected value and rollback/smoke procedure also belong in docs/operations/deployment-configuration.md and the Product deployment runbook under the repository's env/config rule.
There was a problem hiding this comment.
Addressed. You were right that the documented deploy could never enable this. Resolved by keeping it off but making that a deliberate, documented choice rather than an omission: nothing reads beacons yet, so publishing room records to a public chain would be exposure with no consumer, and the publish path has no live host evidence. VITE_DOTIFY_ROOM_BEACONS=off is now explicit in .env.product-devnet with the reasoning, npm run build:product-devnet:beacons / deploy:product-devnet:beacons provide the opt-in path, and both deployment-configuration.md and the runbook now carry the selected value, prerequisites, enabled-build command, live evidence procedure, and rollback. The PR is described as a dormant capability.
| const beacon = parseRoomBeacon(statement.data); | ||
| if (!beacon) return; | ||
| if (options.roomCode && beacon.room !== options.roomCode.toUpperCase()) return; | ||
| seen.set(beacon.room, beacon); |
There was a problem hiding this comment.
[P1] Expired statements remain in this map forever. In SDK 0.6.2 the subscription callback includes the statement's expiry, but BeaconClient narrows the callback to { data }, so this listener cannot prune by TTL. The Statement Store removes an expired record; it does not send this code a deletion event. Consequently, after one beacon is received, beacons() and onChange can continue returning that room for the lifetime of the page even if the host stops and the 90-second statement expires. Preserve the expiry metadata, evict records at expiry (and notify onChange), and cover it with a fake-timer test before this is used by discovery UI.
There was a problem hiding this comment.
Fixed, and you identified the mechanism exactly: the store removes an expired record but sends no deletion event, and BeaconClient was narrowing expiry away. Expiry is now preserved and decoded (seconds live in the upper 32 bits), entries are filtered on read and swept on an interval, onChange fires only when the visible set actually changes, and an already-expired statement is ignored on arrival rather than shown then swept. A transport that omits expiry falls back to an assumed lifetime instead of reintroducing the never-evicting behaviour. Three fake-timer tests cover it.
| // not what is live now, so the overflowing beacon is the one refused. | ||
| const next = new Map(live); | ||
| next.set(beacon.room, beacon); | ||
| const budget = assertBeaconBudget([...next.values()]); |
There was a problem hiding this comment.
[P2] This is not an account-budget preflight. live only knows about successful writes made through this one publisher instance; it cannot see statements from another tab, another Dotify instance, or any other use of the same allowance account. In the shipped hook this publisher owns at most one current room, so the 12-room test exercises a state the app cannot create while real account-wide contention remains invisible. The reason is also discarded and the hook ignores false, so the refusal is not explainable to an operator or user. Please either replace this with account-aware evidence from the host/SDK, or scope it honestly as a per-instance heuristic and add observability for network quota rejection.
There was a problem hiding this comment.
Agreed on all three points, and I have scoped it honestly rather than claiming more.
The comment now states plainly that this only sees writes from this instance and cannot observe another tab, another Dotify instance, or any other use of the same allowance account. announce returns an outcome instead of a bare boolean: quota-local is named as the per-instance guard, and a network refusal is reported separately as rejected, with the account-wide quota called out as the likely cause. The hook logs refusals rather than discarding them, so an operator debugging an undiscoverable room has something to read.
I kept the local check because it does catch the one case this instance can cause, but the network is the authority and the code now says so.
| /** Host display name, already public to anyone holding the link. */ | ||
| host: string; | ||
| /** Listener count. Aggregate only - never identities. */ | ||
| n: number; |
There was a problem hiding this comment.
Better to use meaningful fields/variable names (ex: listenerCount or nbListeners - maybe the first one, to have accurate naming across RoomBeaconInput as well). This should be a rule of thumb
There was a problem hiding this comment.
Agreed, and applied as a rule rather than a one-off: v/n/t/a are now version/listenerCount/title/artist across the type, builder, parser, and tests. The names cost roughly 40 bytes of keys against the 512-byte ceiling, which the budget can afford — and a wire format is read by people debugging a live room, so the saving was not worth the opacity. Noted in a comment so the trade-off is not silently reversed later.
| if (byteLength(beacon) > MAX_BEACON_BYTES) delete beacon.a; | ||
| if (byteLength(beacon) > MAX_BEACON_BYTES) delete beacon.t; |
There was a problem hiding this comment.
Couldn't those two line be rewritten in a single || (OR) conditioned one ?
There was a problem hiding this comment.
Not merged, and I want to explain why rather than just decline. The two lines delete different fields, and each condition must be re-evaluated after the previous deletion — an OR would collapse them into one branch and drop only one field. What I did instead is remove the duplication you were pointing at: both now read if (tooLarge(beacon)), so the repeated measurement expression is gone even though the two statements stay separate.
| beacon.host = beacon.host.slice(0, Math.floor(beacon.host.length / 2)); | ||
| } | ||
|
|
||
| return byteLength(beacon) <= MAX_BEACON_BYTES ? beacon : null; |
There was a problem hiding this comment.
we called several times byteLength(beacon) in this code. Maybe worth extract/snapshot it in a single variable at the beginning ? unless this is a dynamic value we need to keep track of ?
Furthermore, why not reusing the beaconByteLength function ?
There was a problem hiding this comment.
Both applied. beaconByteLength is now the single measurement entry point — the call sites use a tooLarge() helper built on it, and assertBeaconBudget uses it too. It is deliberately not snapshotted into one variable: every shed step changes the serialized bytes, so a cached length would be stale by the next check. That reasoning is now in the comment so it does not look like an oversight.
Six review points, plus the documentation gaps called out in the review body. [P1] The capability was unreachable through the documented deploy. `.env.product-devnet` omitted the flag and `deploy:product-devnet` always rebuilds in that mode, so the published build announced nothing. Resolved by keeping it off but making that explicit and deliberate: nothing reads beacons yet, so publishing room records to a public chain would be exposure with no consumer, and the publish path has no live host evidence. The value is now set in the tracked profile with the reasoning, opt-in build/deploy commands exist, and deployment-configuration.md plus the runbook carry the selected value, prerequisites, enabled-build command, live evidence procedure, and rollback. [P1] The reader never evicted expired rooms. SDK 0.6.2 exposes `expiry` on the subscription callback, but BeaconClient narrowed it away, and the store removes an expired record without sending a deletion event - so a stopped room could be listed for the lifetime of the page. Expiry is now preserved and decoded, entries are filtered on read and swept on an interval, onChange fires only when the visible set changes, and an already-expired statement is ignored on arrival. A transport that omits expiry falls back to an assumed lifetime rather than reintroducing the never-evicting behaviour. [P2] The budget check was presented as an account preflight when it can only see this instance's own writes - not another tab, another instance, or any other use of the same allowance account. It is now scoped honestly in the comment and in the type: announce returns a reason, `quota-local` is named as per-instance, and a network refusal is reported separately as `rejected` with the account-wide quota called out. The hook logs refusals instead of discarding them, so an operator debugging an undiscoverable room has something to read. Naming: single-letter wire fields are replaced with spelled-out names. They cost about 40 bytes of keys against a 512-byte ceiling, which the budget can afford, and a wire format is read by people debugging a live room. Byte measurement is consolidated behind beaconByteLength via a `tooLarge` helper. It is deliberately not snapshotted: every shed step changes the serialized bytes, so a cached length would be stale by the next check. The hook lifecycle is extracted into startRoomBeaconLoop so start, refresh, cancellation, and the stop-during-connect race are testable as plain logic, matching the repository's existing hook-test convention rather than adding a renderer dependency for one file. Roadmap and architecture no longer describe Statement Store presence as proposed. 278 tests pass, up from 269. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Outcome
Hosted rooms are announced on the Statement Store, so a room can be discovered without Dotify's signaling server. Step 3 of the sequence in #116.
Off by default. Stacked on #118.
The constraint that shaped everything
This is discovery only, and that boundary is load-bearing rather than a limitation I settled for.
A WebRTC offer is 1.5–4 KB against a 512-byte statement ceiling, and the per-account total is 1024 bytes — so a peer cannot hold even one SDP. But the arithmetic is the lesser problem. A guest would have to publish an answer to complete a handshake, and publishing requires an identity and an Individuality allowance.
Moving the join path here would convert every listener into a registered person. That does not degrade the product — it deletes the gesture it exists to protect. So only the host publishes, because the host is already identified, and the share link stays the way in: no wallet, no account, no chain.
Design consequences of the byte budget
One account holds at most two full statements, so naive publishing would exhaust it in a couple of heartbeats.
Privacy is a choice here, not a default
A beacon carries the room code, host display name, and an aggregate listener count — never listener identities.
Now-playing is opt-in per host. A beacon is globally readable and outlives the room by up to the retention window, so publishing what someone is listening to is a materially different exposure than sharing a link with a friend. It stays a deliberate act.
Everything received is re-parsed as untrusted — statements come from arbitrary accounts, so a malformed beacon is dropped rather than allowed to break a discovery list, and remote values are clamped before display.
Host container and cost
Host mode signs through the product's allowance account via the RFC-10 sponsored path, so hosting does not require the listener to hold an Individuality allowance of their own.
The client runs only inside the Product host container, so
validateProductionEnvironmentrejectsVITE_DOTIFY_ROOM_BEACONS=onwithout a host mode — otherwise the build ships chain code that can never connect.off)VITE_DOTIFY_ROOM_BEACONS=onEnabling costs ~24 KB. A build with it off still carries a ~69 KB statement-store chunk that is never fetched — Rollup emits a chunk for the nested dynamic import before it can prove the build-time guard makes it unreachable. That is ~1.5% of the bundle and never executes, but I said I would keep it at zero, so it is measured and documented rather than claimed away.
Review guide
web/src/features/rooms/roomBeacon.ts— the encoding and the budget rules.web/src/features/rooms/roomBeaconPublisher.ts— scope limits and fail-quiet behaviour.web/src/hooks/useRoomBeacon.ts— kept out ofuseSessionso the room lifecycle stays readable; the ref is written in an effect, not during render.Verify carefully
Validation
cd web && npm run test:unitcd web && npm run test:signalcd web && npm run lintcd web && npm run buildcd web && npm run build:product-devnetcd web && npm run smoke:production-envnode scripts/backlog-sync.mjs --check --offlinegit diff --checkKnown limitations and follow-ups
Not exercised against a live Statement Store. Every path is tested through an injected transport; the SDK client cannot run outside a Product host container, so real publish/subscribe evidence needs a run inside the host — the same gate as the rest of the Product work.
Discovery is not yet surfaced in the UI.
subscribeRoomBeaconsexists and is tested, but nothing renders a discovered-rooms list. That is deliberate: the reading side should land with a real design for how a discovered room is presented, rather than a debug list.Retention outlives the room. A beacon persists up to the store's retention window after a room closes, so a discovery list must treat entries as hints and confirm liveness on join. Refresh is 30 s against a 90 s TTL, so one missed beat does not drop a room.
Review round: all six comments addressed
Thanks — the two P1s were both real, and the second one would have shipped a visible bug.
[P1] The capability was unreachable through the documented deploy
Correct, and worth stating plainly:
.env.product-devnetomitted the flag anddeploy:product-devnetalways rebuilds in that mode, so the published build announced nothing.Resolved by keeping it off — but as a deliberate, documented choice rather than an omission. Nothing reads beacons yet, so publishing room records to a public chain would be exposure with no consumer, and the publish path has no live host evidence. Enabling also costs ~24 KB against the Bulletin quota.
VITE_DOTIFY_ROOM_BEACONS=offis now explicit in the tracked profile, with the reasoning inlinenpm run build:product-devnet:beacons/deploy:product-devnet:beaconsgive the opt-in pathdeployment-configuration.mdcarries the selected value and rollbackThis PR is a dormant capability, not an active outcome.
[P1] Expired rooms were never evicted
You identified the mechanism exactly: the store removes an expired record but sends no deletion event, and
BeaconClientwas narrowingexpiryaway.Expiry is now preserved and decoded (seconds in the upper 32 bits), entries are filtered on read and swept on an interval,
onChangefires only when the visible set actually changes, and an already-expired statement is ignored on arrival rather than shown then swept. A transport that omits expiry falls back to an assumed lifetime instead of reintroducing the never-evicting behaviour. Three fake-timer tests cover it.[P2] The budget check was over-claimed
Scoped honestly rather than defended. The comment now states it sees only this instance's writes.
announcereturns an outcome:quota-localis named as the per-instance guard, and a network refusal is reported separately asrejectedwith the account-wide quota called out. The hook logs refusals instead of discarding them. Kept the local check — it catches the one case this instance can cause — but the network is the authority and the code says so.Naming, and the two code-quality points
Single-letter wire fields are gone:
version,listenerCount,title,artist. They cost ~40 bytes of keys against a 512-byte ceiling, which the budget affords, and a wire format gets read by people debugging a live room. Noted in a comment so the trade-off is not silently reversed.beaconByteLengthis now the single measurement entry point, used via atooLarge()helper. Not snapshotted into one variable, and that is deliberate: every shed step changes the serialized bytes, so a cached length would be stale by the next check. The reasoning is in the comment.On the OR suggestion — not merged, and the reason is substantive: the two lines delete different fields and each condition must be re-evaluated after the previous deletion, so an OR would drop only one. The duplication you were pointing at is gone anyway, since both now read
if (tooLarge(beacon)).From the review body
startRoomBeaconLoop, so start, refresh, cancellation, and the stop-during-connect race are testable as plain logic. Done this way rather than adding a renderer dependency for one file, matching the repo's existing hook-test convention.Re-verified
npm run test:unitnpm run test:signalnpm run lintnpm run buildVITE_DOTIFY_ROOM_BEACONS=on npm run build:product-devnetnpm run smoke:production-envnode scripts/backlog-sync.mjs --check --offlinegit diff --check