NFT: get_approved panics with NonExistentToken for nonexistent tokens

[ozgunozerk]

Fixes #934

get_approved now panics with NonExistentToken for token IDs that don't exist (never minted or burned), as its trait docs say. Before this change it returned None, the same answer as a live token with no approval.

The default ContractOverrides::get_approved now checks that the token exists through Self::owner_of before reading the approval. It uses the contract type's own owner_of rather than Base::owner_of, because Consecutive doesn't store a per-token owner entry for batch-minted tokens, and Base::owner_of would reject them. This is the same pattern RoyaltySupport uses (#846). No contract type overrides get_approved, so the change covers Base, Enumerable, Consecutive, NonFungibleVotes and the combined types.

The low-level Base::get_approved is unchanged, so the internal approval check in transfer_from behaves as before.

Tests added:

• Base: a never-minted ID fails with #200.
• Consecutive: an ID past the last minted one and a burned ID both fail with #200.
• Consecutive: a live token from a batch returns None, then Some once approved (the existence check doesn't reject tokens whose owner comes from the batch).

PR Checklist

• Tests
• Documentation (no change needed, the trait docs already describe this behavior)

Summary by CodeRabbit

• Bug Fixes
  • Approval lookups now validate that a token exists before returning its approved address, including for consecutively minted tokens.
  • Queries for nonexistent or burned tokens return contract error #200.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Essentials
• Run ID: 27e87497-9379-435d-b956-a14c11229ac6

📥 Commits

Reviewing files that changed from the base of the PR and between ce5c568 and 295a129.

📒 Files selected for processing (3)

• packages/tokens/src/non_fungible/extensions/consecutive/test.rs
• packages/tokens/src/non_fungible/overrides.rs
• packages/tokens/src/non_fungible/test.rs

Included review availability: This review used your included allowance. 4 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

---

Walkthrough

ContractOverrides::get_approved now checks token ownership before reading approval state. Tests cover approval results and contract error #200 for nonexistent tokens, including never-minted and burned tokens.

Changes

NFT approval queries

Layer / File(s)	Summary
Token existence check and approval query tests packages/tokens/src/non_fungible/overrides.rs, packages/tokens/src/non_fungible/test.rs, packages/tokens/src/non_fungible/extensions/consecutive/test.rs	get_approved calls Self::owner_of before Base::get_approved. Tests check results for approved and unapproved minted tokens, and expect error #200 for nonexistent tokens. Consecutive-token tests include the next token ID and a burned token.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 295a1

Approval queries now reject nonexistent or burned tokens while preserving results for live tokens, including batch-minted Consecutive tokens. No material merge-blocking risk is established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: nonexistent NFT token IDs now cause get_approved to panic with NonExistentToken.
Description check		The description follows the repository template. It includes the linked issue, change summary, implementation context, test coverage, and completed checklist items.
Linked Issues check		Issue [#934] requires get_approved to reject never-minted and burned token IDs with NonExistentToken, and to use the contract type's ownership model. The change calls Self::owner_of before `Base…
Out of Scope Changes check		The changes stay within issue [#934]. The implementation changes only the default approval lookup, and the added tests verify the required Base and Consecutive behavior. No unrelated production behavi…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{A rabbit checks each token’s name,
Then reads approval all the same.
For minted tokens, results appear;
For missing ones, error #200 is clear.
The bunny hops away at ease.}

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!