feat: v4 tokenomics + count trees (YAPP token, buy/moderation UX)#291
feat: v4 tokenomics + count trees (YAPP token, buy/moderation UX)#291PastaPastaPasta wants to merge 11 commits into
Conversation
Migrate to a protocol-v12 social contract with count-tree indexes and a native YAPP token. Posts/replies/likes/reposts now cost YAPP (revenue to the contract owner), bought via a minimum-100 direct-purchase bond, with owner-controlled freeze/slash moderation. Counts (likes/replies/followers/following/reposts) now use O(1) count trees instead of O(n) pagination. Token payment is auto-attached on all paid writes. Adds a Buy-YAPP modal, YAPP balance + Buy button, and insufficient-balance prompts across post/reply/like/repost. Reposts migrated to a dedicated 'repost' doctype. Bumps @dashevo/evo-sdk to 4.0.0-rc.2. Adds a legacy-app escape link on empty/end-of-feed states and scripts/set-yapp-price.mjs for tiered pricing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Warning Review limit reached
Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds YAPP token payment, purchase, and moderation flows; switches several counters to indexed counts; migrates reposts to dedicated documents; removes hashtag-based post fields and queries; and adds legacy-app links in empty and end-of-feed states. ChangesYAPP Token Economy
Indexed Counting and Repost Migration
Legacy App Link in Empty States
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant BuyYappModal
participant TokenService
participant EvoSDK
User->>BuyYappModal: enter amount and confirm
BuyYappModal->>TokenService: buyYapp(buyerId, amount, maxTotalCost)
TokenService->>EvoSDK: directPurchase(...)
EvoSDK-->>TokenService: result or error
TokenService-->>BuyYappModal: TokenResult
BuyYappModal->>TokenService: getBalance(identityId)
BuyYappModal->>User: success or error state
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Review complete (commit bf695e1) |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Deploying yappr with
|
| Latest commit: |
bf695e1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://98498aad.yappr.pages.dev |
| Branch Preview URL: | https://feat-v4-tokenomics-count-tre.yappr.pages.dev |
Deploying yappr-v2 with
|
| Latest commit: |
bf695e1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d5673650.yappr-v2.pages.dev |
| Branch Preview URL: | https://feat-v4-tokenomics-count-tre.yappr-v2.pages.dev |
There was a problem hiding this comment.
Code Review
The token-service Map lookups almost certainly miss the SDK's Identifier-keyed results, which breaks the new YAPP balance/price/buy UX end-to-end. Beyond that, the feed-batch stats path still relies on capped document queries despite the PR advertising a count-tree cutover, and the new YAPP dropdown row traps the Buy button behind a disabled menu item. Everything else is minor cleanup.
Red: 1 blocking | Yellow: 2 suggestion(s) | Nit: 3 nitpick(s)
Findings not posted inline (3)
These findings could not be anchored to the current diff, but they are still part of this review.
- [NITPICK]
lib/services/token-service.ts:183-209: findMatchingAuthKey can reject a valid CRITICAL/HIGH key when a lower-security key matches first —findMatchingKeyIndexscans the candidate list and returns the first WIF match, then this code post-filters that single result down to CRITICAL/HIGH. If an identity carries multiple AUTHENTICATION keys derived from the same WIF (for example, a MEDIUM key at a lower id and a CRITICAL key at a hig... - [NITPICK]
lib/services/post-service.ts:20-21: Stale firstMentionId / primaryHashtag properties left on PostDocument — The PR intentionally drops writes and reads offirstMentionId/primaryHashtagbecause the v2 schema usesadditionalProperties: false, butPostDocumenthere (andPostDocumentDataintypes/sdk.ts:170-171) still declares them. They're harmless (optional, unused) but they misrepresent th... - [NITPICK]
components/post/post-card.tsx:393-400: Repost toast + optimistic count drift when a repost already existed —repostService.repostPostreturnstrueearly when an existing repost is found on-chain, but the caller has already appliedreposts + 1optimistically and shows the 'Reposted!' success toast on any truthy return. If the localrepostedstate was stale and the count fetched via the byPost cou...
Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `lib/services/token-service.ts`:
- [BLOCKING] lib/services/token-service.ts:30-62: YAPP balance and price lookups miss Identifier-keyed SDK Maps
`sdk.tokens.identityBalances` and `sdk.tokens.directPurchasePrices` are declared in the evo-sdk facade as `Promise<Map<wasm.Identifier, bigint>>` and `Promise<Map<wasm.Identifier, TokenPriceInfo>>` — the Maps are keyed by wasm `Identifier` objects, not the base58 strings this code passes in. `Map.get(tokenId)` uses SameValueZero equality on those object keys, so `balances.get(tokenId)` and `prices.get(tokenId)` return `undefined` even when the SDK returned the requested entry. The account dropdown then shows 0 YAPP, `getPricePerToken()` returns `null`, and `buyYapp` errors out with 'YAPP is not currently for sale' — the new buy/balance UX never works. Iterate the Map entries and compare `key.toBase58()` (or equivalent) against `tokenId`, or convert `tokenId` to an `Identifier` before lookup.
In `lib/services/post-stats-helpers.ts`:
- [SUGGESTION] lib/services/post-stats-helpers.ts:140-183: Feed batch stats still use capped document fetches instead of count trees
The PR migrates single-post `countLikes` / `countReposts` / `countReplies` to count trees (with a comment on the reply path noting 'also removes the old 100-cap undercount'), but `fetchBatchPostStats` — reached by feed cards via `postService.getBatchPostStats` in `useProgressiveEnrichment` — still delegates to `getLikesByPostIds`, `getRepostsByPostIds`, and `countRepliesByParentIds`, which each hit `sdk.documents.query` with a hard `limit: 100`. That means the same undercount the count-tree cutover eliminates for single-post views is preserved in the main feed once a batch of posts collectively has more than ~100 matching engagement documents. Since count-tree migration is a headline of this PR, the batch stats path should either issue per-post count-tree calls or a grouped `sdk.documents.count` query rather than continue truncating at 100.
In `components/token/yapp-balance-item.tsx`:
- [SUGGESTION] components/token/yapp-balance-item.tsx:31-45: Disabled DropdownMenu.Item hides the Buy button from keyboard users
The Buy button is nested inside a `DropdownMenu.Item` marked `disabled` with `onSelect={(e) => e.preventDefault()}`. Radix removes disabled items from the roving focus, so keyboard-only users navigating the account dropdown cannot reach the Buy button at all — clicks work, but there is no way to open the buy modal via keyboard. Nesting an interactive `<button>` inside a `role="menuitem"` also violates ARIA's no-interactive-in-interactive rule. Either make the item itself the actionable target (`onSelect={() => openBuy()}` and drop the inner button) or lift the Buy control out of the DropdownMenu.Item so it participates in the menu's focus order.
- token-service: robust token-id Map lookup (SDK Maps may be Identifier-keyed) fixing balance/price/buy; pre-filter to CRITICAL/HIGH auth keys before matching so a lower-security key can't mask a valid one - yapp-balance-item: make the Buy row a keyboard-operable DropdownMenu.Item instead of a button nested in a disabled item - post-stats-helpers: batch feed stats now use per-post O(1) count trees (removes the 100-cap undercount on the feed path) - remove stale firstMentionId/primaryHashtag from PostDocument/PostDocumentData types (v2 schema drops them) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the review — addressed in
Build + lint pass. |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
All six prior findings from the 6c6632d review are addressed: five are fixed in code (tokenMapGet handles Identifier-keyed Maps; fetchBatchPostStats uses per-post count trees; the YAPP row is a keyboard-actionable DropdownMenu.Item; findMatchingAuthKey pre-filters CRITICAL/HIGH; stale firstMentionId/primaryHashtag properties removed from source), and the repost toast drift nit is intentionally deferred with a reasonable rationale. Two new concerns emerge from the current buy path: buyYapp uses a freshly-refetched chain price as the maxTotalCost cap rather than the price the user confirmed, so a mid-flight SetPrices bump can silently authorize a higher spend; and purchase amounts are round-tripped through JS number, which the review skill explicitly warns against for token/credit values.
🔴 1 blocking | 🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `lib/services/token-service.ts`:
- [BLOCKING] lib/services/token-service.ts:96-125: maxTotalCost is set from a fresh chain price, not the price the user confirmed
The buy modal shows the user a cost derived from `pricePerToken` loaded once when the modal opens (`components/token/buy-yapp-modal.tsx:40,54`). When the user then clicks Confirm & Buy, `buyYapp()` deliberately refetches the current direct-purchase price and uses `price * amount` as `maxTotalCost` (lines 103-107), with a comment framing this as a safety property. In practice this inverts the guard: if the token authority raises the YAPP SetPrices tier between the modal render and the user's confirmation, the state transition is signed authorizing the *new* higher spend without the user ever seeing that number, and there is no on-chain rejection because `maxTotalCost` already accommodates it. Per the review skill's token/payment rule, `maxTotalCost` should represent the user-approved spend: pass the quoted price (or quoted total) into `buyYapp()` and use that as the cap, so a mid-flight price increase surfaces as an error the user can re-quote instead of silently going through at the higher price.
In `components/token/buy-yapp-modal.tsx`:
- [SUGGESTION] components/token/buy-yapp-modal.tsx:53-82: YAPP purchase amount and cost are coerced through JS number before signing
`amountNum = parseInt(amount, 10)` (line 53) parses user input into a JS `number`, and that value is later converted back to `BigInt` when calling `tokenService.buyYapp(user.identityId, BigInt(amountNum))` (line 82). `costCredits` is similarly funneled through `Number(costCredits)` for display (line 56). While realistic YAPP purchase amounts stay well below `Number.MAX_SAFE_INTEGER`, the review skill explicitly calls out this class of round-trip: 'Do not coerce token/credit values through `Number` when precision can matter; prefer string/BigInt-safe handling.' For token/credit paths, keep the input parsed directly to `BigInt` (rejecting non-numeric strings) and format credits via BigInt-safe helpers so the signed amount can never diverge from what the UI showed.
- buyYapp now takes the user-approved maxTotalCost (the quoted total the modal showed) instead of refetching a fresh price at sign time, so a mid-flight YAPP price increase is rejected on-chain rather than silently overspending; classify the price-too-low rejection as a re-quote prompt - buy modal keeps amount and cost as BigInt end-to-end (no parseInt/Number round-trip through the signed path) and formats credits->DASH with a BigInt-safe helper Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Round 2 addressed in
Build + lint pass. (Prior round's three findings were auto-resolved by the review system.) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Both prior findings from 0652fc9 are FIXED at head 911ee70: buyYapp now signs with a caller-supplied maxTotalCost matching the exact quote the user confirmed in the modal, and the purchase amount + cost stay BigInt end-to-end via a strict /^\d+$/ regex and the new BigInt-safe formatCreditsAsDash helper. No new blocking or suggestion-level defects found in the latest delta; the only new finding is a docs nit that a leftover comment on getPricePerToken misattributes the maxTotalCost-staleness protection to that function even though the fix moved the guarantee to the caller-supplied cap.
💬 1 nitpick(s)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/settings/page.tsx (1)
750-754: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHeader title doesn't account for the moderation section.
getSectionTitleonly searchessettingsSections, which never includesMODERATION_SECTION(it's spread in locally at Line 215 insiderenderMainSettings). Visiting the moderation section falls back to "Settings" instead of "Moderation".🏷️ Proposed fix
const getSectionTitle = () => { if (activeSection === 'main') return 'Settings' - const section = settingsSections.find(s => s.id === activeSection) + const section = [...settingsSections, MODERATION_SECTION].find(s => s.id === activeSection) return section?.label || 'Settings' }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/settings/page.tsx` around lines 750 - 754, The header title logic in getSectionTitle does not handle the moderation section because it only looks up settingsSections, so activeSection values for MODERATION_SECTION fall back to “Settings.” Update getSectionTitle to also recognize the moderation section by checking the same moderation identifier used in renderMainSettings and returning the proper “Moderation” label before falling back to the existing settingsSections lookup.lib/services/reply-service.ts (1)
354-366: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winError swallowed silently — no logging, unlike sibling count methods.
countFollowers/countFollowing(follow-service.ts),countLikes(like-service.ts), andcountReposts(repost-service.ts) alllogger.error(...)before returning 0 on failure. Thiscatch {}block returns 0 with no log, making failures here (e.g., missing index, bad query) invisible in production.🪵 Add logging for consistency
async countReplies(parentId: string): Promise<number> { try { const sdk = await getEvoSdk(); // O(1) count tree on the `byParent` index [parentId] (also removes the old 100-cap undercount). return await documentCount(sdk, { dataContractId: this.contractId, documentTypeName: 'reply', where: [['parentId', '==', parentId]], }); - } catch { + } catch (error) { + logger.error('Error counting replies:', error); return 0; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/services/reply-service.ts` around lines 354 - 366, The `countReplies` method in `reply-service.ts` is swallowing errors silently, unlike the sibling count methods. Update the `catch` block in `countReplies` to log the failure with `logger.error(...)` before returning 0, using the same pattern as `countFollowers`, `countFollowing`, `countLikes`, and `countReposts` so failures in `getEvoSdk` or `documentCount` are visible.
🧹 Nitpick comments (10)
components/settings/moderation-settings.tsx (2)
22-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReset the form after a successful action.
targetId/noteremain populated after freeze/unfreeze/destroyFrozen succeeds, risking accidental resubmission against the same identity.🧹 Proposed fix to clear inputs after success
if (result.success) { toast.success( action === 'freeze' ? 'Identity frozen (suspended)' : action === 'unfreeze' ? 'Identity unfrozen (reinstated)' : 'Frozen balance destroyed (slashed)' ) + setTargetId('') + setNote('') } else {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/settings/moderation-settings.tsx` around lines 22 - 49, The moderation action flow in run() leaves targetId and note filled after a successful tokenService call, so clear both inputs only when result.success is true. Update the ModerationSettings component logic around run(), using the existing setBusy/result handling, to reset the form state after the success toast before returning.
63-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared
Inputcomponent here
Replace the two raw<input>elements withcomponents/ui/input.tsxso both moderation fields use the app’s standard styling and focus behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/settings/moderation-settings.tsx` around lines 63 - 69, The moderation form still uses raw HTML inputs instead of the shared standard field component. Update the two input fields in ModerationSettings to use the shared Input from components/ui/input.tsx, keeping the existing value/onChange/placeholder behavior but moving styling and focus handling to the reusable component so both moderation fields match the app’s UI conventions.Source: Coding guidelines
package.json (1)
16-16: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPin the release-candidate SDK while token APIs are still settling.
This PR depends on new evo-sdk token/direct-purchase APIs; using
^4.0.0-rc.2can pull a newer RC/final API during lock refreshes. Consider pinning exactly until the v4 API is stable.Proposed dependency pin
- "`@dashevo/evo-sdk`": "^4.0.0-rc.2", + "`@dashevo/evo-sdk`": "4.0.0-rc.2",Please also verify with the project build before committing. As per coding guidelines, “Run build with
npm run buildto ensure TypeScript compilation succeeds and static export works correctly before committing.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 16, The dependency range for `@dashevo/evo-sdk` is too loose and can float to newer RC/final API changes during lockfile refreshes. Pin the version exactly in package.json where the evo-sdk dependency is declared, and keep the token/direct-purchase code aligned with that fixed SDK version. After updating the dependency, run the project build through npm run build to confirm TypeScript compilation and static export still succeed before committing.Source: Coding guidelines
components/post/post-card.tsx (1)
369-373: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating the repeated insufficient-token branch.
The
isInsufficientTokenError(error) ? open buy modal : toast.error(...)pattern is now duplicated here (twice) and once more incomponents/compose/compose-modal.tsx. A small shared helper (e.g.handleTokenAwareError(error, reason, fallbackMessage)inlib/error-utils.tsor auseTokenAwareErrorHandlerhook) would remove the triplication and reduce the chance of a future call site forgetting the insufficient-token check.♻️ Example consolidated helper
// lib/error-utils.ts export function handleTokenAwareError(error: unknown, reason: string, fallbackMessage: string) { if (isInsufficientTokenError(error)) { useBuyYappModal.getState().open(reason) } else { toast.error(fallbackMessage) } }- if (isInsufficientTokenError(error)) { - useBuyYappModal.getState().open('You need YAPP to like posts. Buy some to continue.') - } else { - toast.error('Failed to update like. Please try again.') - } + handleTokenAwareError(error, 'You need YAPP to like posts. Buy some to continue.', 'Failed to update like. Please try again.')Also applies to: 406-410
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/post/post-card.tsx` around lines 369 - 373, The insufficient-token handling is duplicated in PostCard and again in the compose flow, so extract the repeated `isInsufficientTokenError(error)` branch into a shared helper or hook such as `handleTokenAwareError` or `useTokenAwareErrorHandler`. Update the current `PostCard` error path and the matching call site in `ComposeModal` to use that shared utility, passing the modal reason and fallback toast message so `useBuyYappModal` and `toast.error` behavior stays consistent in one place.scripts/set-yapp-price.mjs (4)
32-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded contract/token constants duplicate
lib/constants.ts.
CONTRACT_IDandTOKEN_POSmirrorYAPPR_CONTRACT_ID/YAPP_TOKEN_POSITIONreferenced intoken-service.ts(per the upstream contract snippet). Duplicating them here risks drift if the contract is redeployed (a common occurrence pre-mainnet) and only this script is updated, or vice versa.♻️ Reuse shared constants
-const CONTRACT_ID = 'AyBNQV6qSLY8kZYHxbvhLrsDMSzWTKrWJBE6B7NYSxSh'; -const TOKEN_POS = 0; +import { YAPPR_CONTRACT_ID, YAPP_TOKEN_POSITION } from '../lib/constants.js'; +const CONTRACT_ID = YAPPR_CONTRACT_ID; +const TOKEN_POS = YAPP_TOKEN_POSITION;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/set-yapp-price.mjs` around lines 32 - 37, The hardcoded contract and token identifiers in the price-setting script duplicate shared values and can drift from the rest of the codebase. Update the script to reuse the existing constants from the shared constants module instead of defining its own CONTRACT_ID and TOKEN_POS, keeping set-yapp-price.mjs aligned with token-service.ts and the contract config.
110-125: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueFragile error detection via lowercase substring match.
Distinguishing the "unverifiable proof for a just-registered contract" case from real broadcast failures relies on
msg.includes('unknown contract'). If the SDK's error wording changes in a futureevo-sdkrelease, this script will misclassify a genuine failure as the benign case and report false success (it proceeds to read back prices without confirming the transition actually applied via a return-code check).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/set-yapp-price.mjs` around lines 110 - 125, The try/catch around sdk.stateTransitions.broadcastStateTransition and sdk.stateTransitions.waitForResponse is using a fragile lowercase substring match on the error message to detect the benign “just-registered contract” case. Replace the text-based check with a more stable signal from the evo-sdk error object or a dedicated error code/type if available, and only fall back to message inspection as a last resort. Keep the existing confirm-via-price-read path in the benign case, but ensure real broadcast failures still hit the BROADCAST ERROR path and exit.
37-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded personal file path reduces portability.
IDENTITY_FILEpoints to a fixed~/Downloads/...path tied to one developer's machine. Anyone else running this one-shot script needs to know to place the identity JSON there, or edit the source. Consider an env var or CLI arg (e.g.,process.argv[2] ?? join(homedir(), 'Downloads', ...)).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/set-yapp-price.mjs` at line 37, The hardcoded IDENTITY_FILE path in set-yapp-price.mjs ties the script to one machine’s Downloads folder and hurts portability. Update the script to accept the identity JSON location from an environment variable or CLI argument first, and keep the current joined homedir fallback only as a default. Use the existing IDENTITY_FILE constant and the script’s argument parsing/startup logic to wire this in without requiring source edits for other users.
36-36: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo runtime check that the resolved key actually has CRITICAL security level.
The comment on Line 36 states token config transitions require a CRITICAL auth key, and
securityLevelis logged (Line 54) but never validated before signing. If key id 2 on a given identity happens to be a lower security level, this fails only at broadcast/validation time with a less clear on-chain error, rather than failing fast with an actionable message. This mirrors the review-fix intoken-service.tswhere auth keys are filtered to CRITICAL/HIGH before matching.🔒 Fail fast on wrong security level
const signingKey = (identityJson.identityKeys || []).find((k) => k.id === SIGNING_KEY_ID); if (!signingKey?.privateKeyWif) throw new Error(`No privateKeyWif for key id ${SIGNING_KEY_ID}`); +if (signingKey.securityLevel !== 'CRITICAL') { + throw new Error(`Key id ${SIGNING_KEY_ID} is ${signingKey.securityLevel}, expected CRITICAL`); +}Also applies to: 52-54, 99-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/set-yapp-price.mjs` at line 36, The signing flow in set-yapp-price.mjs assumes SIGNING_KEY_ID 2 is CRITICAL, but the resolved key’s securityLevel is only logged and never enforced. Update the key lookup/signing path around the key resolution and logging logic to validate that the selected auth key is actually CRITICAL before any signing happens, and fail fast with a clear error if it is not. Mirror the token-service.ts approach by filtering or checking for CRITICAL (and only proceeding when the matched key meets that level) in the code that resolves the signer and prepares the transaction.lib/services/post-stats-helpers.ts (1)
158-173: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftPer-post fan-out: 3×N individual count-tree calls per batch.
fetchBatchPostStatsnow issuespostIds.length * 3separatedocumentCountround trips (likes/reposts/replies each queried per post). This fixes the prior 100-item undercount, but for larger feed pages this is a meaningful increase in concurrent DAPI requests compared to the old batchedin-queries.The
documentCountdoc comment itself notes grouped counts are supported (groupBy: ['postId']withpostId in [...], reading per-key entries fromsdk.documents.countdirectly). Worth exploring a grouped-count helper here to collapse each stat type into a single request across the batch, ifgroupBysemantics on the count-tree behave as documented.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/services/post-stats-helpers.ts` around lines 158 - 173, `fetchBatchPostStats` in `post-stats-helpers.ts` is doing 3×N per-post count calls via `likeService.countLikes`, `repostService.countReposts`, and `replyService.countReplies`; refactor this batch path to use a grouped-count helper with `groupBy: ['postId']` and `postId in [...]` so each stat type is fetched in one request per batch. Keep the existing per-post assignment logic that fills `result` from the returned counts, but update the lookup to read grouped counts from the new helper instead of calling the count services inside `Promise.all`.lib/services/repost-service.ts (1)
184-206: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSame
'in'+limit:100undercount risk asgetLikesByPostIds, but no caveat comment here.
like-service.ts'sgetLikesByPostIdscarries an explicit TODO noting'in'queries don't paginate reliably and can silently truncate once the batch exceeds the limit.getRepostsByPostIdshas the identical query shape (postId in postIds,limit: 100) but no equivalent note, so a future reader could miss this known SDK limitation when batches of heavily-reposted posts are queried together.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/services/repost-service.ts` around lines 184 - 206, `getRepostsByPostIds` has the same `where: [['postId', 'in', postIds]]` plus `limit: 100` truncation risk as `getLikesByPostIds`, but it lacks the warning note. Add the same caveat/TODO comment near `RepostService.getRepostsByPostIds` so readers know the SDK’s `'in'` query may silently undercount when the batch exceeds the limit, matching the guidance already present in `like-service.ts`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/settings/moderation-settings.tsx`:
- Around line 61-70: The form fields in moderation-settings.tsx have unlabeled
inputs because the <label> elements are not associated with their corresponding
<input> elements. Update the input groups in this component so each label/input
pair is linked with matching htmlFor and id values, using the existing fields
like targetId and the other related inputs in this section. Ensure each label
text remains tied to its specific input for accessibility and screen readers.
In `@lib/services/token-service.ts`:
- Around line 238-239: Fix the under-minimum matcher typo in token-service’s
minimum purchase check. In the logic that normalizes the error string and
matches against `undeminimum`, `under minimum`, and `minimum sale`, update the
compact variant so it correctly matches `underminimum` (or equivalent normalized
form) and still returns the `BELOW_MINIMUM` response from this branch instead of
falling through to `NETWORK_ERROR`.
In `@scripts/set-yapp-price.mjs`:
- Line 84: The SetPrices call in the price-setting script is converting the
price to a Number even though TokenPricingSchedule.SetPrices expects bigint
values. Update the schedule construction to pass CREDITS_PER_TOKEN directly in
the SetPrices mapping, and keep the MIN_PURCHASE key conversion as-is so the
price stays precise in set-yapp-price.mjs.
---
Outside diff comments:
In `@app/settings/page.tsx`:
- Around line 750-754: The header title logic in getSectionTitle does not handle
the moderation section because it only looks up settingsSections, so
activeSection values for MODERATION_SECTION fall back to “Settings.” Update
getSectionTitle to also recognize the moderation section by checking the same
moderation identifier used in renderMainSettings and returning the proper
“Moderation” label before falling back to the existing settingsSections lookup.
In `@lib/services/reply-service.ts`:
- Around line 354-366: The `countReplies` method in `reply-service.ts` is
swallowing errors silently, unlike the sibling count methods. Update the `catch`
block in `countReplies` to log the failure with `logger.error(...)` before
returning 0, using the same pattern as `countFollowers`, `countFollowing`,
`countLikes`, and `countReposts` so failures in `getEvoSdk` or `documentCount`
are visible.
---
Nitpick comments:
In `@components/post/post-card.tsx`:
- Around line 369-373: The insufficient-token handling is duplicated in PostCard
and again in the compose flow, so extract the repeated
`isInsufficientTokenError(error)` branch into a shared helper or hook such as
`handleTokenAwareError` or `useTokenAwareErrorHandler`. Update the current
`PostCard` error path and the matching call site in `ComposeModal` to use that
shared utility, passing the modal reason and fallback toast message so
`useBuyYappModal` and `toast.error` behavior stays consistent in one place.
In `@components/settings/moderation-settings.tsx`:
- Around line 22-49: The moderation action flow in run() leaves targetId and
note filled after a successful tokenService call, so clear both inputs only when
result.success is true. Update the ModerationSettings component logic around
run(), using the existing setBusy/result handling, to reset the form state after
the success toast before returning.
- Around line 63-69: The moderation form still uses raw HTML inputs instead of
the shared standard field component. Update the two input fields in
ModerationSettings to use the shared Input from components/ui/input.tsx, keeping
the existing value/onChange/placeholder behavior but moving styling and focus
handling to the reusable component so both moderation fields match the app’s UI
conventions.
In `@lib/services/post-stats-helpers.ts`:
- Around line 158-173: `fetchBatchPostStats` in `post-stats-helpers.ts` is doing
3×N per-post count calls via `likeService.countLikes`,
`repostService.countReposts`, and `replyService.countReplies`; refactor this
batch path to use a grouped-count helper with `groupBy: ['postId']` and `postId
in [...]` so each stat type is fetched in one request per batch. Keep the
existing per-post assignment logic that fills `result` from the returned counts,
but update the lookup to read grouped counts from the new helper instead of
calling the count services inside `Promise.all`.
In `@lib/services/repost-service.ts`:
- Around line 184-206: `getRepostsByPostIds` has the same `where: [['postId',
'in', postIds]]` plus `limit: 100` truncation risk as `getLikesByPostIds`, but
it lacks the warning note. Add the same caveat/TODO comment near
`RepostService.getRepostsByPostIds` so readers know the SDK’s `'in'` query may
silently undercount when the batch exceeds the limit, matching the guidance
already present in `like-service.ts`.
In `@package.json`:
- Line 16: The dependency range for `@dashevo/evo-sdk` is too loose and can float
to newer RC/final API changes during lockfile refreshes. Pin the version exactly
in package.json where the evo-sdk dependency is declared, and keep the
token/direct-purchase code aligned with that fixed SDK version. After updating
the dependency, run the project build through npm run build to confirm
TypeScript compilation and static export still succeed before committing.
In `@scripts/set-yapp-price.mjs`:
- Around line 32-37: The hardcoded contract and token identifiers in the
price-setting script duplicate shared values and can drift from the rest of the
codebase. Update the script to reuse the existing constants from the shared
constants module instead of defining its own CONTRACT_ID and TOKEN_POS, keeping
set-yapp-price.mjs aligned with token-service.ts and the contract config.
- Around line 110-125: The try/catch around
sdk.stateTransitions.broadcastStateTransition and
sdk.stateTransitions.waitForResponse is using a fragile lowercase substring
match on the error message to detect the benign “just-registered contract” case.
Replace the text-based check with a more stable signal from the evo-sdk error
object or a dedicated error code/type if available, and only fall back to
message inspection as a last resort. Keep the existing confirm-via-price-read
path in the benign case, but ensure real broadcast failures still hit the
BROADCAST ERROR path and exit.
- Line 37: The hardcoded IDENTITY_FILE path in set-yapp-price.mjs ties the
script to one machine’s Downloads folder and hurts portability. Update the
script to accept the identity JSON location from an environment variable or CLI
argument first, and keep the current joined homedir fallback only as a default.
Use the existing IDENTITY_FILE constant and the script’s argument
parsing/startup logic to wire this in without requiring source edits for other
users.
- Line 36: The signing flow in set-yapp-price.mjs assumes SIGNING_KEY_ID 2 is
CRITICAL, but the resolved key’s securityLevel is only logged and never
enforced. Update the key lookup/signing path around the key resolution and
logging logic to validate that the selected auth key is actually CRITICAL before
any signing happens, and fail fast with a clear error if it is not. Mirror the
token-service.ts approach by filtering or checking for CRITICAL (and only
proceeding when the matched key meets that level) in the code that resolves the
signer and prepares the transaction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5161e7e0-ff6a-4b84-b95b-880820e79427
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (29)
app/hashtag/page.tsxapp/settings/page.tsxcomponents/compose/compose-modal.tsxcomponents/feed/feed-post-list.tsxcomponents/layout/sidebar.tsxcomponents/post/post-card.tsxcomponents/providers.tsxcomponents/settings/moderation-settings.tsxcomponents/token/buy-yapp-modal.tsxcomponents/token/yapp-balance-item.tsxcomponents/ui/legacy-yappr-link.tsxcomponents/ui/loading-state.tsxcontracts/yappr-social-contract-v2.jsonhooks/use-buy-yapp-modal.tslib/constants.tslib/dash-platform-client.tslib/error-utils.tslib/services/follow-service.tslib/services/like-service.tslib/services/pagination-utils.tslib/services/post-service.tslib/services/post-stats-helpers.tslib/services/reply-service.tslib/services/repost-service.tslib/services/state-transition-service.tslib/services/token-service.tspackage.jsonscripts/set-yapp-price.mjstypes/sdk.ts
💤 Files with no reviewable changes (3)
- types/sdk.ts
- lib/dash-platform-client.ts
- lib/services/post-service.ts
| }); | ||
| console.log('built TokenBaseTransition'); | ||
|
|
||
| const schedule = TokenPricingSchedule.SetPrices({ [MIN_PURCHASE.toString()]: Number(CREDITS_PER_TOKEN) }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
@dashevo/evo-sdk TokenPricingSchedule.SetPrices value type BigInt or Number
💡 Result:
In the @dashevo/evo-sdk, the values used within the TokenPricingSchedule.SetPrices structure are expected to be of type BigInt [1]. The underlying protocol implementation (wasm-dpp) represents these prices as credit amounts, which are inherently 64-bit integers [1]. While some internal components or lenient parsing paths might accept other types like Number or String for convenience during JS-level input, relying on these can lead to truncation, precision loss, or runtime errors [1]. Specifically, passing a JavaScript Number greater than 2^53-1 will cause precision issues, as Number types cannot safely represent the full range of 64-bit integers [1]. To ensure compatibility with the protocol's expected BigInt values and to avoid issues with silent truncation or type-checking failures, you should use BigInt for all credit price values in your price tiers [1]. If you are working with TypeScript, the SDK expects these values to be mapped as a Record<string, bigint> [1].
Citations:
Pass the schedule price as bigint
TokenPricingSchedule.SetPrices expects bigint values here, so Number(CREDITS_PER_TOKEN) is unnecessary and can lose precision if this value grows. Use CREDITS_PER_TOKEN directly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/set-yapp-price.mjs` at line 84, The SetPrices call in the
price-setting script is converting the price to a Number even though
TokenPricingSchedule.SetPrices expects bigint values. Update the schedule
construction to pass CREDITS_PER_TOKEN directly in the SetPrices mapping, and
keep the MIN_PURCHASE key conversion as-is so the price stays precise in
set-yapp-price.mjs.
…typo) - settings header title now includes the moderation section (was falling back to 'Settings') - fix 'underminimum' matcher typo so compact under-minimum errors classify correctly - associate moderation form labels with inputs via htmlFor/id - set-yapp-price: keep Number for SetPrices (the wasm binding rejects BigInt) but guard against the >2^53 precision cliff Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks @coderabbitai — addressed in
Build + lint pass. |
|
Tip For best results, initiate chat on the files or code changes.
Nice work getting this across the finish line through two review rounds. (ᵔᴥᵔ) glad this one's wrapped up ✏️ Learnings added
|
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Carried-forward: prior-1 (stale JSDoc on getPricePerToken at lib/services/token-service.ts:65-68) is STILL VALID at head 29db286 — the comment still misattributes the maxTotalCost staleness protection to getPricePerToken, while the actual guarantee is enforced by buyYapp's caller-supplied cap (lines 92-98). New in latest delta: no new defects — the four small changes (settings section-title lookup, moderation-settings a11y htmlFor/id pairing, 'undeminimum'→'underminimum' matcher typo fix, and safe-integer guard in scripts/set-yapp-price.mjs) are all correct in-scope fixes to the CodeRabbit round-2 nits. All three agents (codex, opus, sonnet5) converge on the same single carried-forward nit.
💬 1 nitpick(s)
Carried-forward findings already raised (1)
These findings were not re-posted as new inline comments because an existing review thread already covers them.
- [NITPICK] (deduped existing open thread)
lib/services/token-service.ts:65-68: Stale doc comment on getPricePerToken after the maxTotalCost fix — The JSDoc ongetPricePerTokenstill reads: "Fetched fresh each call somaxTotalCostnever uses a stale price." That was accurate whenbuyYappcalledgetPricePerToken()internally right before signing, but the safety property has since moved:buyYapp(buyerId, amount, maxTotalCost)at lin...
Correctness: surface tip announcement-reply failures for 0-YAPP tippers (#7); user like/repost state now queries the user's own docs via composite indexes instead of capped all-user queries (#8); getBalance no longer swallows failures to 0 (#11); broaden insufficient-credits error classification and stop misclassifying network/proof errors as bad-key (#9,#10); gate the settings Moderation title by isAuthority (#12); bound the per-post count fan-out concurrency (#1); log missing count-tree totals (#13); log invalid repost postIds (#4). Cleanup: extract shared handleInsufficientYapp helper (was copy-pasted 3x, #5); remove dead quoteCost (#6) and dead reply-batch methods (#14). Deferred: cross-service key-matching dedup (#2) — a broad refactor of pre-existing signing code, out of scope. Grouped-count optimization for #1 deferred until the hex group-key layout can be verified against live data. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fetchBatchPostStats fired 3xN individual count-tree calls per feed page (one per like/repost/reply per post), previously only bounded to 6-concurrent rather than fixed. Replace with a single grouped sdk.documents.count({ groupBy }) call per stat type, decoding the SDK's hex-keyed grouped response back to base58 postIds via the new identifierToHex helper.
Falls back to the proven per-post reads if the grouped response doesn't decode against expected keys (unverified against live testnet data), so a wrong hex-encoding assumption degrades to the old behavior instead of silently reporting zero counts.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/services/tip-service.ts (1)
332-351: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGeneric error swallowing loses failure-cause detail consumed by the UI.
createTipPostcatches all errors uniformly and returnsfalse, so callers (e.g.,tip-modal.tsx) cannot distinguish "insufficient YAPP" from other failures (network, validation). The UI currently hardcodes a YAPP-specific message for anyfalseresult — see companion comment incomponents/post/tip-modal.tsx(lines 208-214).If precise user messaging matters, consider returning/propagating a reason (e.g., an error code or message) alongside the boolean instead of only logging it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/services/tip-service.ts` around lines 332 - 351, createTipPost in tip-service.ts is swallowing all failures by logging and returning only false, which prevents callers like tip-modal.tsx from telling insufficient YAPP apart from network or validation errors. Update createTipPost to propagate a reason along with the success state, such as an error code/message or a typed result, and preserve the original error details from the catch block instead of collapsing everything to a generic boolean. Keep the existing flow in replyService.createReply and the logger.error path, but make the return value from createTipPost specific enough for the UI to choose the correct user-facing message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/post/tip-modal.tsx`:
- Around line 208-214: The toast message in `tip-modal` is assuming the public
tip note failed because of YAPP, but `createTipPost` in `tip-service` returns
`announcementPosted === false` for any error. Update the messaging in the tip
flow to avoid naming a specific cause unless it is explicitly known, and adjust
`createTipPost` to surface a distinguishable failure reason if you want precise
user-facing wording. Use the `announcementPosted` check in `TipModal` and the
generic catch in `createTipPost` as the main points to revise.
In `@lib/services/like-service.ts`:
- Around line 185-195: getUserLikedPostIds in like-service.ts is limited by the
fixed sdk.documents.query limit of 100, so requests with more than 100 postIds
can miss matches. Update this method to process postIds in chunks of 100 (or
smaller), run the query for each chunk using the same userId and contractId
filters, and merge the results into the returned Set<string> so all liked posts
are captured.
In `@lib/services/repost-service.ts`:
- Around line 177-187: The getUserRepostedPostIds helper is artificially capped
at 100 IDs, which can miss reposts for larger input batches. Update this method
to chunk the postIds array and query each chunk separately, then merge the
results into the returned Set<string>. Keep the existing query shape in
RepostService.sdk()/documents.query and preserve the early return for empty
input.
---
Outside diff comments:
In `@lib/services/tip-service.ts`:
- Around line 332-351: createTipPost in tip-service.ts is swallowing all
failures by logging and returning only false, which prevents callers like
tip-modal.tsx from telling insufficient YAPP apart from network or validation
errors. Update createTipPost to propagate a reason along with the success state,
such as an error code/message or a typed result, and preserve the original error
details from the catch block instead of collapsing everything to a generic
boolean. Keep the existing flow in replyService.createReply and the logger.error
path, but make the return value from createTipPost specific enough for the UI to
choose the correct user-facing message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4148c772-b026-4b5b-8cc8-d0cb054730d1
📒 Files selected for processing (15)
app/settings/page.tsxcomponents/compose/compose-modal.tsxcomponents/post/post-card.tsxcomponents/post/tip-modal.tsxcomponents/settings/moderation-settings.tsxhooks/use-buy-yapp-modal.tslib/services/like-service.tslib/services/pagination-utils.tslib/services/post-stats-helpers.tslib/services/reply-service.tslib/services/repost-service.tslib/services/sdk-helpers.tslib/services/tip-service.tslib/services/token-service.tsscripts/set-yapp-price.mjs
🚧 Files skipped from review as they are similar to previous changes (3)
- app/settings/page.tsx
- components/settings/moderation-settings.tsx
- scripts/set-yapp-price.mjs
There was a problem hiding this comment.
Code Review
Prior finding prior-1 is STILL VALID and carried forward: the JSDoc on getPricePerToken still misattributes the maxTotalCost overspend protection, which now lives in buyYapp's caller-supplied cap. New in latest delta: (a) partial thread-post failures don't route through the new handleInsufficientYapp prompt, and (b) the tip modal's new announcement-failure toast hard-codes '(needs YAPP)' but createTipPost returns false for any thrown error. The Codex retry-cache concern is a real design weakness that this PR amplifies by adding token cost to create-paths, but the caching logic itself pre-exists this PR (introduced in #260) and the most common timeout path (waitForResponse timeout) returns optimistic success without retry, so it's downgraded to a suggestion. Sonnet5's getUserLikedPostIds index-order blocker is not confirmable: the identical postId in [...] + $ownerId == pattern is used in the pre-existing getLike (feed page) and in the shipped batch fetch, so if the pattern silently failed likes would already be broken in production — dropping as speculative.
🟡 3 suggestion(s) | 💬 1 nitpick(s)
Findings not posted inline (2)
These findings could not be anchored to the current diff, but they are still part of this review.
- [SUGGESTION]
components/compose/compose-modal.tsx:888-893: Thread partial failures bypass the new Buy-YAPP prompt — The new insufficient-YAPP handling only runs in the outer catch at line 906 —handleInsufficientYapp(error, ...). That path fires when the FIRST post fails. In the partial-success branch here, a thread has created one or more posts and then failed on a later token-paid reply; if that later failure is due to insufficient YAPP, the code only shows a generictoast.error(...failed: ${errorMsg}. Press Post to retry.)and never opens the Buy-YAPP modal. That leaves the user without the intended recovery path even though the PR's stated goal is surfacing insufficient-balance prompts across post/reply flows. Threading `handleInsufficientYapp(fail... - [SUGGESTION]
lib/services/state-transition-service.ts:350-413: Retry cache miss on regenerated document id — duplicate token-paid creates possible on broadcast-error retry — The pending-ST cache is keyed bydocumentId, but for normal post/reply/like/repost creates callers do not passoptions.documentIdoroptions.entropy, sodocumentBuilderService.buildDocumentForCreate(line 350) generates fresh entropy and a fresh id on every call. IfretryPostCreationre-invokescreateDocumentafter abroadcastStateTransitionthrows a retryable error (line 501), the second attempt computes a differentdocumentId, misses the cache load at line 364, and signs a brand-new ST with a new nonce. If the original broadcast actually landed on Platform despite the thrown error, the retry creates a duplicate document — an...
Carried-forward findings already raised (1)
These findings were not re-posted as new inline comments because an existing review thread already covers them.
- [NITPICK] (deduped existing open thread)
lib/services/token-service.ts:64-67: Stale doc comment on getPricePerToken after the maxTotalCost fix (carried forward from prior-1) — STILL VALID from prior-1 at head 5efbafa. The JSDoc ongetPricePerTokenstill reads that it is fetched fresh each call somaxTotalCostnever uses a stale price. That was accurate whenbuyYappfetched the price internally right before signing, but the safety property was moved:buyYapp(buyerId, amount, maxTotalCost)(lines 84-113) now takes the cap from the caller — the UI-confirmed quote — and its JSDoc correctly explains that a mid-flight price bump is rejected on-chain because the signed cap is the one the user approved. Leaving this comment risks misleading a future caller into thinking a fresh call here alone is what keeps purcha...
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `components/post/tip-modal.tsx`:
- [SUGGESTION] components/post/tip-modal.tsx:211-213: Tip-announcement failure toast blames YAPP for every failure mode
The new toast renders `Tip sent — but the public tip note couldn't be posted (needs YAPP).` whenever `result.announcementPosted === false`. But `TipService.createTipPost` (lib/services/tip-service.ts:326-351) sets that flag to `false` on *any* thrown error — the try/catch swallows everything from `replyService.createReply`, not just insufficient-YAPP failures. A DAPI timeout, signer error, or any other network/contract-level rejection during the announcement reply will surface as a false claim that the user needs YAPP when they may hold plenty, which is misleading and unactionable. The tip-service already has `isInsufficientTokenError` — either narrow `announcementPosted:false` to insufficient-YAPP failures only (returning true on other errors since the tip transfer itself succeeded and the announcement is best-effort), or thread an `announcementError` code back up and let the modal pick the right toast wording. Minor because the money-path (credit transfer) is unaffected, but the wording is user-facing on a paid flow.
In `components/compose/compose-modal.tsx`:
- [SUGGESTION] components/compose/compose-modal.tsx:888-893: Thread partial failures bypass the new Buy-YAPP prompt
The new insufficient-YAPP handling only runs in the outer catch at line 906 — `handleInsufficientYapp(error, ...)`. That path fires when the FIRST post fails. In the partial-success branch here, a thread has created one or more posts and then failed on a later token-paid reply; if that later failure is due to insufficient YAPP, the code only shows a generic `toast.error(...failed: ${errorMsg}. Press Post to retry.)` and never opens the Buy-YAPP modal. That leaves the user without the intended recovery path even though the PR's stated goal is surfacing insufficient-balance prompts across post/reply flows. Threading `handleInsufficientYapp(failureError, 'You need YAPP to finish posting this thread. Buy some to continue.')` into this branch (falling through to the current toast when it doesn't match) would cover the partial-thread case without changing the retry semantics.
In `lib/services/state-transition-service.ts`:
- [SUGGESTION] lib/services/state-transition-service.ts:350-413: Retry cache miss on regenerated document id — duplicate token-paid creates possible on broadcast-error retry
The pending-ST cache is keyed by `documentId`, but for normal post/reply/like/repost creates callers do not pass `options.documentId` or `options.entropy`, so `documentBuilderService.buildDocumentForCreate` (line 350) generates fresh entropy and a fresh id on every call. If `retryPostCreation` re-invokes `createDocument` after a `broadcastStateTransition` throws a retryable error (line 501), the second attempt computes a different `documentId`, misses the cache load at line 364, and signs a brand-new ST with a new nonce. If the original broadcast actually landed on Platform despite the thrown error, the retry creates a duplicate document — and under this PR's v4 tokenomics each duplicate now costs YAPP twice (posts, replies, likes, and reposts all have `tokenCost.create`). The caching design itself pre-exists this PR (introduced in #260 `fix: prevent double-posting on DAPI timeout`), so this is not a new bug — and the more common `waitForResponse` timeout path returns optimistic `success:true, confirmed:false` without throwing (lines 518-538), which means retries only fire on rarer broadcast-side errors. Still, this PR meaningfully raises the cost of the failure mode (duplicate content + double YAPP debit), so it's worth revisiting: either persist the entropy into the retry closure (compute `entropy` once in the caller and pass it via `options.entropy` on every attempt), or key the cache on a stable per-operation identity (contract+type+ownerId+contentHash) rather than the fresh docId. If out of scope, at minimum consider disabling `retryPostCreation`'s auto-retry for token-paid creates and letting the outer UI drive a user-initiated retry through the cache path.
In `lib/services/token-service.ts`:
- [NITPICK] lib/services/token-service.ts:64-67: Stale doc comment on getPricePerToken after the maxTotalCost fix (carried forward from prior-1)
STILL VALID from prior-1 at head 5efbafa8. The JSDoc on `getPricePerToken` still reads that it is fetched fresh each call so `maxTotalCost` never uses a stale price. That was accurate when `buyYapp` fetched the price internally right before signing, but the safety property was moved: `buyYapp(buyerId, amount, maxTotalCost)` (lines 84-113) now takes the cap from the caller — the UI-confirmed quote — and its JSDoc correctly explains that a mid-flight price bump is rejected on-chain because the signed cap is the one the user approved. Leaving this comment risks misleading a future caller into thinking a fresh call here alone is what keeps purchases safe; a new call site that skips threading a user-confirmed cap through `buyYapp` would reintroduce the original overspend window. The latest delta touched other parts of this file (getBalance rework at 57-62, the INSUFFICIENT_CREDITS/INVALID_KEY matcher refinements) but did not touch lines 64-67.
…al opens The on-chain error reads 'does not have enough balance for token X: required N, actual 0' which none of the isInsufficientTokenError patterns matched, so failed posts showed the raw broadcast error instead of the Buy-YAPP prompt. Also route mid-thread partial failures through the same handler. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds a 'See action costs' disclosure listing the per-action YAPP prices from the contract (post 10, reply 3, like/repost 1) and a dynamic 'covers about N posts, M replies, or K likes' line under the cost on both the input and confirm steps, so buyers know how long a bundle lasts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…y is HIGH Drive rejects token direct purchases signed with a HIGH auth key (Invalid public key security level HIGH, requires CRITICAL). buyYapp now matches the stored WIF against CRITICAL keys only and returns NEEDS_CRITICAL_KEY when it can't; the Buy-YAPP modal then shows an Authorize Purchase step where the user pastes their CRITICAL key, which signs locally and is never stored. The key is cleared on close, back, success, and any exit from the authorize flow so a retry can never silently reuse it. Freeze/unfreeze authority actions keep their existing CRITICAL|HIGH matching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| "@blocknote/mantine": "^0.47.0", | ||
| "@blocknote/react": "^0.47.0", | ||
| "@dashevo/evo-sdk": "3.1.0-dev.8", | ||
| "@dashevo/evo-sdk": "^4.0.0-rc.2", |
There was a problem hiding this comment.
Use actual release version. And pin (so you don't have to worry about breaking changes in a patch version or smth)?
| "@dashevo/evo-sdk": "^4.0.0-rc.2", | |
| "@dashevo/evo-sdk": "4.0.0", |
…p warnings Reset and stale-guard the sidebar YAPP balance on identity switch; chunk per-user like/repost/bookmark lookups and grouped counts to respect Platform's 100-value in-clause cap; rethrow from identityService.getBalance so fetch failures aren't treated as a zero balance (call sites hardened); return structured announcement-failure detail from sendTip so the tip toast only blames missing YAPP when that was the cause. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/services/repost-service.ts (1)
231-253: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid a single 100-row
byPostquery here.load-for-you-feedonly passes 20 IDs, so chunking bypostIdswon’t help. Thelimit: 100on this non-unique query can still drop reposts for a popular batch, which leaves some posts withoutrepostedBydata. Fetch one/latest repost per post instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/services/repost-service.ts` around lines 231 - 253, The current getRepostsByPostIds batch lookup in RepostService uses a single non-unique byPost query with a hard limit of 100, which can omit reposts for some postIds. Update getRepostsByPostIds to fetch one/latest repost per postId instead of relying on the batch limit, and keep the existing mapping/normalizeSDKResponse flow inside the method so every requested post gets its repostedBy data when available.components/token/buy-yapp-modal.tsx (1)
108-134: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard
handleBuyagainst double-submission.
handleBuyhas no re-entrancy check, and the "Confirm & Buy" (line 283) and "Authorize & Buy" (line 324) buttons aren't disabled while an invocation is in-flight — onlystate === 'processing'removes them from the DOM after a re-render. A double-click or rapid repeated Enter before that re-render commits could firehandleBuytwice, potentially signing and broadcasting two separatedirectPurchasetransitions for real DASH credits. This is unlikely in practice (React typically flushes the state update before the next click registers) but the fix is trivial and the downside (duplicate spend) is high-impact enough to be worth a defensive guard.🛡️ Proposed fix
const handleBuy = async () => { - if (!user || costCredits === null) return + if (!user || costCredits === null || state === 'processing') return setState('processing')Also applies to: 268-286, 288-329
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@lib/services/repost-service.ts`:
- Around line 231-253: The current getRepostsByPostIds batch lookup in
RepostService uses a single non-unique byPost query with a hard limit of 100,
which can omit reposts for some postIds. Update getRepostsByPostIds to fetch
one/latest repost per postId instead of relying on the batch limit, and keep the
existing mapping/normalizeSDKResponse flow inside the method so every requested
post gets its repostedBy data when available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 08f985df-c618-4174-9e33-cd8a49e18abc
📒 Files selected for processing (13)
components/compose/compose-modal.tsxcomponents/layout/sidebar.tsxcomponents/post/tip-modal.tsxcomponents/token/buy-yapp-modal.tsxcomponents/token/yapp-balance-item.tsxlib/error-utils.tslib/services/bookmark-service.tslib/services/identity-service.tslib/services/like-service.tslib/services/pagination-utils.tslib/services/repost-service.tslib/services/tip-service.tslib/services/token-service.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- lib/error-utils.ts
- components/post/tip-modal.tsx
- components/token/yapp-balance-item.tsx
- lib/services/like-service.ts
- lib/services/pagination-utils.ts
- lib/services/token-service.ts
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Latest delta (5efbafa..1248873) cleanly resolves prior-2 and prior-3: tip-modal now distinguishes INSUFFICIENT_YAPP from generic announcement failures, and compose-modal routes thread partial-failures through the Buy-YAPP prompt. New CRITICAL-key entry flow for YAPP purchases is well-scoped. Only carry-forward: prior-1 stale JSDoc on getPricePerToken (nitpick). Prior-4 (retry cache regenerates documentId) remains an intentionally deferred pre-existing design.
Source: reviewers: opus (general), claude-sonnet-5 (general), gpt-5.5[high] (general, failed); verifier: opus
💬 1 nitpick(s)
| /** | ||
| * Price in credits to buy one YAPP (the SetPrices tier value). Null if not for | ||
| * sale. Fetched fresh each call so `maxTotalCost` never uses a stale price. | ||
| */ |
There was a problem hiding this comment.
💬 Nitpick: Stale JSDoc on getPricePerToken misattributes maxTotalCost overspend protection
Carried forward from prior-1 — unchanged at head 1248873. The comment reads that getPricePerToken is fetched fresh each call "so maxTotalCost never uses a stale price." That was true when buyYapp fetched the price internally, but the safety property now lives in buyYapp(buyerId, amount, maxTotalCost, criticalKeyWif?) (lines 95-121): the cap is caller-supplied (the UI-confirmed quote) and enforced on-chain, and its JSDoc at lines 84-94 correctly documents this. Leaving the stale note here risks misleading a future caller into assuming a fresh call to getPricePerToken alone is what keeps purchases safe; a new call site that skipped threading a user-confirmed cap through buyYapp would reintroduce the overspend window. The rest of this file was touched substantially (getBalance rethrow, NEEDS_CRITICAL_KEY marker, getAuthSigner overrideWif) but lines 64-67 were not.
| /** | |
| * Price in credits to buy one YAPP (the SetPrices tier value). Null if not for | |
| * sale. Fetched fresh each call so `maxTotalCost` never uses a stale price. | |
| */ | |
| /** | |
| * Price in credits to buy one YAPP (the SetPrices tier value). Null if not for | |
| * sale. Not cached, since the price can change at any time. Overspend | |
| * protection lives in `buyYapp`, which enforces the caller-supplied | |
| * `maxTotalCost` cap on-chain; callers holding a quote across a user | |
| * confirmation should re-quote if the confirmation window may be long. | |
| */ |
source: ['claude', 'codex']
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Carried-forward: prior-1 (stale JSDoc on getPricePerToken at lib/services/token-service.ts:64-67) is STILL VALID — the comment still credits freshness of getPricePerToken with overspend protection, when the actual cap enforcement lives in buyYapp via caller-supplied maxTotalCost. New in latest delta (bf695e1): sonnet5 flagged the void Promise.all(...) added at components/token/buy-yapp-modal.tsx:60 to silence @typescript-eslint/no-floating-promises — this pattern is explicitly listed as prohibited in the project's CLAUDE.md, which requires a proper .catch() instead. Both are suggestion-level; no blockers.
Source: reviewers: opus (general), claude-sonnet-5 (general), gpt-5.5[high] (general, failed); verifier: opus
🟡 1 suggestion(s) | 💬 1 nitpick(s)
Carried-forward findings already raised (1)
These findings were not re-posted as new inline comments because an existing review thread already covers them.
- [NITPICK] (deduped existing open thread)
lib/services/token-service.ts:64-67: Stale JSDoc on getPricePerToken misattributes maxTotalCost overspend protection — Carried forward from prior review — lines 64-67 are unchanged at head bf695e1. The comment reads thatgetPricePerTokenis fetched fresh each call "somaxTotalCostnever uses a stale price." That framing is misleading: the actual overspend protection lives in `buyYapp(buyerId, amount, maxTota...
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `components/token/buy-yapp-modal.tsx`:
- [SUGGESTION] components/token/buy-yapp-modal.tsx:60-64: `void` prefix silences floating-promise lint in a way the project's CLAUDE.md prohibits
The mount effect now uses `void Promise.all([...]).finally(...)` to silence the `@typescript-eslint/no-floating-promises` warn-level rule. The project's own CLAUDE.md explicitly forbids this suppression pattern — it lists `void someAsyncFn()` among the things NOT to do, and directs contributors to `add proper .catch() error handling` instead. Each inner promise already has its own `.catch()`, so the outer chain just needs `.catch(() => {})` (or restructuring as an async IIFE with a catch) to satisfy the rule without using the disallowed shortcut. Small change, but the guideline is explicit enough that it's worth honoring here so future contributors don't take this as precedent.
| void Promise.all([ | ||
| tokenService.getBalance(user.identityId).then(setYappBalance).catch(() => setYappBalance(null)), | ||
| identityService.getBalance(user.identityId).then(b => setCreditBalance(b.confirmed)).catch(() => setCreditBalance(null)), | ||
| tokenService.getPricePerToken().then(setPricePerToken).catch(() => setPricePerToken(null)), | ||
| ]).finally(() => setLoading(false)) |
There was a problem hiding this comment.
🟡 Suggestion: void prefix silences floating-promise lint in a way the project's CLAUDE.md prohibits
The mount effect now uses void Promise.all([...]).finally(...) to silence the @typescript-eslint/no-floating-promises warn-level rule. The project's own CLAUDE.md explicitly forbids this suppression pattern — it lists void someAsyncFn() among the things NOT to do, and directs contributors to add proper .catch() error handling instead. Each inner promise already has its own .catch(), so the outer chain just needs .catch(() => {}) (or restructuring as an async IIFE with a catch) to satisfy the rule without using the disallowed shortcut. Small change, but the guideline is explicit enough that it's worth honoring here so future contributors don't take this as precedent.
| void Promise.all([ | |
| tokenService.getBalance(user.identityId).then(setYappBalance).catch(() => setYappBalance(null)), | |
| identityService.getBalance(user.identityId).then(b => setCreditBalance(b.confirmed)).catch(() => setCreditBalance(null)), | |
| tokenService.getPricePerToken().then(setPricePerToken).catch(() => setPricePerToken(null)), | |
| ]).finally(() => setLoading(false)) | |
| Promise.all([ | |
| tokenService.getBalance(user.identityId).then(setYappBalance).catch(() => setYappBalance(null)), | |
| identityService.getBalance(user.identityId).then(b => setCreditBalance(b.confirmed)).catch(() => setCreditBalance(null)), | |
| tokenService.getPricePerToken().then(setPricePerToken).catch(() => setPricePerToken(null)), | |
| ]).finally(() => setLoading(false)).catch(() => {}) |
source: ['claude']
3 participants
Summary
Adopts Dash Platform v4 (protocol v12) for Yappr: count trees (O(1) proof-backed counts) and a native YAPP token with pay-per-action pricing, a minimum bulk buy-in bond (anti-spam), and freeze/slash moderation. Requires a cutover to a new v12 social contract.
New social contract (testnet):
AyBNQV6qSLY8kZYHxbvhLrsDMSzWTKrWJBE6B7NYSxSh— count-tree indexes, dedicatedrepostdoctype, YAPP token (owner-controlled),tokenCostper action. Contract validated end-to-end via the real DPP deserializer + a full audit of every app query against the indexes.What's included
Counts → count trees
like/reply/follower/following/repostcounts now usesdk.documents.count(O(1)) instead of O(n) pagination (also fixes the old 100-cap reply undercount).Tokenomics
post/reply/like/repost(central instate-transition-service.createDocument), guarded bymaximumTokenCost.token-service.ts: balance, price,buyYapp(directPurchase), and authorityfreeze/unfreeze/destroyFrozen.SetPrices, set viascripts/set-yapp-price.mjssince the JS facade is flat-only).UX
LegacyYapprLink) on empty / end-of-feed states → sends users to the previous deployment for pre-cutover content.Reposts
repostdoctype (cheapertokenCost,byPostcount tree,postOwnerAndTimenotification index).Other
@dashevo/evo-sdk→4.0.0-rc.2.primaryHashtag/firstMentionIdpost writes that the v2 schema'sadditionalProperties:falsewould reject.Notes
YAPPR_PROFILE_CONTRACT_ID) untouched by this cutover.Testing
Build + lint pass. End-to-end manual testing on testnet is still pending (post→YAPP debit, counts, buy <100 rejected / ≥100 works, freeze blocks posting, slash burns). See the QA plan discussed in the working notes.
🤖 Generated with Claude Code
Summary by CodeRabbit