feat: add binary prediction client#111
Conversation
|
Warning Review limit reached
Next review available in: 40 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a new Vite/React client application under binary-prediction/app for interacting with the binary prediction Solana program: build tooling, an Anchor IDL, a client library handling PDAs/oracle/transactions, App and PriceChart UI components, a full stylesheet, and README/gitignore documentation updates. ChangesNew Vite/React client app for binary prediction
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: PoemA rabbit hops through code brand new, 🚥 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 |
Local wallets sign every transaction directly, so the gum-SDK session-key flow was unused. placeBet already signed with the user wallet and passed a null session token. Drop the session create/approve UI, backing helpers, stored/snapshot fields, and related styles.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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 `@binary-prediction/app/src/App.tsx`:
- Around line 361-398: The reset dialog in App should support keyboard dismissal
and modal focus behavior. Update the showResetModal block in App so the reset
modal handles Escape to close, moves focus into the dialog when it opens, and
keeps keyboard focus contained while open (or at least returns focus when
closed). Use the existing reset-modal, handleReset, and setShowResetModal logic
to wire this up without changing the current mouse backdrop close behavior.
In `@binary-prediction/app/src/lib/binaryPrediction.ts`:
- Around line 281-299: decodeOraclePrice currently trusts fixed offsets after
only checking Buffer length, so add a discriminator/layout validation before
reading the PriceUpdateV2 fields. Update decodeOraclePrice to verify the account
starts with the expected Pyth Anchor discriminator (or another explicit schema
marker used by this parser), then only read raw, exponent, publishTime, and slot
if the check passes; otherwise throw a clear error so bad accounts fail fast
instead of decoding garbage.
- Around line 76-85: The stored market state still carries a vestigial
pool-authority signer and duplicate snapshot fields from the old custody model.
Remove poolAuthoritySecret from StoredMarket and stop reconstructing it in
keypairs(), then update any callers such as loadOrCreateMarket, bootstrapMarket,
placeBet, and settleBet to rely only on admin/user as used today. Also rename
MarketSnapshot.poolAuthority and poolAuthoritySol in refreshSnapshot to clearer
pool-based names so they no longer imply a separate authority signer.
- Around line 372-403: sendSignedTransaction currently returns after
sendRawTransaction with skipPreflight enabled, so state-changing calls can
appear successful even when the chain rejects them. Update sendSignedTransaction
to at least confirm or poll the signature status for the state-mutating flows
that use it, especially bootstrapMarket, place_bet, and settle-related actions,
while still keeping the initial send latency measurement separate. Use the
existing sendSignedTransaction helper and its callers in binaryPrediction.ts /
App.tsx to ensure failures are surfaced before showing success toasts or
proceeding to dependent transactions.
- Around line 522-567: The bootstrap flow in binaryPrediction.ts sends dependent
transactions too early; initializeTx, initializeBetTx, delegateBetTx, and the
delegateSpl transaction need confirmation before the next step runs. Update the
sequence around sendSignedTransaction, delegateBet, and delegateSpl to wait for
on-chain confirmation between each dependent transaction (for example via
connection.confirmTransaction or equivalent) so the bet PDA and SPL state are
guaranteed to exist before later instructions execute.
- Around line 161-178: loadOrCreateMarket currently trusts localStorage and
calls JSON.parse directly, which can crash App initialization when the stored
value is malformed or outdated. Update loadOrCreateMarket to safely parse the
stored value with error handling, validate that the parsed object matches
StoredMarket/programId before using it, and fall back to removing STORAGE_KEY
and creating a fresh market when parsing or validation fails.
- Around line 577-602: The approveMarket helper is unused dead code, since the
placeBet flow always signs as user with sessionToken set to null and never
relies on SPL approval/delegate setup. Remove the approveMarket function from
binaryPrediction.ts, or if it must remain for future session-key support, add a
clear note near approveMarket explaining that it is intentionally retained and
not used by the current transaction path.
In `@binary-prediction/app/src/styles.css`:
- Around line 1-26: Add the missing blank line before the font-family
declaration in the :root rule to satisfy Stylelint’s
declaration-empty-line-before rule. Update the styles in the :root block of
styles.css so the font-family declaration is separated from the preceding custom
properties, keeping the rest of the variable definitions unchanged.
In `@binary-prediction/programs/binary-prediction/src/lib.rs`:
- Around line 458-461: The pool custody account validation is too permissive in
the PlaceBet and Settle account constraints, since owner/mint checks still allow
a non-canonical token account owned by the Pool PDA. Tighten the
`pool_token_account` constraint in both `PlaceBet` and `Settle` to require the
initialized canonical pool ATA instead of just any matching owner/mint account,
using the existing `pool_token_account` symbol and the pool PDA as the reference
for the expected custody account.
- Around line 220-225: The solvency check in the betting flow is using the
current pool balance before the user’s stake is transferred, so valid bets can
be rejected by `require!` in the stake/payout path. Update the check around
`checked_payout` and `pool_token_account.amount` to account for the incoming
stake by using the post-transfer effective balance (or otherwise including
`stake` in the available liquidity calculation) before comparing against
`required_payout`. Keep the change localized to the betting instruction logic in
`lib.rs` where the pool liquidity is validated.
In `@binary-prediction/README.md`:
- Around line 76-96: The Client section documents localhost defaults that do not
match the actual fallback values used by binaryPrediction.ts. Update the README
text to reflect the real defaults in the client configuration (the base and
ephemeral endpoints), and make the local testing flow explicit by telling
readers to set VITE_PROVIDER_ENDPOINT, VITE_EPHEMERAL_PROVIDER_ENDPOINT, and
VITE_EPHEMERAL_WS_ENDPOINT if they want to use the local cluster after running
setup. Keep the wording aligned with the app/src/lib/binaryPrediction.ts
defaults so the README matches what the client actually uses.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f994f4e2-aab7-46b7-bd00-5b7a726403fa
⛔ Files ignored due to path filters (1)
binary-prediction/app/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
.gitignorebinary-prediction/README.mdbinary-prediction/app/index.htmlbinary-prediction/app/package.jsonbinary-prediction/app/src/App.tsxbinary-prediction/app/src/idl/binary_prediction.jsonbinary-prediction/app/src/lib/binaryPrediction.tsbinary-prediction/app/src/main.tsxbinary-prediction/app/src/polyfills.tsbinary-prediction/app/src/styles.cssbinary-prediction/app/src/vite-env.d.tsbinary-prediction/app/tsconfig.jsonbinary-prediction/app/vite.config.tsbinary-prediction/programs/binary-prediction/src/lib.rsbinary-prediction/programs/binary-prediction/src/state.rsbinary-prediction/programs/binary-prediction/src/utils.rsbinary-prediction/tests/binary-prediction.ts
…rediction-client # Conflicts: # binary-prediction/programs/binary-prediction/src/lib.rs # binary-prediction/tests/binary-prediction.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
binary-prediction/app/src/lib/binaryPrediction.ts (1)
713-735: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid using
PublicKey.defaultas the pool placeholder before bootstrap. Whenmintis missing, this still fetches a balance and renders the system address as the pool. Use a sentinel like"-"and skip the pool balance lookup until the market has a mint.🤖 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 `@binary-prediction/app/src/lib/binaryPrediction.ts` around lines 713 - 735, The pool placeholder in binaryPrediction should not use PublicKey.default when mint is missing, because it causes the system address to be treated as a real pool and still queried for balance. Update the logic around the pool/publicKey handling in binaryPrediction.ts so that the pool is represented by a sentinel like "-" until mint is available, and make the Promise.all balance lookup skip the pool entry when no mint exists while keeping admin/user balance behavior unchanged.binary-prediction/app/src/App.tsx (2)
281-284: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAvoid deriving the settlement result from a stale subscribed price.
Line 282 reuses
oraclePricewhenever it exists, but the transaction settles against the oracle account at send/execution time. A delayed account-change update can make the toast/log report the wrong win/loss/refund. Fetch a fresh oracle value for settlement display, or derive the result from the post-settle snapshot if available.Minimal safer change
- const livePrice = oraclePrice ?? (await fetchOraclePrice()); + const livePrice = await fetchOraclePrice(); const result = await settleBet(market);🤖 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 `@binary-prediction/app/src/App.tsx` around lines 281 - 284, The settlement flow in settleCurrentTicket is using oraclePrice as the displayed settlement input, which can be stale relative to the actual on-chain execution. Update the logic to use a fresh oracle value fetched at settlement time, or derive the outcome from the post-settle snapshot if that is available, so the toast/log reflects the true win/loss/refund. Keep the fix localized around settleCurrentTicket and the settlementOutcome calculation in App.tsx.
250-278: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRefresh with the newly bootstrapped market, not the stale closure.
run()always callsrefresh()captured with the pre-taskmarket. InhandleBootstrap,setMarket(nextMarket)happens before that final refresh, so the post-bootstrap snapshot can be overwritten by a refresh using the old market state.Proposed direction
+ const marketRef = useRef(market); + + useEffect(() => { + marketRef.current = market; + }, [market]); + - const refresh = useCallback(async () => { - const nextSnapshot = await refreshSnapshot(market); + const refresh = useCallback(async () => { + const nextSnapshot = await refreshSnapshot(marketRef.current); setSnapshot(nextSnapshot); - }, [market]); + }, []);const nextMarket = await bootstrapMarket(market, { seedAmount: 10_000, userAmount: 1_000, durationSeconds: numberOrDefault(duration, 8), minStake: 10, payoutBps: 19_000, onLog: pushLog, }); + marketRef.current = nextMarket; setMarket(nextMarket);Also applies to: 302-313
🤖 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 `@binary-prediction/app/src/App.tsx` around lines 250 - 278, The final refresh in run() is using a stale refresh closure captured before the market update, which can overwrite the newly bootstrapped state; update the post-task refresh flow so it uses the latest market snapshot after setMarket(nextMarket) in handleBootstrap (and any similar caller), or make refresh accept the fresh market/value explicitly. Adjust the run() callback and the handleBootstrap path so the refresh after bootstrap is driven by the new market state rather than the pre-task closure.
♻️ Duplicate comments (1)
binary-prediction/app/src/lib/binaryPrediction.ts (1)
161-163: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winValidate stored key material before returning cached market.
Line 163 still accepts any same-program object, so partial storage like
{ "programId": "..." }can pass and crash later duringkeypairs()reconstruction. Validate the stored shape and secret arrays before returning it. This overlaps with the earlier stored-market validation review.🛡️ Proposed fix
+function isSecretKey(value: unknown): value is number[] { + return ( + Array.isArray(value) && + value.length === 64 && + value.every((byte) => Number.isInteger(byte) && byte >= 0 && byte <= 255) + ); +} + +function isStoredMarket(value: unknown): value is StoredMarket { + if (!value || typeof value !== "object") return false; + const market = value as Partial<StoredMarket>; + return ( + market.version === 1 && + market.programId === PROGRAM_ID.toBase58() && + isSecretKey(market.adminSecret) && + isSecretKey(market.userSecret) && + (market.mint === undefined || typeof market.mint === "string") && + (market.userAta === undefined || typeof market.userAta === "string") && + (market.poolAta === undefined || typeof market.poolAta === "string") + ); +} + export function loadOrCreateMarket(): StoredMarket { const stored = localStorage.getItem(STORAGE_KEY); if (stored) { try { - const market = JSON.parse(stored) as StoredMarket; - if (market.programId === PROGRAM_ID.toBase58()) return market; + const market = JSON.parse(stored); + if (isStoredMarket(market)) return market;🤖 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 `@binary-prediction/app/src/lib/binaryPrediction.ts` around lines 161 - 163, The cached market path in binaryPrediction.ts still returns any object with a matching programId, so incomplete stored data can slip through and fail later in keypairs() reconstruction. Tighten the validation inside the JSON.parse/stored market check by verifying the full StoredMarket shape before returning from the market cache branch, including the presence and array structure of the key material/secret fields. Use the existing StoredMarket handling in the same function to reject partial objects and only return when all required fields are valid.
🤖 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 `@binary-prediction/app/src/components/PriceChart.tsx`:
- Around line 138-174: The chart update logic in pushPriceSample only stores the
smoothed chartValue and stops emitting new points when the price prop is stable,
so the series can get stuck away from the real oracle price. Update
pushPriceSample in PriceChart so it also stores and converges toward the actual
nextPrice (for example by keeping the latest oracle value in the series/refs and
continuing to advance the displayed point over time), and make sure the chart
can keep producing samples even when useEffect only fires once per prop change.
---
Outside diff comments:
In `@binary-prediction/app/src/App.tsx`:
- Around line 281-284: The settlement flow in settleCurrentTicket is using
oraclePrice as the displayed settlement input, which can be stale relative to
the actual on-chain execution. Update the logic to use a fresh oracle value
fetched at settlement time, or derive the outcome from the post-settle snapshot
if that is available, so the toast/log reflects the true win/loss/refund. Keep
the fix localized around settleCurrentTicket and the settlementOutcome
calculation in App.tsx.
- Around line 250-278: The final refresh in run() is using a stale refresh
closure captured before the market update, which can overwrite the newly
bootstrapped state; update the post-task refresh flow so it uses the latest
market snapshot after setMarket(nextMarket) in handleBootstrap (and any similar
caller), or make refresh accept the fresh market/value explicitly. Adjust the
run() callback and the handleBootstrap path so the refresh after bootstrap is
driven by the new market state rather than the pre-task closure.
In `@binary-prediction/app/src/lib/binaryPrediction.ts`:
- Around line 713-735: The pool placeholder in binaryPrediction should not use
PublicKey.default when mint is missing, because it causes the system address to
be treated as a real pool and still queried for balance. Update the logic around
the pool/publicKey handling in binaryPrediction.ts so that the pool is
represented by a sentinel like "-" until mint is available, and make the
Promise.all balance lookup skip the pool entry when no mint exists while keeping
admin/user balance behavior unchanged.
---
Duplicate comments:
In `@binary-prediction/app/src/lib/binaryPrediction.ts`:
- Around line 161-163: The cached market path in binaryPrediction.ts still
returns any object with a matching programId, so incomplete stored data can slip
through and fail later in keypairs() reconstruction. Tighten the validation
inside the JSON.parse/stored market check by verifying the full StoredMarket
shape before returning from the market cache branch, including the presence and
array structure of the key material/secret fields. Use the existing StoredMarket
handling in the same function to reject partial objects and only return when all
required fields are valid.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3111d20b-9de0-47ac-b7fb-4b11c0a42d85
📒 Files selected for processing (6)
binary-prediction/README.mdbinary-prediction/app/src/App.tsxbinary-prediction/app/src/components/PriceChart.tsxbinary-prediction/app/src/idl/binary_prediction.jsonbinary-prediction/app/src/lib/binaryPrediction.tsbinary-prediction/app/src/styles.css
Summary
Testing
anchor buildyarn lintyarn buildNotes
6iL4bfFA7WAkRaRhS4XTRE6khacQ6QGBoKhg1ufXEUBTSummary by CodeRabbit
New Features
Bug Fixes
Documentation