Skip to content

refactor: credential check as ProofRequest method#411

Open
chipp wants to merge 1 commit into
mainfrom
vladimirburdukov/refactor-credential-check-as-proofrequest-method
Open

refactor: credential check as ProofRequest method#411
chipp wants to merge 1 commit into
mainfrom
vladimirburdukov/refactor-credential-check-as-proofrequest-method

Conversation

@chipp
Copy link
Copy Markdown
Contributor

@chipp chipp commented May 18, 2026

Summary

  • Moves check_credentials_against_proof_request free function into ProofRequest::check_credentials method (addresses Paolo's review on add credential pre-flight check against proof requests #404)
  • Converts requests.rsrequests/mod.rs with credential_check submodule
  • Replaces manual constraint validation with self.0.validate_constraints()? now that world-id-core 0.11.x is available (resolves Dzejkop's TODO)
  • Adds From<ValidationError> for CredentialConstraintsCheckError
  • Drops constraint expression tests that duplicate world-id-primitives coverage; retains 6 walletkit-specific tests

Test plan

  • cargo test -p walletkit-core requests::credential_check
  • cargo check --all-features

Copy link
Copy Markdown
Contributor Author

chipp commented May 18, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@chipp chipp marked this pull request as ready for review May 18, 2026 15:47
@chipp chipp requested a review from paolodamico May 18, 2026 15:47
Comment thread walletkit-core/src/requests/credential_check.rs Outdated
Comment thread walletkit-core/src/requests/credential_check.rs Outdated
Comment thread walletkit-core/src/requests/credential_check.rs Outdated
Comment thread walletkit-core/src/requests/credential_check.rs Outdated
Comment thread walletkit-core/src/requests/credential_check.rs Outdated
@chipp chipp force-pushed the vladimirburdukov/refactor-credential-check-as-proofrequest-method branch 2 times, most recently from 887653b to a95f92b Compare May 18, 2026 19:13
@chipp chipp requested a review from Dzejkop May 18, 2026 19:14
@chipp chipp force-pushed the vladimirburdukov/refactor-credential-check-as-proofrequest-method branch from a95f92b to f5ae23c Compare May 18, 2026 19:17
Dzejkop
Dzejkop previously approved these changes May 19, 2026
/// - [`CredentialConstraintsCheckError::Storage`] if the credential store query fails.
/// - [`CredentialConstraintsCheckError::ConstraintTooDeep`] if the constraint tree exceeds depth 2.
/// - [`CredentialConstraintsCheckError::ConstraintTooLarge`] if the constraint tree exceeds the node limit.
pub fn check_credentials(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is implementing duplicate logic from https://github.com/worldcoin/world-id-protocol/blob/ada9887a40255ce7e8576cbca4e0442c049d887d/crates/primitives/src/request/mod.rs#L374. is this performing any additional action from just exposing that method to foreign code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will reuse this method for constraints evaluation, thanks for sharing.

however, check_credentials applies time constraints per item and returns per-item diagnostics for UI before triggering ZK proof generation.
there are two options (or more, idk) here:
— keep in walletkit: time checks stay here, can't reuse protocol's since they're buried in circuit input validation.
— add to protocol primitives: expose time checks as a reusable method, walletkit just maps storage records → protocol types.

- move check_credentials_against_proof_request into ProofRequest::check_credentials under requests/credential_check.rs
- replace manual constraint depth/node checks with validate_constraints()
- drop tests that duplicate world-id-primitives coverage
@chipp chipp force-pushed the vladimirburdukov/refactor-credential-check-as-proofrequest-method branch from f5ae23c to 43314c8 Compare May 19, 2026 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants