feat: Add revenue distribution #72
Conversation
WalkthroughThe changes implement a new revenue distribution model for cipher purchases, splitting payments into 88% for the prize pool and 12% for the admin. This affects the smart contract, TypeScript bindings, tests, and documentation. The admin wallet is now explicitly included in relevant transactions and tests, and a new constant defines the prize pool percentage. Changes
Sequence Diagram(s)sequenceDiagram
participant Player
participant Frontend
participant Program
participant PrizePool
participant AdminWallet
Player->>Frontend: Initiate cipher purchase (amount)
Frontend->>Program: purchaseCiphers(amount, accounts)
Program->>PrizePool: Transfer 88% of amount
Program->>AdminWallet: Transfer 12% of amount
Program-->>Frontend: Transaction confirmation
Frontend-->>Player: Update balances/UI
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Clippy (1.86.0)warning: failed to write cache, path: /usr/local/registry/index/index.crates.io-1949cf8c6b5b557f/.cache/an/ch/anchor-lang, error: Permission denied (os error 13) Caused by: 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (11)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsREADME.md (1)🪛 markdownlint-cli2 (0.17.2)README.md46-46: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) 53-53: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) 62-62: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) 72-72: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) 79-79: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) 96-96: Bare URL used (MD034, no-bare-urls) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
README.md (1)
68-70: Minor grammar: “fewer” for countable nouns- 3. **Swift** - The next step costs two less ciphers than it would. + 3. **Swift** - The next step costs two fewer ciphers than it would.tests/helpers/constants.ts (2)
5-7: Consider using a deterministic test admin keypair
Keypair.generate()creates a fresh keypair every time tests start.
While Node's module cache makes sure the same instance is reused within a single Jest/Mocha run, the private key differs between runs, so balances/history accumulated on a local validator will not survive a restart.
If you rely on fixtures or wish to debug fund flows between runs, consider:-export const ADMIN_KEYPAIR = Keypair.generate(); +export const ADMIN_KEYPAIR = Keypair.fromSeed( + Uint8Array.from(Array(32).fill(1)) // <–– any 32-byte seed you like +);This keeps tests reproducible and avoids “unknown signer” surprises when re-using the same ledger.
13-13: Potential precision loss when parsing on-chain constants
prizePoolPercentageis an on-chainu64, but it is cast to JSNumber, which is IEEE-754 and loses precision above 2⁵³-1.
88 is safe, yet if the constant ever becomes something larger (e.g. 10 000 = 100.00 %), the cast would still be safe but the pattern invites future mistakes. PreferBNfor unsigned 64-bit ints to stay type-aligned:-export const PRIZE_POOL_PERCENTAGE = Number(getConstantOrThrow("prizePoolPercentage")); +export const PRIZE_POOL_PERCENTAGE = new BN( + getConstantOrThrow("prizePoolPercentage") +).toNumber(); // or expose the BN directlyapp/src/idl/blockrunners.ts (1)
513-516: Account list lacks explicitsignerfield – intentional?Other signer accounts (e.g.
player) explicitly declare"signer": true.
Leaving the field out defaults tofalse, which is correct foradminWallet, but adding it explicitly improves self-documentation and avoids accidental behavioural changes if Anchor’s defaults ever evolve.tests/move.ts (1)
2-3: RedundantBNimportYou import
BNon line 2 but still instantiateanchor.BNthroughout the file (e.g. line 94).
Either:
- drop the extra import, or
- switch to the standalone
BNfor consistency.This avoids confusion and dead imports.
app/src/context/BlockrunnersProvider.tsx (1)
219-223: Guard clause is good – surface the error to the UI as wellEarly-returning on missing
gameState.authorityprevents a crash, but the user receives no feedback.
Consider bubbling the error up (toast/snackbar, state flag, etc.) so the front-end isn’t silently inert.- console.error("Purchase ciphers: Game state or admin authority not found"); + const msg = "Game not initialised – please refresh or try again later."; + console.error(msg); + // expose a setter or toast helper heretests/purchase_ciphers.ts (1)
101-113: Remove noisy console logging or guard it behind a debug flag
These eleven consecutive logs clutter test output and slow CI. Either delete them or emit them only whenprocess.env.DEBUGis set.programs/blockrunners/src/instructions/purchase_ciphers.rs (1)
100-102: Avoid float math in on-chain programs
Casting tof64and back sacrifices determinism and can truncate large values unexpectedly. Compute the percentage with integer math:let increase_percentage = (prize_pool_amount * 100) / old_prize_pool;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
README.md(1 hunks)app/src/context/BlockrunnersProvider.tsx(1 hunks)app/src/idl/blockrunners.json(2 hunks)app/src/idl/blockrunners.ts(2 hunks)programs/blockrunners/src/constants.rs(1 hunks)programs/blockrunners/src/instructions/purchase_ciphers.rs(5 hunks)tests/helpers/constants.ts(1 hunks)tests/initialize_game.ts(2 hunks)tests/initialize_player.ts(2 hunks)tests/join_game.ts(2 hunks)tests/move.ts(9 hunks)tests/purchase_ciphers.ts(15 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~70-~70: Did you mean “fewer”? The noun “ciphers” is countable.
Context: ... 3. Swift - The next step costs two less ciphers than it would. **Balancing Mec...
(FEWER_LESS)
🪛 markdownlint-cli2 (0.17.2)
README.md
46-46: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
53-53: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
62-62: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
72-72: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
79-79: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
96-96: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (6)
tests/initialize_game.ts (1)
7-21: Great move to a sharedADMIN_KEYPAIRCentralising the admin identity removes noise from the tests and mirrors the on-chain assumption of a single admin wallet.
No further issues spotted here.tests/join_game.ts (1)
6-18: Consistent admin usage across testsImporting
ADMIN_KEYPAIRkeeps the test matrix predictable and avoids mismatched PDAs/accounts. Looks good.tests/initialize_player.ts (1)
6-18: Aligned with the new revenue split flowUsing the shared admin keypair ensures that any purchase-related side effects in later tests reflect the real admin wallet. Implementation is sound.
app/src/idl/blockrunners.ts (1)
1139-1146:u64for percentage looks wastefulStoring an 8-bit percentage in a 64-bit constant costs 7 extra bytes in every instruction that serialises the constant. Using
u8(consistent withMOVE_SUCCESS_PROBABILITY) would tighten account data and instruction payloads.tests/move.ts (1)
95-101: ```shell
#!/bin/bash
set -e
rg ".purchaseCiphers" -n tests/move.ts
rg "playerStatePda" -n tests/move.ts
rg "gameStatePda" -n tests/move.ts
rg ".accounts\s*(" -n tests/move.ts</details> <details> <summary>app/src/idl/blockrunners.json (1)</summary> `1075-1081`: **`PRIZE_POOL_PERCENTAGE` could be a `u8`** Similar to the TS IDL, a full 64-bit integer for an 0-100 percentage seems excessive. Down-sizing to `u8` saves 7 bytes per constant read and matches semantic intent. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Closes #27
Summary by CodeRabbit