feat(token_lite): single-game gas-optimized token component (denshokan lite) - #123
Open
starknetdev wants to merge 29 commits into
Open
feat(token_lite): single-game gas-optimized token component (denshokan lite)#123starknetdev wants to merge 29 commits into
starknetdev wants to merge 29 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>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📝 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 Review[MEDIUM] packages/economy/src/tokenomics/buyback/buyback.cairo:558 - The strict config check lives in |
GPT Code Reviewlgtm |
Claude Code ReviewReview execution failed. Spending cap reached resets 11:40am |
Claude Code ReviewReview execution failed. Spending cap reached resets 11:40am |
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
Comment on lines
+4
to
+7
| // 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.
Comment on lines
+113
to
+216
|
|
||
| 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.
Suggested change
| 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
Comment on lines
+103
to
+334
| 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
Comment on lines
+27
to
+34
| /// 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
Comment on lines
+38
to
+85
| 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>
Member
Author
|
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.
Suggested change
| ``` |
🧰 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
Comment on lines
+266
to
+273
| 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
…ted as one reserved high-half region The lite token now owns its 251-bit token-id layout (token_lite/packing.cairo) instead of reusing the full token's pack_token_id. The full layout in token/structs.cairo is untouched — it keeps serving legacy denshokan. Indexers must branch their token-id decode by contract generation. Low u128 (128 bits): bits [0-34] minted_at 35 (unix seconds) bits [35-59] start_delay 25 bits [60-84] end_delay 25 (0 = no expiration) bits [85-100] settings_id 16 (ABI stays Option<u32>; value <= 0xFFFF) bits [101-126] minted_by 26 (minter id must fit 26 bits) bit [127] soulbound 1 High u128 (123 bits): bits [0-9] tx_hash 10 (last 10 bits of tx hash) bits [10-25] salt 16 (batch bound: salt + sum(counts) - 1 <= 0xFFFF) bits [26-122] reserved 97 (component-owned, ALWAYS packed as zero) Reserved-region rule: the 97 spare bits are one component-owned region with no pack parameter and no public unpack accessor. Future protocol- or game-facing fields are carved from it later; every id minted under this layout provably decodes the region as 0, so carve-outs are non-breaking by construction. The IMinigameTokenLite ABI is unchanged, so IMINIGAME_TOKEN_LITE_ID stays 0x2dc0909ee1d6854df56adcced7d2cd9c3ce2f8d5aa788a754f0ffde901fd5e7. Behavioral changes: settings_id > 0xFFFF is now rejected at mint; the batch salt bound widens from 0x3FF to 0xFFFF. LiteGameMock's soulbound hook and the token_lite tests migrate to the new packing helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… and read views stay Strip principle: delete dead machinery and compat shims; keep capability (writes) and cheap client-facing read views. The lite token's external ABI is no longer an IMinigameToken mirror. Removed surface: - mint / mint_batch_recipients trimmed to 7 args: the dead full-token params (game_address, objective_id, context, client_url, renderer_address, skills_address, paymaster, metadata) are gone along with their reject-asserts - game_address / game_registry_address views deleted — consumers SRC5-probe the lite id; metagame::assert_game_registered now probes IMINIGAME_TOKEN_LITE_ID first and falls through to the unchanged full-token registry path - assert_is_playable / assert_owner_and_playable moved off the ABI to InternalTrait — the embedding game's own pre-action guard (zero syscalls); clients read is_playable - refresh_metadata_batch deleted (a multicall of singles) - legacy IMINIGAME_TOKEN_ID SRC5 registration dropped — SRC5 is honest: a lite token does not implement IMinigameToken Kept with identical semantics: token_metadata, is_playable, settings_id, player_name, minted_by, minted_by_address, is_soulbound, update_player_name, refresh_metadata. New interface id (derived over the surface minus refresh_metadata, per the refresh-exclusion convention): IMINIGAME_TOKEN_LITE_ID = 0x2ec4714e0b5610e5cffd262be7c69b721a6865f9a8ce7e1094c8211f3beaa37 LiteGameMock rewires IMinigame::mint_game/mint_game_batch to the trimmed mint (dead params rejected at the mock) and re-exposes the internal guard for tests. token_lite 40/40, metagame 92/92. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…adata mint params — metadata widened to 65 bits Reverses part of the earlier ABI strip per user direction: the five mint parameters return with their ORIGINAL full-token behaviors, and the lite id's high half is now fully allocated (no reserved region). New high-u128 layout (low half unchanged): tx_hash(10) | salt(16) | paymaster(1) | has_context(1) | objective_id(30) | metadata(65). The spare bits were merged into the single writable metadata field, in line with the original layout's single-field design; a future protocol field would require a new contract generation (accepted trade-off). * mint / mint_batch_recipients: 12-arg shape (player_name, settings_id, start, end, objective_id, context, client_url, to|recipients, soulbound, paymaster, salt, metadata: u128). * objective_id: packed, INERT data the game interprets — no completion machinery; completed_objective stays always-false. objective_id view restored. * context: sets the has_context bit only; data NOT stored (full-token parity). Batch shares the bit. * client_url: storage-backed map + view, empty default; batch writes the url per token. * paymaster: packed bit. * metadata: u128 param packed into the 65-bit field; mint_metadata view. TokenMetadata.metadata (u16, deployed full-token ABI) stays 0 — never a truncation. * IMINIGAME_TOKEN_LITE_ID rederived over the 14-function surface (minus refresh_metadata): 0x15951d6d145a5a13c454bd75f0787e43e531a80a4bfb42a01fc4859e6fb7aea * LiteGameMock forwards the standard 15-arg mint_game naturally now; only renderer/skills remain asserted-None (u16 metadata widens via .into()). * Tests: roundtrip/boundary/reject coverage for the restored fields, client_url storage + default, has_context without storage, batch sharing; bit-exact layout proof updated. 47 token_lite + 92 metagame green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Brings the exact O(1) payout work (#120) and the Geometric/Tiered Distribution variants onto the token-lite branch, so consumers pinned to this branch get the same distribution surface as v1.1.12. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nal token→legacy Source-level rename only: every interface-id VALUE and selector is identical — deployed contracts registered them on-chain and the values are frozen. Naming map (old → new / legacy): - IMinigameTokenLite(*Dispatcher*) → IMinigameToken(*Dispatcher*) - IMINIGAME_TOKEN_LITE_ID → IMINIGAME_TOKEN_ID (value 0x15951d…7aea unchanged) - IMinigameToken (original full trait) → IMinigameTokenLegacy - IMINIGAME_TOKEN_ID (original) → IMINIGAME_TOKEN_LEGACY_ID (value 0x246f61…9906 unchanged) - interfaces: token/lite.cairo → token/core.cairo (new standard); original core trait moved to token/legacy.cairo - embeddable_game_standard: src/token_lite → src/token; src/token → src/token_legacy (everything inside token_legacy keeps its names — CoreTokenComponent, structs::PackedTokenId, … — the path is the marker) - CoreTokenLiteComponent → MinigameTokenComponent (CoreTokenLiteImpl → MinigameTokenImpl) - token::packing: LitePackedTokenId → PackedTokenId, pack_lite_token_id → pack_token_id, unpack_lite_token_id → unpack_token_id - error strings: "MinigameTokenLite: …" → "MinigameToken: …", "LitePackedTokenId: …" → "PackedTokenId: …" - test_common: lite_game_mock/LiteGameMock → standard_game_mock/StandardGameMock The minter is standard, not optional — absorbed into MinigameTokenComponent with storage-name compatibility (minter_counter, minter_addresses, minter_id_by_address), the same IMinigameTokenMinter surface + id and the same MinterRegistryUpdate event; OptionalMinter remains only in token_legacy. The standard token's SRC5 id is NOT rederived (its trait did not change). Consumers of the original token (minigame/metagame/registry libs and components, renderer utilities, examples) now target the *Legacy* names and token_legacy:: paths. metagame::assert_game_registered probes IMINIGAME_TOKEN_ID (the standard) first; the legacy registry fallback is unchanged. CI matrices: module token → token_legacy (ubuntu-latest-32), token_lite → token (ubuntu-latest-8); codecov build count unchanged (18). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eTokenCreator)
The registry's game_fee_info role moves onto the self-bound token itself.
With the registry retired, v2 monetization platforms (Budokan) had no way
to resolve a game's payee or minimum fee — both old paths (registry
game_fee_info navigation, registry-NFT-owner payee) are gone. The identity
now lives on the token standard, set at initialization.
New surface (packages/interfaces/src/token/creator.cairo):
- GameCreatorInfo { creator, license, fee_numerator } — the registry's
GameFeeInfo plus a payee; DEFAULT_GAME_FEE_BPS (500) and
default_license() reused as the initializer defaults.
- IMinigameTokenCreator: game_creator_info / game_creator_address (reads),
set_game_creator_address / set_game_fee (writes).
- IMINIGAME_TOKEN_CREATOR_ID =
0x21531ca59c09f4a8554a0c390d8054188d27b19148c9039f0279f2b66a86de7
(src5_rs; registered by the initializer alongside the minter id).
Authorization: the game contract's OZ Ownable OWNER administers the surface
(assert_only_owner on both setters) — the stored creator is a payout sink,
not an admin. This is a HARD OwnableComponent::HasComponent bound on
CreatorImpl: BREAKING for integrators — every contract embedding
MinigameTokenComponent must now also embed OwnableComponent, and the
initializer gains (game_creator, license: Option, fee_numerator: Option)
with a non-zero creator assert. Rotation to zero is rejected (must never
brick the payee); fee capped at FEE_DENOMINATOR. Consumers must resolve the
payee LIVE at claim time so rotation is honoured.
StandardGameMock embeds Ownable and takes (game_creator, owner) constructor
params. IMINIGAME_TOKEN_ID unchanged (core trait untouched). 7 new tests;
token 54 / metagame 92 / minigame 214 / presets 187 all green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rd-split unpack Same method next-death-mountain uses for its model packing, ported to the standard token id codec. The bit LAYOUT is unchanged — ids, ABI, interface ids and downstream decoders are all untouched; this is implementation-only. - pack_token_id: pure felt252 arithmetic (a valid id is <= 251 bits < P, so felt add/mul is exact) — no u128 multiplications, no u256 assembly. - unpack_token_id: each u128 half splits ONCE at a field-aligned boundary (low at bit 60, high at bit 58 where 65-bit metadata falls out as the quotient), then all field extractions run as u64 DivRems. 3 u128 + 6 u64 DivRems, was 10 u128. - per-field helpers: one shift-to-bottom u128 DivRem + at most two u64 ops (soulbound/metadata collapse to a single quotient). Bench (l2_gas): mint 2,580,763 -> 2,455,483 (-4.9%); batch mint -125k/token; guard -14.5k/call; post_action -12.5k/call. token 54/54, metagame 92/92. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`assert_game_registered` was widened to accept self-bound standard tokens, but every path behind that gate still spoke the legacy ABI — so an accepted game passed the gate and reverted at mint. The component's initializer also rejected standard tokens outright, so such a metagame could not even be constructed. All four entry points now branch on SRC5: * `initializer` accepts a default token supporting either `IMINIGAME_TOKEN_LEGACY_ID` or `IMINIGAME_TOKEN_ID`. * `mint` / `mint_batch` route to the standard 12-arg `mint` for standard tokens. The unsupported `renderer_address` / `skills_address` params are rejected loudly rather than silently dropped. * `get_game_fee_info` / `pay_game_fee` read the token's creator surface (`game_creator_info` / `game_creator_address`) when it advertises `IMINIGAME_TOKEN_CREATOR_ID`, keeping the registry -> NFT-owner walk for legacy tokens. * The zero-registry legacy branch now compares the token's own `game_address()` to the game, instead of asserting token == game. A legacy token is a separate contract from its game, so the old equality could never hold for a single-game legacy deployment. Every remaining legacy dispatcher call sits in a legacy-only fallback branch. Tests: 10 new cases in `metagame::tests::test_libs` — the standard-token mint/fee/registration paths run against the real merged game+token contract (`StandardGameMock`) rather than a mock ABI, plus the legacy single-game pairing including its mispaired negative case. Known limitation: `MetagameComponent::mint` still takes `metadata: u16`, so a metagame cannot reach the standard token's 65-bit metadata field. Widening it is a breaking metagame ABI change, left as a follow-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every game brings its own token, so a default token had nothing to point at. The token is now resolved from `game_address` on every mint. BREAKING: * `IMetagame` loses `default_token_address()`, so IMETAGAME_ID changes from 0x7997c74299c045696726f0f7f0165f85817acbb0964e23ff77e11e34eff6f2 to 0x1363c8de5144122290d663c4c7a10d09518fbe76475610a7027ea4770b9c179 (rederived with src5_rs; regenerating the old two-method trait reproduces the old constant exactly, confirming the derivation). Consumers probing the old id must update. * `initializer(context_address)` no longer takes a token, and no longer SRC5-validates one at construction — a bad game address now surfaces at first mint instead. * `mint`, `mint_batch` and `MintMetagameParams.game_address` take a REQUIRED `ContractAddress`. The blank-game mint (game_address = 0 against a default token) no longer exists as a capability. * `MetagameCallbackComponent::initializer(token_address)` binds its own legacy token. Callbacks fire from `update_game()`, which the standard token does not have, so the extension is legacy-only and owns the binding — it no longer depends on MetagameComponent, and its guard is a storage read rather than a cross-component call. `context_address()` was verified unused outside the component: the legacy token takes context as a mint parameter and never resolves a provider address. Tests: full package 1197 passed / 0 failed. The obsolete `test_minigame_token_address_view` is removed and a zero-token guard test added for the callback initializer, leaving the marker count unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors `MinigameTokenComponent`: the embedding contract IS the metagame, so it holds no addresses at all. `context_address` had no consumers. A metagame that provides context embeds `ContextComponent` itself, registering IMETAGAME_CONTEXT_ID on its own address — nothing ever resolved a provider through a stored address, because the legacy token takes context as a mint parameter and dispatches to `ContextOpt::on_context_set`. BREAKING: * `MetagameComponent` has empty storage and no `initializer`. Neither address it used to hold exists, so there was nothing left to initialize. * `IMetagame` and `IMETAGAME_ID` are REMOVED, not renumbered. With both views gone the trait had no methods, and an SRC5 id cannot be derived from an empty selector set. Nothing probed the id: the component registered it and two tests asserted the registration, with no production consumer. Discover a metagame through the surfaces that still carry meaning — IMETAGAME_CONTEXT_ID for a context provider, IMETAGAME_CALLBACK_ID for a legacy callback receiver. The component is now internal helpers over `metagame::metagame` (`libs`), each branching on SRC5 to serve both token generations. Removes the four tests covering the deleted views (T001.1, T001.2 and the two context_address view tests), leaving a note in their place. Tests: full package 1193 passed / 0 failed (1197 minus those four). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ctor CI fails the build on any test-compilation diagnostic; my local runs were filtered to `Tests:|FAIL`, which hid them. All five came from the preceding two commits: * `test_mint_batch_empty_array` bound a game it never used — an empty batch needs no game at all. * `test_mint_batch_mixed_game_addresses` had a duplicated binding that shadowed the first, so both entries minted against the SAME game and the test no longer tested anything mixed. It now deploys game_a and game_b and asserts each token carries its own game address; the leftover 'NoGame' assertion belonged to the deleted default-token arm. * `Zero` and `ISRC5Dispatcher`/`Trait` were orphaned in test_metagame_component when the four IMetagame view tests were removed. Also refreshes comments still describing the removed default-token path, and renames `test_mint_defaults_to_standard_token` (a duplicate after the change) to `test_mint_standard_token_minimal`. Verified with CI's own gate — `grep -qE '^ --> .*\.cairo:[0-9]+:[0-9]+'` over the raw test log — across all five packages: no diagnostics anywhere. embeddable_game_standard 1193 passed / 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… it paid `pay_game_fee` ignored the bool returned by `IERC20.transfer`. An ERC20 that signals failure by returning false rather than reverting would leave the component returning a non-zero `fee_amount`, so callers would proceed as if the game creator had been paid. Pre-existing, but the new standard-token creator-fee path inherits it. The lib cannot defend against this from the outside — there is no catchable external call here — so the assert belongs at the transfer site. Adds the first `pay_game_fee` tests (there were none): the false-return rejection, the happy path returning 5% of revenue at the default 500 bps, and the zero-revenue short circuit, via a MockFeePayer exposing the component method. Reported by Codex review. Full package: 1196 passed / 0 failed, no compilation diagnostics. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n path The standard-token branches I added trusted `game_address.token_address()` without checking that the game IS its token. The legacy paths were implicitly protected — they resolve through the registry, which rejects unregistered games — but the standard paths had no equivalent gate. A standard token is self-bound, so `token_address() == game_address` is its registration check. `assert_game_registered` already applied it; `mint`, `get_game_fee_info` and `get_game_creator_address` did not. Impact: an attacker deploys a standard token naming themselves as creator with `fee_numerator` at 10000, plus a trivial contract whose `token_address()` returns it. A metagame calling `pay_game_fee` with that address transfers 100% of revenue to the attacker. The same gap let a hostile contract have a metagame mint on a standard token it does not own, poisoning `minted_by`. All three paths now call a shared `assert_self_bound` before trusting the token. Adds four regression tests: a hostile game blocked on each path, plus a legitimate self-bound game confirmed to still pass all three — three should_panic tests would also pass if the guard were simply too broad. Reported by Codex review. Full package: 1200 passed / 0 failed, no compilation diagnostics. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…onent buy_back is permissionless and _get_effective_config falls back to the global config for any token without a per-token override — so every token is buyable-back by default, with no way for a deployment to say "only tokens I have explicitly configured". Ekubo's revenue_buybacks expresses this as an Option<Config> default; that exact shape does not transfer here because sweep_buy_token_to_treasury reads buy_token/treasury identity off the global config, which must stay mandatory. Only the trading POLICY is gated: - Buyback_require_token_config storage flag (default false — no-op for every existing consumer; purely opt-in strictness) - _get_effective_config asserts 'No config for token' in the fallback arm when the flag is on - IBuybackAdmin::set_require_token_config + RequireTokenConfigUpdated event; owner-gated in AutonomousBuyback (and the test mock) Rationale (Budokan): protocol-fee revenue arrives in whatever token a tournament host charged; with the flag on, governance decides which of those are tradeable rather than anyone opening TWAMM orders into whatever pool happens to exist. Replaces a rejected allowlist-wrapper design in token_buybacks — config-level gating belongs in the component. tokenomics 221/221 (3 new), presets 187/187. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… surface The initializer registers IMINIGAME_TOKEN_ID, IMINIGAME_TOKEN_MINTER_ID and IMINIGAME_TOKEN_CREATOR_ID unconditionally, but MinigameTokenImpl / MinterImpl / CreatorImpl were separate embeds — a contract wiring only some of them would advertise SRC5 surfaces it does not expose, and consumers that probe-then-dispatch (e.g. metagame's get_game_fee_info) would revert on the missing entrypoint. Fix, ERC20MixinImpl-style: new MinigameTokenABI trait (interfaces token/core) combining all 22 entrypoints, and MinigameTokenMixinImpl forwarding to the three inner impls — one embed, honest SRC5 by construction. NOT used for id derivation; the three ids are unchanged. Separate impls stay exported for contracts that wire them individually (SDM GameCore does today — adoption optional). StandardGameMock switches to the mixin; all existing dispatcher-based tests exercise it unchanged. token 54 / metagame 105 / presets 187. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… embedders The SRC5-honesty gap is closed by construction only for mixin-wired contracts; individually-wired ones are honest by convention. Put the invariant where an embedder actually looks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ot deploy Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…batch
`mint_batch` loops over `mint`: one cross-contract call per token, with the
`context` struct (which holds an Array) re-serialised each time. That made the
component unadoptable for budokan v2, whose tournament entry mints every
entrant through the token's own `mint_batch_recipients` in a single call —
adopting would have turned 1 dispatch into N on its hottest path.
Adds `libs::mint_batch_recipients` and the matching component method, routing
one dispatch to whichever generation the game's token is. `mint_batch` stays:
it serves heterogeneous batches where each entry names a different game.
`metadata` is `u128`, not `u16`, because that is what the consumer actually
passes — budokan widened metadata_value to u128 (their 67875f1), so a u16
passthrough would have compiled and been useless. The legacy token's field is
u16, so the legacy path asserts the value fits instead of truncating silently.
The standard path takes the same `assert_self_bound` gate as every other
standard-token path, and rejects renderer/skills loudly rather than dropping
them.
Tests (10, own file): recipient ordering and per-recipient counts, distinct
ids across the global salt counter, the wide-metadata round trip verified
through `mint_metadata`, hostile-game rejection, renderer/skills rejection,
and the legacy path both accepting a renderer and rejecting over-wide
metadata. `mint_batch_recipients` was `panic!("not implemented")` in both
legacy test mocks — implemented for real so the legacy branch is exercised
rather than stubbed.
Also refreshes the module AGENTS.md: the embedding example still showed the
removed `MetagameImpl` ABI, the InternalTrait table was missing several
methods, and MintMetagameParams was missing four fields.
Full package: 1210 passed / 0 failed, no compilation diagnostics, fmt clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`mint` took `metadata: u16` while `mint_batch_recipients` took `u128`, so the
two paths disagreed about how much metadata a caller could carry. Budokan
threads a u128 metadata_value through all three of its entrypoints; adopting
`mint` as-is would have narrowed its single-mint path to 16 bits while its
batch path kept 128 — a regression on work that had just landed.
BREAKING: `libs::mint`, `MetagameComponent::mint` and
`MintMetagameParams.metadata` all take `u128`. The legacy token's field is
still `u16`, so the legacy branch narrows with the same
`try_into().expect('Metagame: metadata exceeds u16')` the batch path already
used — both paths now reject identically rather than one truncating.
Only the metagame surfaces widened. Most `metadata: u16` occurrences in these
files belong to legacy TOKEN mocks implementing `IMinigameTokenLegacy`, whose
field genuinely is u16; widening those would have made the mocks lie about the
interface they implement. `IMinigame::mint_game` also keeps u16 — the minigame
helper layer is legacy-only (MinigameComponent asserts
IMINIGAME_TOKEN_LEGACY_ID), so u16 is correct there.
Two tests that discriminate rather than decorate: `test_mint_carries_wide_metadata`
round-trips 0x100000000 through `mint` and reads it back via `mint_metadata`
(would not compile under u16, would fail under silent truncation), and
`test_mint_rejects_wide_metadata_on_legacy_token` proves the single path
narrows exactly as the batch path does.
Raised by the budokan session, which hit it while evaluating adoption.
Full package: 1212 passed / 0 failed, no compilation diagnostics, fmt clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes the self-bound single-game token THE minigame token standard. The original multi-game, registry-backed token is preserved unchanged as
token_legacyfor the deployed denshokan.The new standard is built for deployments that never used the multi-game registry, objectives machinery, context storage, skills, per-token renderers or enumerable — and that keep game-over / objective-completion authority in the game contract itself.
Breaking changes
token/is now the standard token; the original moves totoken_legacy/token_legacyand their SRC5 probe toIMINIGAME_TOKEN_LEGACY_IDgame_idresolution, no SRC5 probes on mint. There is nogame_addressview or mint parameter at all; consumers identify a standard token by SRC5 (IMINIGAME_TOKEN_ID)update_game, no metagame callbacks, no game_over latch.is_playable= lifecycle window only, zero storage reads. Games gate dead runs themselves and callrefresh_metadata(ERC-4906) after actionsIMinigameTokenLegacy-compatiblegame_address/renderer_address/skills_addressmint params are gone, along with thegame_addressandgame_registry_addressviews. Cheap client-facing read views staytoken::packing)settings_idwidened to 16 bits,saltto 16,metadatato 65. Indexers must branch their decoder by contract generationIMinigameTokenMintersurface, sameMinterRegistryUpdateevent.OptionalMinterindirection remains only intoken_legacyIMinigameTokenCreator)game_fee_inforole moves to the token:game_creatorpayout sink, license and fee (bps) set at init, served viaIMINIGAME_TOKEN_CREATOR_ID. Setters gated on the game contract's OZ Ownable OWNER — the stored creator is a payee, not an adminMinigameComponentis legacy-onlyIMINIGAME_TOKEN_LEGACY_IDand callsgame_registry_address(). A standard-token game embedsMinigameTokenComponentdirectly (the one-address shape) instead of wiring a separate token throughMinigameComponentContext: super-death-mountain's 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.Metagame support for both token generations
MetagameComponentand themetagamelib serve both generations, branching on SRC5:assert_game_registered— a standard token is self-bound, so "registered" reduces to the mutual pairing (game.token_address() == game); registry-backed legacy tokens still ask the registry, and a zero-registry legacy token falls back to the same pairing check.mint/mint_batchroute to the standard 12-argmintfor standard tokens. The unsupportedrenderer_address/skills_addressparams are rejected loudly, never silently dropped.get_game_fee_info/pay_game_feeread the token's creator surface (game_creator_info()/game_creator_address()) when it advertisesIMINIGAME_TOKEN_CREATOR_ID, and keep the registry → NFT-owner walk for legacy tokens.Breaking: the metagame component is self-bound
MetagameComponentnow mirrorsMinigameTokenComponent— the embedding contract IS the metagame, and it holds no addresses at all:game_addresson each mint.mint/mint_batchtake a requiredgame_address: ContractAddress, andMintMetagameParams.game_addressis no longer anOption. The "blank game" mint (game_address = 0against a default token) no longer exists as a capability.ContextComponentitself, registeringIMETAGAME_CONTEXT_IDon its own address. Nothing ever resolved a provider through a stored address — the legacy token takes context as a mint parameter.IMetagameandIMETAGAME_IDare REMOVED, not renumbered. With both views gone the trait had no methods left, and an SRC5 id cannot be derived from an empty selector set. Nothing probed the old0x7997c74299c045696726f0f7f0165f85817acbb0964e23ff77e11e34eff6f2: the component registered it and two tests asserted the registration, with no production consumer. Discover a metagame throughIMETAGAME_CONTEXT_ID(context provider) orIMETAGAME_CALLBACK_ID(legacy callback receiver).initializer— nothing left to initialize, and therefore no SRC5 validation at construction. A bad game address now surfaces at first mint.MetagameCallbackComponent::initializer(token_address)binds its own legacy token. Callbacks fire fromupdate_game(), which the standard token does not have, so the extension is legacy-only, owns the binding, and no longer depends onMetagameComponentat all.Known limitation:
MetagameComponent::mintstill takesmetadata: u16, so a metagame cannot reach the standard token's full 65-bit metadata field. Likewisemint_batchis a per-token loop rather than a passthrough to the token'smint_batch_recipients. Both are breaking metagame ABI changes — tracked as follow-ups.Changes
packages/interfaces/src/token/core.cairo—IMinigameToken+IMINIGAME_TOKEN_ID;creator.cairo—IMinigameTokenCreator+IMINIGAME_TOKEN_CREATOR_ID(both derived viasrc5_rs, excludingrefresh_metadata*per convention)packages/embeddable_game_standard/src/token/—MinigameTokenComponent, standard packing codec, module AGENTS.mdpackages/embeddable_game_standard/src/token_legacy/— the original token, untouched behaviourpackages/embeddable_game_standard/src/metagame/— dual-generation SRC5 branching (above)packages/test_common/src/mocks/standard_game_mock.cairo— the merged game+token contract, declarable downstream viabuild-external-contractsAGENTS.md— architecture section split into standard vs legacy flows; module matrix refreshed to 18codecov.ymlat 18 modulesTest plan
scarb build --workspacescarb fmt --checksnforge test -p game_components_embeddable_game_standard(full, unfiltered) — 1193 passed / 0 failed, incl. new tests covering the standard-token mint / fee / registration paths against the real merged mock, the legacy single-game pairing, and the callback zero-token guardDownstream
Budokan v2 (Provable-Games/budokan#313) runs against this branch and is verified E2E on Sepolia against SDM's stack. It flips from the branch pin to a release tag once this merges and is tagged.
🤖 Generated with Claude Code