fix: isolate failed royalty recipients - #146
Conversation
✅ Deploy Preview for muzinga canceled.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2712f25f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| listener, | ||
| recipient, | ||
| amountWei, | ||
| settlement: 'claimable', |
There was a problem hiding this comment.
Reconcile claims before labeling accruals as claimable
After musicRoyClaim succeeds, the pending balance is cleared and MusicRoyRoyaltyClaimed is emitted, but this reader never fetches claim events and permanently maps every historical accrual to settlement: 'claimable'. Refreshing after a claim therefore leaves those rows labeled Claimable and excludes the claimed amounts from the settled total, even while the current claimable metric is zero. Consume claim events to reconcile the ledger, or represent these rows as historical accruals rather than current pending funds.
Useful? React with 👍 / 👎.
| title: 'Claiming royalties', | ||
| message: 'Submitting the pending royalty claim from your SmartRuntime.' | ||
| }); | ||
| const txHash = await runtimeWriter.claimRoyalty(artistRuntimeAddress, activeEvmAddress); |
There was a problem hiding this comment.
Let split recipients select the runtime holding their funds
When the connected wallet is a collaborator listed in another artist's royalty splits, artistRuntimeAddress was resolved through directory.runtimeOf(activeEvmAddress) at useArtistConsole.ts:361, so it is null or points to the collaborator's own runtime rather than the runtime that accrued the royalty. The new claim path consequently cannot reach the balance held in the originating artist's runtime, and ArtistShell.tsx:33 shows onboarding instead of the claim UI when the collaborator has no runtime. The claim flow needs a way to discover or select runtimes where the connected recipient has pending funds.
Useful? React with 👍 / 👎.
| const [paidLogs, claimableLogs] = await Promise.all([ | ||
| client().getLogs({ | ||
| address: runtimeAddress, | ||
| event: musicRoyRoyaltyPaidEvent, |
There was a problem hiding this comment.
Preserve pre-upgrade royalty history
For an existing runtime, including one upgraded in place, purchases made before W05 emitted only MusicRoyAccessPaid; the new per-recipient events are not retroactive. Because the reader now queries only MusicRoyRoyaltyPaid and MusicRoyRoyaltyClaimable, all pre-upgrade payment rows disappear permanently after this frontend change, even after the facet is upgraded. Retain a legacy access-payment query and present those records with an explicit legacy/unknown-settlement state rather than reporting an empty ledger.
Useful? React with 👍 / 👎.
|
Review follow-up pushed in Addressed the three inline comments:
Also added the upgrade/migration operator path:
Validated with contracts/web format checks, |
|
Follow-up for the code-hash mismatch seen during the live dry-run. The error was correct: Pushed
Correct sequence for the runtime from the failed attempt: cd contracts/evm
npm run runtime:export:testnet -- --runtime 0xB60e91CcAcD08B6cb0Ddb2E678F90791901e9338 --recipient <ARTIST_OR_SPLIT_RECIPIENT> --out /tmp/dotify-runtime-snapshot.json
npm run runtime:deploy-royalties-facet:testnet
npm run runtime:deploy-royalties-facet:testnet -- --execute --confirm-chain-id 420420417 --confirm-code-hash <LOCAL_CODE_HASH_FROM_DRY_RUN> --out /tmp/dotify-royalties-facet.json
npm run runtime:royalties-upgrade:testnet -- --runtime 0xB60e91CcAcD08B6cb0Ddb2E678F90791901e9338 --facet <FACET_FROM_/tmp/dotify-royalties-facet.json> --out /tmp/dotify-royalties-upgrade-plan.json
npm run runtime:royalties-upgrade:testnet -- --runtime 0xB60e91CcAcD08B6cb0Ddb2E678F90791901e9338 --facet <FACET_FROM_/tmp/dotify-royalties-facet.json> --execute --confirm-plan <PLAN_DIGEST_FROM_DRY_RUN> --out /tmp/dotify-royalties-upgrade-final.jsonLatest local validation includes |
Outcome
W05 prevents one failed royalty recipient from reverting an otherwise valid Classic access purchase. Normal recipients still receive native-token shares immediately, while rejecting or gas-consuming recipients get a claimable balance in the artist runtime.
Issue and context
Closes #145.
Local scope document:
docs/backlog/implementation/W05-royalty-failure-isolation.mdBefore this PR,
musicRoyPayAccessused unbounded recipient calls and required every transfer to succeed. A single collaborator contract with a revertingreceive()hook, an expensive hook, or an incompatible native-token path could block the listener's purchase and all other recipients.Dotify needs Classic support to remain simple for the listener, but payment submission, access finality, and per-recipient receipt are distinct facts. This PR keeps those facts separate.
Architecture and key concepts
Classic payment now has one access-payment fact and multiple per-recipient settlement facts:
MusicRoyAccessPaidremains the listener access-payment event. It is not used as proof that every recipient was paid. The artist ledger now readsMusicRoyRoyaltyPaid,MusicRoyRoyaltyClaimable,MusicRoyRoyaltyClaimed, and pre-W05MusicRoyAccessPaidlegacy rows.totalRoyaltyWeisums only paid rows plus claimable rows that were later cleared by claim events.How it works
Contracts:
LibMusicRoyaltiesappendsclaimableto the existing namespaced Diamond storage and exposes bounded native transfer/accounting helpers.MusicRoyaltiesPalletsettles each share through that helper, emits paid/failed/claimable events, and exposesmusicRoyClaimable(address)plusmusicRoyClaim(address).ArtistRuntimeFactoryinstalls the two new royalties selectors for newly created runtimes.Frontend:
getRoyaltyClaimableandclaimRoyalty.MusicRoyAccessPaidas recipient income.MusicRoyRoyaltyClaimedFIFO per recipient so a refreshed ledger does not keep cleared accruals labeled claimable.musicRoyClaim; historical Product event reads remain unsupported until an indexer/event API exists.Operations:
runtime:exportsaves one SmartRuntime catalogue, royalty splits, track-state hash, and optional recipient claimable balance.runtime:deploy-royalties-facetdeploys the current W05MusicRoyaltiesPalletas a stateless facet with code-hash confirmation and durable evidence.runtime:royalties-upgradeplans, simulates, and optionally executes an owner-signed W05 royalties facet cut with an exact digest and durable evidence file.runtime:migration-planrenders replay calldata from an export snapshot for clean-redeploy fallback, while blocking runtime-bound encrypted audio refs by default.Design decisions and tradeoffs
Chosen: immediate distribution plus bounded-gas claimable fallback.
This is smaller and less disruptive than converting all royalty settlement to a pull-only model. Artists and compatible collaborators keep immediate settlement, while incompatible recipients no longer block access purchases.
Chosen upgrade path: in-place Diamond facet cut when the artist still controls the runtime.
This preserves the runtime address, protected-audio key binding, catalogue storage, paid-access state, royalty splits, and claimable balances. Clean redeploy remains a fallback only; replaying track registrations to a new runtime cannot move paid-access grants or claimable native-token balances, and encrypted
dotify:enc:v2:audio requires re-encryption or explicit key recovery.The first live operator attempt showed why the explicit facet-deploy step matters:
runtime:royalties-upgradecorrectly rejects a stale target facet whendeployments.jsonstill points to an older on-chainMusicRoyaltiesPallet. The corrected sequence is deploy current facet, then pass its manifestfacetaddress toruntime:royalties-upgrade --facet <NEW_FACET>.Deferred:
Security, failure, and operations
musicRoyClaim(recipient)requiresrecipient == msg.sender, so a helper cannot drain another recipient's pending balance.MusicRoyRoyaltyClaimFailed, allowing the failure to be auditable.runtime:deploy-royalties-facetandruntime:royalties-upgradeare dry-run by default. The deploy task refuses execution without chain/code-hash confirmation and--out. The upgrade task refuses execution without--out, verifies target facet bytecode, simulates the owner call, persists signed/broadcast evidence, waits for finality, verifies selector routing, and compares the post-upgrade catalogue hash with the pre-upgrade hash.Review guide
Suggested order
contracts/evm/contracts/libraries/LibMusicRoyalties.solandcontracts/evm/contracts/pallets/MusicRoyaltiesPallet.sol- verify settlement accounting, gas-bounded calls, claim restore behavior, and non-reentrancy assumptions.contracts/evm/tasks/registryUpgrade.tsandcontracts/evm/test/RegistryUpgradeTasks.test.ts- verify runtime export, royalties facet deployment, royalties upgrade evidence, finality/readback, and migration-plan guardrails.contracts/evm/test/ArtistRuntime.test.tsandcontracts/evm/contracts/test/RoyaltyFailureRecipients.sol- verify the failure fixtures, repeated purchase accounting, dust/rounding, and forkless upgrade rehearsal.contracts/evm/contracts/ArtistRuntimeFactory.solplus generated ABI files - verify new runtime selector installation and binding drift.web/src/features/runtime/*- verify viem/Product CDM ports preserve adapter boundaries, claim reconciliation, split-recipient runtime discovery, and legacy history.web/src/hooks/useArtistConsole.ts,ArtistStudioProvider.tsx, and artist view files - verify claimable values are never displayed as settled income.Verify carefully
paidAccessremain accurate when one recipient share becomes claimable?Validation
npm --prefix contracts/evm test-> 66 passingnpm --prefix contracts/evm run fmt:check-> passnpm --prefix contracts/evm run compile-> passnpm exec -- hardhat help runtime:export-> passnpm exec -- hardhat help runtime:deploy-royalties-facet-> passnpm exec -- hardhat help runtime:royalties-upgrade-> passnpm exec -- hardhat help runtime:migration-plan-> passnpm --prefix contracts/evm run generate:abis-> passnpm --prefix web run generate:cdm-> passnpm --prefix web run generate:cdm-metadata-> passnpm --prefix web run fmt:check-> passnpm --prefix web run test:unit-> 52 files, 400 tests passingnpm --prefix web run test:unit -- src/features/runtime/viemRuntimeAdapter.test.ts src/features/runtime/royaltyRuntimeClaims.test.ts-> 12 passingnpm --prefix web run lint-> exit 0, 3 warningsApp.tsxandArtistShell.tsxnpm --prefix web run build-> passnpm --prefix web run generate:product-catalog-bootstrap:strict -- --input fixtures/product-devnet-catalog.json-> passCATALOG_API_URL=http://127.0.0.1:9 npm --prefix web run build:product-devnet-> passnode scripts/backlog-sync.mjs --check --offline-> pass with existing warningsgit diff --check-> passDetailed handoff:
docs/backlog/implementation/evidence/W05.mdKnown limitations and follow-ups
runtime:deploy-royalties-facet:testnet, thenruntime:royalties-upgrade:testnet -- --facet <NEW_FACET>for old SmartRuntimes owned by the artist. Useruntime:exportplusruntime:migration-planonly for clean-redeploy fallback planning.Metadata checklist
Dotify sprints)