feat: bump metagame_extensions to v0.1.4 (context-owner namespacing)#105
feat: bump metagame_extensions to v0.1.4 (context-owner namespacing)#105starknetdev merged 3 commits intomainfrom
Conversation
metagame_extensions v0.1.4 namespaces extension storage by (context_owner, context_id) so that different consumer contracts can use the same context_id on a shared validator without collision. See Provable-Games/metagame_extensions#19 for the full interface change. ### Interface updates consumed here - Read methods on IEntryRequirementExtension / IEntryFeeExtension / IPrizeExtension gained a leading `context_owner: ContractAddress` parameter. - `context_owner(context_id)` was removed; replaced by `is_context_registered(context_owner, context_id) -> bool`. - Write methods are unchanged. ### Changes in this PR - Bump `metagame_extensions_interfaces` to `tag = "v0.1.4"`. - `entry_requirement_store.cairo`: dispatcher calls to `valid_entry` and `entries_left` now pass `starknet::get_contract_address()` as the `context_owner` — the metagame contract was the caller at `add_config` time, so it is the owner of the corresponding namespace on the validator. - Mocks under `metagame/src/entry_requirement/tests/mocks/` and `test_common/src/mocks/` updated to the new interface: added `is_context_registered`, removed `context_owner`, and threaded `context_owner` through the `bannable`, `valid_entry`, `should_ban`, and `entries_left` signatures. Mock storage rekeyed where needed so per-context maps are keyed by `(ContractAddress, u64)`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 41 minutes and 26 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR updates the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @starknetdev's task —— View job Claude Review - General Engineering Reviewrun=24853170227 attempt=1 sha=8e39f9806f2c2afbad1ff514edea26c4122d75bf scope=general Review Progress
AnalysisFiles in scope: Only Change reviewed:
Review ResultsNo issues found. The dependency version bump is:
The CI configuration properly handles dependency bumps:
Summary0 CRITICAL, 0 HIGH, 0 MEDIUM, 0 LOW, 0 INFO |
Codex Review - General Engineering ReviewReview process failed to complete. |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @starknetdev's task —— View job Claude Review - General Engineering Reviewrun=24853327443 attempt=1 sha=2e3844f4b24da55a68e39051d824bbd6980c7f0a scope=general Review Progress
AnalysisFiles in scope: Only Change reviewed:
Review ResultsNo issues found. The dependency version bump is:
The CI configuration properly handles dependency bumps:
Summary0 CRITICAL, 0 HIGH, 0 MEDIUM, 0 LOW, 0 INFO |
Codex Review - General Engineering ReviewReview process failed to complete. |
|
Claude finished @starknetdev's task —— View job Claude Review - Cairo/Starknet Contract Reviewrun=24853327443 attempt=1 sha=2e3844f4b24da55a68e39051d824bbd6980c7f0a scope=packages Review Progress
AnalysisFiles in scope: 6 files in packages/ directory with interface changes for context-owner namespacing Change reviewed: Interface update from metagame_extensions v0.1.3 to v0.1.4, which adds Review ResultsNo issues found. Dependency Management:
Interface Usage:
Mock Implementations:
Security & Correctness:
Summary0 CRITICAL, 0 HIGH, 0 MEDIUM, 0 LOW, 0 INFO |
Codex Review - Cairo/Starknet Contract ReviewReview process failed to complete. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the metagame_extensions_interfaces dependency to version 0.1.4 and refactors the entry requirement logic to include a context_owner parameter in extension calls. Correspondingly, mock validators were updated to support this new parameter and now use a composite key (ContractAddress, u64) for the bannable storage map. The review feedback suggests inlining the get_contract_address() call within EntryRequirementStoreImpl to improve code conciseness where the context_owner variable is only utilized once.
| let caller_address = get_caller_address(); | ||
| let context_owner = get_contract_address(); |
There was a problem hiding this comment.
While defining context_owner explicitly can improve readability, it is only used once in the following valid_entry call. You could consider passing get_contract_address() directly to the dispatcher call to be more concise, similar to how other parameters are handled if they don't need pre-processing.
let caller_address = get_caller_address();
let display_extension_address: felt252 = extension_config.address.into();
assert!(
extension_dispatcher
.valid_entry(get_contract_address(), context_id, caller_address, qualification),
| let caller_address = get_caller_address(); | ||
| let context_owner = get_contract_address(); |
There was a problem hiding this comment.
Similar to the validate_qualification function, context_owner is only used once in the entries_left call. Inlining get_contract_address() would make the code slightly more succinct.
let caller_address = get_caller_address();
let qualification = match qualifier {
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @starknetdev's task —— View job Claude Review - General Engineering Reviewrun=24853998869 attempt=1 sha=c186e3bb77e16c27795e4e94739278899e9ac4f2 scope=general Review Progress
AnalysisFiles in scope: Only Change reviewed:
Review ResultsNo issues found. The dependency version bump is:
The CI configuration properly handles dependency bumps:
Summary0 CRITICAL, 0 HIGH, 0 MEDIUM, 0 LOW, 0 INFO |
Codex Review - General Engineering ReviewReview process failed to complete. |
|
Claude finished @starknetdev's task —— View job Claude Review - Cairo/Starknet Contract Reviewrun=24853998869 attempt=1 sha=c186e3bb77e16c27795e4e94739278899e9ac4f2 scope=packages Review Progress
AnalysisFiles in scope: 6 files in packages/ directory with interface changes for context-owner namespacing Change reviewed: Interface update from metagame_extensions v0.1.3 to v0.1.4, which adds Review ResultsNo issues found. Dependency Management:
Interface Usage:
Mock Implementations:
Security & Correctness:
Summary0 CRITICAL, 0 HIGH, 0 MEDIUM, 0 LOW, 0 INFO |
Codex Review - Cairo/Starknet Contract ReviewReview process failed to complete. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…me_extensions v0.1.4) (#221) Bumps dependencies to pick up the extension interface change that namespaces per-context storage on extension validators by (context_owner, context_id) rather than just context_id. Under the previous model, two Budokan deployments that both asked a validator to configure tournament_id=1 would collide on the second call with "Only context owner can call"; each deployment now has its own private namespace on a shared validator contract. See: - Provable-Games/metagame_extensions#19 - Provable-Games/game-components#105 ### Contract changes - `budokan.cairo`: - `bannable(tournament_id + 1)` -> `bannable(get_contract_address(), tournament_id + 1)` - `should_ban(tournament_id, ...)` -> `should_ban(get_contract_address(), tournament_id, ...)` The contract passes its own address as the context_owner because it was the caller at `add_config` time. - `entry_validator_mock.cairo` and `tournament_validator_mock.cairo`: - IEntryRequirementExtension impl updated to the new signatures: added `is_context_registered`, removed `context_owner`, threaded `context_owner: ContractAddress` through `bannable`, `valid_entry`, `should_ban`, and `entries_left`. - Storage rekeyed so per-context maps are `Map<(ContractAddress, u64), _>`. - `add_config` / `add_entry` / `remove_entry` now key storage by `get_caller_address()` so the mock matches the namespacing behavior of real validators. Co-authored-by: starknetdev <starknetdev@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
Bumps
metagame_extensions_interfacesto the newly-releasedv0.1.4, which namespaces extension storage by(context_owner, context_id). Under the previous model, the first caller to register acontext_idon an extension contract took permanent ownership — two different consumer contracts could not share the samecontext_idon the same validator.See Provable-Games/metagame_extensions#19 for the full interface change and rationale.
Interface updates consumed here
IEntryRequirementExtension/IEntryFeeExtension/IPrizeExtensiongained a leadingcontext_owner: ContractAddressparameter.context_owner(context_id)was removed; replaced byis_context_registered(context_owner, context_id) -> bool.add_config,add_entry,remove_entry,set_entry_fee_config,pay_entry_fee,claim_entry_fee,add_prize,claim_prize) are unchanged.Changes in this PR
Scarb.toml— bumpmetagame_extensions_interfacesfromv0.1.3tov0.1.4.entry_requirement_store.cairo— the two dispatcher read calls (valid_entry,entries_left) now passstarknet::get_contract_address()as thecontext_owner. The metagame contract was the caller atadd_configtime, so it owns the matching namespace on the validator.metagame/src/entry_requirement/tests/mocks/*.cairo,test_common/src/mocks/*.cairo) — updated to the new interface shape. Addedis_context_registered, removedcontext_owner, threadedcontext_ownerthroughbannable/valid_entry/should_ban/entries_left. Mock storage rekeyed so per-context maps are keyed by(ContractAddress, u64).Test plan
scarb buildcleanscarb fmt --checkcleansnforge test --workspacepasses (reported clean locally by the repo owner)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Chores