Skip to content

Commit 33a202d

Browse files
starknetdevclaude
andauthored
perf(entry-requirement): drop redundant entries_left cross-call (#109)
* perf(entry-requirement): drop redundant entries_left cross-call The extension branch of update_qualification_entries dispatched a second cross-contract call into the validator's entries_left just to assert > 0. For validators with expensive view paths (Opus walks user troves end-to-end twice) this duplicated the work valid_entry already performed. The IEntryRequirementExtension contract is now: valid_entry MUST enforce both eligibility and remaining-entry quota. The framework trusts that single signal on the entry path. entries_left remains for off-chain consumers. See companion PR in metagame_extensions for matching docs and the in-tree validators that now bundle quota. Updates the three update_qualification_entries extension-path tests to assert no-op behavior (previously they asserted on the entries_left panic, which has been removed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: scarb fmt — move comment to fn doc scarb fmt collapses comments inside an empty match-arm body onto the same line as the braces. Moving the explanation up to the fn doc keeps it readable and keeps the formatter happy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 82b9ce5 commit 33a202d

2 files changed

Lines changed: 24 additions & 69 deletions

File tree

packages/metagame/src/entry_requirement/entry_requirement_store.cairo

Lines changed: 8 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -183,43 +183,21 @@ pub impl EntryRequirementStoreImpl<T, +Store<T>, +Drop<T>> of EntryRequirementSt
183183
}
184184
}
185185

186+
/// Track a successful entry against the configured limit.
187+
///
188+
/// For extension-typed requirements this is a no-op: extensions enforce both eligibility
189+
/// and remaining-entry quota inside their own `valid_entry` (already called by
190+
/// `validate_qualification`). The framework deliberately does NOT make a second
191+
/// `entries_left` cross-contract call here — it would walk the same on-chain state a
192+
/// second time. See `IEntryRequirementExtension` docs for the contract.
186193
fn update_qualification_entries(
187194
ref self: T,
188195
context_id: u64,
189196
qualifier: QualificationProof,
190197
entry_requirement: EntryRequirement,
191198
) {
192199
match entry_requirement.entry_requirement_type {
193-
EntryRequirementType::extension(extension_config) => {
194-
let extension_address = extension_config.address;
195-
let extension_dispatcher = IEntryRequirementExtensionDispatcher {
196-
contract_address: extension_address,
197-
};
198-
let display_extension_address: felt252 = extension_address.into();
199-
let caller_address = get_caller_address();
200-
let context_owner = get_contract_address();
201-
202-
let qualification = match qualifier {
203-
QualificationProof::Extension(qual) => qual,
204-
_ => panic!(
205-
"EntryRequirement: Provided qualification proof is not of type 'Extension'",
206-
),
207-
};
208-
209-
let entries_left = extension_dispatcher
210-
.entries_left(context_owner, context_id, caller_address, qualification);
211-
212-
match entries_left {
213-
Option::Some(entries_left) => {
214-
assert!(
215-
entries_left > 0,
216-
"EntryRequirement: No entries left according to extension {}",
217-
display_extension_address,
218-
);
219-
},
220-
Option::None => {},
221-
}
222-
},
200+
EntryRequirementType::extension(_) => {},
223201
_ => {
224202
let entry_limit = entry_requirement.entry_limit;
225203
if entry_limit != 0 {

packages/metagame/src/entry_requirement/tests/test_entry_requirement_component.cairo

Lines changed: 16 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -639,9 +639,11 @@ fn deploy_accepting_limited_entry_validator_mock(owner: ContractAddress) -> Cont
639639
// ============================================================================
640640

641641
#[test]
642-
fn test_update_qualification_entries_extension_none_entries_left() {
643-
// EntryValidatorMock returns Option::None from entries_left
644-
// This means no limit check - should succeed silently
642+
fn test_update_qualification_entries_extension_is_noop() {
643+
// The framework no longer cross-checks the extension's `entries_left` here — extensions
644+
// enforce both eligibility and remaining-entry quota inside their own `valid_entry`.
645+
// `update_qualification_entries` must therefore complete silently for any extension
646+
// requirement, regardless of what the extension would have reported.
645647
let mock = deploy_entry_requirement_mock();
646648
let caller = make_address(0x555);
647649
let validator_addr = deploy_entry_validator_mock(caller);
@@ -657,44 +659,16 @@ fn test_update_qualification_entries_extension_none_entries_left() {
657659
snforge_std::cheat_caller_address(
658660
mock.contract_address, caller, snforge_std::CheatSpan::TargetCalls(1),
659661
);
660-
// Should not panic - entries_left returns None (no limit)
661662
mock
662663
.update_qualification_entries(
663664
context_id, QualificationProof::Extension(array![].span()), req,
664665
);
665666
}
666667

667668
#[test]
668-
fn test_update_qualification_entries_extension_some_entries_remaining() {
669-
// AcceptingLimitedEntryValidatorMock returns Option::Some(5)
670-
// entries_left > 0 - should succeed
671-
let mock = deploy_entry_requirement_mock();
672-
let caller = make_address(0x555);
673-
let validator_addr = deploy_accepting_limited_entry_validator_mock(caller);
674-
let context_id: u64 = 81;
675-
676-
let req = EntryRequirement {
677-
entry_limit: 10,
678-
entry_requirement_type: EntryRequirementType::extension(
679-
ExtensionConfig { address: validator_addr, config: array![].span() },
680-
),
681-
};
682-
683-
snforge_std::cheat_caller_address(
684-
mock.contract_address, caller, snforge_std::CheatSpan::TargetCalls(1),
685-
);
686-
// Should not panic - entries_left returns Some(5) which is > 0
687-
mock
688-
.update_qualification_entries(
689-
context_id, QualificationProof::Extension(array![].span()), req,
690-
);
691-
}
692-
693-
#[test]
694-
#[should_panic(expected: "EntryRequirement: No entries left according to extension")]
695-
fn test_update_qualification_entries_extension_zero_entries_left() {
696-
// RejectingEntryValidatorMock returns Option::Some(0)
697-
// entries_left == 0 - should panic
669+
fn test_update_qualification_entries_extension_noop_even_when_extension_would_reject() {
670+
// Same as above with a rejecting/zero-entries-left mock. Pre-change this would have
671+
// panicked with "No entries left"; post-change the framework never asks the extension.
698672
let mock = deploy_entry_requirement_mock();
699673
let caller = make_address(0x555);
700674
let validator_addr = deploy_rejecting_entry_validator_mock(caller);
@@ -717,10 +691,11 @@ fn test_update_qualification_entries_extension_zero_entries_left() {
717691
}
718692

719693
#[test]
720-
#[should_panic(
721-
expected: "EntryRequirement: Provided qualification proof is not of type 'Extension'",
722-
)]
723-
fn test_update_qualification_entries_extension_wrong_proof_type() {
694+
fn test_update_qualification_entries_extension_accepts_any_proof_type() {
695+
// Pre-change the framework destructured the qualifier inside the extension branch and
696+
// panicked on a non-Extension proof. Post-change the branch is a no-op, so any proof
697+
// shape is accepted (the qualifier is consumed by `validate_qualification`, which runs
698+
// earlier in `enter_tournament`).
724699
let mock = deploy_entry_requirement_mock();
725700
let caller = make_address(0x555);
726701
let validator_addr = deploy_entry_validator_mock(caller);
@@ -733,7 +708,9 @@ fn test_update_qualification_entries_extension_wrong_proof_type() {
733708
),
734709
};
735710

736-
// Pass NFT proof for extension requirement - should panic
711+
snforge_std::cheat_caller_address(
712+
mock.contract_address, caller, snforge_std::CheatSpan::TargetCalls(1),
713+
);
737714
mock
738715
.update_qualification_entries(
739716
context_id, QualificationProof::NFT(NFTQualification { token_id: 0x123 }), req,

0 commit comments

Comments
 (0)