Markee buy-flow modal on Explore#338
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Gaston Review
Verdict: Approved
Score: ███████░░░ 7/10
Pull Request Summary
This PR replaces the old Markee banner (a link to markee.xyz) with a full in-app buy/boost modal. It moves data fetching from server-side (which blocked page load in Promise.all) to client-side, adds three proxy API routes to markee.xyz (leaderboards, moderation, views), introduces a MarkeeSign component for the explore page sign, and a MarkeeModal with buy and boost tabs backed by on-chain transactions via wagmi.
Review Summary
🟡 Warning — Health endpoint is a hardcoded lie
The /api/markee/health route always returns { overall: "ok" } with three sub-checks all returning ok, regardless of whether markee.xyz is actually reachable. If this is consumed by monitoring, it will mask real outages. Either remove it, make it a static endpoint that doesn't pretend to check things, or actually probe the upstream services.
🔵 Suggestion — Duplicate moderation fetch between MarkeeSign and MarkeeModal
Both MarkeeSign and MarkeeModal independently fetch /api/markee/moderation on mount and maintain separate flagged state. The moderation data could be fetched once in MarkeeSign and passed as a prop to MarkeeModal, avoiding the redundant request when the dialog opens.
🟡 Warning — Views POST proxy is an open relay with no rate limiting
The /api/markee/views endpoint accepts any POST with an address and message field and forwards it to markee.xyz. There's no authentication, no CSRF protection, no rate limiting. Anyone can call this endpoint to inflate view counts for arbitrary markee addresses. If markee.xyz doesn't rate-limit by IP, this could be abused trivially.
The overall direction is good — moving from a blocking server-side fetch to a client-side flow with a richer UX. Code is well-structured with proper error handling throughout. I have a few concerns about the health endpoint being misleading, duplicate moderation fetches, and the open views proxy, but none are blockers.
📌 4 inline comments
🟡 Warning: 1 · 🔵 Suggestion: 2 · ⚪ Nitpick: 1
🔍 Reviewed by Gaston
There was a problem hiding this comment.
Gaston Review
Verdict: Approved
Score: ████████░░ 8/10
Pull Request Summary
This PR replaces the old Markee banner (a link to markee.xyz) with a full in-app buy/boost modal. It moves data fetching from server-side to client-side, adds three proxy API routes plus a real health check, introduces a MarkeeSign component for the explore page, and a MarkeeModal with buy and boost tabs backed by on-chain transactions via wagmi.
Clean follow-up push. All three actionable items from my prior review are addressed: health check now actually probes upstream with timeouts, views validation properly checks types with typeof, and the duplicate moderation fetch is eliminated by passing flagged as a prop. No new issues to flag. Ship it.
🔍 Reviewed by Gaston
Replaces the display-only Markee banner on the Explore page with an embedded buy flow.
Verified with headless-browser checks against the dev server: SSR data-markee-address attribute, boost list read from Base, preset amounts, 8-digit input cap, and the connect round trip.