feat(token_lite): single-game gas-optimized token component (denshokan lite) - #123
feat(token_lite): single-game gas-optimized token component (denshokan lite)#123starknetdev wants to merge 9 commits into
Conversation
…hokan lite) Adds a CoreTokenLiteComponent for single-game deployments (e.g. super-death-mountain) that never used the multi-game registry, objectives, context, skills, per-token renderers, or enumerable, and that keep game-over/objective authority in the game contract: - No mutable token state: no update_game, no metagame callbacks, no game_over latch. is_playable/assert_is_playable check the lifecycle window only — zero storage reads (pure unpack of the packed token id). - New assert_owner_and_playable merges the per-action owner_of + assert_is_playable pair into one external call. - Mint does no SRC5 probe, no registry lookup, no settings/objective validation; keeps the exact IMinigameToken::mint ABI and rejects unsupported params loudly. 251-bit pack_token_id layout is unchanged. - Registers IMINIGAME_TOKEN_LITE_ID plus the legacy IMINIGAME_TOKEN_ID and exposes a zero game_registry_address() so MinigameComponent::initializer accepts a lite token unchanged. Includes IMinigameTokenLite in the interfaces package, a wiring example contract, 30 tests, CI matrix + codecov updates (18 modules), and doc refreshes (root AGENTS.md matrix table was stale). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a self-bound, single-game ChangesToken Lite interface and component
CI and migration documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GameContract
participant LiteGameMock
participant CoreTokenLiteComponent
participant ERC721Component
GameContract->>LiteGameMock: Call mint or mint_batch_recipients
LiteGameMock->>CoreTokenLiteComponent: Validate game address and lifecycle
CoreTokenLiteComponent->>CoreTokenLiteComponent: Pack token ID and register minter
CoreTokenLiteComponent->>ERC721Component: Mint token(s)
ERC721Component-->>GameContract: Return minted token ID(s)
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
GPT Code ReviewReview execution failed (exit code 1). The Codex CLI log was not posted because it can contain the full review prompt. |
GPT Code ReviewReview execution failed (exit code 1). The Codex CLI log was not posted because it can contain the full review prompt. |
Claude Code Review[LOW] AGENTS.md:131 - The illustrative [LOW] codecov.yml:4 - The comment says "Must equal package count in ... matrix" and line 5 says "when adding packages", but the CI check at Note on the PR's assumptions: I verified the matrix now has 18 |
Claude Code ReviewThat's in a different test file (not the settings/objectives libs tests). The new skip branch in Here's my review: [MEDIUM] packages/embeddable_game_standard/src/minigame/extensions/settings/libs.cairo:52-55 (and objectives/libs.cairo:31-34) - The new
[LOW] packages/embeddable_game_standard/src/minigame/tests/test_settings_libs.cairo (and test_objectives_libs.cairo) - The new skip branch (
[LOW] packages/embeddable_game_standard/src/minigame/tests/test_objectives_libs.cairo:46-47 (representative; many sites) - Each single-call test queues two one-shot
Note: |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Baseline/op-x10 test pairs measuring warm mint, per-action ownership+ playability guard, and post-action sync on the lite component against FullTokenContract in its deployed-denshokan configuration (registry-backed multi-game, all extensions). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Free-function helpers in minigame::lite keep game code's familiar shape against a lite token — the module path carries the semantic shift: pre_action folds the assert_token_ownership + pre_action pair into one assert_owner_and_playable call; post_action is refresh_metadata only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
packages/embeddable_game_standard/src/token_lite/tests/test_gas_bench.cairo (1)
31-280: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDocument all functions in this Cairo file.
Add documentation for
addr,ALICE,OWNER, deployment helpers, setup helpers, mint helpers, and benchmark tests. Describe the purpose, parameter constraints, return values, and benchmark operation where applicable.As per coding guidelines, “Every function must include clear explanation of what it does and why, parameter descriptions with types and constraints, return value documentation, and example usage when appropriate.”
🤖 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 `@packages/embeddable_game_standard/src/token_lite/tests/test_gas_bench.cairo` around lines 31 - 280, Document every function in the file, including addr, ALICE, OWNER, deploy_mock_game, setup_lite, setup_full, mint_lite, mint_full, and each benchmark test. Add Cairo documentation describing purpose, rationale, typed parameters and constraints, return values, and representative usage where appropriate; for benchmark tests, describe the operation and iteration count being measured while preserving the existing behavior.Source: Coding guidelines
packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo (1)
18-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the test helper contracts.
Add documentation for
addr,GAME,ALICE,BOB,MINTER,deploy_token_lite, andmint_basic. Document each parameter constraint, return value, and the neutral-value assumptions inmint_basic.As per coding guidelines, “Every function must include clear explanation of what it does and why, parameter descriptions with types and constraints, return value documentation, and example usage when appropriate.”
🤖 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 `@packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo` around lines 18 - 88, Add Cairo documentation comments to addr, GAME, ALICE, BOB, MINTER, deploy_token_lite, and mint_basic explaining each helper’s purpose and rationale, parameter types and constraints, return values, and representative usage where appropriate. For mint_basic, explicitly document that unsupported mint parameters are passed as Option::None, metadata is 0, paymaster is false, and the salt is supplied by the caller.Source: Coding guidelines
packages/embeddable_game_standard/src/minigame/lite.cairo (1)
21-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply the function-documentation standard to both new Cairo surfaces.
The new helper, hook, and constructor functions do not document their purpose, parameter constraints, return behavior, and examples where appropriate.
packages/embeddable_game_standard/src/minigame/lite.cairo#L21-L35: Add argument and return sections, plus a usage example for the action lifecycle.packages/embeddable_game_standard/src/token_lite/tests/examples/token_lite_contract.cairo#L70-L106: Document the hook behavior, the no-opafter_update, and constructor constraints.As per coding guidelines, every function must include an explanation, parameter constraints, return documentation, and examples when appropriate.
🤖 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 `@packages/embeddable_game_standard/src/minigame/lite.cairo` around lines 21 - 35, The functions in packages/embeddable_game_standard/src/minigame/lite.cairo lines 21-35 (anchor) need complete Cairo documentation: update pre_action and post_action with purpose, parameter constraints, return behavior, and an action-lifecycle usage example. Also document the affected hook and constructor functions in packages/embeddable_game_standard/src/token_lite/tests/examples/token_lite_contract.cairo lines 70-106 (sibling), covering hook behavior, the no-op after_update behavior, constructor constraints, parameters, returns, and examples where appropriate.Source: Coding guidelines
🤖 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
`@packages/embeddable_game_standard/src/token_lite/tests/examples/token_lite_contract.cairo`:
- Line 83: Define a descriptive module-scoped constant for the soulbound
transfer error in TokenLiteContract’s module, preserving the exact existing
wording, then update the panic! call to use that constant instead of the inline
string.
In `@packages/embeddable_game_standard/src/token_lite/tests/test_gas_bench.cairo`:
- Around line 4-7: Add setup-plus-mint baseline tests for each token type used
by bench_*_guard_x10 and bench_*_post_action_x10, then subtract those baselines
when calculating the corresponding guard and post-action costs. Retain the
existing deployment baselines only for mint measurements, and ensure each paired
calculation still divides the difference by 10.
In
`@packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo`:
- Around line 113-216: Add snforge fuzz tests alongside
test_mint_packs_expected_fields and the lifecycle tests, varying valid
settings_id, salt, start/end delays, and other packed inputs across their
supported ranges. Assert unpack_token_id and token_metadata preserve the
expected fields after minting, including lifecycle clamping and reconstruction.
Add fuzz cases for invalid lifecycle bounds and assert minting rejects them,
reusing mint_basic and the existing deployment setup.
In `@packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo`:
- Around line 103-334: Replace every raw string passed to assert! in the
MinigameTokenLite component with descriptive named error constants, covering
ownership, unsupported mint parameters, lifecycle validation, and initialization
checks. Define the constants in the component’s established constants section
and update the affected assertions in functions such as assert_is_owner, mint,
assert_lifecycle_open, and initializer to reference them consistently.
- Line 40: Update the import of IMINIGAME_TOKEN_ID in token_lite_component to
use game_components_interfaces::token::IMINIGAME_TOKEN_ID instead of the local
crate::token::interface path, keeping the interfaces package as the direct
source for this shared SRC5 definition.
In `@packages/interfaces/src/token/lite.cairo`:
- Around line 27-34: The EFS comment above IMINIGAME_TOKEN_LITE_ID is missing
the src5_rs output. Run src5_rs against the trait with refresh_metadata and
refresh_metadata_batch excluded, then document every extended function selector
and the final XOR value in the comment while keeping the constant unchanged.
- Around line 38-85: Document every public interface method in
packages/interfaces/src/token/lite.cairo (lines 38-85), including purpose,
parameter types and constraints, return values, and examples where useful;
preserve the existing API contract. Add matching documentation for each
implementation and internal lifecycle method in
packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo
(lines 77-336), ensuring descriptions accurately reflect behavior and
constraints across both sites.
---
Nitpick comments:
In `@packages/embeddable_game_standard/src/minigame/lite.cairo`:
- Around line 21-35: The functions in
packages/embeddable_game_standard/src/minigame/lite.cairo lines 21-35 (anchor)
need complete Cairo documentation: update pre_action and post_action with
purpose, parameter constraints, return behavior, and an action-lifecycle usage
example. Also document the affected hook and constructor functions in
packages/embeddable_game_standard/src/token_lite/tests/examples/token_lite_contract.cairo
lines 70-106 (sibling), covering hook behavior, the no-op after_update behavior,
constructor constraints, parameters, returns, and examples where appropriate.
In `@packages/embeddable_game_standard/src/token_lite/tests/test_gas_bench.cairo`:
- Around line 31-280: Document every function in the file, including addr,
ALICE, OWNER, deploy_mock_game, setup_lite, setup_full, mint_lite, mint_full,
and each benchmark test. Add Cairo documentation describing purpose, rationale,
typed parameters and constraints, return values, and representative usage where
appropriate; for benchmark tests, describe the operation and iteration count
being measured while preserving the existing behavior.
In
`@packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo`:
- Around line 18-88: Add Cairo documentation comments to addr, GAME, ALICE, BOB,
MINTER, deploy_token_lite, and mint_basic explaining each helper’s purpose and
rationale, parameter types and constraints, return values, and representative
usage where appropriate. For mint_basic, explicitly document that unsupported
mint parameters are passed as Option::None, metadata is 0, paymaster is false,
and the salt is supplied by the caller.
🪄 Autofix
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 Plus
Run ID: dc177cb2-2a08-40a6-955b-52ea407d0fea
📒 Files selected for processing (19)
.github/workflows/main-ci.yml.github/workflows/pr-ci.ymlAGENTS.mdcodecov.ymlpackages/embeddable_game_standard/src/lib.cairopackages/embeddable_game_standard/src/minigame.cairopackages/embeddable_game_standard/src/minigame/lite.cairopackages/embeddable_game_standard/src/token_lite.cairopackages/embeddable_game_standard/src/token_lite/AGENTS.mdpackages/embeddable_game_standard/src/token_lite/interface.cairopackages/embeddable_game_standard/src/token_lite/tests.cairopackages/embeddable_game_standard/src/token_lite/tests/examples.cairopackages/embeddable_game_standard/src/token_lite/tests/examples/token_lite_contract.cairopackages/embeddable_game_standard/src/token_lite/tests/test_gas_bench.cairopackages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairopackages/embeddable_game_standard/src/token_lite/token_lite_component.cairopackages/interfaces/src/AGENTS.mdpackages/interfaces/src/token.cairopackages/interfaces/src/token/lite.cairo
| let current_owner = self._owner_of(token_id); | ||
| if !current_owner.is_zero() && !to.is_zero() { | ||
| if unpack_soulbound(token_id.try_into().unwrap()) { | ||
| panic!("Token is soulbound and cannot be transferred"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 \
'const .*ERROR|panic!\(' \
packages/embeddable_game_standard/srcRepository: Provable-Games/game-components
Length of output: 16523
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/embeddable_game_standard/src/token_lite/tests/examples/token_lite_contract.cairo"
echo "== file line count =="
wc -l "$file"
echo "== relevant file section =="
sed -n '1,110p' "$file" | cat -n
echo "== token lite module errors/constants snippets =="
rg -n -C 2 'const .*=' packages/embeddable_game_standard/src/token_lite -g '*.cairo' || trueRepository: Provable-Games/game-components
Length of output: 5955
Move the soulbound transfer error into a module-scoped constant.
TokenLiteContract currently embeds "Token is soulbound and cannot be transferred" directly in panic! at line 83. Define a descriptive module-level constant and pass it to panic! to satisfy the error-message constant rule while preserving the existing wording.
🤖 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
`@packages/embeddable_game_standard/src/token_lite/tests/examples/token_lite_contract.cairo`
at line 83, Define a descriptive module-scoped constant for the soulbound
transfer error in TokenLiteContract’s module, preserving the exact existing
wording, then update the panic! call to use that constant instead of the inline
string.
Source: Coding guidelines
| // Method: paired tests. Each `*_baseline` test performs setup only; each op | ||
| // test repeats the measured operation 10 times on top of the same setup. | ||
| // Per-op cost = (op_test_l2_gas - baseline_l2_gas) / 10. Deployment noise | ||
| // cancels out within a pair; snforge prints l2_gas per test. |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Add minted-token baselines for guard and post-action measurements.
bench_*_guard_x10 and bench_*_post_action_x10 mint one token before their loops. The deployment baselines do not include this mint. The stated calculation therefore includes one-tenth of the mint cost in every reported guard or post-action cost.
Add one setup + mint baseline for each token type. Subtract that baseline from the corresponding guard and post-action tests. Keep the deployment baseline for mint measurements only.
🤖 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 `@packages/embeddable_game_standard/src/token_lite/tests/test_gas_bench.cairo`
around lines 4 - 7, Add setup-plus-mint baseline tests for each token type used
by bench_*_guard_x10 and bench_*_post_action_x10, then subtract those baselines
when calculating the corresponding guard and post-action costs. Retain the
existing deployment baselines only for mint measurements, and ensure each paired
calculation still divides the difference by 10.
| #[test] | ||
| fn test_mint_packs_expected_fields() { | ||
| let (token, erc721, minter) = deploy_token_lite(); | ||
| start_cheat_block_timestamp(token.contract_address, 1000); | ||
|
|
||
| cheat_caller_address(token.contract_address, MINTER(), CheatSpan::TargetCalls(1)); | ||
| let token_id = mint_basic( | ||
| token, | ||
| Option::Some('alice'), | ||
| Option::Some(42), | ||
| Option::Some(2000), | ||
| Option::Some(3000), | ||
| ALICE(), | ||
| true, | ||
| 7, | ||
| ); | ||
|
|
||
| let packed = unpack_token_id(token_id); | ||
| assert!(packed.game_id == 0, "game_id must be 0 for single game"); | ||
| assert!(packed.settings_id == 42, "settings_id mismatch"); | ||
| assert!(packed.minted_at == 1000, "minted_at mismatch"); | ||
| assert!(packed.start_delay == 1000, "start_delay mismatch"); | ||
| assert!(packed.end_delay == 1000, "end_delay mismatch"); | ||
| assert!(packed.objective_id == 0, "objective_id must be 0"); | ||
| assert!(packed.soulbound, "soulbound flag should be set"); | ||
| assert!(!packed.has_context, "has_context must be 0"); | ||
| assert!(!packed.paymaster, "paymaster must be 0"); | ||
| assert!(packed.salt == 7, "salt mismatch"); | ||
| assert!(packed.metadata == 0, "metadata must be 0"); | ||
|
|
||
| // Views resolve from the packed id / minter map | ||
| assert!(token.settings_id(token_id) == 42, "settings_id view mismatch"); | ||
| assert!(token.is_soulbound(token_id), "is_soulbound view mismatch"); | ||
| assert!(token.player_name(token_id) == 'alice', "player_name mismatch"); | ||
| assert!(token.minted_by(token_id) == 1, "First minter should get id 1"); | ||
| assert!(token.minted_by_address(token_id) == MINTER(), "minted_by_address mismatch"); | ||
| assert!(minter.get_minter_address(1) == MINTER(), "Minter registry mismatch"); | ||
| assert!(erc721.owner_of(token_id.into()) == ALICE(), "Owner mismatch"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_mint_defaults_and_metadata_view() { | ||
| let (token, _, _) = deploy_token_lite(); | ||
| start_cheat_block_timestamp(token.contract_address, 1000); | ||
|
|
||
| let token_id = mint_basic( | ||
| token, Option::None, Option::None, Option::None, Option::None, ALICE(), false, 0, | ||
| ); | ||
|
|
||
| let metadata = token.token_metadata(token_id); | ||
| assert!(metadata.game_id == 0, "game_id should be 0"); | ||
| assert!(metadata.settings_id == 0, "settings_id should default 0"); | ||
| assert!(metadata.minted_at == 1000, "minted_at mismatch"); | ||
| assert!(metadata.lifecycle.start == 1000, "start clamps to mint time"); | ||
| assert!(metadata.lifecycle.end == 0, "no end means immortal"); | ||
| assert!(!metadata.soulbound, "not soulbound"); | ||
| // No mutable state exists — these are unconditionally false/0 | ||
| assert!(!metadata.game_over, "game_over must always be false"); | ||
| assert!(!metadata.completed_objective, "completed_objective must always be false"); | ||
| assert!(metadata.completed_at == 0, "completed_at must always be 0"); | ||
| assert!(token.player_name(token_id) == 0, "No player name set"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_mint_past_start_clamps_to_now() { | ||
| let (token, _, _) = deploy_token_lite(); | ||
| start_cheat_block_timestamp(token.contract_address, 1000); | ||
|
|
||
| let token_id = mint_basic( | ||
| token, Option::None, Option::None, Option::Some(500), Option::Some(2000), ALICE(), false, 0, | ||
| ); | ||
|
|
||
| let metadata = token.token_metadata(token_id); | ||
| assert!(metadata.lifecycle.start == 1000, "Past start should clamp to mint time"); | ||
| assert!(metadata.lifecycle.end == 2000, "End must reconstruct to the caller's value"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_mint_unique_ids_by_salt_and_minter() { | ||
| let (token, _, _) = deploy_token_lite(); | ||
| start_cheat_block_timestamp(token.contract_address, 1000); | ||
|
|
||
| // Same params, same block, same caller — salt must disambiguate | ||
| let id_a = mint_basic( | ||
| token, Option::None, Option::None, Option::None, Option::None, ALICE(), false, 0, | ||
| ); | ||
| let id_b = mint_basic( | ||
| token, Option::None, Option::None, Option::None, Option::None, ALICE(), false, 1, | ||
| ); | ||
| assert!(id_a != id_b, "Salt must produce distinct token ids"); | ||
|
|
||
| // Second distinct caller gets minter id 2 | ||
| cheat_caller_address(token.contract_address, MINTER(), CheatSpan::TargetCalls(1)); | ||
| let id_c = mint_basic( | ||
| token, Option::None, Option::None, Option::None, Option::None, ALICE(), false, 0, | ||
| ); | ||
| assert!(token.minted_by(id_c) == 2, "Second minter should get id 2"); | ||
| // Repeat caller keeps its id | ||
| cheat_caller_address(token.contract_address, MINTER(), CheatSpan::TargetCalls(1)); | ||
| let id_d = mint_basic( | ||
| token, Option::None, Option::None, Option::None, Option::None, ALICE(), false, 1, | ||
| ); | ||
| assert!(token.minted_by(id_d) == 2, "Repeat minter keeps id"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What is the supported Starknet Foundry fuzz-test syntax for Cairo 2.16.1 and snforge?
💡 Result:
In Starknet Foundry (snforge), fuzz testing is enabled by adding the #[fuzzer] attribute to a test function that accepts parameters [1][2]. The fuzzer generates random values for these parameters, and the test is executed multiple times [1]. Syntax for Fuzz Testing To implement a fuzz test, you must include the #[fuzzer] attribute above your test function [1][2]. Any arguments defined in the function signature will be automatically populated with random values by the fuzzer [1]: #[cfg(test)] mod tests { #[test] #[fuzzer] fn test_sum(x: felt252, y: felt252) { assert_eq!(x + y, x + y); } } Configuration You can configure the number of runs and the seed for reproducibility directly within the attribute or globally [1]. 1. Per-test configuration: #[fuzzer(runs: 100, seed: 12345)] 2. Global configuration via Scarb.toml: [tool.snforge] fuzzer_runs = 256 fuzzer_seed = 12345 3. Command-line overrides: snforge test --fuzzer-runs 1234 --fuzzer-seed 1111 Supported Types Starknet Foundry provides built-in support for basic Cairo types, including integers (e.g., u8, u64, u128, u256, i8, etc.) [1]. For custom structs or enums, you can derive the Fuzzable trait if all fields implement it, or manually implement the Fuzzable trait to define custom generation logic [1][3]. Note: The requirement to use the #[fuzzer] attribute is mandatory for fuzz testing in modern versions of Starknet Foundry [4]. Ensure that any custom types used as parameters have a valid Fuzzable implementation, otherwise, a compilation error will occur [1][3].
Citations:
- 1: https://foundry-rs.github.io/starknet-foundry/snforge-advanced-features/fuzz-testing.html
- 2: https://foundry-rs.github.io/starknet-foundry/testing/test-attributes.html
- 3: https://foundry-rs.github.io/starknet-foundry/appendix/snforge-library/fuzzable.html
- 4: https://github.com/foundry-rs/starknet-foundry/releases/tag/v0.38.0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file and sizes =="
git ls-files | rg '^packages/embeddable_game_standard/src/token_lite/tests/test_token_lite\.cairo$' || true
wc -l packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo 2>/dev/null || true
echo "== outline fuzz/test attributes in test file =="
rg -n '#\[test\]|#\[fuzzer|#\[fork|fn test_|ft252|u[0-9]+' packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo 2>/dev/null | sed -n '1,220p'
echo "== relevant sections around mint boundary tests =="
sed -n '1,130p' packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo 2>/dev/null
sed -n '215,310p' packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo 2>/dev/null
sed -n '310,430p' packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo 2>/dev/null
sed -n '430,550p' packages/embeddable_game_standard/src token_lite/tests/test_token_lite.cairo 2>/dev/null || sed -n '430,550p' packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo 2>/dev/null
sed -n '700,750p' packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo 2>/dev/null || true
echo "== token lite source relevant identifiers =="
fd -a '.*token_lite.*\.cairo$' packages/embeddable_game_standard/src/token_lite 2>/dev/null | sed -n '1,40p'
rg -n 'struct TokenMetadata|token_metadata|settings_id|pack_token_id|unpack_token_id|is_playable|assert_is_playable|mint\(' packages/embeddable_game_standard/src/token_lite -g '*.cairo' | sed -n '1,220p'Repository: Provable-Games/game-components
Length of output: 25908
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== token structs with TokenMetadata and pack/unpack =="
rg -n 'struct TokenMetadata|fn to_token_metadata|fn pack_token_id|fn unpack_token_id|fn unpack_settings_id|fn unpack_minted_by|fn unpack_soulbound|impl Lifecycle|is_playable|game_over|completed_objective|completed_at' packages/embeddable_game_standard/src -g '*.cairo' | sed -n '1,260p'
echo "== source relevant implementation snippets =="
# Find files containing the exact implementations from previous hits
sed -n '1,260p' packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo
for f in $(git ls-files packages/embeddable_game_standard/src | rg '\.cairo$'); do
if rg -q 'struct TokenMetadata|fn to_token_metadata|fn pack_token_id|impl Lifecycle|is_playable\(self' "$f"; then
echo "--- $f"
rg -n -A 5 'struct TokenMetadata|fn to_token_metadata|fn pack_token_id|impl Lifecycle|is_playable\(' "$f" | sed -n '1,220p'
fi
done
echo "== deterministic parse/equivalence probe for current test cases =="
python3 - <<'PY'
from pathlib import Path
p = Path('packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo')
s = p.read_text()
checks = {
'uses fuzzer attribute': '#[fuzzer]' in s,
'test_count': s.count('fn test_'),
'mint_pack_boundary_values': '"game_id must be 0 for single game"' in s and '"metadata must be 0"' in s,
'lifecycle_window_tests': s.count('test_playability_follows_lifecycle_window') == 1,
'immortal_test': 'fn test_immortal_token_always_playable' in s,
'rejection_past_end': 'test_mint_rejects_past_end' in s,
'rejection_start_after_end': 'test_mint_rejects_start_after_end' in s,
}
for k,v in checks.items():
print(f'{k}: {v}')
print('line_count', p.stat().st_size, len(s.splitlines()))
PYRepository: Provable-Games/game-components
Length of output: 50386
Add fuzz coverage for mint boundaries and packed fields.
test_token_lite.cairo currently tests fixed settings_id/salt values and a few lifecycle points only. Add snforged fuzz tests over valid/invalid lifecycle bounds and packed-field inputs so token-ID round trips and expected rejections are exercised across ranges.
🤖 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 `@packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo`
around lines 113 - 216, Add snforge fuzz tests alongside
test_mint_packs_expected_fields and the lifecycle tests, varying valid
settings_id, salt, start/end delays, and other packed inputs across their
supported ranges. Assert unpack_token_id and token_metadata preserve the
expected fields after minting, including lifecycle clamping and reconstruction.
Add fuzz cases for invalid lifecycle bounds and assert minting rejects them,
reusing mint_basic and the existing deployment setup.
Source: Coding guidelines
| Map, StoragePathEntry, StoragePointerReadAccess, StoragePointerWriteAccess, | ||
| }; | ||
| use starknet::{ContractAddress, get_block_timestamp, get_caller_address, get_tx_info}; | ||
| use crate::token::interface::IMINIGAME_TOKEN_ID; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Import the legacy interface ID from the interfaces package.
Replace the local crate::token::interface::IMINIGAME_TOKEN_ID import with game_components_interfaces::token::IMINIGAME_TOKEN_ID. The interfaces package must remain the direct source for shared SRC5 definitions.
Proposed fix
-use crate::token::interface::IMINIGAME_TOKEN_ID;
+use game_components_interfaces::token::IMINIGAME_TOKEN_ID;As per coding guidelines, “The interfaces package is the single source of truth for all game-component interface definitions; other packages must import cross-contract interfaces and SRC5 definitions from it.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use crate::token::interface::IMINIGAME_TOKEN_ID; | |
| use game_components_interfaces::token::IMINIGAME_TOKEN_ID; |
🤖 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 `@packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo`
at line 40, Update the import of IMINIGAME_TOKEN_ID in token_lite_component to
use game_components_interfaces::token::IMINIGAME_TOKEN_ID instead of the local
crate::token::interface path, keeping the interfaces package as the direct
source for this shared SRC5 definition.
Source: Coding guidelines
| assert!(!expected_owner.is_zero(), "MinigameTokenLite: Expected owner cannot be zero"); | ||
| let contract = self.get_contract(); | ||
| let erc721_component = ERC721::get_component(contract); | ||
| // _owner_of returns zero for a nonexistent token, which can never | ||
| // equal the asserted-non-zero expected_owner — so this also | ||
| // guarantees existence. | ||
| let token_owner = erc721_component._owner_of(token_id.into()); | ||
| assert!( | ||
| token_owner == expected_owner, | ||
| "MinigameTokenLite: Address is not owner of token {}", | ||
| token_id, | ||
| ); | ||
| self.assert_lifecycle_open(token_id); | ||
| } | ||
|
|
||
| fn settings_id(self: @ComponentState<TContractState>, token_id: felt252) -> u32 { | ||
| unpack_settings_id(token_id) | ||
| } | ||
|
|
||
| fn player_name(self: @ComponentState<TContractState>, token_id: felt252) -> felt252 { | ||
| self.token_player_names.entry(token_id).read() | ||
| } | ||
|
|
||
| fn minted_by(self: @ComponentState<TContractState>, token_id: felt252) -> felt252 { | ||
| let minted_by_val: u64 = unpack_minted_by(token_id); | ||
| minted_by_val.into() | ||
| } | ||
|
|
||
| fn minted_by_address( | ||
| self: @ComponentState<TContractState>, token_id: felt252, | ||
| ) -> ContractAddress { | ||
| let minted_by_id: u64 = unpack_minted_by(token_id); | ||
| let contract_ref = self.get_contract(); | ||
| MinterOpt::get_minter_address(contract_ref, minted_by_id) | ||
| } | ||
|
|
||
| fn is_soulbound(self: @ComponentState<TContractState>, token_id: felt252) -> bool { | ||
| unpack_soulbound(token_id) | ||
| } | ||
|
|
||
| fn game_address(self: @ComponentState<TContractState>) -> ContractAddress { | ||
| self.game_address.read() | ||
| } | ||
|
|
||
| fn game_registry_address(self: @ComponentState<TContractState>) -> ContractAddress { | ||
| // Compat shim: MinigameComponent::initializer queries this before | ||
| // deciding whether to register with a registry. Zero = no registry. | ||
| Zero::zero() | ||
| } | ||
|
|
||
| fn mint( | ||
| ref self: ComponentState<TContractState>, | ||
| game_address: ContractAddress, | ||
| player_name: Option<felt252>, | ||
| settings_id: Option<u32>, | ||
| start: Option<u64>, | ||
| end: Option<u64>, | ||
| objective_id: Option<u32>, | ||
| context: Option<GameContextDetails>, | ||
| client_url: Option<ByteArray>, | ||
| renderer_address: Option<ContractAddress>, | ||
| skills_address: Option<ContractAddress>, | ||
| to: ContractAddress, | ||
| soulbound: bool, | ||
| paymaster: bool, | ||
| salt: u16, | ||
| metadata: u16, | ||
| ) -> felt252 { | ||
| // The signature matches IMinigameToken::mint so existing call | ||
| // sites work unchanged, but unsupported features must not be | ||
| // silently dropped — reject them loudly. | ||
| assert!(objective_id.is_none(), "MinigameTokenLite: objectives not supported"); | ||
| assert!(context.is_none(), "MinigameTokenLite: context not supported"); | ||
| assert!(client_url.is_none(), "MinigameTokenLite: client_url not supported"); | ||
| assert!( | ||
| renderer_address.is_none(), "MinigameTokenLite: per-token renderer not supported", | ||
| ); | ||
| assert!(skills_address.is_none(), "MinigameTokenLite: skills not supported"); | ||
| assert!(!paymaster, "MinigameTokenLite: paymaster flag not supported"); | ||
| assert!(metadata == 0, "MinigameTokenLite: metadata field not supported"); | ||
|
|
||
| // Single game — no SRC5 probe, no registry resolution. The | ||
| // parameter is kept (and checked) purely for call-site parity. | ||
| assert!( | ||
| game_address == self.game_address.read(), | ||
| "MinigameTokenLite: Game address does not match configured game", | ||
| ); | ||
|
|
||
| let caller = get_caller_address(); | ||
| let current_time = get_block_timestamp(); | ||
|
|
||
| // Same lifecycle rules as CoreTokenComponent::mint_game: a | ||
| // non-zero end must be in the future and after start (end_delay 0 | ||
| // means "no expiration", so a past window must not collapse into | ||
| // an immortal token), and a start at or before now clamps to now | ||
| // so the packed delays reconstruct the caller's intended end. | ||
| let lifecycle = token_state::create_lifecycle_with_defaults(start, end); | ||
| lifecycle.validate(); | ||
| assert!( | ||
| lifecycle.end == 0 | ||
| || (lifecycle.end > current_time && lifecycle.end > lifecycle.start), | ||
| "MinigameTokenLite: Lifecycle end must be in the future and after start", | ||
| ); | ||
| let effective_start = if lifecycle.start > current_time { | ||
| lifecycle.start | ||
| } else { | ||
| current_time | ||
| }; | ||
| let start_delay: u32 = (effective_start - current_time).try_into().unwrap(); | ||
| let end_delay: u32 = if lifecycle.end > effective_start { | ||
| (lifecycle.end - effective_start).try_into().unwrap() | ||
| } else { | ||
| 0 | ||
| }; | ||
|
|
||
| let tx_hash_bits = extract_tx_hash_bits(get_tx_info().unbox().transaction_hash); | ||
|
|
||
| let mut contract_self = self.get_contract_mut(); | ||
| let minted_by = MinterOpt::add_minter(ref contract_self, caller); | ||
|
|
||
| let final_token_id = pack_token_id( | ||
| 0, // game_id: always 0 — single game | ||
| minted_by, | ||
| settings_id.unwrap_or(0), | ||
| current_time, | ||
| start_delay, | ||
| end_delay, | ||
| 0, // objective_id | ||
| soulbound, | ||
| false, // has_context | ||
| false, // paymaster | ||
| tx_hash_bits, | ||
| salt, | ||
| 0 // metadata | ||
| ); | ||
|
|
||
| if let Option::Some(name) = player_name { | ||
| self.token_player_names.entry(final_token_id).write(name); | ||
| } | ||
|
|
||
| let mut contract = self.get_contract_mut(); | ||
| let mut erc721_component = ERC721::get_component_mut(ref contract); | ||
| erc721_component.mint(to, final_token_id.into()); | ||
|
|
||
| final_token_id | ||
| } | ||
|
|
||
| /// Emits an ERC-4906 `MetadataUpdate` without touching state. Same | ||
| /// deliberate no-existence-check trade-off as | ||
| /// `CoreTokenComponent::refresh_metadata`: the event is advisory, | ||
| /// consumers resolve token ids against their own mint records, and | ||
| /// the check would cost ~52k gas on the cheap path without stopping | ||
| /// spam anyway. | ||
| fn refresh_metadata(ref self: ComponentState<TContractState>, token_id: felt252) { | ||
| self.emit(MetadataUpdate { token_id: token_id.into() }); | ||
| } | ||
|
|
||
| fn refresh_metadata_batch( | ||
| ref self: ComponentState<TContractState>, token_ids: Span<felt252>, | ||
| ) { | ||
| assert!(token_ids.len() > 0, "MinigameTokenLite: token_ids array cannot be empty"); | ||
| let mut i: u32 = 0; | ||
| while i < token_ids.len() { | ||
| self.emit(MetadataUpdate { token_id: (*token_ids.at(i)).into() }); | ||
| i += 1; | ||
| } | ||
| } | ||
|
|
||
| fn update_player_name( | ||
| ref self: ComponentState<TContractState>, token_id: felt252, name: felt252, | ||
| ) { | ||
| assert!(!name.is_zero(), "MinigameTokenLite: Player name is empty"); | ||
| let contract = self.get_contract(); | ||
| let erc721_component = ERC721::get_component(contract); | ||
| let token_owner = erc721_component._owner_of(token_id.into()); | ||
| assert!( | ||
| token_owner == get_caller_address(), | ||
| "MinigameTokenLite: Caller is not owner of token", | ||
| ); | ||
| self.token_player_names.entry(token_id).write(name); | ||
| self.emit(MetadataUpdate { token_id: token_id.into() }); | ||
| } | ||
| } | ||
|
|
||
| #[generate_trait] | ||
| pub impl InternalImpl< | ||
| TContractState, | ||
| +HasComponent<TContractState>, | ||
| impl SRC5: SRC5Component::HasComponent<TContractState>, | ||
| impl ERC721: ERC721Component::HasComponent<TContractState>, | ||
| impl MinterOpt: OptionalMinter<TContractState>, | ||
| +Drop<TContractState>, | ||
| +ERC721Component::ERC721HooksTrait<TContractState>, | ||
| > of InternalTrait<TContractState> { | ||
| fn initializer(ref self: ComponentState<TContractState>, game_address: ContractAddress) { | ||
| assert!(!game_address.is_zero(), "MinigameTokenLite: Game address is zero"); | ||
| self.game_address.write(game_address); | ||
|
|
||
| let mut contract = self.get_contract_mut(); | ||
| let mut src5_component = SRC5::get_component_mut(ref contract); | ||
| src5_component.register_interface(IMINIGAME_TOKEN_LITE_ID); | ||
| // Also advertise the full-token id: MinigameComponent::initializer | ||
| // hard-asserts it before wiring a game to its token. The lite | ||
| // token implements the subset of IMinigameToken that game-side | ||
| // components actually call (mint, assert_is_playable, player_name, | ||
| // refresh_metadata, game_registry_address); anything else reverts | ||
| // with ENTRYPOINT_NOT_FOUND rather than misbehaving silently. | ||
| src5_component.register_interface(IMINIGAME_TOKEN_ID); | ||
| } | ||
|
|
||
| /// Lifecycle-window check only — there is deliberately no token-side | ||
| /// game_over / completed_objective state to consult. Games gate dead | ||
| /// runs themselves; they are the source of truth. | ||
| fn assert_lifecycle_open(self: @ComponentState<TContractState>, token_id: felt252) { | ||
| let packed = unpack_token_id(token_id); | ||
| let empty_state = TokenMutableState { | ||
| game_over: false, completed_objective: false, completed_at: 0, | ||
| }; | ||
| let metadata = to_token_metadata(packed, empty_state); | ||
| let current_time = get_block_timestamp(); | ||
| let lifecycle = metadata.lifecycle; | ||
| assert!( | ||
| lifecycle.can_start(current_time), | ||
| "MinigameTokenLite: Token is not playable - game has not started (now={}, start={})", | ||
| current_time, | ||
| lifecycle.start, | ||
| ); | ||
| assert!( | ||
| !lifecycle.has_expired(current_time), | ||
| "MinigameTokenLite: Token is not playable - game has expired (now={}, end={})", | ||
| current_time, | ||
| lifecycle.end, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace raw assertion messages with named error constants.
The component embeds raw error strings in each assert!, including the ownership, unsupported-parameter, lifecycle, and initialization checks. Define descriptive constants and use them consistently.
As per coding guidelines, “All error messages must be implemented as descriptive constants.”
🤖 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 `@packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo`
around lines 103 - 334, Replace every raw string passed to assert! in the
MinigameTokenLite component with descriptive named error constants, covering
ownership, unsupported mint parameters, lifecycle validation, and initialization
checks. Define the constants in the component’s established constants section
and update the affected assertions in functions such as assert_is_owner, mint,
assert_lifecycle_open, and initializer to reference them consistently.
Source: Coding guidelines
| /// SNIP-5 interface ID derived via src5_rs: XOR of extended function selectors. | ||
| /// | ||
| /// Surface is the trait below minus `refresh_metadata`/`refresh_metadata_batch`, | ||
| /// mirroring their exclusion from `IMINIGAME_TOKEN_ID`. Run `src5_rs parse` | ||
| /// against a stripped copy of this trait (see packages/interfaces/src/AGENTS.md) | ||
| /// to rederive. | ||
| pub const IMINIGAME_TOKEN_LITE_ID: felt252 = | ||
| 0x3ea3d599077fbe09ddbe82ff33c1abc87aef52d8609d8bf3508fdba8dd92056; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Record the extended function selectors.
The comment above IMINIGAME_TOKEN_LITE_ID does not contain the EFS output from src5_rs. Add the selectors and final XOR value that produced this constant.
As per coding guidelines, “Always update the EFS comment above an interface ID constant to match the output from src5_rs.”
🤖 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 `@packages/interfaces/src/token/lite.cairo` around lines 27 - 34, The EFS
comment above IMINIGAME_TOKEN_LITE_ID is missing the src5_rs output. Run src5_rs
against the trait with refresh_metadata and refresh_metadata_batch excluded,
then document every extended function selector and the final XOR value in the
comment while keeping the constant unchanged.
Source: Coding guidelines
| fn token_metadata(self: @TState, token_id: felt252) -> TokenMetadata; | ||
| fn is_playable(self: @TState, token_id: felt252) -> bool; | ||
| fn assert_is_playable(self: @TState, token_id: felt252); | ||
| /// Combined ownership + playability guard: one external call instead of | ||
| /// `owner_of` followed by `assert_is_playable`. `expected_owner` is the | ||
| /// game contract's caller (must be non-zero); panics unless it owns the | ||
| /// token and the lifecycle window is open. | ||
| fn assert_owner_and_playable(self: @TState, token_id: felt252, expected_owner: ContractAddress); | ||
| fn settings_id(self: @TState, token_id: felt252) -> u32; | ||
| fn player_name(self: @TState, token_id: felt252) -> felt252; | ||
| fn minted_by(self: @TState, token_id: felt252) -> felt252; | ||
| fn minted_by_address(self: @TState, token_id: felt252) -> ContractAddress; | ||
| fn is_soulbound(self: @TState, token_id: felt252) -> bool; | ||
| fn game_address(self: @TState) -> ContractAddress; | ||
| /// Always returns the zero address — the lite token has no registry. Kept | ||
| /// so `MinigameComponent::initializer`, which unconditionally queries the | ||
| /// registry address before deciding whether to register the game, works | ||
| /// against a lite deployment without modification. | ||
| fn game_registry_address(self: @TState) -> ContractAddress; | ||
|
|
||
| /// Signature-compatible with `IMinigameToken::mint`. `game_address` must be | ||
| /// the single configured game; `objective_id`, `context`, `client_url`, | ||
| /// `renderer_address`, `skills_address` must be `None`, `paymaster` must be | ||
| /// `false`, and `metadata` must be `0`. | ||
| fn mint( | ||
| ref self: TState, | ||
| game_address: ContractAddress, | ||
| player_name: Option<felt252>, | ||
| settings_id: Option<u32>, | ||
| start: Option<u64>, | ||
| end: Option<u64>, | ||
| objective_id: Option<u32>, | ||
| context: Option<GameContextDetails>, | ||
| client_url: Option<ByteArray>, | ||
| renderer_address: Option<ContractAddress>, | ||
| skills_address: Option<ContractAddress>, | ||
| to: ContractAddress, | ||
| soulbound: bool, | ||
| paymaster: bool, | ||
| salt: u16, | ||
| metadata: u16, | ||
| ) -> felt252; | ||
| /// Emits an ERC-4906 `MetadataUpdate` for `token_id` — see | ||
| /// `IMinigameToken::refresh_metadata` for the spam/existence trade-offs; | ||
| /// identical semantics here. | ||
| fn refresh_metadata(ref self: TState, token_id: felt252); | ||
| fn refresh_metadata_batch(ref self: TState, token_ids: Span<felt252>); | ||
| fn update_player_name(ref self: TState, token_id: felt252, name: felt252); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Document the complete Token Lite API.
The interface and implementation add public methods without the required API documentation. Keep the interface contract and component behavior documented together.
packages/interfaces/src/token/lite.cairo#L38-L85: document each interface method, including behavior, parameter constraints, return values, and examples where useful.packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo#L77-L336: document each implementation and internal lifecycle method with matching behavior and constraints.
As per coding guidelines, “Every function must include clear explanation of what it does and why, parameter descriptions with types and constraints, return value documentation, and example usage when appropriate.”
📍 Affects 2 files
packages/interfaces/src/token/lite.cairo#L38-L85(this comment)packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo#L77-L336
🤖 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 `@packages/interfaces/src/token/lite.cairo` around lines 38 - 85, Document
every public interface method in packages/interfaces/src/token/lite.cairo (lines
38-85), including purpose, parameter types and constraints, return values, and
examples where useful; preserve the existing API contract. Add matching
documentation for each implementation and internal lifecycle method in
packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo
(lines 77-336), ensuring descriptions accurately reflect behavior and
constraints across both sites.
Source: Coding guidelines
Stage 0 for metagame (tournament-platform) compatibility with lite tokens: - mint_batch_recipients on CoreTokenLiteComponent, ABI-compatible with the full token (same global salt counter, salt + sum(counts) - 1 <= 0x3FF); batch work hoisted, unsupported params rejected like mint. Lite interface id rederived to include it. - metagame::metagame::assert_game_registered now accepts registry-less tokens: when game_registry_address() is zero (single-game full tokens and lite tokens), registered means the game <-> token pairing is mutual. Previously this path dispatched to address 0 and reverted. - Deployable TokenLiteContract example moved to test_common so downstream suites can declare it via build-external-contracts; embeddable_game_standard tests now consume it from there. End-to-end test covers MinigameComponent::initializer + assert_game_registered against a lite token. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Stage 0 for tournament-platform (budokan) compatibility pushed:
New coverage: 5 batch-mint tests + an end-to-end |
…okens The game-side settings/objectives extensions unconditionally dispatched create_settings/create_objective to the token — entrypoints a lite token does not have — bricking settings creation (including constructors that create default settings) for any game wired to a lite token. The token-side call stores nothing; it is an indexer announcement, and the game remains the source of truth. Probe SRC5 for the token extension id and skip the announcement when the surface is absent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The lite token and its game contract need each other's address at construction (the game's MinigameComponent::initializer SRC5-checks the token; the token binds its single game). Split the lite initializer into register_interfaces + bind_game (one-time) so real deployments can break the cycle: deploy the token unbound, deploy the game pointing at it, then bind. An unbound token cannot mint. Adds the production preset (ERC721 + CoreTokenLite + Minter + soulbound guard + Ownable + Upgradeable, Option<game_address> constructor, owner-gated bind_game) and openzeppelin_upgrades to workspace deps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/presets/src/lib.cairo (1)
3-16: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRestore the crate-level doc block and list the new preset.
The
///block at lines 5-15 now sits between module declarations, so it documentsminigame_token_liteinstead of the crate. The preset list also omits the new preset. Move the block above the module declarations, convert it to//!crate docs, and addMinigameTokenLite.📝 Proposed fix
-pub mod autonomous_buyback; -pub mod leaderboard; -/// # Game Components Presets -/// -/// Ready-to-deploy contracts built with game components. -/// These presets provide simple, generic implementations suitable for -/// common gaming use cases without requiring custom contract development. -/// -/// ## Available Presets -/// - **Leaderboard**: Tournament leaderboard management with scoring and ranking -/// - **AutonomousBuyback**: Autonomous token buyback via Ekubo TWAMM -/// - **StreamToken**: ERC20 token with built-in TWAMM distribution - -pub mod minigame_token_lite; +//! # Game Components Presets +//! +//! Ready-to-deploy contracts built with game components. +//! These presets provide simple, generic implementations suitable for +//! common gaming use cases without requiring custom contract development. +//! +//! ## Available Presets +//! - **Leaderboard**: Tournament leaderboard management with scoring and ranking +//! - **AutonomousBuyback**: Autonomous token buyback via Ekubo TWAMM +//! - **StreamToken**: ERC20 token with built-in TWAMM distribution +//! - **MinigameTokenLite**: Single-game lite ERC721 game token + +pub mod autonomous_buyback; +pub mod leaderboard; +pub mod minigame_token_lite;🤖 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 `@packages/presets/src/lib.cairo` around lines 3 - 16, Update the crate-level documentation in lib.cairo by moving the existing descriptive block above all module declarations and converting each doc comment from /// to //!; extend the “Available Presets” list with MinigameTokenLite, while preserving the existing module declarations and descriptions.
🧹 Nitpick comments (6)
packages/embeddable_game_standard/src/metagame/metagame.cairo (2)
33-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse a descriptive constant for the registration error.
The new zero-registry branch adds a second
"Game is not registered"literal. Define one error constant and reuse it at Line [34] and Line [41].As per coding guidelines, all Cairo error messages must use descriptive constants.
🤖 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 `@packages/embeddable_game_standard/src/metagame/metagame.cairo` around lines 33 - 35, Define a descriptive constant for the “Game is not registered” error and replace both duplicate string literals in the zero-registry branch and the existing registration check near line 41 with that constant.Source: Coding guidelines
17-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComplete the public helper documentation.
These functions explain their behavior but omit parameter types, constraints, and explicit return behavior.
packages/embeddable_game_standard/src/metagame/metagame.cairo#L17-L24: documentgame_addressand the revert conditions.packages/embeddable_game_standard/src/minigame/extensions/objectives/libs.cairo#L10-L16: document all parameter types and the unsupported-interface no-op.packages/embeddable_game_standard/src/minigame/extensions/settings/libs.cairo#L27-L37: document all parameter types and the unsupported-interface no-op.As per coding guidelines, every function must document parameter types, constraints, and return 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 `@packages/embeddable_game_standard/src/metagame/metagame.cairo` around lines 17 - 24, The public helper documentation is incomplete. In packages/embeddable_game_standard/src/metagame/metagame.cairo:17-24, document the game_address parameter type, its constraints, return behavior, and all revert conditions. In packages/embeddable_game_standard/src/minigame/extensions/objectives/libs.cairo:10-16 and packages/embeddable_game_standard/src/minigame/extensions/settings/libs.cairo:27-37, document every parameter’s type and constraints, the return behavior, and that unsupported interfaces are handled as no-ops.Source: Coding guidelines
packages/embeddable_game_standard/src/minigame/tests/test_objectives_libs.cairo (1)
46-47: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest the unsupported optional-token surfaces.
Both test files cover only the supported-interface branch. Add one negative case at each site.
packages/embeddable_game_standard/src/minigame/tests/test_objectives_libs.cairo#L46-L47: returnfalsefromsupports_interfaceand verify nocreate_objectivedispatch.packages/embeddable_game_standard/src/minigame/tests/test_settings_libs.cairo#L119-L120: returnfalsefromsupports_interfaceand verify nocreate_settingsdispatch.Based on the PR objective, these tests protect lite-token compatibility.
🤖 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 `@packages/embeddable_game_standard/src/minigame/tests/test_objectives_libs.cairo` around lines 46 - 47, Extend the tests at packages/embeddable_game_standard/src/minigame/tests/test_objectives_libs.cairo:46-47 and packages/embeddable_game_standard/src/minigame/tests/test_settings_libs.cairo:119-120 with negative optional-token cases: configure supports_interface to return false, then verify no create_objective dispatch in the objectives tests and no create_settings dispatch in the settings tests.packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo (2)
846-899: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared minigame initializer wiring.
deploy_initialized_minigame_mockandtest_minigame_initializer_and_game_registered_with_lite_tokenrepeat the same 14-argumentinitializercall. Split the helper into a deploy step and aninitialize_minigame_mock(game_address, token_address)step, then reuse it in both places.♻️ Proposed refactor
+fn initialize_minigame_mock(game_address: ContractAddress, token_address: ContractAddress) { + game_components_test_common::mocks::minigame_mock::IMinigameMockInitDispatcher { + contract_address: game_address, + } + .initializer( + ALICE(), + "Game", + "d", + "dev", + "pub", + "genre", + "img", + Option::None, + Option::None, + Option::None, + Option::None, + Option::None, + token_address, + Option::None, + ); +} + fn deploy_initialized_minigame_mock(token_address: ContractAddress) -> ContractAddress { let contract = declare("minigame_mock").unwrap().contract_class(); let (game_address, _) = contract.deploy(`@array`![]).unwrap(); - game_components_test_common::mocks::minigame_mock::IMinigameMockInitDispatcher { - contract_address: game_address, - } - .initializer( - ALICE(), - "Game", - "d", - "dev", - "pub", - "genre", - "img", - Option::None, - Option::None, - Option::None, - Option::None, - Option::None, - token_address, - Option::None, - ); + initialize_minigame_mock(game_address, token_address); game_address }🤖 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 `@packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo` around lines 846 - 899, Extract the repeated 14-argument initializer call into a helper named initialize_minigame_mock(game_address, token_address). Keep deploy_initialized_minigame_mock focused on declaring and deploying the contract, then call the new helper there and from test_minigame_initializer_and_game_registered_with_lite_token, preserving all existing initializer arguments and behavior.
727-734: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider adding the passing salt boundary case.
The test covers the rejected case at
salt + count - 1 == 1024. Add the accepted case at exactly1023(for examplesalt: 1020, count: 4). That pins the bound on both sides.🤖 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 `@packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo` around lines 727 - 734, Add a passing boundary test alongside test_mint_batch_recipients_rejects_salt_overflow, using batch_neutral with salt 1020 and count 4 so salt + count - 1 equals 1023. Assert the batch mint succeeds and preserve the existing overflow rejection test.packages/presets/src/tests/test_minigame_token_lite.cairo (1)
63-109: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding coverage for the preset-only surfaces.
The tests cover binding and minting. Three preset-specific behaviors remain untested: the
upgradeowner-only guard, the constructor zero-owner assertion, and the soulboundbefore_updateguard in this contract. Each is a short test that reusesdeploy.🤖 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 `@packages/presets/src/tests/test_minigame_token_lite.cairo` around lines 63 - 109, Extend the tests around deploy and admin behavior to cover the preset-only surfaces: add an owner-only upgrade test for the upgrade entry point, a constructor test asserting deployment with a zero owner fails, and a soulbound transfer/update test confirming before_update rejects changes. Reuse deploy and the existing dispatchers/constants, and assert the expected panic messages where established.
🤖 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 `@packages/presets/src/minigame_token_lite.cairo`:
- Around line 104-119: Define a module-scoped descriptive constant for "Token is
soulbound and cannot be transferred" and use it in the panic! call within
ERC721HooksImpl::before_update in packages/presets/src/minigame_token_lite.cairo
lines 104-119. Apply the same constant pattern and wording in the
TokenLiteContract guard at
packages/test_common/src/examples/token_lite_contract.cairo line 86 so both
implementations remain identical.
- Around line 142-147: Update the guards in mint and mint_batch_recipients to
reject a zero caller-supplied game_address using the same non-zero assertion as
bind_game, while preserving the existing bound-address comparison. Add tests
covering zero-address calls to both mint paths before bind_game.
---
Outside diff comments:
In `@packages/presets/src/lib.cairo`:
- Around line 3-16: Update the crate-level documentation in lib.cairo by moving
the existing descriptive block above all module declarations and converting each
doc comment from /// to //!; extend the “Available Presets” list with
MinigameTokenLite, while preserving the existing module declarations and
descriptions.
---
Nitpick comments:
In `@packages/embeddable_game_standard/src/metagame/metagame.cairo`:
- Around line 33-35: Define a descriptive constant for the “Game is not
registered” error and replace both duplicate string literals in the
zero-registry branch and the existing registration check near line 41 with that
constant.
- Around line 17-24: The public helper documentation is incomplete. In
packages/embeddable_game_standard/src/metagame/metagame.cairo:17-24, document
the game_address parameter type, its constraints, return behavior, and all
revert conditions. In
packages/embeddable_game_standard/src/minigame/extensions/objectives/libs.cairo:10-16
and
packages/embeddable_game_standard/src/minigame/extensions/settings/libs.cairo:27-37,
document every parameter’s type and constraints, the return behavior, and that
unsupported interfaces are handled as no-ops.
In
`@packages/embeddable_game_standard/src/minigame/tests/test_objectives_libs.cairo`:
- Around line 46-47: Extend the tests at
packages/embeddable_game_standard/src/minigame/tests/test_objectives_libs.cairo:46-47
and
packages/embeddable_game_standard/src/minigame/tests/test_settings_libs.cairo:119-120
with negative optional-token cases: configure supports_interface to return
false, then verify no create_objective dispatch in the objectives tests and no
create_settings dispatch in the settings tests.
In
`@packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo`:
- Around line 846-899: Extract the repeated 14-argument initializer call into a
helper named initialize_minigame_mock(game_address, token_address). Keep
deploy_initialized_minigame_mock focused on declaring and deploying the
contract, then call the new helper there and from
test_minigame_initializer_and_game_registered_with_lite_token, preserving all
existing initializer arguments and behavior.
- Around line 727-734: Add a passing boundary test alongside
test_mint_batch_recipients_rejects_salt_overflow, using batch_neutral with salt
1020 and count 4 so salt + count - 1 equals 1023. Assert the batch mint succeeds
and preserve the existing overflow rejection test.
In `@packages/presets/src/tests/test_minigame_token_lite.cairo`:
- Around line 63-109: Extend the tests around deploy and admin behavior to cover
the preset-only surfaces: add an owner-only upgrade test for the upgrade entry
point, a constructor test asserting deployment with a zero owner fails, and a
soulbound transfer/update test confirming before_update rejects changes. Reuse
deploy and the existing dispatchers/constants, and assert the expected panic
messages where established.
🪄 Autofix
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 Plus
Run ID: 15b8122b-9306-458c-8aff-1eb1ba646387
📒 Files selected for processing (20)
Scarb.tomlpackages/embeddable_game_standard/Scarb.tomlpackages/embeddable_game_standard/src/metagame/metagame.cairopackages/embeddable_game_standard/src/minigame/extensions/objectives/libs.cairopackages/embeddable_game_standard/src/minigame/extensions/settings/libs.cairopackages/embeddable_game_standard/src/minigame/tests/test_objectives_libs.cairopackages/embeddable_game_standard/src/minigame/tests/test_settings_libs.cairopackages/embeddable_game_standard/src/token_lite.cairopackages/embeddable_game_standard/src/token_lite/AGENTS.mdpackages/embeddable_game_standard/src/token_lite/tests.cairopackages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairopackages/embeddable_game_standard/src/token_lite/token_lite_component.cairopackages/interfaces/src/token/lite.cairopackages/presets/Scarb.tomlpackages/presets/src/lib.cairopackages/presets/src/minigame_token_lite.cairopackages/presets/src/tests.cairopackages/presets/src/tests/test_minigame_token_lite.cairopackages/test_common/src/examples.cairopackages/test_common/src/examples/token_lite_contract.cairo
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/embeddable_game_standard/src/token_lite.cairo
Full change log vs the original architecture with rationale and measured results across game-components #123, SDM #149/#150 and budokan #313, including the Sepolia E2E verification and per-game cost impact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/denshokan-lite-migration.md`:
- Line 17: Update the fenced code block in the migration document to specify the
text language, changing its opening fence to use text while preserving the ASCII
architecture diagram content.
🪄 Autofix
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 Plus
Run ID: c5d1a8c7-e54b-4521-8f24-d7f50078b7f7
📒 Files selected for processing (1)
docs/denshokan-lite-migration.md
|
|
||
| The original stack (mainnet today): | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify a language for the fenced code block.
markdownlint-cli2 reports MD040 at Line 17. Use text for this ASCII architecture diagram.
Proposed fix
-```
+```text📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 17-17: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/denshokan-lite-migration.md` at line 17, Update the fenced code block in
the migration document to specify the text language, changing its opening fence
to use text while preserving the ASCII architecture diagram content.
Source: Linters/SAST tools
Anchors harness action deltas to each stack's measured on-chain overhead (reproduces the measured attack numbers exactly) to estimate explore, surrender and select_stat_upgrades; notes the mock under-charge bias and the light-action/batching interaction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…the token The lite component briefly supported two deployment shapes: embedded in the game contract (one-address) and as a separate token contract paired to a game. Measurements showed the separate shape strictly worse on gas, and supporting it kept dead machinery alive. The component is now self-bound only: - Delete the game_address storage slot; game_address() returns the contract's own address (kept as a view for ecosystem consumers). - mint/mint_batch_recipients keep the game_address parameter for ABI parity; it must equal the contract's own address (same error string). - Collapse InternalTrait to a single no-arg initializer registering the two SRC5 ids; delete bind_game and the two-phase register_interfaces (they only existed to break the removed shape's constructor circularity). - Delete the MinigameTokenLite preset and the minigame::lite pre_action/post_action helpers — in the one-address world the game calls the component internally. - assert_game_registered's registry-less branch becomes a plain token_address == game_address equality, saving a cross-contract call at tournament creation. - Replace the TokenLiteContract example with a merged LiteGameMock (one contract that is both game and token) and rework the token_lite tests and gas bench around it. The IMinigameTokenLite ABI and IMINIGAME_TOKEN_LITE_ID are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo (1)
87-163: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftAdd complete function-level Cairo documentation.
The new functions do not consistently document purpose, parameter types and constraints, return values, and examples where useful.
packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo#L87-L163: document token views and ownership/playability guards.packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo#L165-L261: documentmintparameters, rejected features, and token ID result.packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo#L275-L487: complete documentation for batch minting, metadata updates, player-name updates, initialization, and lifecycle validation.packages/test_common/src/mocks/lite_game_mock.cairo#L18-L25: document the test interface methods and their test-only behavior.packages/test_common/src/mocks/lite_game_mock.cairo#L118-L302: document hook behavior, constructor inputs, game views, token-data views, and state-mutating test helpers.As per coding guidelines, “Every function must include clear explanation of what it does and why, parameter descriptions with types and constraints, return value documentation, and example usage when appropriate.”
🤖 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 `@packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo` around lines 87 - 163, Add complete Cairo doc comments to every function across packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo ranges 87-163, 165-261, and 275-487, covering purpose, rationale, parameter types and constraints, return values, and examples where useful; include token views, ownership/playability guards, minting, batch minting, metadata and player-name updates, initialization, and lifecycle validation. Also document every test interface and helper in packages/test_common/src/mocks/lite_game_mock.cairo ranges 18-25 and 118-302, including test-only behavior, hook behavior, constructor inputs, game/token-data views, and state mutations. Preserve all existing behavior and use the project’s established Cairo documentation style.Source: Coding guidelines
docs/denshokan-lite-migration.md (3)
126-126: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMap each live address to its stack and tournament.
The paragraph reports both stacks but does not identify which tournament uses the
standalone lite token. Label the addresses explicitly, such astournament 1 multi-contract tokenandtournament 2 one-address GameCore/token.🤖 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 `@docs/denshokan-lite-migration.md` at line 126, Update the live E2E proof paragraph to explicitly map each listed address to its stack and tournament, including labeling the standalone lite token as the multi-contract token for tournament 1 or the one-address GameCore/token for tournament 2 as applicable. Preserve the existing address values and execution summary while making the tournament-to-address relationships unambiguous.
9-9: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLimit the “zero changes” claim to the one-address transition.
Lines 70-80 document substantial Budokan v2 changes, including constructor, configuration, fee handling, and viewer fixes. State that no additional Budokan changes were required after the lite-only integration if that is the intended claim.
🤖 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 `@docs/denshokan-lite-migration.md` at line 9, Update the architecture summary near “zero changes” to scope the claim specifically to the one-address transition, stating that no further Budokan changes were needed after the lite-only integration. Ensure it does not imply Budokan v2 required no changes, since the documented constructor, configuration, fee-handling, and viewer updates remain valid.
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRecord the assumptions behind the dollar estimates.
$0.40,$0.35,$0.28, and$0.25cannot be reproduced from gas totals alone. Add the network, fee inputs, ETH/USD rate, and measurement date. Keep the gas figures as the primary comparison.Also applies to: 124-124
🤖 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 `@docs/denshokan-lite-migration.md` at line 7, The cost estimates in the migration document lack reproducible assumptions. Update the section containing the beast-mode cost comparison and client-side batching estimate to document the network, gas-price or fee inputs, ETH/USD conversion rate, and measurement date, while keeping the gas figures as the primary comparison.
🤖 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 `@packages/test_common/src/mocks/lite_game_mock.cairo`:
- Line 131: Define a descriptive named error constant for the soulbound transfer
rejection in the relevant mock module, then update the panic! call in the
transfer logic to use that constant instead of the raw string while preserving
the existing error text.
- Around line 266-273: Update set_score and end_game to call
self.core_token_lite.refresh_metadata(token_id) after writing the score and
completion state, ensuring metadata is refreshed after every state change.
---
Outside diff comments:
In `@docs/denshokan-lite-migration.md`:
- Line 126: Update the live E2E proof paragraph to explicitly map each listed
address to its stack and tournament, including labeling the standalone lite
token as the multi-contract token for tournament 1 or the one-address
GameCore/token for tournament 2 as applicable. Preserve the existing address
values and execution summary while making the tournament-to-address
relationships unambiguous.
- Line 9: Update the architecture summary near “zero changes” to scope the claim
specifically to the one-address transition, stating that no further Budokan
changes were needed after the lite-only integration. Ensure it does not imply
Budokan v2 required no changes, since the documented constructor, configuration,
fee-handling, and viewer updates remain valid.
- Line 7: The cost estimates in the migration document lack reproducible
assumptions. Update the section containing the beast-mode cost comparison and
client-side batching estimate to document the network, gas-price or fee inputs,
ETH/USD conversion rate, and measurement date, while keeping the gas figures as
the primary comparison.
In `@packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo`:
- Around line 87-163: Add complete Cairo doc comments to every function across
packages/embeddable_game_standard/src/token_lite/token_lite_component.cairo
ranges 87-163, 165-261, and 275-487, covering purpose, rationale, parameter
types and constraints, return values, and examples where useful; include token
views, ownership/playability guards, minting, batch minting, metadata and
player-name updates, initialization, and lifecycle validation. Also document
every test interface and helper in
packages/test_common/src/mocks/lite_game_mock.cairo ranges 18-25 and 118-302,
including test-only behavior, hook behavior, constructor inputs, game/token-data
views, and state mutations. Preserve all existing behavior and use the project’s
established Cairo documentation style.
🪄 Autofix
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 Plus
Run ID: c06d01b0-c27c-4661-a08f-3c71b07f0bf9
📒 Files selected for processing (16)
docs/denshokan-lite-migration.mdpackages/embeddable_game_standard/Scarb.tomlpackages/embeddable_game_standard/src/metagame/metagame.cairopackages/embeddable_game_standard/src/token_lite.cairopackages/embeddable_game_standard/src/token_lite/AGENTS.mdpackages/embeddable_game_standard/src/token_lite/tests.cairopackages/embeddable_game_standard/src/token_lite/tests/test_gas_bench.cairopackages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairopackages/embeddable_game_standard/src/token_lite/token_lite_component.cairopackages/interfaces/src/AGENTS.mdpackages/interfaces/src/token/lite.cairopackages/presets/Scarb.tomlpackages/presets/src/lib.cairopackages/test_common/src/AGENTS.mdpackages/test_common/src/mocks.cairopackages/test_common/src/mocks/lite_game_mock.cairo
💤 Files with no reviewable changes (2)
- packages/presets/Scarb.toml
- packages/presets/src/lib.cairo
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/embeddable_game_standard/src/token_lite/tests.cairo
- packages/embeddable_game_standard/src/token_lite.cairo
- packages/interfaces/src/AGENTS.md
- packages/embeddable_game_standard/src/metagame/metagame.cairo
- packages/embeddable_game_standard/src/token_lite/tests/test_gas_bench.cairo
- packages/embeddable_game_standard/src/token_lite/tests/test_token_lite.cairo
- packages/interfaces/src/token/lite.cairo
| let current_owner = self._owner_of(token_id); | ||
| if !current_owner.is_zero() && !to.is_zero() { | ||
| if unpack_soulbound(token_id.try_into().unwrap()) { | ||
| panic!("Token is soulbound and cannot be transferred"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace the raw panic text with a named error constant.
Define a descriptive error constant for the soulbound transfer rejection. Use that constant in panic!.
🤖 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 `@packages/test_common/src/mocks/lite_game_mock.cairo` at line 131, Define a
descriptive named error constant for the soulbound transfer rejection in the
relevant mock module, then update the panic! call in the transfer logic to use
that constant instead of the raw string while preserving the existing error
text.
Source: Coding guidelines
| fn set_score(ref self: ContractState, token_id: felt252, score: u64) { | ||
| self.scores.entry(token_id).write(score); | ||
| } | ||
|
|
||
| fn end_game(ref self: ContractState, token_id: felt252, score: u64) { | ||
| self.scores.entry(token_id).write(score); | ||
| self.game_over.entry(token_id).write(true); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Emit metadata updates after score or completion changes.
set_score and end_game change data returned by IMinigameTokenData. Neither method calls refresh_metadata. Indexers can retain stale game metadata after either call.
Call self.core_token_lite.refresh_metadata(token_id) after each state update.
🤖 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 `@packages/test_common/src/mocks/lite_game_mock.cairo` around lines 266 - 273,
Update set_score and end_game to call
self.core_token_lite.refresh_metadata(token_id) after writing the score and
completion state, ensuring metadata is refreshed after every state change.
Source: Coding guidelines
1 participant
Summary
Adds a
token_litemodule toembeddable_game_standard: a single-game, storage-minimal replacement for the full minigame token, designed for deployments like super-death-mountain that never used the multi-game registry, objectives, context, skills, per-token renderers, client urls, or enumerable — and that keep game-over / objective-completion authority in the game contract itself.Design
game_id_from_addresson mint, no SRC5 probes anywhereupdate_game, no metagame callbacks, no game_over latch;is_playable= lifecycle window only, zero storage reads (pure unpack of the packed token id)assert_owner_and_playable(token_id, expected_owner)owner_of+assert_is_playablepair into one external call (one storage read total)mintkeeps the exactIMinigameToken::mintABIminigame::minthelper work unchanged; unsupported params are rejected loudly, never silently ignoredpack_token_idlayout unchangedsettings_id/minted_by/lifecycle from the id, and indexers, keep working; unused fields are written as zeroIMINIGAME_TOKEN_LITE_ID+ legacyIMINIGAME_TOKEN_ID, exposes zerogame_registry_address()MinigameComponent::initializeraccepts a lite token without modificationContext: super-death-mountain's own gas bench measures
update_gameas a ~6.73M L2 gas subtree, dominated by the twogame_over()/score()callbacks (~1.56M each). With no mutable state there is nothing to sync — games gate dead runs themselves and callrefresh_metadata(ERC-4906) after actions.Changes
packages/interfaces/src/token/lite.cairo—IMinigameTokenLite+IMINIGAME_TOKEN_LITE_ID(derived viasrc5_rs, excludingrefresh_metadata*per convention)packages/embeddable_game_standard/src/token_lite/—CoreTokenLiteComponent, module AGENTS.md, wiring example contract, 30 testscodecov.ymlbumped to 18 modules; root AGENTS.md matrix table refreshed (was stale at 16, missingmerkledrop)Test plan
scarb build --workspacesnforge test -p game_components_embeddable_game_standard "::token_lite::"— 30/30 passing (packing fields, lifecycle windows/clamping, all rejected params, soulbound transfer guard, combined owner+playable guard, minter map, ERC-4906 events, SRC5 registration)scarb fmt --check🤖 Generated with Claude Code
Summary by CodeRabbit