feat(adventurer-oracle): item-set objectives (hold N of a set of item ids)

[starknetdev]

Summary

Stacks on the adventurer oracle (#31) to add a new objective kind: "hold N of these item ids", per the product ask. A single objective_id can now represent an item-set goal, so every consumer (the entry validator, Budokan gating, the beast-mode rewards) can check it with one completed_objective call instead of ANDing several per-item objectives.

What's added

• interface: three ItemSet* metrics — ItemSetHeldAnywhere, ItemSetEquipped, ItemSetInBag — plus a dedicated create_item_set_objective(name, description, config, items: Span<u8>) entrypoint and a get_objective_items view. For a set metric, config.target is the required count, compared via config.comparator: "hold all 3 of {A,B,C}" = AtLeast 3; "hold at least one of {A,B,C}" = AtLeast 1.
• oracle_lib (pure): is_item_set_metric, set_item_held (zero ids never count — empty slots share id 0), count_items_held, evaluate_item_set. evaluate/scalar_value now explicitly reject set metrics so they can never silently fall through to the scalar path.
• oracle: stores the item id set alongside the objective (shared sequential id space with create_objective); completed_objective routes set metrics to evaluate_item_set. Guardrails: create_objective rejects set metrics; the set creator rejects scalar metrics, empty sets, and zero item ids.

Tests

9 new snforge tests (count-per-variant, zero-id handling, hold-all vs hold-any end to end, all four guardrail panics, shared id space) — 28 total green. scarb build + scarb fmt --check clean.

Notes

• Base is feat/adventurer-oracle (stacked on the in-review Phase 1 PR) to keep review unified; retarget to main if feat: adventurer oracle — general objective checker for Death Mountain adventurers #31 lands first.
• The Phase 2 entry validator (separate PR) needs no change — a set objective is just another objective_id it already delegates.
• Not merged/deployed.

🤖 Generated with Claude Code

[github-actions]

GPT Code Review

Review execution failed (exit code 1).

The Codex CLI log was not posted because it can contain the full review prompt.
See the workflow run for job status: https://github.com/Provable-Games/metagame_extensions/actions/runs/28880605044

[github-actions]

Claude Code Review

Now I have full context. Let me analyze the key findings:

1. Duplicate item ids: create_item_set_objective validates non-zero but doesn't reject duplicates. The count_items_held comment says "Duplicate ids counted once each" — meaning {10,10,10} with AtLeast 3 could be satisfied by holding only item 10 once (counts 3 times? No — it counts each list entry independently, so {10,10,10} → 3 if item 10 held). That's a real correctness/misconfiguration issue where a "hold 3 distinct" objective can be satisfied by 1 item. Let me verify: count_items_held iterates over each item_id in items and increments if held. For [10,10,10], if item 10 is held, count=3. So AtLeast 3 passes with a single item. This is a genuine footgun but it's owner-configured. Worth flagging as medium.

2. get_objective_items / read_objective_items duplication — two nearly identical functions. Minor.

3. aux not validated to be 0 for set metrics — the config doc says aux "must be 0 otherwise". Not enforced. Minor.

4. Storage read in loop: read_objective_items reads len once, fine.

Let me check the most impactful: the duplicate-id issue is the strongest correctness finding. Let me confirm there's no dedup guard.

The duplicate concern is real but owner-controlled config. Let me weigh severity. The comparator flexibility (Equal, NotEqual, etc.) combined with duplicates makes it a genuine footgun. I'll flag as medium.

Let me also verify the objective_item_len/objective_items reading correctness and gas. completed_objective for set metrics calls read_objective_items which loops storage reads — acceptable.

Now let me write up findings.

Findings:

[MEDIUM] Duplicate item ids inflate the held count — create_item_set_objective rejects empty/zero ids but not duplicates. Since count_items_held counts each list entry independently (oracle_lib.cairo:146-154), a set like [10, 10, 10] with AtLeast 3 is satisfied by holding item 10 alone, defeating a "hold 3 distinct items" objective. Owner-controlled, but a silent misconfiguration footgun with no guard/test.

Let me double check whether there are other reviewers might flag. The aux must be 0 clarity note isn't enforced for set metrics — low signal. I'll include duplicate as the primary, and note the code duplication of read functions only if it's actionable — it is minor refactor, I'll mention briefly.

Let me finalize.

Based on my review of the scoped diff:

[MEDIUM] packages/adventurer_oracle/src/oracle_lib.cairo:146-154 - count_items_held counts each list entry independently, and create_item_set_objective (oracle.cairo:236-239) validates non-empty/non-zero but never rejects duplicate ids.
Impact: An owner registering items = [10, 10, 10] with ItemSet* + AtLeast 3 creates an objective satisfied by holding item 10 once — the count reaches 3 from a single held item, silently defeating a "hold 3 distinct items" goal. Same class of bug affects Equal/NotEqual/AtMost targets, where duplicates skew the count away from the number of distinct items actually held. There is no dedup guard and no test covering duplicate ids.
Fix: Reject duplicates at registration (reverting is safest given the owner-config surface), e.g. in the create_item_set_objective loop check the id was not already seen before writing:

for i in 0..items.len() {
    let id = *items.at(i);
    assert(id != 0, Errors::ZERO_ITEM_ID);
    let mut j = 0;
    while j != i {
        assert(*items.at(j) != id, Errors::DUPLICATE_ITEM_ID);
        j += 1;
    }
}

Alternatively, if duplicates are intended to be tolerated, dedup them (or count distinct held ids) and document/test the chosen semantics.

[LOW] packages/adventurer_oracle/src/oracle.cairo:258-268,327-336 - get_objective_items and read_objective_items are byte-for-byte identical except return type (Array<u8> vs Span<u8>).
Impact: Duplicated storage-read loop; future changes to the item-read layout must be kept in sync in two places.
Fix: Have the public get_objective_items delegate to the internal reader: self.read_objective_items(objective_id).into() (or return Span and adapt callers), keeping a single loop implementation.

[gemini-code-assist]

Code Review

This pull request introduces item-set objectives to the adventurer oracle, allowing verification of whether an adventurer holds a specific set of items (equipped, in their bag, or anywhere). It adds new metrics, contract methods to register and retrieve these sets, and corresponding library evaluation logic and tests. The review feedback focuses on improving robustness and code quality by recommending: 1) validating that registered item sets do not contain duplicate item IDs (along with adding a corresponding error constant and unit test) to prevent logical exploits; 2) refactoring read_objective_items to return Array<u8> so that get_objective_items can reuse it, eliminating duplicate storage-reading loops; and 3) correcting a comment in oracle_lib.cairo to accurately state that duplicate IDs in items are counted multiple times.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Add a new error constant DUPLICATE_ITEM_ID to represent the failure when duplicate item IDs are registered in an item-set objective.

        pub const SET_METRIC_NEEDS_ITEMS: felt252 = 'Oracle: use item_set create';
        pub const NOT_SET_METRIC: felt252 = 'Oracle: not an item-set metric';
        pub const EMPTY_ITEM_SET: felt252 = 'Oracle: item set is empty';
        pub const ZERO_ITEM_ID: felt252 = 'Oracle: item id is zero';
        pub const DUPLICATE_ITEM_ID: felt252 = 'Oracle: duplicate item id';

[gemini-code-assist]

Add validation to ensure that the registered item set does not contain duplicate item IDs. This prevents logical errors and potential exploits where an adventurer holding a single item can satisfy an objective requiring multiple distinct items.

            let mut i: u32 = 0;
            while i < items.len() {
                let item_id = *items.at(i);
                assert(item_id != 0, Errors::ZERO_ITEM_ID);
                let mut j: u32 = i + 1;
                while j < items.len() {
                    assert(item_id != *items.at(j), Errors::DUPLICATE_ITEM_ID);
                    j += 1;
                };
                i += 1;
            };

[gemini-code-assist]

Since read_objective_items now returns Array<u8>, call .span() on the returned array when passing it to evaluate_item_set.

            if oracle_lib::is_item_set_metric(config.metric) {
                let items = self.read_objective_items(objective_id);
                oracle_lib::evaluate_item_set(adventurer, bag, items.span(), config)
            } else {
                oracle_lib::evaluate(adventurer, bag, config)
            }

[gemini-code-assist]

Simplify get_objective_items by reusing the refactored read_objective_items function, removing the duplicate storage reading loop.

        fn get_objective_items(self: @ContractState, objective_id: u32) -> Array<u8> {
            assert(self.objective_exists(objective_id), Errors::OBJECTIVE_MISSING);
            self.read_objective_items(objective_id)
        }

[gemini-code-assist]

Refactor read_objective_items to return Array<u8> instead of Span<u8>. This allows us to reuse this function in get_objective_items and eliminate the duplicated loop.

        /// Read the stored item id set for an objective as an array.
        fn read_objective_items(self: @ContractState, objective_id: u32) -> Array<u8> {
            let len = self.objective_item_len.read(objective_id);
            let mut items: Array<u8> = array![];
            let mut index: u32 = 0;
            while index != len {
                items.append(self.objective_items.read((objective_id, index)));
                index += 1;
            }
            items
        }

[gemini-code-assist]

Update the comment to accurately reflect that duplicate IDs in items will be counted multiple times, rather than "counted once each".

/// Assumes unique ids in `items` (duplicates in `items` will be counted multiple times).

[gemini-code-assist]

Add a unit test to verify that create_item_set_objective correctly panics when duplicate item IDs are supplied.

}

#[test]
#[should_panic(expected: 'Oracle: duplicate item id')]
fn test_create_item_set_rejects_duplicate_id() {
    let (_mock, oracle) = setup();
    start_cheat_caller_address(oracle.contract_address, owner_addr());
    oracle
        .create_item_set_objective(
            "n",
            "d",
            config(Metric::ItemSetHeldAnywhere, Comparator::AtLeast, 1, 0),
            array![1_u8, 2, 1].span(),
        );
}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[starknetdev]

Superseded by the composite-conditions refactor now in #31 (commit ca080f0). Per the design discussion, objectives are unified into a single create_objective(name, description, settings_id, conditions: Array<Condition>, items: Option<Array<u8>>) — an objective is an AND of conditions plus an optional 'own all these items' set. 'Hold a set of item ids' is now the items list rather than a dedicated ItemSet* metric + second endpoint, so this branch's approach is no longer needed. Closing in favor of #31.