-
Notifications
You must be signed in to change notification settings - Fork 179
merge dev to main #657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
merge dev to main #657
Conversation
* remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]>
* implement self uups upgradeable * small changes in identityVerificationHubImplV2 * delete aderyn.toml * chore: add custom verifier * chnage return output * feat: use self structs and a Generic output struct * feat: add userIdentifier, nullifier, forbiddencountries to returned output * add root view functions from registry * fix: build and compilation errors * add userDefined data into selfVerificationRoot * "resolve conflicts" * fix compilation problem * fix how to register verification config * test: CustomVerifier * fix verification root and hub integration * add scope check in hub impl * replace poseidon hash to ripemd+sha256 * add todo list * feat: refactor and add test cases for generic formatter * add performUserIdentifierCheck in basicVerification * change how to handle additionalData and fix stack too deep * start adding test codes * fix dependency problems in monorepo * fix: forbidden countries (#612) LGTM! * able to run test code * pass happy path * delete unused codes * change error code name, add caller address validation and add scripts to run test and build in monorepo * add all test cases in vcAndDisclose flow * remove comment out * chore: use actual user identifier outputs * success in registration tests * cover all cases * pass contractVersion instead of circuitVersion * fix disclose test * chore: add natspecs for ImplHubV2, CustomVerifier and GenericFormatter * change val name and remove unused lines * add val name change * remove userIdentifier from return data * feat: use GenericDiscloseOutput struct in verfication hook fix test cases for user identifier * chore: change the function order for Hub Impl V2 (#625) * fix nat specs * add nat spec in SelfStructs --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Nesopie <[email protected]>
* add missed files * add NFCMethodSelectionScreen * bump android build --------- Co-authored-by: Justin Hernandez <[email protected]>
* Simplify developer mode gesture * Enable dev mode on MockData screen with five taps
* fix: vc_and_disclose_id test * chore: yarn prettier
* Add help button and error modal actions * fix the screen management * yarn nice
* bump version and build numbers * remove tamagui/toast * fix marketing version
* update gesture version and bump android build * bump and fix ios build * update lock files * fixes * fix fotoapparat library source
* refactor: update HappyBirthday contract to V2 with support for E-Passport and EUID cards, introduce bonus multipliers, and enhance verification logic * refactor: update Airdrop contract to V2 with support for E-Passport and EU ID Card attestations * refactor: remove BASIS_POINTS constant from Airdrop contract * feat: introduce SelfIdentityERC721 contract for issuing NFTs based on verified identity credentials, replacing SelfPassportERC721 * fix: update verification functions in Airdrop, HappyBirthday, and SelfIdentityERC721 contracts to use customVerificationHook
* merge dev branch into main (#624) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> * update contracts (#628) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) * implement self uups upgradeable (#592) * implement self uups upgradeable * small changes in identityVerificationHubImplV2 * delete aderyn.toml * chore: add custom verifier * chnage return output * feat: use self structs and a Generic output struct * feat: add userIdentifier, nullifier, forbiddencountries to returned output * add root view functions from registry * fix: build and compilation errors * add userDefined data into selfVerificationRoot * "resolve conflicts" * fix compilation problem * fix how to register verification config * test: CustomVerifier * fix verification root and hub integration * add scope check in hub impl * replace poseidon hash to ripemd+sha256 * add todo list * feat: refactor and add test cases for generic formatter * add performUserIdentifierCheck in basicVerification * change how to handle additionalData and fix stack too deep * start adding test codes * fix dependency problems in monorepo * fix: forbidden countries (#612) LGTM! * able to run test code * pass happy path * delete unused codes * change error code name, add caller address validation and add scripts to run test and build in monorepo * add all test cases in vcAndDisclose flow * remove comment out * chore: use actual user identifier outputs * success in registration tests * cover all cases * pass contractVersion instead of circuitVersion * fix disclose test * chore: add natspecs for ImplHubV2, CustomVerifier and GenericFormatter * change val name and remove unused lines * add val name change * remove userIdentifier from return data * feat: use GenericDiscloseOutput struct in verfication hook fix test cases for user identifier * chore: change the function order for Hub Impl V2 (#625) * fix nat specs * add nat spec in SelfStructs --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Nesopie <[email protected]> * prettier (#629) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> Co-authored-by: Nesopie <[email protected]> * fix: vc_and_disclose_id test (#640) * fix: vc_and_disclose_id test * chore: yarn prettier * fix: check if a config id exists * chore: change the function where the config not set verification is happening * fix: add await * feat: add getConfigId function in SelfVerificationRoot (#650) * feat: add getConfigId function in SelfVerificationRoot * update comment --------- Co-authored-by: motemotech <[email protected]> * chore: fix ofac end index in eu id cards * chore: fix tests * fix: example contracts and tests --------- Co-authored-by: turnoffthiscomputer <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]>
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 72 files out of 179 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughThis update introduces new features, refactors existing logic, and improves configuration across mobile, backend, and smart contract components. Key changes include enhanced gesture handling and error modals in the app, stricter branch merging policies and npm publishing workflows, expanded NFC and passport scanning options, new and refactored Solidity contracts for identity and birthday rewards, and improved test coverage for verification flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant PassportReader
participant Store
participant Modal
User->>App: Selects NFC scan method
App->>App: If 'mrzCorrection', show MRZ input fields
User->>App: Edits MRZ fields
App->>Store: Update passportNumber, dateOfBirth, dateOfExpiry
User->>App: Initiates scan
App->>PassportReader: reset()
App->>PassportReader: scan({ canNumber, useCan, ... })
PassportReader-->>App: Scan result or error
App->>Modal: On error, show error modal with help option
User->>Modal: Dismiss or tap Help
Modal->>App: Navigate to NFC Trouble Screen (if Help)
sequenceDiagram
participant Dev
participant GitHub
participant Workflow
Dev->>GitHub: Open PR to main (from branch)
GitHub->>Workflow: Trigger block-non-dev-to-main
Workflow->>GitHub: Check if source branch == dev
alt Not dev
Workflow->>GitHub: Block PR, output error
else dev
Workflow->>GitHub: Allow PR
end
sequenceDiagram
participant User
participant SelfHappyBirthday
participant IdentityVerificationHub
participant USDC
User->>SelfHappyBirthday: Submit ZK proof (birthday)
SelfHappyBirthday->>IdentityVerificationHub: Validate config ID
alt Config valid and within window
SelfHappyBirthday->>USDC: Transfer USDC to user
SelfHappyBirthday->>User: Emit USDCClaimed event
else
SelfHappyBirthday->>User: Revert with error
end
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 18
🧹 Nitpick comments (21)
common/index.ts (1)
18-25: Group imports from./src/utils/trees.js.You’re importing from
trees.jstwice—once forgetLeafCscaTree/getLeafDscTreeand again forbuildSMT. Consider consolidating into a single import for clarity and maintainability:-import { getLeafCscaTree, getLeafDscTree } from './src/utils/trees.js'; -import { buildSMT } from './src/utils/trees.js'; +import { getLeafCscaTree, getLeafDscTree, buildSMT } from './src/utils/trees.js';app/android/react-native-passport-reader/android/build.gradle (1)
37-38: Remove legacy commented dependencies.The previous jmrtd versions are commented out, which clutters the dependency block now that
0.8.1is confirmed. Please remove the commented lines to improve clarity.circuits/circuits/utils/passport/disclose/disclose_id.circom (1)
67-71: Adjust revealedData array size
Updatedsignal revealedData[94];to accommodate 90 MRZ bytes + 2 older-than bytes + 2 OFAC bytes.
Consider updating the comment fromofac: 2 byte→ofac: 2 bytesfor consistency.contracts/test/unit/GenericFormatter.test.ts (1)
15-165: Consider adding edge case tests.The current test suite covers basic functionality well but could benefit from additional edge cases:
Consider adding tests for:
- Empty or zero values in configuration structs
- Maximum values for numeric fields
- Invalid configurations (if applicable)
- Array bounds testing for forbiddenCountriesListPacked
Example additional test:
it("should handle edge case values", async function () { const edgeCaseConfig: SelfStructs.VerificationConfigV2Struct = { olderThanEnabled: false, olderThan: 0, forbiddenCountriesEnabled: true, forbiddenCountriesListPacked: [BigInt(2**256-1), 0n, 0n, 0n], // Max value ofacEnabled: [true, true, true], }; // Test encoding/decoding with edge values const bytes = await testGenericFormatter.testFormatV2Config(edgeCaseConfig); const decoded = await testGenericFormatter.testVerificationConfigFromBytes(bytes); expect(decoded.forbiddenCountriesListPacked[0]).to.equal(edgeCaseConfig.forbiddenCountriesListPacked[0]); });contracts/contracts/libraries/GenericFormatter.sol (1)
6-9: Consider moving struct definition to SelfStructs.sol for consistency.The
GenericVerificationStructis defined in this file but doesn't appear to be used. For consistency with the import pattern, consider moving it toSelfStructs.solor removing it if unused.If the struct is needed, move it to
SelfStructs.sol:-struct GenericVerificationStruct { - uint8 attestationId; - bytes verificationConfig; -}And add it to the SelfStructs library instead.
app/src/screens/passport/NFCMethodSelectionScreen.tsx (1)
58-65: Document MRZ correction use caseThe MRZ correction feature is useful for development and testing, but could be misused in production. Consider adding a comment explaining when this should be used.
{ key: 'mrzCorrection', label: 'Edit MRZ', description: - 'Edit the MRZ fields manually. This allows to correct the MRZ if it is incorrect.', + 'Edit the MRZ fields manually. This allows to correct the MRZ if OCR scanning produced errors or for testing purposes.', platform: ['ios', 'android'], params: {}, },contracts/scripts/test.sh (2)
186-186: Add newline at end of fileFiles should end with a newline character for POSIX compliance and to avoid issues with various tools.
# Run main function with all arguments main "$@" +
77-145: Consider validating hardhat installation before running testsThe script assumes
npx hardhatis available but doesn't check if dependencies are installed. This could lead to cryptic errors for users.Add a validation check before running tests:
# Function to run tests run_test() { local test_type=$1 + + # Check if hardhat is available + if ! npx hardhat --version >/dev/null 2>&1; then + print_error "Hardhat is not installed. Please run 'npm install' or 'yarn install' first." + exit 1 + fi + print_status "Running $test_type tests..."contracts/contracts/libraries/Formatter.sol (2)
157-157: Address the TODO commentThe TODO suggests this function needs further review. Please complete the review or provide more context about what specifically needs to be examined.
Would you like me to help analyze the function logic or open an issue to track this task?
175-178: Extract duplicated byte reversal logicThe byte reversal logic is duplicated in multiple places. Consider extracting it to a helper function for better maintainability.
+ function reverseBytes3(uint256 packedData) private pure returns (uint24) { + return uint24( + ((packedData & 0xff) << 16) | + ((packedData & 0xff00)) | + ((packedData & 0xff0000) >> 16) + ); + } + function extractForbiddenCountriesFromPacked( uint256[4] memory publicSignals ) internal pure returns (string[MAX_FORBIDDEN_COUNTRIES_LIST_LENGTH] memory forbiddenCountries) { // ... existing validation code ... for (uint256 j = 0; j < MAX_FORBIDDEN_COUNTRIES_LIST_LENGTH; j++) { uint256 byteIndex = (j * 3) % 93; uint256 index = j / 31; if (byteIndex + 2 < 31) { uint256 shift = byteIndex * 8; uint256 mask = 0xFFFFFF; uint256 packedData = (publicSignals[index * 3] >> shift) & mask; - uint256 reversedPackedData = ((packedData & 0xff) << 16) | - ((packedData & 0xff00)) | - ((packedData & 0xff0000) >> 16); - forbiddenCountries[j] = string(abi.encodePacked(uint24(reversedPackedData))); + forbiddenCountries[j] = string(abi.encodePacked(reverseBytes3(packedData)));Also applies to: 190-193, 205-208
contracts/test/v2/hubOther.test.ts (1)
76-88: Verify specific access control error messagesThe access control tests expect reverts but don't verify the specific error message. Consider checking for the expected
OwnableUnauthorizedAccounterror.- await expect(nonOwnerHub.updateRegistry(attestationIdBytes32, ethers.ZeroAddress)).to.be.reverted; // Should revert due to onlyOwner modifier + await expect(nonOwnerHub.updateRegistry(attestationIdBytes32, ethers.ZeroAddress)) + .to.be.revertedWithCustomError(deployedActors.hub, "OwnableUnauthorizedAccount") + .withArgs(deployedActors.user1.address);contracts/test/v2/disclosePassport.test.ts (2)
17-24: Consider extracting date formatting helper to a shared utilityThe
formatDateForPassportfunction could be useful in other test files. Consider moving it to a shared test utility module.Create a new file
contracts/test/utils/dateHelpers.ts:export function formatDateForPassport(date: Date): string { const year = date.getUTCFullYear().toString().slice(-2); const month = (date.getUTCMonth() + 1).toString().padStart(2, "0"); const day = date.getUTCDate().toString().padStart(2, "0"); return year + month + day; }Then import it in this test file:
-// Helper function to format date for passport (YYMMDD format) -function formatDateForPassport(date: Date): string { - const year = date.getUTCFullYear().toString().slice(-2); // Get last 2 digits of year - const month = (date.getUTCMonth() + 1).toString().padStart(2, "0"); // Month is 0-indexed - const day = date.getUTCDate().toString().padStart(2, "0"); - return year + month + day; -} +import { formatDateForPassport } from "../utils/dateHelpers";
40-46: Consider moving user identifier hash calculation to a utilityThe
calculateUserIdentifierHashfunction implements a specific hashing scheme that might be reused elsewhere. Consider extracting it to a shared utility.contracts/test/utils/deploymentV2.ts (1)
185-186: Use imported constants for attestation IDsThe attestation IDs are hardcoded here but could be imported from the constants file for consistency.
+import { ATTESTATION_ID } from "../utils/constants"; + // Register verifiers with the hub - const E_PASSPORT = ethers.hexlify(ethers.zeroPadValue(ethers.toBeHex(1), 32)); - const EU_ID_CARD = ethers.hexlify(ethers.zeroPadValue(ethers.toBeHex(2), 32)); + const E_PASSPORT = ethers.hexlify(ethers.zeroPadValue(ethers.toBeHex(ATTESTATION_ID.E_PASSPORT), 32)); + const EU_ID_CARD = ethers.hexlify(ethers.zeroPadValue(ethers.toBeHex(ATTESTATION_ID.EU_ID_CARD), 32));contracts/test/v2/discloseId.test.ts (1)
77-78: Consider documenting the test country codes.The forbidden countries list includes non-standard codes like "ABC", "CBA", "AAA". While acceptable for testing, consider adding a comment to clarify these are test values to avoid confusion.
- forbiddenCountriesList = [countries.AFGHANISTAN, "ABC", "CBA", "AAA"] as Country3LetterCode[]; + // Include real country (AFGHANISTAN) and test country codes for comprehensive testing + forbiddenCountriesList = [countries.AFGHANISTAN, "ABC", "CBA", "AAA"] as Country3LetterCode[];contracts/contracts/example/SelfIdentityERC721.sol (2)
24-24: Consider starting token IDs from 1 instead of 0.Most NFT collections start token IDs from 1 for better UX and compatibility with marketplaces. Consider initializing
_tokenIdCounterto 1.- uint256 private _tokenIdCounter; + uint256 private _tokenIdCounter = 1;Also applies to: 135-135
89-92: Use OpenZeppelin's _requireOwned instead of custom _exists.OpenZeppelin's ERC721 provides
_requireOwnedwhich throws a more descriptive error. Consider using it for consistency.- function getIdentityAttributes( - uint256 tokenId - ) external view returns (ISelfVerificationRoot.GenericDiscloseOutputV2 memory) { - require(_exists(tokenId), "Token does not exist"); - return _identityAttributes[tokenId]; - } + function getIdentityAttributes( + uint256 tokenId + ) external view returns (ISelfVerificationRoot.GenericDiscloseOutputV2 memory) { + _requireOwned(tokenId); + return _identityAttributes[tokenId]; + } - function _exists(uint256 tokenId) internal view returns (bool) { - return _ownerOf(tokenId) != address(0); - }Also applies to: 152-154
contracts/contracts/libraries/CustomVerifier.sol (1)
2-2: Use exact Solidity version for consistency.Other contracts use
pragma solidity 0.8.28;. Use the same exact version instead of^0.8.0for consistency across the codebase.-pragma solidity ^0.8.0; +pragma solidity 0.8.28;contracts/contracts/abstract/SelfVerificationRoot.sol (1)
167-174: Consider using a custom error instead of string revert.While the default implementation correctly enforces override requirement, consider defining a custom error for gas efficiency:
+ /// @notice Error thrown when getConfigId is not overridden + error GetConfigIdNotOverridden(); function getConfigId( bytes32 destinationChainId, bytes32 userIdentifier, bytes memory userDefinedData ) public view virtual returns (bytes32) { - // Default implementation reverts; must be overridden in derived contract - revert("SelfVerificationRoot: getConfigId must be overridden"); + revert GetConfigIdNotOverridden(); }contracts/contracts/IdentityVerificationHubImplV2.sol (2)
567-575: Document the assumption about msg.sender.The function assumes
msg.senderimplementsISelfVerificationRoot. Consider adding a comment to document this requirement:function _handleVerificationResult(uint256 destChainId, bytes memory output, bytes memory userDataToPass) internal { if (destChainId == block.chainid) { + // msg.sender is expected to be a contract implementing ISelfVerificationRoot ISelfVerificationRoot(msg.sender).onVerificationSuccess(output, userDataToPass); } else {
788-790: Use custom error instead of string revert for consistency.The contract uses custom errors everywhere except here. Consider adding a custom error:
+ /// @notice Thrown when registry is not set for an attestation ID + error RegistryNotSet(); if (registryAddress == address(0)) { - revert("Registry not set for attestation ID"); + revert RegistryNotSet(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
app/ios/Podfile.lockis excluded by!**/*.locksdk/tests/api-server/bun.lockbis excluded by!**/bun.lockbsdk/tests/web-app/src/app/favicon.icois excluded by!**/*.icosdk/tests/web-app/src/app/fonts/GeistMonoVF.woffis excluded by!**/*.woffsdk/tests/web-app/src/app/fonts/GeistVF.woffis excluded by!**/*.woffsdk/tests/web-app/src/app/verified/skeleton.gifis excluded by!**/*.gifyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (107)
.github/actions/mobile-setup/action.yml(1 hunks).github/actions/yarn-install/action.yml(1 hunks).github/workflows/artifacts.yml(2 hunks).github/workflows/block-non-dev-to-main.yml(1 hunks).github/workflows/npm-publish.yml(4 hunks)app/android/android-passport-reader/app/build.gradle(1 hunks)app/android/app/build.gradle(2 hunks)app/android/build.gradle(1 hunks)app/android/gradle.properties(2 hunks)app/android/react-native-passport-reader/android/build.gradle(1 hunks)app/index.js(1 hunks)app/ios/OpenPassport/Info.plist(1 hunks)app/ios/Self.xcodeproj/project.pbxproj(4 hunks)app/package.json(3 hunks)app/src/layouts/SimpleScrolledTitleLayout.tsx(2 hunks)app/src/screens/dev/MockDataScreen.tsx(2 hunks)app/src/screens/misc/LaunchScreen.tsx(2 hunks)app/src/screens/passport/NFCMethodSelectionScreen.tsx(5 hunks)app/src/screens/passport/PassportNFCScanScreen.tsx(6 hunks)app/src/screens/passport/PassportNFCTroubleScreen.tsx(4 hunks)app/src/screens/settings/SettingsScreen.tsx(2 hunks)app/src/types/react-native-passport-reader.d.ts(1 hunks)app/src/utils/nfcScanner.ts(1 hunks)app/src/utils/utils.ts(1 hunks)circuits/circuits/utils/crypto/bigInt/bigInt.circom(5 hunks)circuits/circuits/utils/crypto/hasher/shaBytes/dynamic/sha1Bytes.circom(4 hunks)circuits/circuits/utils/crypto/hasher/shaBytes/shaBytesDynamic.circom(3 hunks)circuits/circuits/utils/passport/disclose/disclose_id.circom(3 hunks)circuits/scripts/build/build_disclose_circuits.sh(1 hunks)circuits/scripts/build/common.sh(2 hunks)circuits/tests/disclose/vc_and_disclose_id.test.ts(4 hunks)common/index.ts(2 hunks)common/package.json(1 hunks)contracts/contracts/IdentityVerificationHubImplV2.sol(10 hunks)contracts/contracts/abstract/SelfVerificationRoot.sol(2 hunks)contracts/contracts/constants/CircuitConstantsV2.sol(2 hunks)contracts/contracts/example/Airdrop.sol(1 hunks)contracts/contracts/example/HappyBirthday.sol(6 hunks)contracts/contracts/example/SelfIdentityERC721.sol(1 hunks)contracts/contracts/example/SelfPassportERC721.sol(1 hunks)contracts/contracts/interfaces/IIdentityVerificationHubV2.sol(3 hunks)contracts/contracts/interfaces/ISelfVerificationRoot.sol(2 hunks)contracts/contracts/libraries/CircuitAttributeHandler.sol(3 hunks)contracts/contracts/libraries/CircuitAttributeHandlerV2.sol(9 hunks)contracts/contracts/libraries/CustomVerifier.sol(1 hunks)contracts/contracts/libraries/Formatter.sol(2 hunks)contracts/contracts/libraries/GenericFormatter.sol(1 hunks)contracts/contracts/libraries/IdCardAttributeHandler.sol(3 hunks)contracts/contracts/libraries/SelfStructs.sol(1 hunks)contracts/contracts/registry/IdentityRegistryIdCard.sol(0 hunks)contracts/contracts/registry/IdentityRegistryIdCardImplV1.sol(0 hunks)contracts/contracts/tests/TestCustomVerifier.sol(1 hunks)contracts/contracts/tests/TestSelfVerificationRoot.sol(1 hunks)contracts/contracts/tests/testGenericFormatter.sol(1 hunks)contracts/contracts/verifiers/disclose/Verifier_vc_and_disclose_id.sol(1 hunks)contracts/contracts/verifiers/register_id/Verifier_register_id_sha256_sha256_sha256_rsa_65537_4096.sol(4 hunks)contracts/hardhat.config.ts(2 hunks)contracts/ignition/modules/deployTestSelfVerificationRoot.ts(1 hunks)contracts/ignition/modules/hub/deployHub.ts(1 hunks)contracts/ignition/modules/verifiers/deployAllVerifiers.ts(3 hunks)contracts/package.json(2 hunks)contracts/scripts/README.md(1 hunks)contracts/scripts/dev.sh(1 hunks)contracts/scripts/setHubV2.ts(3 hunks)contracts/scripts/setRegistry.ts(2 hunks)contracts/scripts/setRegistryId.ts(3 hunks)contracts/scripts/test.sh(1 hunks)contracts/test/example/airdrop.test.ts(4 hunks)contracts/test/integration/commitmentRegistration.test.ts(3 hunks)contracts/test/integration/endToEnd.test.ts(3 hunks)contracts/test/integration/vcAndDisclose.test.ts(4 hunks)contracts/test/integration/verifyAll.test.ts(14 hunks)contracts/test/unit/CustomVerifier.test.ts(1 hunks)contracts/test/unit/GenericFormatter.test.ts(1 hunks)contracts/test/unit/IdentityRegistry.test.ts(1 hunks)contracts/test/unit/formatter.test.ts(1 hunks)contracts/test/utils/constants.ts(1 hunks)contracts/test/utils/deploymentV2.ts(1 hunks)contracts/test/utils/formatter.ts(1 hunks)contracts/test/utils/generateProof.ts(7 hunks)contracts/test/utils/pubkeys/serialized_csca_tree.json(0 hunks)contracts/test/utils/pubkeys/serialized_dsc_tree.json(0 hunks)contracts/test/utils/types.ts(2 hunks)contracts/test/v2/discloseId.test.ts(1 hunks)contracts/test/v2/disclosePassport.test.ts(1 hunks)contracts/test/v2/hubOther.test.ts(1 hunks)contracts/test/v2/registerId.test.ts(1 hunks)contracts/test/v2/registerPassport.test.ts(1 hunks)package.json(0 hunks)sdk/tests/api-server/.env.sample(0 hunks)sdk/tests/api-server/.gitignore(0 hunks)sdk/tests/api-server/README.md(0 hunks)sdk/tests/api-server/package.json(0 hunks)sdk/tests/api-server/src/app.routes.ts(0 hunks)sdk/tests/api-server/src/app.ts(0 hunks)sdk/tests/api-server/src/contracts/infrastructure/contracts.controller.ts(0 hunks)sdk/tests/api-server/tsconfig.json(0 hunks)sdk/tests/web-app/.env.sample(0 hunks)sdk/tests/web-app/.eslintrc.json(0 hunks)sdk/tests/web-app/.gitignore(0 hunks)sdk/tests/web-app/README.md(0 hunks)sdk/tests/web-app/next.config.mjs(0 hunks)sdk/tests/web-app/package.json(0 hunks)sdk/tests/web-app/postcss.config.mjs(0 hunks)sdk/tests/web-app/src/app/api/verify/route.ts(0 hunks)sdk/tests/web-app/src/app/disclose/logo.ts(0 hunks)sdk/tests/web-app/src/app/disclose/page.tsx(0 hunks)
⛔ Files not processed due to max files limit (7)
- sdk/tests/web-app/src/app/globals.css
- sdk/tests/web-app/src/app/layout.tsx
- sdk/tests/web-app/src/app/page.tsx
- sdk/tests/web-app/src/app/verified/page.module.css
- sdk/tests/web-app/src/app/verified/page.tsx
- sdk/tests/web-app/tailwind.config.ts
- sdk/tests/web-app/tsconfig.json
💤 Files with no reviewable changes (23)
- sdk/tests/web-app/.env.sample
- sdk/tests/web-app/next.config.mjs
- sdk/tests/web-app/.eslintrc.json
- package.json
- sdk/tests/api-server/src/app.ts
- sdk/tests/api-server/.env.sample
- sdk/tests/api-server/tsconfig.json
- contracts/test/utils/pubkeys/serialized_csca_tree.json
- sdk/tests/api-server/README.md
- contracts/test/utils/pubkeys/serialized_dsc_tree.json
- sdk/tests/api-server/src/app.routes.ts
- sdk/tests/api-server/.gitignore
- sdk/tests/web-app/src/app/disclose/logo.ts
- sdk/tests/web-app/postcss.config.mjs
- sdk/tests/web-app/src/app/disclose/page.tsx
- sdk/tests/web-app/.gitignore
- sdk/tests/web-app/README.md
- contracts/contracts/registry/IdentityRegistryIdCard.sol
- sdk/tests/web-app/package.json
- sdk/tests/api-server/src/contracts/infrastructure/contracts.controller.ts
- contracts/contracts/registry/IdentityRegistryIdCardImplV1.sol
- sdk/tests/api-server/package.json
- sdk/tests/web-app/src/app/api/verify/route.ts
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/block-non-dev-to-main.yml
12-12: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions for more details
(expression)
🪛 Biome (1.9.4)
contracts/test/unit/CustomVerifier.test.ts
[error] 4-8: Do not export from a test file.
(lint/suspicious/noExportsInTest)
🔇 Additional comments (138)
common/package.json (1)
3-3: Bump package version
Updated version from0.0.2to0.0.3.app/ios/OpenPassport/Info.plist (1)
24-24: iOS app version bump
UpdatedCFBundleShortVersionStringto2.5.4to align with Android and Xcode project settings.circuits/scripts/build/common.sh (1)
75-77: Update circom include paths
Switched-lflags to../node_modulesto match the updated directory layout.circuits/circuits/utils/crypto/bigInt/bigInt.circom (1)
1-169: No substantive changes
Only whitespace/formatting tweaks were applied; functionality is unchanged.app/ios/Self.xcodeproj/project.pbxproj (1)
426-427: Synchronize iOS project versioning
CURRENT_PROJECT_VERSIONset to133andMARKETING_VERSIONto2.5.4in both Debug and Release. Ensure CI tagging, App Store metadata, and Android counterparts are in sync.Also applies to: 541-542, 564-565, 678-679
app/android/build.gradle (2)
39-39: LGTM: Repository reordering.Moving
jcenter()to immediately followmavenCentral()is a reasonable change for dependency resolution priority.
51-51: LGTM: Corrected Fotoapparat dependency coordinate.The fix from
com.github.RedApparat:Fotoapparat:2.7.0tocom.github.fotoapparat:fotoapparat:2.7.0aligns with the correct GitHub organization name, ensuring proper dependency resolution.app/src/layouts/SimpleScrolledTitleLayout.tsx (4)
5-5: LGTM: Import added for SecondaryButton.The import is properly added to support the new secondary button functionality.
10-16: LGTM: Well-designed interface extension.The interface extension properly adds optional properties for secondary button functionality while maintaining backward compatibility.
22-23: LGTM: Props properly destructured.The new props are correctly added to the function parameter destructuring.
37-41: LGTM: Proper conditional rendering with validation.The implementation correctly validates that both
secondaryButtonTextandonSecondaryButtonPressmust be provided together, preventing incomplete configurations.app/android/gradle.properties (3)
13-13: LGTM: Improved JVM configuration for better build performance.The increased heap size (4GB) and additional JVM flags (heap dumps, UTF-8 encoding, parallel GC) are appropriate optimizations for large Android projects.
19-19: LGTM: Enabled parallel builds.Enabling parallel execution will improve build performance for multi-module projects.
48-58: Note the security trade-off in dependency verification.The build optimizations (caching, configuration on demand) are excellent for performance. However, disabling dependency verification (
org.gradle.dependency.verification=off) reduces security by skipping checksum validation of dependencies.Consider the security implications of this change in your build pipeline and ensure you have other security measures in place.
contracts/contracts/libraries/SelfStructs.sol (6)
1-9: LGTM: Proper library structure and documentation.The library follows Solidity best practices with appropriate license, pragma version, and comprehensive NatSpec documentation.
16-36: LGTM: Well-structured data types for verification outputs.The
HubInputHeaderandPassportOutputstructs are properly defined with clear field documentation and appropriate data types.
46-52: LGTM: Consistent EU ID output structure.The
EuIdOutputstruct maintains consistency withPassportOutputstructure, which is good for unified handling.
54-59: LGTM: Clear OFAC verification mode constants.The constants are well-named and documented, providing clear options for different OFAC verification approaches.
77-91: LGTM: Comprehensive generic disclosure structure.The
GenericDiscloseOutputV2struct provides a thorough representation of identity attributes with proper typing and documentation.
101-123: Clarify the difference between VerificationConfigV1 and VerificationConfigV2.Both structs appear identical in structure and fields. If they're truly identical, consider if both versions are necessary or if V2 is intended for future extensions.
Could you clarify the intended differences between these two configuration versions? If they're placeholder for future changes, consider adding a comment to explain this.
contracts/test/utils/constants.ts (1)
4-4: NewEU_ID_CARDconstant looks correct.The 32-byte hex identifier is well-formed and aligns with the existing attestation IDs.
app/index.js (1)
23-25: Verify the removal ofToastProvider.The import and usage of
ToastProviderfrom@tamagui/toasthas been removed, yet the dependency persists. Ensure that toast context is now provided elsewhere or that toast handling has been refactored to prevent runtime errors or missing UI feedback.contracts/ignition/modules/hub/deployHub.ts (1)
21-21: Initialize data formatting simplified.The
encodeFunctionDatacall has been condensed into a single-line array without altering the arguments or logic. LGTM.app/package.json (3)
3-3: Version bump to 2.5.4
The app version was correctly updated to 2.5.4, matching native build configurations.
74-74: Verify@tamagui/toastusage
The@tamagui/toastdependency was added but toast imports were removed fromapp/index.js. Please confirm if this package is required or should be removed to avoid unused dependencies.
97-97: Pinreact-native-gesture-handlerto 2.24.0
Locking this dependency to an exact version improves reproducible builds..github/actions/yarn-install/action.yml (1)
18-19: Pin Yarn to version 4.6.0
Explicitly preparing and setting Yarn 4.6.0 in the action ensures CI consistency across workflows.contracts/ignition/modules/verifiers/deployAllVerifiers.ts (3)
10-10: Consistent arrow function formatting
Adding parentheses around thekeyparameter in.filteraligns with the project's arrow function style.
25-25: Consistent arrow function formatting
WrappingfilePathin parentheses for the.somecallback maintains stylistic consistency.
67-72: Multi-line console.log formatting
Refactoring the deployment summary logs into multi-line calls enhances readability without altering functionality.app/android/android-passport-reader/app/build.gradle (1)
77-77: Fix Fotoapparat dependency coordinates
Updating the group/artifact ID tocom.github.fotoapparat:fotoapparat:2.7.0corrects the casing and aligns with upstream library naming.contracts/test/unit/IdentityRegistry.test.ts (1)
195-195: Standardize dynamic import arrow function
Adding parentheses around themodparameter in the dynamic import callback matches style changes across the test suite.circuits/scripts/build/build_disclose_circuits.sh (1)
19-19: LGTM! New ID card circuit integration looks good.The addition of
"vc_and_disclose_id:20:true"follows the correct format and aligns with the broader system enhancement for ID card attestations mentioned in the summary.circuits/circuits/utils/crypto/hasher/shaBytes/dynamic/sha1Bytes.circom (1)
31-31: LGTM! Formatting cleanup improves consistency.The whitespace and formatting changes enhance code readability and consistency without affecting the cryptographic circuit logic.
Also applies to: 69-71, 89-89, 95-95, 118-118
app/src/utils/nfcScanner.ts (2)
37-37: Good addition of reset call for clean state.Adding
PassportReader.reset()at the start ensures the passport reader state is cleared before scanning, which is a best practice for hardware interactions.
42-43: LGTM! New parameters enhance scanning capabilities.The addition of
canNumberanduseCanparameters with sensible defaults aligns with the iOS implementation and expands Android NFC scanning capabilities..github/workflows/npm-publish.yml (3)
6-6: LGTM! Branch trigger change aligns with development workflow.Changing the trigger from
maintodevbranch aligns with the updated development workflow mentioned in the summary.
66-66: Good switch from yarn to npm for publishing.The change from
yarn npm publishtonpm publishsimplifies the publishing process and may resolve potential authentication issues.Also applies to: 94-94, 121-121
69-69: Verify the necessity of dual authentication tokens.Both
NODE_AUTH_TOKENandNPM_AUTH_TOKENare now set with the same value. Please confirm if both are needed or if this redundancy could cause conflicts.Do npm publish commands require both NODE_AUTH_TOKEN and NPM_AUTH_TOKEN environment variables, or is one sufficient?Also applies to: 97-97, 124-124
contracts/scripts/setRegistry.ts (2)
19-21: LGTM! Cleaner object key formatting.Removing quotes from object keys improves readability when keys are valid JavaScript identifiers.
33-36: Good formatting improvement for readability.The multi-line
path.joincall improves readability when dealing with multiple path arguments.app/src/screens/misc/LaunchScreen.tsx (2)
29-33: LGTM! Improved gesture accessibility.The change from a two-finger tap to a five-tap gesture makes the developer mode activation more accessible and easier to use, while maintaining the same functionality.
40-40: Gesture detector updated correctly.The GestureDetector properly uses the new
devModeTapgesture, maintaining consistency with the gesture definition above.app/android/app/build.gradle (2)
89-90: Version updates look correct.The version code increment from 69 to 72 and version name update to "2.5.4" follow standard Android versioning practices for a release build.
163-163: Verify the Fotoapparat dependency group correction.The group ID was changed from
"com.github.RedApparat"to"com.github.fotoapparat". Please ensure this is the correct group identifier for the Fotoapparat library.#!/bin/bash # Description: Verify the correct group ID for Fotoapparat library on Maven Central or GitHub # Expected: Confirm "com.github.fotoapparat" is the correct group ID # Check if there are other references to this library in the codebase rg -A 2 -B 2 "fotoapparat|RedApparat" --type gradleapp/src/screens/settings/SettingsScreen.tsx (2)
139-143: Consistent gesture improvement across screens.The change to a five-tap gesture aligns with similar updates in other screens (like LaunchScreen.tsx), ensuring consistent developer mode activation patterns throughout the app.
200-200: GestureDetector properly updated.The GestureDetector correctly references the new
devModeTapgesture, maintaining consistency with the gesture definition..github/actions/mobile-setup/action.yml (1)
55-55: Good practice: Yarn version pinning.Adding
yarn set version 4.6.0ensures consistent Yarn behavior across different CI environments, which helps prevent build inconsistencies and dependency resolution issues.contracts/contracts/constants/CircuitConstantsV2.sol (2)
64-64: Clear documentation of field usage.The updated comment clearly explains that
passportNoSmtRootIndexis only used for passports and has a special value (99) for ID cards, improving code maintainability.
101-101: Verify circuit constant change impact across the system.The change from
0to99forpassportNoSmtRootIndexin EU ID card attestation is significant. Ensure that all verification logic, circuits, and related components properly handle this sentinel value.#!/bin/bash # Description: Verify that the passportNoSmtRootIndex value 99 is handled correctly across the codebase # Expected: Find references to this field and ensure proper handling of the sentinel value # Search for references to passportNoSmtRootIndex ast-grep --pattern 'passportNoSmtRootIndex' # Search for hardcoded references to the value 99 in circuit-related files rg -A 3 -B 3 "99" --type sol contracts/ # Search for EU_ID_CARD attestation handling rg -A 5 -B 5 "EU_ID_CARD" contracts/.github/workflows/artifacts.yml (2)
43-45: Consistent Yarn setup
Pinning Yarn to version 4.6.0 and enabling Corepack ensures deterministic dependency management across CI runs.
63-64: Streamline Cpp circuits build with proper line continuations
The use of&&operators and explicit line continuations makes the build steps more readable and reliable.app/src/screens/dev/MockDataScreen.tsx (2)
263-269: Update gesture to single-finger five-tap for advanced mode
Switched from a two-finger triple tap to a one-finger five-tap (Gesture.Tap().numberOfTaps(5)). Ensure that using.onStartfires at the correct moment after the fifth tap for the intended UX.
276-276: Wrap header in GestureDetector
Encapsulating the header container in<GestureDetector gesture={devModeTap}>correctly applies the new tap gesture to toggle advanced mode.circuits/circuits/utils/passport/disclose/disclose_id.circom (2)
45-47: Introduce OFAC selector input and include in assertions
Addedsignal input selector_ofac;and included it in the selectors check to allow two bytes of OFAC data inrevealedData.
94-94: Unchanged closing brace
No action needed for the trailing}.contracts/scripts/setHubV2.ts (3)
16-18: Remove quotes from NETWORK_TO_CHAIN_ID keys for consistency
Converted string-quoted keys to unquoted identifiers while retaining their string values, which is valid and cleaner in modern TS/JS.
37-40: Reformat contractAbiPath with multiline path.join
Stylistic update to split thepath.joinarguments across lines, improving readability without altering functionality.
228-231: Skip indentation tweak
The indentation change onmain().catch(...)is purely stylistic and has no effect on logic.contracts/contracts/libraries/IdCardAttributeHandler.sol (3)
16-16: Standardize error identifier casing
RenamedINSUFFICIENT_CHARCODE_LENtoInsufficientCharcodeLento align with PascalCase error naming conventions.
179-181: Update doc comment for extractStringAttribute
Revised the@devcomment to reference the newInsufficientCharcodeLenerror name.
192-192: Update revert statement to new error
Changedrevert INSUFFICIENT_CHARCODE_LEN();torevert InsufficientCharcodeLen();matching the renamed error.contracts/scripts/setRegistryId.ts (2)
19-21: LGTM - Clean code style improvement.Removing quotes around object keys follows modern JavaScript/TypeScript conventions and improves readability.
33-36: LGTM - Improved readability with multi-line formatting.Breaking complex function calls and object instantiations into multiple lines enhances code readability and maintainability.
Also applies to: 89-93
contracts/contracts/libraries/CircuitAttributeHandler.sol (1)
15-15: LGTM - Consistent error naming convention update.The renaming from
INSUFFICIENT_CHARCODE_LENtoInsufficientCharcodeLenfollows modern Solidity conventions using PascalCase for custom errors. The change is applied consistently across the error declaration, documentation, and usage.Also applies to: 182-182, 194-194
contracts/test/example/airdrop.test.ts (2)
35-35: LGTM - Consistent arrow function formatting.Adding parentheses around arrow function parameters in dynamic imports improves consistency and follows TypeScript best practices.
Also applies to: 290-290
77-83: LGTM - Enhanced readability with multi-line formatting.Breaking chained method calls into multiple lines improves code readability and makes the deployment configuration more maintainable.
Also applies to: 352-352
app/src/types/react-native-passport-reader.d.ts (1)
6-7: LGTM - Well-defined type extensions for CAN support.The new
canNumberanduseCanproperties extend the scanning options to support Card Access Number functionality, and thereset()method provides necessary cleanup capability. The type definitions are properly structured.Verify that the implementation in
app/src/utils/nfcScanner.tsproperly utilizes these new interface members:#!/bin/bash # Description: Verify usage of new PassportReader interface members # Expected: Find usage of canNumber, useCan properties and reset() method echo "Checking usage of new PassportReader interface members..." # Search for canNumber usage echo "=== canNumber usage ===" rg -A 3 -B 3 "canNumber" # Search for useCan usage echo "=== useCan usage ===" rg -A 3 -B 3 "useCan" # Search for reset() method usage echo "=== reset() method usage ===" rg -A 3 -B 3 "\.reset\(\)"Also applies to: 13-13
circuits/circuits/utils/crypto/hasher/shaBytes/shaBytesDynamic.circom (3)
6-7: LGTM - Necessary imports for range checking functionality.The new imports support the range checking feature and are properly included from established circuit libraries.
24-26: Excellent security enhancement with input validation.Adding a range check using
Num2Bits(log2Ceil(max_num_bytes * 8))ensures thatin_len_padded_bytesfits within the expected bit width, preventing potential overflow issues and invalid parameter attacks. This is a critical security improvement for the cryptographic circuit.
37-37: LGTM - Minor formatting cleanup.The formatting adjustments improve code consistency without affecting functionality.
Also applies to: 44-44
contracts/package.json (2)
18-18: LGTM! Package management migration is well-executed.The consistent migration from
npx hardhattoyarn hardhatacross all scripts properly aligns with the project's upgrade to Yarn 4.x. The addition of thetest:v2script and thepackageManagerfield specification supports the new testing infrastructure and standardized package management.Also applies to: 34-34, 37-60
53-53: New V2 test script properly configured.The
test:v2script correctly targets the new V2 test suite and specifies the localhost network, which is appropriate for integration testing of the upgraded verification flows.contracts/test/integration/endToEnd.test.ts (3)
12-14: LGTM! Import statement improvements enhance maintainability.The removal of the
.jsextension from the import and the direct import ofLeanIMTfrom the package improve code clarity and follow TypeScript best practices. The updated path forserialized_dsc_tree.jsoncorrectly reflects the project structure.
89-89: Dynamic import formatting improved for consistency.The addition of parentheses around the parameter in the arrow function aligns with consistent formatting standards across the project.
186-197: Contract deployment refactoring improves readability.The single chained call approach for the Airdrop contract deployment maintains all the original parameters while improving code readability and consistency with modern JavaScript/TypeScript patterns.
app/src/utils/utils.ts (1)
5-11: ```shell
#!/bin/bashSearch for MRZ validation patterns or test cases across TypeScript files
rg -n "mrz" --glob '.ts'
rg -n "TD1_REGEX" --glob '.ts'
rg -n "TD3_line_2_REGEX" --glob '.ts'
rg -n "describe" --glob '.spec.ts' --glob '*.test.ts'</details> <details> <summary>contracts/test/unit/formatter.test.ts (1)</summary> `153-173`: **Test properly updated to reflect new function signature and behavior.** The modification to use four longer hexadecimal inputs and the addition of the `"id"` parameter to the TypeScript implementation correctly aligns with the updated `extractForbiddenCountriesFromPacked` function. The transformation logic that concatenates every three consecutive elements properly formats the TypeScript result for comparison with the contract output. </details> <details> <summary>contracts/test/integration/verifyAll.test.ts (2)</summary> `44-44`: **Formatting improvement enhances code consistency.** The addition of parentheses around the parameter in the arrow function for the dynamic import aligns with consistent formatting standards across the project test files. --- `181-181`: **Type annotation formatting cleaned up throughout the file.** The removal of extra spaces in type annotations (`VcAndDiscloseHubProof`) improves code readability and maintains consistent formatting standards across the test suite. Also applies to: 202-202, 225-225, 248-248, 269-269, 307-307, 332-332, 353-353, 381-381, 405-405, 429-429, 482-482, 505-505 </details> <details> <summary>contracts/contracts/verifiers/register_id/Verifier_register_id_sha256_sha256_sha256_rsa_65537_4096.sol (2)</summary> `25-35`: **LGTM: Improved constant alignment.** The alignment of constant declarations enhances code readability without affecting functionality. --- `63-68`: **LGTM: Better function signature formatting.** The multi-line parameter formatting improves readability for this complex function signature. </details> <details> <summary>contracts/test/integration/commitmentRegistration.test.ts (2)</summary> `8-9`: **LGTM: Import path standardization.** Moving imports to the common directory improves maintainability and aligns with the codebase refactoring. --- `62-62`: **LGTM: Consistent dynamic import style.** The arrow function with parentheses provides a consistent style for dynamic imports across the test suite. Also applies to: 246-246 </details> <details> <summary>circuits/tests/disclose/vc_and_disclose_id.test.ts (3)</summary> `84-84`: **LGTM: Correct removal of passport number SMT.** Passing `null` instead of `passportNo_smt` aligns with the ID card verification logic where passport number SMT is not applicable. Also applies to: 268-268 --- `140-140`: **LGTM: Updated loop bounds for expanded data structure.** The extension from 88 to 90 iterations correctly reflects the updated circuit data structure with 90 bytes for the MRZ portion. --- `186-187`: **LGTM: Updated assertions for new data indices.** The updated null byte assertions at indices 90 and 91 correctly correspond to the expanded 90-byte MRZ data structure. </details> <details> <summary>contracts/contracts/tests/TestCustomVerifier.sol (1)</summary> `1-15`: **LGTM: Well-designed test wrapper contract.** The contract provides a clean interface for testing the `CustomVerifier.customVerify` function. The structure is minimal, focused, and appropriate for its testing purpose. </details> <details> <summary>contracts/test/integration/vcAndDisclose.test.ts (4)</summary> `13-13`: **LGTM: Consolidated imports improve organization.** Combining related imports into a single line reduces clutter and improves code organization. --- `46-46`: **LGTM: Consistent dynamic import style.** The standardized arrow function syntax for dynamic imports maintains consistency across the test suite. --- `92-92`: **LGTM: Added missing semicolon.** Good catch on the missing semicolon, improving code style consistency. --- `136-141`: **LGTM: Enhanced type safety with explicit tuple typing.** The explicit tuple type `[BigNumberish, BigNumberish, BigNumberish, BigNumberish]` provides better type safety by ensuring exactly 4 elements are passed, rather than allowing the entire array. </details> <details> <summary>contracts/ignition/modules/deployTestSelfVerificationRoot.ts (1)</summary> `22-22`: **Verify the hardcoded hub address for different environments.** The hardcoded address `0x3e2487a250e2A7b56c7ef5307Fb591Cc8C83623D` appears to be environment-specific. Ensure this address is correct for all intended deployment environments. ```shell #!/bin/bash # Description: Verify if the hardcoded hub address is used consistently across deployment files # Expected: Find other references to this address or similar deployment patterns echo "Searching for the hardcoded hub address..." rg "0x3e2487a250e2A7b56c7ef5307Fb591Cc8C83623D" -A 2 -B 2 echo -e "\nSearching for other deployment modules with similar patterns..." fd "deploy.*\.ts" --exec rg -l "IdentityVerificationHub" {} echo -e "\nSearching for environment-specific configuration patterns..." rg "getParameter.*Address" -A 1 -B 1contracts/contracts/example/SelfPassportERC721.sol (1)
1-220: ```shell
#!/bin/bashRe-verify references to the deprecated contract across all relevant files
echo "Searching for any occurrences of 'SelfPassportERC721' in .sol, .js, .ts files..."
rg "SelfPassportERC721" -g ".sol" -g ".js" -g "*.ts" -A2 -B2 || trueecho -e "\nChecking for import statements referencing SelfPassportERC721..."
rg "import.SelfPassportERC721" -g ".sol" -g ".js" -g ".ts" -A1 -B1 || trueecho -e "\nLooking in deployment scripts or test files for usage..."
rg "SelfPassportERC721" -g ".ts" -g ".js" -A2 -B2 || true</details> <details> <summary>app/src/screens/passport/PassportNFCTroubleScreen.tsx (5)</summary> `3-3`: **LGTM: YStack import added for layout improvements.** The import aligns with the layout restructuring later in the component. --- `17-17`: **LGTM: String formatting standardized.** Consistent use of straight apostrophes improves code consistency across the component. Also applies to: 25-25, 33-33, 37-37 --- `52-57`: **Consider the implications of simplifying the developer gesture.** The gesture was changed from a two-finger, five-tap to a single-finger, five-tap gesture. While this makes developer access easier, it might increase the risk of accidental activation by regular users. Consider if this change maintains the right balance between developer accessibility and preventing accidental activation. The single-finger gesture might be more prone to accidental triggers during normal app usage. --- `63-64`: **LGTM: Secondary button improves user experience.** The addition of the "Open NFC Options" button provides users with a clear, discoverable way to access NFC options without relying on hidden gestures. --- `66-79`: **LGTM: Layout improvements with YStack wrapper.** The YStack wrapper provides better layout control, and the gesture detector now only wraps the first caption line, which is a more targeted approach. The text updates improve clarity for users. </details> <details> <summary>contracts/hardhat.config.ts (6)</summary> `4-4`: **LGTM: Trailing comma added for consistency.** The trailing comma in the dotenv configuration improves code consistency. --- `57-57`: **LGTM: Environment variable standardization.** Changing from `CELO_KEY` to `PRIVATE_KEY` provides consistent environment variable naming across networks. --- `59-63`: **LGTM: Network configuration renamed and updated.** The network name change from `celoAlfajores` to `alfajores` simplifies the configuration and aligns with common naming conventions. --- `70-70`: **LGTM: Etherscan API key added for alfajores network.** The API key configuration aligns with the network renaming and ensures contract verification works properly. --- `77-89`: **LGTM: Custom chains configuration updated appropriately.** The API URL changes to use Etherscan's v2 API with chain ID parameters and the addition of the alfajores custom chain configuration support the network updates properly. --- `16-23`: **Verify impact of removing Solidity optimizer details.** The optimizer configuration was simplified by removing the `details` object and `viaIR` flag. This might affect contract optimization and gas costs. ```shell #!/bin/bash # Description: Check if other config files or documentation mention specific optimizer settings # Expected: Find references to viaIR or optimizer details that might need updating echo "Searching for references to viaIR in the codebase..." rg "viaIR" -A 2 -B 2 echo -e "\nSearching for optimizer details configuration..." rg "optimizer.*details" -A 5 -B 2 echo -e "\nSearching for any gas optimization documentation..." rg -i "gas.*optimi" -g "*.md" -A 2 -B 2Consider documenting why these advanced optimizer features were removed and whether this impacts contract deployment costs.
contracts/scripts/README.md (1)
1-154: Excellent documentation for utility scripts.This README provides comprehensive documentation for the utility scripts with:
- Clear categorization of test and development scripts
- Practical usage examples and workflows
- Helpful troubleshooting section
- Good explanation of script features
- Integration details with existing package.json scripts
The documentation will significantly improve the developer experience for working with the contracts.
contracts/scripts/dev.sh (1)
1-141: LGTM! Well-structured development script with good practices.The script demonstrates excellent shell scripting practices including:
- Proper error handling with
set -e- Colored output functions for better UX
- Directory validation logic
- Comprehensive help documentation
- Clear command structure
The implementation is solid and follows bash best practices.
contracts/contracts/libraries/GenericFormatter.sol (1)
17-72: LGTM! Well-implemented conversion utilities.The library functions are well-structured with:
- Proper documentation
- Efficient use of memory structs
- Appropriate use of ABI encoding/decoding
- Pure functions for gas efficiency
- Clear naming conventions
The implementation correctly handles version conversions and serialization needs.
contracts/contracts/tests/testGenericFormatter.sol (1)
1-37: LGTM! Clean test contract wrapper.The test contract properly exposes the
GenericFormatterlibrary functions for testing:
- All functions correctly marked as
pure- Proper import structure
- Clean function naming with
testprefix- Appropriate use of memory parameters
This provides good isolation for testing the library functionality.
app/src/screens/passport/PassportNFCScanScreen.tsx (3)
73-78: Consider the security implications of the simplified developer mode gestureChanging from a two-finger five-tap to a single-finger five-tap gesture makes it significantly easier to accidentally trigger developer mode. This could expose development features to end users.
Consider reverting to the two-finger gesture or implementing additional safeguards like:
- Requiring a longer press duration
- Adding a confirmation dialog
- Checking for a debug build flag
80-93: Good refactoring of error handlingThe centralized error handling with
openErrorModalimproves code maintainability and ensures consistent error presentation across all NFC scan failures.Also applies to: 221-221
310-324: Nice UX improvement with the help buttonAdding a visible help button next to the title improves discoverability of the NFC troubleshooting screen.
contracts/test/v2/registerId.test.ts (1)
1-266: Well-structured and comprehensive test suiteThe test suite provides excellent coverage for the V2 identity registration functionality with:
- Proper test isolation using snapshots
- Comprehensive error scenario testing
- Clear test descriptions and assertions
- Good verification of state changes
contracts/test/unit/CustomVerifier.test.ts (1)
10-411: Comprehensive test coverage for CustomVerifierExcellent test suite with thorough coverage of:
- Both passport and ID card verification flows
- OFAC compliance checks
- Age verification logic
- Forbidden countries validation
- Proper error handling for invalid inputs
app/src/screens/passport/NFCMethodSelectionScreen.tsx (1)
94-104: Ensure MRZ field updates are validatedThe update handlers directly pass user input to the store without validation. This could propagate invalid data throughout the app.
Consider adding validation before updating the store:
const onDateOfBirthChange = (text: string) => { const cleaned = text.replace(/[^0-9]/g, ''); if (cleaned.length === 6) { // Basic date validation const year = parseInt(cleaned.substring(0, 2)); const month = parseInt(cleaned.substring(2, 4)); const day = parseInt(cleaned.substring(4, 6)); if (month >= 1 && month <= 12 && day >= 1 && day <= 31) { updatePassport({ dateOfBirth: cleaned }); } } else if (cleaned.length < 6) { updatePassport({ dateOfBirth: cleaned }); } };contracts/contracts/libraries/Formatter.sol (1)
169-169: Verify the index calculation logicThe index calculation
uint256 index = j / 31;combined withpublicSignals[index * 3]on line 174 could potentially access out-of-bounds array elements.#!/bin/bash # Description: Check the maximum possible index value to ensure array bounds are respected # Calculate maximum index value max_j=39 # MAX_FORBIDDEN_COUNTRIES_LIST_LENGTH - 1 max_index=$((max_j / 31)) # = 1 max_array_access=$((max_index * 3)) # = 3 echo "Maximum j value: $max_j" echo "Maximum index value: $max_index" echo "Maximum array access: publicSignals[$max_array_access]" echo "publicSignals array size: 4" if [ $max_array_access -ge 4 ]; then echo "WARNING: Array access out of bounds!" else echo "Array access is within bounds" ficontracts/test/v2/discloseId.test.ts (2)
19-25: LGTM! Well-implemented user identifier hash calculation.The helper function correctly implements the two-step hashing process (SHA256 → RIPEMD160) to generate a 20-byte identifier, which aligns with Ethereum address format.
158-898: Excellent test coverage!The test suite comprehensively covers:
- Success flow with proper proof encoding
- All relevant failure scenarios with appropriate custom error assertions
- Edge cases for date validation, cross-chain verification, and authorization
- Proper isolation using snapshot/revert pattern
The tests are well-structured and follow best practices.
contracts/contracts/verifiers/disclose/Verifier_vc_and_disclose_id.sol (1)
1-307: Auto-generated verifier contract looks correct.This snarkJS-generated Groth16 verifier for ID card attestations correctly:
- Accepts 21 public signals as expected for ID card circuits
- Validates all field elements are within the scalar field
- Implements standard elliptic curve operations and pairing checks
- Uses assembly for gas optimization
The structure follows the expected pattern for snarkJS-generated verifiers.
contracts/test/v2/registerPassport.test.ts (1)
1-268: Well-structured passport registration tests.The test suite provides comprehensive coverage for both DSC and identity commitment registration:
- Proper separation of concerns between DSC and identity registration tests
- Thorough failure scenario testing with appropriate custom error assertions
- Good use of test fixtures and snapshot isolation
- Clear test naming and organization
contracts/test/utils/generateProof.ts (3)
90-139: Well-implemented ID registration proof generation.The function correctly:
- Dynamically selects the appropriate circuit based on document type
- Maintains backward compatibility with passport circuits
- Follows the established pattern for proof generation and verification
308-308: Correct implementation with proper ID card adaptations.The function properly handles ID card specifics:
- Uses 90-element selector for ID cards (vs 93 for passports)
- Correctly sets document type and category
- Uses ID-specific circuit artifacts
The implementation aligns with the ID card circuit requirements.
Also applies to: 369-372
420-427: Good enhancement for Solidity compatibility.The modification to convert hex strings to decimal ensures proper compatibility with Solidity contracts that expect decimal string inputs for BigNumber values.
contracts/contracts/libraries/CircuitAttributeHandlerV2.sol (3)
17-17: Good naming convention update.The error renaming from
INSUFFICIENT_CHARCODE_LENtoInsufficientCharcodeLenfollows Solidity's PascalCase convention for custom errors, improving code consistency.Also applies to: 298-298
199-267: Excellent refactoring of OFAC check functions.The changes improve the code by:
- Using boolean return types for OFAC checks (more semantically correct)
- Simplifying the logic in
compareOfacwith proper boolean operations- Making the code more intuitive:
true= no OFAC violationThe refactoring maintains the same functionality while improving readability.
323-325: Good backward compatibility maintenance.The legacy function properly converts the boolean result to uint256 (1 or 0) to maintain compatibility with existing integrations while allowing the main implementation to use the improved boolean type.
contracts/contracts/interfaces/ISelfVerificationRoot.sol (1)
4-73: Well-structured interface with comprehensive documentation.The interface properly defines the V2 verification system with clear struct definitions and function signatures. The NatSpec documentation is thorough and helpful.
contracts/contracts/tests/TestSelfVerificationRoot.sol (1)
8-108: Well-structured test contract implementation.The test contract properly implements the abstract
SelfVerificationRootwith appropriate helper functions for testing the V2 verification flow.contracts/contracts/example/HappyBirthday.sol (1)
35-36: Verify the 50x increase in default claimable amount.The default claimable amount increased from 1 USDC to 50 USDC. Please confirm this significant increase is intentional and that the contract will be adequately funded.
contracts/contracts/abstract/SelfVerificationRoot.sol (3)
20-22: LGTM! Clear version constant definition.The CONTRACT_VERSION constant is well-documented and properly scoped as internal.
95-134: Well-structured verification function with proper input validation.The refactored
verifySelfProoffunction correctly:
- Validates minimum lengths for both input parameters
- Uses assembly safely for efficient calldata reading
- Constructs the verification input with the documented format
- Properly extracts destination chain ID, user identifier, and user-defined data
The data format documentation is clear and helpful.
144-157: Secure callback implementation with proper access control.The
onVerificationSuccesscallback correctly enforces that only the identity verification hub V2 can invoke it, preventing unauthorized calls. The delegation tocustomVerificationHookprovides good extensibility.contracts/contracts/interfaces/IIdentityVerificationHubV2.sol (1)
6-181: Well-designed interface supporting the V2 architecture.The interface effectively:
- Provides clear separation between external functions, view functions, and public functions
- Supports attestation ID-based operations throughout
- Includes comprehensive getter functions for all storage mappings
- Properly documents all functions with clear parameter descriptions
The structured approach with
SelfStructsand unifiedverifyfunction provides good extensibility.contracts/contracts/example/Airdrop.sol (1)
26-27: Good use of basis points constant.Using BASIS_POINTS constant improves readability and reduces magic numbers in calculations.
contracts/contracts/IdentityVerificationHubImplV2.sol (4)
19-41: Excellent use of ERC-7201 storage pattern.The implementation correctly uses namespaced storage with proper slot calculations, ensuring upgrade safety and preventing storage collisions.
285-297: Well-structured main verification function.The
verifyfunction provides a clean orchestration of the verification process with proper separation of concerns. The cross-chain support placeholder with clear error is appropriate.
1027-1044: Correct implementation of user identifier validation.The function properly validates the user identifier by:
- Using the same hash chain as the circuit (SHA256 + RIPEMD160)
- Correctly excluding the configId from the hash calculation
- Providing clear error when validation fails
504-506: Simple and effective config ID generation.Using SHA256 hash of the encoded config struct is a good choice for generating deterministic, collision-resistant config IDs.
| # local RAND_STR=$(get_random_string) | ||
| # echo $RAND_STR | yarn snarkjs zkey contribute \ | ||
| # ${OUTPUT_DIR}/${CIRCUIT_NAME}/${CIRCUIT_NAME}.zkey \ | ||
| # ${OUTPUT_DIR}/${CIRCUIT_NAME}/${CIRCUIT_NAME}_final.zkey |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review security impact of disabling randomness
Commenting out the zkey contribute step removes entropy from the ceremony, weakening security. If deterministic builds are needed, integrate a trusted randomness source for production.
🤖 Prompt for AI Agents
In circuits/scripts/build/common.sh around lines 88 to 91, the zkey contribute
step that adds randomness is commented out, which removes entropy and weakens
security. To fix this, restore the zkey contribute command to include randomness
in the ceremony. If deterministic builds are required, replace the disabled
randomness with a trusted, secure randomness source instead of disabling it
entirely.
| - name: Block PRs not from dev | ||
| run: | | ||
| if [[ "${{ github.head_ref }}" != "dev" ]]; then | ||
| echo "You can only merge from dev to main." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security: Avoid direct use of potentially untrusted input in shell scripts.
Using ${{ github.head_ref }} directly in shell scripts poses a security risk as it can contain malicious commands. Use an environment variable instead.
Apply this diff to fix the security vulnerability:
- - name: Block PRs not from dev
- run: |
- if [[ "${{ github.head_ref }}" != "dev" ]]; then
- echo "You can only merge from dev to main."
- exit 1
- fi
+ - name: Block PRs not from dev
+ env:
+ HEAD_REF: ${{ github.head_ref }}
+ run: |
+ if [[ "$HEAD_REF" != "dev" ]]; then
+ echo "You can only merge from dev to main."
+ exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Block PRs not from dev | |
| run: | | |
| if [[ "${{ github.head_ref }}" != "dev" ]]; then | |
| echo "You can only merge from dev to main." | |
| exit 1 | |
| fi | |
| - name: Block PRs not from dev | |
| env: | |
| HEAD_REF: ${{ github.head_ref }} | |
| run: | | |
| if [[ "$HEAD_REF" != "dev" ]]; then | |
| echo "You can only merge from dev to main." | |
| exit 1 | |
| fi |
🧰 Tools
🪛 actionlint (1.7.7)
12-12: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions for more details
(expression)
🤖 Prompt for AI Agents
In .github/workflows/block-non-dev-to-main.yml around lines 11 to 16, avoid
using the GitHub context expression ${{ github.head_ref }} directly in the shell
script. Instead, assign it to an environment variable in the workflow step and
reference that variable in the shell script to prevent potential command
injection vulnerabilities.
| "identityVerificationHubV2Address", | ||
| "0x3e2487a250e2A7b56c7ef5307Fb591Cc8C83623D", | ||
| ); | ||
| const scopeValue = m.getParameter("scopeValue", 12345); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Replace the temporary scope value with a proper implementation.
The comment explicitly states this is a "TEMPORARY VALUE" that requires manual calculation after deployment. Consider implementing a proper scope calculation or making this a required parameter without a default.
- const scopeValue = m.getParameter("scopeValue", 12345);
+ const scopeValue = m.getParameter("scopeValue"); // Remove default to force explicit valueOr add validation:
const scopeValue = m.getParameter("scopeValue", 12345);
+
+ // Log warning for temporary value
+ if (scopeValue === 12345) {
+ console.warn("⚠️ Using temporary scope value. Calculate proper scope after deployment.");
+ }🤖 Prompt for AI Agents
In contracts/ignition/modules/deployTestSelfVerificationRoot.ts at line 24, the
current use of a hardcoded temporary scopeValue (12345) should be replaced with
a proper implementation. Either calculate the scopeValue dynamically based on
deployment context or make it a required parameter without a default to ensure
it is explicitly provided. Additionally, add validation to check that the
scopeValue is valid and not left as a placeholder.
| static extractForbiddenCountriesFromPacked( | ||
| revealedData_packed: string | string[], | ||
| id_type: "passport" | "id", | ||
| ): string[] { | ||
| // If revealedData_packed is not an array, convert it to an array | ||
| const packedArray = Array.isArray(revealedData_packed) ? revealedData_packed : [revealedData_packed]; | ||
|
|
||
| const bytesCount = id_type === "passport" ? [31, 31, 31] : [31, 31, 31, 27]; // nb of bytes in each of the first three field elements | ||
| const bytesArray = packedArray.flatMap((element: string, index: number) => { | ||
| const bytes = bytesCount[index] || 31; // Use 31 as default if index is out of range | ||
| const elementBigInt = BigInt(element); | ||
| const byteMask = BigInt(255); // 0xFF | ||
| const bytesOfElement = [...Array(bytes)].map((_, byteIndex) => { | ||
| return (elementBigInt >> (BigInt(byteIndex) * BigInt(8))) & byteMask; | ||
| }); | ||
| return bytesOfElement; | ||
| }); | ||
|
|
||
| return bytesArray.map((byte: bigint) => String.fromCharCode(Number(byte))); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Verify the bigint conversion logic and add input validation.
The refactored function handles both single strings and arrays, but there are potential issues:
- The
bytesCountarray access at line 100 could be unsafe if the array is longer than expected - No input validation for empty arrays or invalid
id_typevalues - The bigint conversion assumes valid numeric strings
Consider adding input validation:
static extractForbiddenCountriesFromPacked(
revealedData_packed: string | string[],
id_type: "passport" | "id",
): string[] {
+ if (Array.isArray(revealedData_packed) && revealedData_packed.length === 0) {
+ throw new Error("Empty packed data array");
+ }
+
// If revealedData_packed is not an array, convert it to an array
const packedArray = Array.isArray(revealedData_packed) ? revealedData_packed : [revealedData_packed];
const bytesCount = id_type === "passport" ? [31, 31, 31] : [31, 31, 31, 27];
const bytesArray = packedArray.flatMap((element: string, index: number) => {
- const bytes = bytesCount[index] || 31; // Use 31 as default if index is out of range
+ const bytes = index < bytesCount.length ? bytesCount[index] : 31;
+
+ // Validate that element is a valid numeric string
+ try {
const elementBigInt = BigInt(element);
+ } catch (error) {
+ throw new Error(`Invalid numeric string at index ${index}: ${element}`);
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In contracts/test/utils/formatter.ts around lines 91 to 110, the
extractForbiddenCountriesFromPacked function lacks input validation and has
unsafe array access for bytesCount. Add validation to ensure revealedData_packed
is not empty, id_type is either "passport" or "id", and each element in
revealedData_packed is a valid numeric string before converting to BigInt. Also,
handle cases where packedArray length exceeds bytesCount length safely by
limiting iteration or providing defaults. This will prevent runtime errors and
ensure robust bigint conversion.
| genericDiscloseOutputV2Bytes, | ||
| ); | ||
|
|
||
| expect(genericDiscloseOutputV2.attestationId.toString()).to.equal(genericDiscloseOutputV2.attestationId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix redundant assertion in the test.
Line 151 has a redundant assertion that compares attestationId to itself:
- expect(genericDiscloseOutputV2.attestationId.toString()).to.equal(genericDiscloseOutputV2.attestationId);
+ expect(genericDiscloseOutputLatest[0][0]).to.equal(genericDiscloseOutputV2.attestationId);This should compare the decoded value from the contract with the original input value.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(genericDiscloseOutputV2.attestationId.toString()).to.equal(genericDiscloseOutputV2.attestationId); | |
| expect(genericDiscloseOutputLatest[0][0]).to.equal(genericDiscloseOutputV2.attestationId); |
🤖 Prompt for AI Agents
In contracts/test/unit/GenericFormatter.test.ts at line 151, the assertion
redundantly compares attestationId to itself. Update the test to compare the
decoded attestationId value from genericDiscloseOutputV2 with the original
expected input value to properly validate the decoding logic.
| // Calculate final amount based on attestation type | ||
| uint256 finalAmount = claimableAmount; | ||
|
|
||
| // Apply bonus multiplier for EUID card users | ||
| if (output.attestationId == AttestationId.EU_ID_CARD) { | ||
| finalAmount = (claimableAmount * euidBonusMultiplier) / BASIS_POINTS; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passport bonus multiplier is defined but never applied.
The contract defines passportBonusMultiplier but only applies bonus for EUID cards. E-Passport users don't receive their configured bonus.
- // Apply bonus multiplier for EUID card users
- if (output.attestationId == AttestationId.EU_ID_CARD) {
- finalAmount = (claimableAmount * euidBonusMultiplier) / BASIS_POINTS;
- }
+ // Apply bonus multiplier based on attestation type
+ if (output.attestationId == AttestationId.EU_ID_CARD) {
+ finalAmount = (claimableAmount * euidBonusMultiplier) / BASIS_POINTS;
+ } else if (output.attestationId == AttestationId.E_PASSPORT) {
+ finalAmount = (claimableAmount * passportBonusMultiplier) / BASIS_POINTS;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Calculate final amount based on attestation type | |
| uint256 finalAmount = claimableAmount; | |
| // Apply bonus multiplier for EUID card users | |
| if (output.attestationId == AttestationId.EU_ID_CARD) { | |
| finalAmount = (claimableAmount * euidBonusMultiplier) / BASIS_POINTS; | |
| } | |
| // Calculate final amount based on attestation type | |
| uint256 finalAmount = claimableAmount; | |
| // Apply bonus multiplier based on attestation type | |
| if (output.attestationId == AttestationId.EU_ID_CARD) { | |
| finalAmount = (claimableAmount * euidBonusMultiplier) / BASIS_POINTS; | |
| } else if (output.attestationId == AttestationId.E_PASSPORT) { | |
| finalAmount = (claimableAmount * passportBonusMultiplier) / BASIS_POINTS; | |
| } |
🤖 Prompt for AI Agents
In contracts/contracts/example/HappyBirthday.sol around lines 143 to 149, the
code applies a bonus multiplier only for EUID card users but neglects to apply
the passportBonusMultiplier for E-Passport users. Add a conditional check for
the E-PASSPORT attestationId and apply the passportBonusMultiplier to
finalAmount in a similar manner as done for the EUID card, ensuring both user
types receive their respective bonuses.
| // Create birthday in current year (format: YYMMDD) | ||
| string memory dobInThisYear = string(abi.encodePacked("25", month, day)); | ||
|
|
||
| uint256 dobInThisYearTimestamp = SelfCircuitLibrary.dateToTimestamp(dobInThisYear); | ||
| uint256 dobInThisYearTimestamp = Formatter.dateToUnixTimestamp(dobInThisYear); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove hardcoded year "25" - will break in 2026.
The birthday validation hardcodes "25" for the year 2025, which will cause incorrect behavior starting in 2026. Calculate the current year dynamically from block.timestamp.
- // Create birthday in current year (format: YYMMDD)
- string memory dobInThisYear = string(abi.encodePacked("25", month, day));
+ // Calculate current year from timestamp
+ uint256 currentYear = (block.timestamp / 365 days) + 1970;
+ string memory yearStr = Formatter.uint2str(currentYear % 100); // Get last 2 digits
+
+ // Create birthday in current year (format: YYMMDD)
+ string memory dobInThisYear = string(abi.encodePacked(yearStr, month, day));Note: You'll need to add a helper function to convert uint to string or use an existing library.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In contracts/contracts/example/HappyBirthday.sol around lines 185 to 187, the
code hardcodes the year as "25" when creating the birthday string, which will
break after 2025. Replace the hardcoded "25" with a dynamically calculated
current year derived from block.timestamp. To do this, extract the current year
from block.timestamp, convert it to a string (using a helper function or
existing library), and concatenate it with the month and day to form
dobInThisYear. This ensures the birthday validation works correctly beyond 2025.
| /// @notice Bonus multiplier for EUID card users (in basis points) | ||
| uint256 public euidBonusMultiplier = 200; // 200% = 100% bonus | ||
|
|
||
| /// @notice Bonus multiplier for E-Passport card users (in basis points) | ||
| uint256 public passportBonusMultiplier = 100; // 100% = 50% bonus | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect bonus multiplier comments and math.
The comments incorrectly describe the bonus percentages:
- 200 basis points (200%) = 2x the base amount = 100% bonus, not "100% bonus"
- 100 basis points (100%) = 1x the base amount = 0% bonus, not "50% bonus"
- /// @notice Bonus multiplier for EUID card users (in basis points)
- uint256 public euidBonusMultiplier = 200; // 200% = 100% bonus
+ /// @notice Bonus multiplier for EUID card users (in basis points)
+ uint256 public euidBonusMultiplier = 200; // 200% of base = 100% bonus
- /// @notice Bonus multiplier for E-Passport card users (in basis points)
- uint256 public passportBonusMultiplier = 100; // 100% = 50% bonus
+ /// @notice Bonus multiplier for E-Passport users (in basis points)
+ uint256 public passportBonusMultiplier = 150; // 150% of base = 50% bonus📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// @notice Bonus multiplier for EUID card users (in basis points) | |
| uint256 public euidBonusMultiplier = 200; // 200% = 100% bonus | |
| /// @notice Bonus multiplier for E-Passport card users (in basis points) | |
| uint256 public passportBonusMultiplier = 100; // 100% = 50% bonus | |
| /// @notice Bonus multiplier for EUID card users (in basis points) | |
| uint256 public euidBonusMultiplier = 200; // 200% of base = 100% bonus | |
| /// @notice Bonus multiplier for E-Passport users (in basis points) | |
| uint256 public passportBonusMultiplier = 150; // 150% of base = 50% bonus |
🤖 Prompt for AI Agents
In contracts/contracts/example/HappyBirthday.sol around lines 38 to 43, the
comments for euidBonusMultiplier and passportBonusMultiplier incorrectly
describe the bonus percentages and their basis points. Update the comments to
correctly reflect that 200 basis points equals 2x the base amount, which is a
100% bonus, and 100 basis points equals 1x the base amount, which is a 0% bonus.
Adjust the comments to accurately describe these values and their corresponding
bonuses.
| function _isWithinBirthdayWindow(bytes32 attestationId, string memory dobFromProof) internal view returns (bool) { | ||
| // DOB comes in format "DD-MM-YY" from the proof system | ||
| bytes memory dobBytes = bytes(dobFromProof); | ||
| require(dobBytes.length == 8, "Invalid DOB format"); // "DD-MM-YY" = 8 chars | ||
|
|
||
| // Check if user identifier has already been registered | ||
| if (_registeredUserIdentifiers[userIdentifier]) { | ||
| revert UserIdentifierAlreadyRegistered(); | ||
| } | ||
| // Extract day and month from "DD-MM-YY" format | ||
| string memory day = Formatter.substring(dobFromProof, 0, 2); // DD | ||
| string memory month = Formatter.substring(dobFromProof, 3, 5); // MM (skip hyphen at index 2) | ||
|
|
||
| _nullifierToUserIdentifier[nullifier] = userIdentifier; | ||
| _registeredUserIdentifiers[userIdentifier] = true; | ||
| // Create birthday in current year (format: YYMMDD) | ||
| string memory dobInThisYear = string(abi.encodePacked("25", month, day)); | ||
| uint256 dobInThisYearTimestamp = Formatter.dateToUnixTimestamp(dobInThisYear); | ||
|
|
||
| // Emit registration event | ||
| emit UserIdentifierRegistered(userIdentifier, nullifier); | ||
| } | ||
| uint256 currentTime = block.timestamp; | ||
| uint256 timeDifference; | ||
|
|
||
| // ==================================================== | ||
| // Internal Functions | ||
| // ==================================================== | ||
| if (currentTime > dobInThisYearTimestamp) { | ||
| timeDifference = currentTime - dobInThisYearTimestamp; | ||
| } else { | ||
| timeDifference = dobInThisYearTimestamp - currentTime; | ||
| } | ||
|
|
||
| /** | ||
| * @notice Internal function to mark the caller as having claimed their tokens. | ||
| * @dev Updates the claimed mapping. | ||
| */ | ||
| function _setClaimed() internal { | ||
| claimed[msg.sender] = true; | ||
| return timeDifference <= claimableWindow; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded year "25" will cause issues after 2025.
The function hardcodes "25" for the year when creating dobInThisYear. This will break when the current year changes to 2026 or beyond.
Consider dynamically calculating the current year:
- // Create birthday in current year (format: YYMMDD)
- string memory dobInThisYear = string(abi.encodePacked("25", month, day));
+ // Create birthday in current year (format: YYMMDD)
+ uint256 currentYear = ((block.timestamp / 365 days) + 1970) % 100;
+ string memory yearStr = currentYear < 10
+ ? string(abi.encodePacked("0", Strings.toString(currentYear)))
+ : Strings.toString(currentYear);
+ string memory dobInThisYear = string(abi.encodePacked(yearStr, month, day));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function _isWithinBirthdayWindow(bytes32 attestationId, string memory dobFromProof) internal view returns (bool) { | |
| // DOB comes in format "DD-MM-YY" from the proof system | |
| bytes memory dobBytes = bytes(dobFromProof); | |
| require(dobBytes.length == 8, "Invalid DOB format"); // "DD-MM-YY" = 8 chars | |
| // Check if user identifier has already been registered | |
| if (_registeredUserIdentifiers[userIdentifier]) { | |
| revert UserIdentifierAlreadyRegistered(); | |
| } | |
| // Extract day and month from "DD-MM-YY" format | |
| string memory day = Formatter.substring(dobFromProof, 0, 2); // DD | |
| string memory month = Formatter.substring(dobFromProof, 3, 5); // MM (skip hyphen at index 2) | |
| _nullifierToUserIdentifier[nullifier] = userIdentifier; | |
| _registeredUserIdentifiers[userIdentifier] = true; | |
| // Create birthday in current year (format: YYMMDD) | |
| string memory dobInThisYear = string(abi.encodePacked("25", month, day)); | |
| uint256 dobInThisYearTimestamp = Formatter.dateToUnixTimestamp(dobInThisYear); | |
| // Emit registration event | |
| emit UserIdentifierRegistered(userIdentifier, nullifier); | |
| } | |
| uint256 currentTime = block.timestamp; | |
| uint256 timeDifference; | |
| // ==================================================== | |
| // Internal Functions | |
| // ==================================================== | |
| if (currentTime > dobInThisYearTimestamp) { | |
| timeDifference = currentTime - dobInThisYearTimestamp; | |
| } else { | |
| timeDifference = dobInThisYearTimestamp - currentTime; | |
| } | |
| /** | |
| * @notice Internal function to mark the caller as having claimed their tokens. | |
| * @dev Updates the claimed mapping. | |
| */ | |
| function _setClaimed() internal { | |
| claimed[msg.sender] = true; | |
| return timeDifference <= claimableWindow; | |
| } | |
| function _isWithinBirthdayWindow(bytes32 attestationId, string memory dobFromProof) internal view returns (bool) { | |
| // DOB comes in format "DD-MM-YY" from the proof system | |
| bytes memory dobBytes = bytes(dobFromProof); | |
| require(dobBytes.length == 8, "Invalid DOB format"); // "DD-MM-YY" = 8 chars | |
| // Extract day and month from "DD-MM-YY" format | |
| string memory day = Formatter.substring(dobFromProof, 0, 2); // DD | |
| string memory month = Formatter.substring(dobFromProof, 3, 5); // MM (skip hyphen at index 2) | |
| // Create birthday in current year (format: YYMMDD) | |
| uint256 currentYear = ((block.timestamp / 365 days) + 1970) % 100; | |
| string memory yearStr = currentYear < 10 | |
| ? string(abi.encodePacked("0", Strings.toString(currentYear))) | |
| : Strings.toString(currentYear); | |
| string memory dobInThisYear = string(abi.encodePacked(yearStr, month, day)); | |
| uint256 dobInThisYearTimestamp = Formatter.dateToUnixTimestamp(dobInThisYear); | |
| uint256 currentTime = block.timestamp; | |
| uint256 timeDifference; | |
| if (currentTime > dobInThisYearTimestamp) { | |
| timeDifference = currentTime - dobInThisYearTimestamp; | |
| } else { | |
| timeDifference = dobInThisYearTimestamp - currentTime; | |
| } | |
| return timeDifference <= claimableWindow; | |
| } |
🤖 Prompt for AI Agents
In contracts/contracts/example/Airdrop.sol around lines 176 to 199, the function
_isWithinBirthdayWindow hardcodes the year as "25" when constructing
dobInThisYear, which will fail after 2025. To fix this, dynamically obtain the
current year (last two digits) from the current block timestamp and use it
instead of the hardcoded "25" when building dobInThisYear. This ensures the
function works correctly across different years.
…ed documentation and library linkage for CustomVerifier. Update initialization process to reflect changes in V2 implementation, ensuring proper setup for proxy deployment. (#658)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
* audit fixes (#645) * merge dev branch into main (#624) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> * update contracts (#628) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) * implement self uups upgradeable (#592) * implement self uups upgradeable * small changes in identityVerificationHubImplV2 * delete aderyn.toml * chore: add custom verifier * chnage return output * feat: use self structs and a Generic output struct * feat: add userIdentifier, nullifier, forbiddencountries to returned output * add root view functions from registry * fix: build and compilation errors * add userDefined data into selfVerificationRoot * "resolve conflicts" * fix compilation problem * fix how to register verification config * test: CustomVerifier * fix verification root and hub integration * add scope check in hub impl * replace poseidon hash to ripemd+sha256 * add todo list * feat: refactor and add test cases for generic formatter * add performUserIdentifierCheck in basicVerification * change how to handle additionalData and fix stack too deep * start adding test codes * fix dependency problems in monorepo * fix: forbidden countries (#612) LGTM! * able to run test code * pass happy path * delete unused codes * change error code name, add caller address validation and add scripts to run test and build in monorepo * add all test cases in vcAndDisclose flow * remove comment out * chore: use actual user identifier outputs * success in registration tests * cover all cases * pass contractVersion instead of circuitVersion * fix disclose test * chore: add natspecs for ImplHubV2, CustomVerifier and GenericFormatter * change val name and remove unused lines * add val name change * remove userIdentifier from return data * feat: use GenericDiscloseOutput struct in verfication hook fix test cases for user identifier * chore: change the function order for Hub Impl V2 (#625) * fix nat specs * add nat spec in SelfStructs --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Nesopie <[email protected]> * prettier (#629) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> Co-authored-by: Nesopie <[email protected]> * fix: vc_and_disclose_id test (#640) * fix: vc_and_disclose_id test * chore: yarn prettier * fix: check if a config id exists * chore: change the function where the config not set verification is happening * fix: add await * feat: add getConfigId function in SelfVerificationRoot (#650) * feat: add getConfigId function in SelfVerificationRoot * update comment --------- Co-authored-by: motemotech <[email protected]> * chore: fix ofac end index in eu id cards * chore: fix tests * fix: example contracts and tests --------- Co-authored-by: turnoffthiscomputer <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> * Update deployment module for Identity Verification Hub V2 with detailed documentation and library linkage for CustomVerifier. Update initialization process to reflect changes in V2 implementation, ensuring proper setup for proxy deployment. (#658) * publish npm-package (#651) * App/eu id updates (#638) * fix build issues * generate disclosure proof with euids * generate disclosure proof with euids * Eu id updates 2 (#648) * update vc_and_disclose_id test (dev branch) (#641) * fix: vc_and_disclose_id test * chore: yarn prettier * Show modal on NFC scan error (#642) * Add help button and error modal actions * fix the screen management * yarn nice * Bump build v2.5.4: ios 132; android 71 (#631) * bump version and build numbers * remove tamagui/toast * fix marketing version * fix: update TD1 and TD3 checks (#643) * bum yarn.lock * add version and user defined data --------- Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> * remove the mock user define data * get the useridentifier as a hash from the user defined data * chore: add version and userDefinedData * feat: use the version in register / dsc proofs as well * update calculateUserIdentifierHash * yarn nice * refactor: consolidate user context data handling and update payload structure * fix typing issues on sha1 * remove console.log(sha1) * fix sha1 import * refactor: streamline userDefinedData handling and adjust payload type for circuit * refactor: update sha1 usage and enhance logging in calculateUserIdentifierHash * yarn nice * yarn lint common * use ts-ignore for sha1 import * fix app ci tests * fix typing issue * remove unused ts-ignore * cast uuid before calling generateinputs * bump qrcode version * add tsup on the qrcode sdk * fix: exports on selfxyz/qrcode * update how we define config.version * fix yarn imports * yarn format --------- Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> Co-authored-by: Ayman <[email protected]> * Hotfix contract compile error (#660) * Fix previous rebase error * Refactor deployment module for Identity Verification Hub V2. * Fix/sdk (#652) * fix: sdk build configs * chore: SelfBackendVerifier (WIP) * feat: add custom verification * feat: consider destination chain in user defined data * chore: export attestation id * chore: export attestation id * chore: export config storage * chore: don't throw an error if the proof is not valid * chore: trim abi and rm typechain types * refactor * chore: rm unnecessary exports * 📝 Add docstrings to `fix/sdk` (#653) Docstrings generation was requested by @remicolin. * https://github.com/selfxyz/self/pull/652#issuecomment-2992046545 The following files were modified: * `sdk/core/src/utils/hash.ts` * `sdk/core/src/utils/proof.ts` * `sdk/core/src/utils/utils.ts` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * review fixes * chore: fix package.json cjs types * chore: add minor changes to checks * feat: add InMemoryConfigStore, allIds constant and verificationResult type * chore: export Verification config * feat: change the verification config types * fix: throw issues early if verification config is null * fix: update yarn.lock file * chore: lint * fix: rm ts expect error directive * fix: contract tests * use excluded countries instead forbidden countries list * chore: change types in constnats --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update npm-publish workflow and bump core package version to 1.0.0 (#661) * update import * Update get verification config visibility (#664) * Update deployment module for Identity Verification Hub V2 to correct file paths and module name for deployment commands. * Add troubleshooting documentation for verification issues in deployHubV2.ts. Include manual verification steps and common failure reasons to assist users during deployment. * Change visibility of getVerificationConfigV2 function from internal to public in IdentityVerificationHubImplV2 contract to allow external access. * Apply BUSL v1.1 license headers to app (#665) * Add BSL license headers to app sources * prettier * fix license reference - https://spdx.org/licenses/BUSL-1.1.html * bump build: android 73 (#659) * Contracts/deploy staging (#668) * update scripts * deploy vc and disclose id * fix the deployment scripts on staging * update yarn.lock * bump ios build and version (#669) * configure coderabbitai (#670) * tweak coderabbit * bump * more thorough test spec * Apply BSL to app codebase (#639) * Clean up root license wording * Simplify SPDX header * simplify license and rename BSL to BUSL * fix merge issues * fix missing method --------- Co-authored-by: Justin Hernandez <[email protected]> * SEL-423 apply xcode build suggestions (#671) * apply recommended app settings from xcode * stick to portrait orientation and update target settings * remove app clip references * Circuit audit fixes (#644) * feat: add range checks before use of LessEqThan and SelectSubArray * fix: Num2Bits_strict to constrain virtualKey * bump core version * bump core version and fix ci * chore: use npm_auth_token in yarnrc * chroe: rm yarnrc changes * chore: update npm publish * chore: run npm publish manually * chore: change hub contract address (#675) * Update npm-publish.yml * merge dev to main (#657) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) * implement self uups upgradeable (#592) * implement self uups upgradeable * small changes in identityVerificationHubImplV2 * delete aderyn.toml * chore: add custom verifier * chnage return output * feat: use self structs and a Generic output struct * feat: add userIdentifier, nullifier, forbiddencountries to returned output * add root view functions from registry * fix: build and compilation errors * add userDefined data into selfVerificationRoot * "resolve conflicts" * fix compilation problem * fix how to register verification config * test: CustomVerifier * fix verification root and hub integration * add scope check in hub impl * replace poseidon hash to ripemd+sha256 * add todo list * feat: refactor and add test cases for generic formatter * add performUserIdentifierCheck in basicVerification * change how to handle additionalData and fix stack too deep * start adding test codes * fix dependency problems in monorepo * fix: forbidden countries (#612) LGTM! * able to run test code * pass happy path * delete unused codes * change error code name, add caller address validation and add scripts to run test and build in monorepo * add all test cases in vcAndDisclose flow * remove comment out * chore: use actual user identifier outputs * success in registration tests * cover all cases * pass contractVersion instead of circuitVersion * fix disclose test * chore: add natspecs for ImplHubV2, CustomVerifier and GenericFormatter * change val name and remove unused lines * add val name change * remove userIdentifier from return data * feat: use GenericDiscloseOutput struct in verfication hook fix test cases for user identifier * chore: change the function order for Hub Impl V2 (#625) * fix nat specs * add nat spec in SelfStructs --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Nesopie <[email protected]> * prettier (#629) * CAN auth - android (#613) * add missed files * add NFCMethodSelectionScreen * bump android build --------- Co-authored-by: Justin Hernandez <[email protected]> * feat: add MRZ correction method to NFCMethodSelectionScreen (#627) * add npm auth token env (#632) * bump sdk version (#633) * publish npm package when merging on dev * bump common sdk version * replace yarn publish by npm publish * update common package version * Simplify dev mode gesture (#635) * Simplify developer mode gesture * Enable dev mode on MockData screen with five taps * add build smt function to common sdk * update vc_and_disclose_id test (dev branch) (#641) * fix: vc_and_disclose_id test * chore: yarn prettier * Show modal on NFC scan error (#642) * Add help button and error modal actions * fix the screen management * yarn nice * Bump build v2.5.4: ios 132; android 71 (#631) * bump version and build numbers * remove tamagui/toast * fix marketing version * fix: update TD1 and TD3 checks (#643) * bum yarn.lock * Bump build: ios 133; android 72 and build fixes (#654) * update gesture version and bump android build * bump and fix ios build * update lock files * fixes * fix fotoapparat library source * Update example contracts to include EUID usage (#656) * refactor: update HappyBirthday contract to V2 with support for E-Passport and EUID cards, introduce bonus multipliers, and enhance verification logic * refactor: update Airdrop contract to V2 with support for E-Passport and EU ID Card attestations * refactor: remove BASIS_POINTS constant from Airdrop contract * feat: introduce SelfIdentityERC721 contract for issuing NFTs based on verified identity credentials, replacing SelfPassportERC721 * fix: update verification functions in Airdrop, HappyBirthday, and SelfIdentityERC721 contracts to use customVerificationHook * cherry pick commit from add-test-self-verification... * block non-dev pr to main branch * audit fixes (#645) * merge dev branch into main (#624) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> * update contracts (#628) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) * implement self uups upgradeable (#592) * implement self uups upgradeable * small changes in identityVerificationHubImplV2 * delete aderyn.toml * chore: add custom verifier * chnage return output * feat: use self structs and a Generic output struct * feat: add userIdentifier, nullifier, forbiddencountries to returned output * add root view functions from registry * fix: build and compilation errors * add userDefined data into selfVerificationRoot * "resolve conflicts" * fix compilation problem * fix how to register verification config * test: CustomVerifier * fix verification root and hub integration * add scope check in hub impl * replace poseidon hash to ripemd+sha256 * add todo list * feat: refactor and add test cases for generic formatter * add performUserIdentifierCheck in basicVerification * change how to handle additionalData and fix stack too deep * start adding test codes * fix dependency problems in monorepo * fix: forbidden countries (#612) LGTM! * able to run test code * pass happy path * delete unused codes * change error code name, add caller address validation and add scripts to run test and build in monorepo * add all test cases in vcAndDisclose flow * remove comment out * chore: use actual user identifier outputs * success in registration tests * cover all cases * pass contractVersion instead of circuitVersion * fix disclose test * chore: add natspecs for ImplHubV2, CustomVerifier and GenericFormatter * change val name and remove unused lines * add val name change * remove userIdentifier from return data * feat: use GenericDiscloseOutput struct in verfication hook fix test cases for user identifier * chore: change the function order for Hub Impl V2 (#625) * fix nat specs * add nat spec in SelfStructs --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Nesopie <[email protected]> * prettier (#629) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> Co-authored-by: Nesopie <[email protected]> * fix: vc_and_disclose_id test (#640) * fix: vc_and_disclose_id test * chore: yarn prettier * fix: check if a config id exists * chore: change the function where the config not set verification is happening * fix: add await * feat: add getConfigId function in SelfVerificationRoot (#650) * feat: add getConfigId function in SelfVerificationRoot * update comment --------- Co-authored-by: motemotech <[email protected]> * chore: fix ofac end index in eu id cards * chore: fix tests * fix: example contracts and tests --------- Co-authored-by: turnoffthiscomputer <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> * Update deployment module for Identity Verification Hub V2 with detailed documentation and library linkage for CustomVerifier. Update initialization process to reflect changes in V2 implementation, ensuring proper setup for proxy deployment. (#658) * publish npm-package (#651) * App/eu id updates (#638) * fix build issues * generate disclosure proof with euids * generate disclosure proof with euids * Eu id updates 2 (#648) * update vc_and_disclose_id test (dev branch) (#641) * fix: vc_and_disclose_id test * chore: yarn prettier * Show modal on NFC scan error (#642) * Add help button and error modal actions * fix the screen management * yarn nice * Bump build v2.5.4: ios 132; android 71 (#631) * bump version and build numbers * remove tamagui/toast * fix marketing version * fix: update TD1 and TD3 checks (#643) * bum yarn.lock * add version and user defined data --------- Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> * remove the mock user define data * get the useridentifier as a hash from the user defined data * chore: add version and userDefinedData * feat: use the version in register / dsc proofs as well * update calculateUserIdentifierHash * yarn nice * refactor: consolidate user context data handling and update payload structure * fix typing issues on sha1 * remove console.log(sha1) * fix sha1 import * refactor: streamline userDefinedData handling and adjust payload type for circuit * refactor: update sha1 usage and enhance logging in calculateUserIdentifierHash * yarn nice * yarn lint common * use ts-ignore for sha1 import * fix app ci tests * fix typing issue * remove unused ts-ignore * cast uuid before calling generateinputs * bump qrcode version * add tsup on the qrcode sdk * fix: exports on selfxyz/qrcode * update how we define config.version * fix yarn imports * yarn format --------- Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> Co-authored-by: Ayman <[email protected]> * Hotfix contract compile error (#660) * Fix previous rebase error * Refactor deployment module for Identity Verification Hub V2. * Fix/sdk (#652) * fix: sdk build configs * chore: SelfBackendVerifier (WIP) * feat: add custom verification * feat: consider destination chain in user defined data * chore: export attestation id * chore: export attestation id * chore: export config storage * chore: don't throw an error if the proof is not valid * chore: trim abi and rm typechain types * refactor * chore: rm unnecessary exports * 📝 Add docstrings to `fix/sdk` (#653) Docstrings generation was requested by @remicolin. * https://github.com/selfxyz/self/pull/652#issuecomment-2992046545 The following files were modified: * `sdk/core/src/utils/hash.ts` * `sdk/core/src/utils/proof.ts` * `sdk/core/src/utils/utils.ts` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * review fixes * chore: fix package.json cjs types * chore: add minor changes to checks * feat: add InMemoryConfigStore, allIds constant and verificationResult type * chore: export Verification config * feat: change the verification config types * fix: throw issues early if verification config is null * fix: update yarn.lock file * chore: lint * fix: rm ts expect error directive * fix: contract tests * use excluded countries instead forbidden countries list * chore: change types in constnats --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update npm-publish workflow and bump core package version to 1.0.0 (#661) * update import * Update get verification config visibility (#664) * Update deployment module for Identity Verification Hub V2 to correct file paths and module name for deployment commands. * Add troubleshooting documentation for verification issues in deployHubV2.ts. Include manual verification steps and common failure reasons to assist users during deployment. * Change visibility of getVerificationConfigV2 function from internal to public in IdentityVerificationHubImplV2 contract to allow external access. * Apply BUSL v1.1 license headers to app (#665) * Add BSL license headers to app sources * prettier * fix license reference - https://spdx.org/licenses/BUSL-1.1.html * bump build: android 73 (#659) * Contracts/deploy staging (#668) * update scripts * deploy vc and disclose id * fix the deployment scripts on staging * update yarn.lock * bump ios build and version (#669) * configure coderabbitai (#670) * tweak coderabbit * bump * more thorough test spec * Apply BSL to app codebase (#639) * Clean up root license wording * Simplify SPDX header * simplify license and rename BSL to BUSL * fix merge issues * fix missing method --------- Co-authored-by: Justin Hernandez <[email protected]> * SEL-423 apply xcode build suggestions (#671) * apply recommended app settings from xcode * stick to portrait orientation and update target settings * remove app clip references * Circuit audit fixes (#644) * feat: add range checks before use of LessEqThan and SelectSubArray * fix: Num2Bits_strict to constrain virtualKey * bump core version * bump core version and fix ci * chore: use npm_auth_token in yarnrc * chroe: rm yarnrc changes * chore: update npm publish * chore: run npm publish manually * chore: change hub contract address (#675) * Update npm-publish.yml --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> Co-authored-by: Nesopie <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Kevin Lin <[email protected]> Co-authored-by: kevinsslin <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Eric Nakagawa <[email protected]> * chore: use proper secret when publishing * feat: enable publishing if workflow was triggered manually * Contracts/update verifier (#673) * update hardhat config * update vc and disclose verifier * update vc and disclose verifier script and run it * update test self verification root * update verifier * bump sdk version and use new hub address * chore: update zk-kit binary merkle root dep (#674) * Dev (#677) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) * implement self uups upgradeable (#592) * implement self uups upgradeable * small changes in identityVerificationHubImplV2 * delete aderyn.toml * chore: add custom verifier * chnage return output * feat: use self structs and a Generic output struct * feat: add userIdentifier, nullifier, forbiddencountries to returned output * add root view functions from registry * fix: build and compilation errors * add userDefined data into selfVerificationRoot * "resolve conflicts" * fix compilation problem * fix how to register verification config * test: CustomVerifier * fix verification root and hub integration * add scope check in hub impl * replace poseidon hash to ripemd+sha256 * add todo list * feat: refactor and add test cases for generic formatter * add performUserIdentifierCheck in basicVerification * change how to handle additionalData and fix stack too deep * start adding test codes * fix dependency problems in monorepo * fix: forbidden countries (#612) LGTM! * able to run test code * pass happy path * delete unused codes * change error code name, add caller address validation and add scripts to run test and build in monorepo * add all test cases in vcAndDisclose flow * remove comment out * chore: use actual user identifier outputs * success in registration tests * cover all cases * pass contractVersion instead of circuitVersion * fix disclose test * chore: add natspecs for ImplHubV2, CustomVerifier and GenericFormatter * change val name and remove unused lines * add val name change * remove userIdentifier from return data * feat: use GenericDiscloseOutput struct in verfication hook fix test cases for user identifier * chore: change the function order for Hub Impl V2 (#625) * fix nat specs * add nat spec in SelfStructs --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Nesopie <[email protected]> * prettier (#629) * CAN auth - android (#613) * add missed files * add NFCMethodSelectionScreen * bump android build --------- Co-authored-by: Justin Hernandez <[email protected]> * feat: add MRZ correction method to NFCMethodSelectionScreen (#627) * add npm auth token env (#632) * bump sdk version (#633) * publish npm package when merging on dev * bump common sdk version * replace yarn publish by npm publish * update common package version * Simplify dev mode gesture (#635) * Simplify developer mode gesture * Enable dev mode on MockData screen with five taps * add build smt function to common sdk * update vc_and_disclose_id test (dev branch) (#641) * fix: vc_and_disclose_id test * chore: yarn prettier * Show modal on NFC scan error (#642) * Add help button and error modal actions * fix the screen management * yarn nice * Bump build v2.5.4: ios 132; android 71 (#631) * bump version and build numbers * remove tamagui/toast * fix marketing version * fix: update TD1 and TD3 checks (#643) * bum yarn.lock * Bump build: ios 133; android 72 and build fixes (#654) * update gesture version and bump android build * bump and fix ios build * update lock files * fixes * fix fotoapparat library source * Update example contracts to include EUID usage (#656) * refactor: update HappyBirthday contract to V2 with support for E-Passport and EUID cards, introduce bonus multipliers, and enhance verification logic * refactor: update Airdrop contract to V2 with support for E-Passport and EU ID Card attestations * refactor: remove BASIS_POINTS constant from Airdrop contract * feat: introduce SelfIdentityERC721 contract for issuing NFTs based on verified identity credentials, replacing SelfPassportERC721 * fix: update verification functions in Airdrop, HappyBirthday, and SelfIdentityERC721 contracts to use customVerificationHook * cherry pick commit from add-test-self-verification... * block non-dev pr to main branch * audit fixes (#645) * merge dev branch into main (#624) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> * update contracts (#628) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) * implement self uups upgradeable (#592) * implement self uups upgradeable * small changes in identityVerificationHubImplV2 * delete aderyn.toml * chore: add custom verifier * chnage return output * feat: use self structs and a Generic output struct * feat: add userIdentifier, nullifier, forbiddencountries to returned output * add root view functions from registry * fix: build and compilation errors * add userDefined data into selfVerificationRoot * "resolve conflicts" * fix compilation problem * fix how to register verification config * test: CustomVerifier * fix verification root and hub integration * add scope check in hub impl * replace poseidon hash to ripemd+sha256 * add todo list * feat: refactor and add test cases for generic formatter * add performUserIdentifierCheck in basicVerification * change how to handle additionalData and fix stack too deep * start adding test codes * fix dependency problems in monorepo * fix: forbidden countries (#612) LGTM! * able to run test code * pass happy path * delete unused codes * change error code name, add caller address validation and add scripts to run test and build in monorepo * add all test cases in vcAndDisclose flow * remove comment out * chore: use actual user identifier outputs * success in registration tests * cover all cases * pass contractVersion instead of circuitVersion * fix disclose test * chore: add natspecs for ImplHubV2, CustomVerifier and GenericFormatter * change val name and remove unused lines * add val name change * remove userIdentifier from return data * feat: use GenericDiscloseOutput struct in verfication hook fix test cases for user identifier * chore: change the function order for Hub Impl V2 (#625) * fix nat specs * add nat spec in SelfStructs --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Nesopie <[email protected]> * prettier (#629) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> Co-authored-by: Nesopie <[email protected]> * fix: vc_and_disclose_id test (#640) * fix: vc_and_disclose_id test * chore: yarn prettier * fix: check if a config id exists * chore: change the function where the config not set verification is happening * fix: add await * feat: add getConfigId function in SelfVerificationRoot (#650) * feat: add getConfigId function in SelfVerificationRoot * update comment --------- Co-authored-by: motemotech <[email protected]> * chore: fix ofac end index in eu id cards * chore: fix tests * fix: example contracts and tests --------- Co-authored-by: turnoffthiscomputer <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> * Update deployment module for Identity Verification Hub V2 with detailed documentation and library linkage for CustomVerifier. Update initialization process to reflect changes in V2 implementation, ensuring proper setup for proxy deployment. (#658) * publish npm-package (#651) * App/eu id updates (#638) * fix build issues * generate disclosure proof with euids * generate disclosure proof with euids * Eu id updates 2 (#648) * update vc_and_disclose_id test (dev branch) (#641) * fix: vc_and_disclose_id test * chore: yarn prettier * Show modal on NFC scan error (#642) * Add help button and error modal actions * fix the screen management * yarn nice * Bump build v2.5.4: ios 132; android 71 (#631) * bump version and build numbers * remove tamagui/toast * fix marketing version * fix: update TD1 and TD3 checks (#643) * bum yarn.lock * add version and user defined data --------- Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> * remove the mock user define data * get the useridentifier as a hash from the user defined data * chore: add version and userDefinedData * feat: use the version in register / dsc proofs as well * update calculateUserIdentifierHash * yarn nice * refactor: consolidate user context data handling and update payload structure * fix typing issues on sha1 * remove console.log(sha1) * fix sha1 import * refactor: streamline userDefinedData handling and adjust payload type for circuit * refactor: update sha1 usage and enhance logging in calculateUserIdentifierHash * yarn nice * yarn lint common * use ts-ignore for sha1 import * fix app ci tests * fix typing issue * remove unused ts-ignore * cast uuid before calling generateinputs * bump qrcode version * add tsup on the qrcode sdk * fix: exports on selfxyz/qrcode * update how we define config.version * fix yarn imports * yarn format --------- Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> Co-authored-by: Ayman <[email protected]> * Hotfix contract compile error (#660) * Fix previous rebase error * Refactor deployment module for Identity Verification Hub V2. * Fix/sdk (#652) * fix: sdk build configs * chore: SelfBackendVerifier (WIP) * feat: add custom verification * feat: consider destination chain in user defined data * chore: export attestation id * chore: export attestation id * chore: export config storage * chore: don't throw an error if the proof is not valid * chore: trim abi and rm typechain types * refactor * chore: rm unnecessary exports * 📝 Add docstrings to `fix/sdk` (#653) Docstrings generation was requested by @remicolin. * https://github.com/selfxyz/self/pull/652#issuecomment-2992046545 The following files were modified: * `sdk/core/src/utils/hash.ts` * `sdk/core/src/utils/proof.ts` * `sdk/core/src/utils/utils.ts` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * review fixes * chore: fix package.json cjs types * chore: add minor changes to checks * feat: add InMemoryConfigStore, allIds constant and verificationResult type * chore: export Verification config * feat: change the verification config types * fix: throw issues early if verification config is null * fix: update yarn.lock file * chore: lint * fix: rm ts expect error directive * fix: contract tests * use excluded countries instead forbidden countries list * chore: change types in constnats --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update npm-publish workflow and bump core package version to 1.0.0 (#661) * update import * Update get verification config visibility (#664) * Update deployment module for Identity Verification Hub V2 to correct file paths and module name for deployment commands. * Add troubleshooting documentation for verification issues in deployHubV2.ts. Include manual verification steps and common failure reasons to assist users during deployment. * Change visibility of getVerificationConfigV2 function from internal to public in IdentityVerificationHubImplV2 contract to allow external access. * Apply BUSL v1.1 license headers to app (#665) * Add BSL license headers to app sources * prettier * fix license reference - https://spdx.org/licenses/BUSL-1.1.html * bump build: android 73 (#659) * Contracts/deploy staging (#668) * update scripts * deploy vc and disclose id * fix the deployment scripts on staging * update yarn.lock * bump ios build and version (#669) * configure coderabbitai (#670) * tweak coderabbit * bump * more thorough test spec * Apply BSL to app codebase (#639) * Clean up root license wording * Simplify SPDX header * simplify license and rename BSL to BUSL * fix merge issues * fix missing method --------- Co-authored-by: Justin Hernandez <[email protected]> * SEL-423 apply xcode build suggestions (#671) * apply recommended app settings from xcode * stick to portrait orientation and update target settings * remove app clip references * Circuit audit fixes (#644) * feat: add range checks before use of LessEqThan and SelectSubArray * fix: Num2Bits_strict to constrain virtualKey * bump core version * bump core version and fix ci * chore: use npm_auth_token in yarnrc * chroe: rm yarnrc changes * chore: update npm publish * chore: run npm publish manually * chore: change hub contract address (#675) * Update npm-publish.yml * chore: use proper secret when publishing * feat: enable publishing if workflow was triggered manually * Contracts/update verifier (#673) * update hardhat config * update vc and disclose verifier * update vc and disclose verifier script and run it * update test self verification root * update verifier * bump sdk version and use new hub address * chore: update zk-kit binary merkle root dep (#674) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> Co-authored-by: Nesopie <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Kevin Lin <[email protected]> Co-authored-by: kevinsslin <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Eric Nakagawa <[email protected]> * refactor deployment scripts (#678) * feat: add register eu id instances (#682) * feat: add register eu id instances * feat: add new instances * chore: update scripts * chore: fix sig alg * chore: rm circuits * update the smart contracts scripts (#684) * remove the && false * fix euid (#685) * keep build and version in sync (#686) * fix env set to null * fix: circuit for register ci (#690) * fix: circuit for register ci * fix: rm duplicate workflow_dispatch * feat: add better error handling (#691) * fix: older than bug (#692) * bump: sdk/[email protected] * fix: config not found bug * decrease parallel circuits to 3 * ci: add prettier check for contract sdk (#602) * Add Prettier check for code formatting in contracts workflow * Update contracts workflow: remove unused checkout action and fix build step name * Run formatter * Run lint fix * chore: update build_cpp to 2 concurrent builds * Contract/fix sdk (#695) * fix contracts sdk * fix contracts sdk * Fix contract example v2 (#694) * feat: add verification config ID functionality to Airdrop, HappyBirthday, and SelfIdentityERC721 contracts * Run formatter * SEL-473: Add lint rule for BUSL headers (#698) * chore(app): enforce license header via eslint * update lock and order * fix formatting * SEL-444: Fix android cloud backup (#697) * feat(android): migrate google backup * update lock and google services config * add bulk format command * backup fixes * working drive settings!!!!!!!! * remove unneeded intent filter * add tests * coderabbit feedback * coderabbit feedback * abstract google method * coderabbit feedback and fix test * more coderabbit suggestions and tests fixes * chore: update relayer verifier enum to include the register circuits (#699) * fix env sample (#700) * Abstract iOS cloud backup logic (#701) * feat(ios): abstract cloud backup logic * prettier and cr feedback * tested on iOS and android and functionality is the same * Fix navigation serialization warnings (#702) * test: cover modal callbacks * coderabbit feedback * feat(app): clarify passport linking (#704) * Show NFC support message (#708) * SEL-425: Add document management analytics events (#706) * Add document management analytics * coderabbit feedback * SEL-447: Improve proof failure feedback (#707) * feat: flag stale proofs as failed * make a constant * format * SEL-330: Add backup check after verification (#711) * route to save phrase if backup disabled * format * SEL-483: Implement recovery backup prompts (#710) * feat: prompt users to back up account * feat: prompt users to back up account * format * Add tests for recovery prompt logic * more lint updates * fix imports * fix unused import * update cursor suggestions * implement coderabbit suggestions and fix tests * SEL-472: Enable production push notifications (#703) * chore: leave sandbox apns token comment * tweak entitlement * coderabbit ai feedback * firebase tweaks * Chore: ensure there is an extra empty line after the license declaration (#712) * ensure there is an extra empty line after the license declaration * ignore adding header to cjs config files * add missing license header * ignore linting metro config * bump version and add mainnet hub address * Bugfix: Show recovery prompt only when user has docs (#714) * feat(app): prompt recovery only when docs exist * cr feedbacl * SEL-487: Prompt user to backup recovery phrase before registering (#715) * feat: prompt backup before registration * coderabbit feedback * fix tests * coderabbitai feedback and fix tests * Remove StartupFlushPolicy (#717) * SEL-479: Multi-ID onboarding mvp flow (#688) * save new launch screen wip * save wip * finalize launch look * replace launch screen * rename * update camera onboarding and scan screen * update tips looks * update nfc scan issue screens * update copy * add launch screen todo * fix casing * update launch screen link, copy and add tracking event * bump project version to match app store * match app store * updated supported bio id link * add dialog message support back in * cr feedback * bump version and build * update images * tweak animation layout * loop with setTimeout * fix onboarding assets (#719) * feat: add flag to use PACEPolling (#680) * feat: add flag to use PACEPolling * fix: santize before storing in store * bump ios build number and update podfile lock * prettier * bump build * feat: add flag to use PACEPolling * fix: santize before storing in store * bump ios build number and update podfile lock * prettier * bump build --------- Co-authored-by: Justin Hernandez <[email protected]> * fix backup button label (#722) * update version to 2.6.0 and bump build numbers (#721) * SEL-179 & SEL-312: Add gitleaks and GitGuardian scanning (#705) * chore: add secret scanning setup * fix: correct GitGuardian action path * cr feedbacak * test husky commit * pr feedback * fix workflows * tweaks * fix versions * upgrade: migrate from husky v8 to v9 - Update husky from ^8.0.0 to ^9.1.7 - Change prepare script from 'husky install' to 'husky' - Remove v8 hook structure (shebang, husky.sh sourcing) - Delete .husky/_/ directory as it's not needed in v9 - Maintain gitleaks pre-commit hook functionality * coderabbitai feedback * add bulk sort command (#723) * feat(app): redirect empty docs to launch (#725) * Apply consistent safe area padding across screens (#726) * Contracts/update verifiers (#729) * update the verifiers * update deployment script * update deployment script and deploy to prod * prettier run write * App/ethcc fixes (#730) * fix mock data screen * increase timout between dsc and register proof * fix the isUserRegisteredWithAlternativeCSCA function * yarn nice * allow people to switch to a mock id (#732) * yarn nice * chore: update default config id method * chore: use named exports * Update README.md * Temporarily disable recovery redirect and reminder prompts (#733) * Revert "SEL-487: Prompt user to backup recovery phrase before registering (#715)" This reverts commit fe14ac655e11b4b9e0c4023002b84fcc79bedd31. * revert update * fix safe area context pkg * Revert "SEL-487: Prompt user to backup recovery phrase before registering (#715)" This reverts commit fe14ac655e11b4b9e0c4023002b84fcc79bedd31. * fix old flow * more silent tests * update lock files * hard code return * SEL-486: Fix unwrap DO (#718) * update podfile: unwrapDO * update lock * bump version and builds * bump build; forgot to enable logs * fix version to not interfere with release --------- Co-authored-by: Justin Hernandez <[email protected]> * SEL-494: Update proving machine event tracking (#734) * Add extensive proof analytics instrumentation * prettier and sort events by key name * remove loading screen race condition redirect (#736) * Chore: new build for v2.6.0 ios 145 android 81 (#737) * bump version and build * properly bump app * bump build * Improve manual mobile deploy workflow and docs (#728) * Add basic Fastlane helper tests * Upgrade fastlane and enhance helper tests (#738) * simplify mobile deploy pipelines and make them manual. update readme * update fastlane dev readme * update tests and add helper script * cr feedback, update tests, revert circuits package.json sort change * tweaks * fix slack * cr feedback and fixes * add better cjs eslint support * save wip. add confirmation check script. update scripts * remove auto increment feature * migrate readme items over to DEV due to fastlane auto regen docs flow * use regular xcode * fix hermes compiler path * coderabbit feedback * reinstall when on local dev * fix upload * simplify * simplify confirmation feedback with tests * fix mobile deploys * cr feedback * test iOS building * fix trigger logic * cr feedback * updates * fix env var * fix order * re-enable upload to testflight for ios * updated notes * chore: update readme * Bugfix: android deeplinks (#742) * bugfix: deep linking * add android manifest test * bump build and version * format readme * fix deeplink genmockiddoc * add the gender to the deeplink optoin * bump version (#743) * fix the female bug * bump build 148 (#744) * SEL-496: Add Firebase Remote Config and dev feature flag screen (#735) * feat: add remote config support * update lock * tweak config logic. add feature flag viewing screen * add tests * allow for local overriding of feature flags * save local override work * save wip * clean up ui * update screen to handle multi value types * fix tests * cr feedback and fix tests * remote config upates. fix tests, codex feedback * Improve AGENTS workflow notes (#747) * clarify workflow instructions * agents feedback * Address minor mobile deployment bugs (#745) * feat: improve deployment tooling * cr feedback * for temp testing * clean build artifacts after deploy * add deploy source * uncomment ios commands * Add tests for minor deployment fixes (#750) * Add test coverage for deployment scripts and Fastfile * format * increase github check to 5 minutes * Extend platform build file tests (#748) * Add build file tests * cr feedback * Add proving machine tests (#749) * Add actor mock helper and tests * format tests * fix tests * wip fix tests * address cr feedback * Add thorough test cases for mobile app (#752) * Add actor mock helper and tests * format tests * fix tests * Revert non-app tests * update tests * fix tests * coderabbit feedback * revert change * remove spurious tests * don't use crypto in core sdk * Start of Web App (#689) * Add .cursorignore to optimize AI editor performance and security (#758) Prevents Cursor AI from accessing sensitive files (keys, credentials, deployment configs) and large generated artifacts that slow down indexing. Keeps source code accessible while excluding build outputs, node_modules, and circuit/contract compilation artifacts across the monorepo. * SEL-504: fix fonts and some styles (#762) * fix fonts and some styles * dry config * fix some warnings * lets start with coverage for app (#763) * lets start with coverage for app * lint * better setup * SEL-559: Update td1 regex (#760) * feat: update td1 regex * update review comments * fix: NPE on expirationDate regex * fix user defined data (#766) * fix: name formatting for middle name * bump: sdk/core to 1.0.7-beta.1 * Feat/retrieve OFAC trees from api (#769) * retrieve the ofac trees from the api * remove the ofac trees from the common repo * fix ofac test * yarn nice * yarn nice * yarn nice * refactor ofac fetching * Release new build v2.6.2 (#779) * bump version and build * ignore podfile * Remove failing version test (#780) * remove version check test * remove test all together * SEL-269: Update ESLint rules & lock prettier config (#781) * Update ESLint config and lock prettier config * Refine ESLint config and fix lint issues * Apply eslint fixes * Use socketIo alias (#782) * move gesture handler * save wip updates * fix svg imports * update tsconfig * eslint updates * eslint fixes * improve ignore folders * coderabbit feedback * Fix style prop shorthands (#787) * Expand view style props * Expand remaining style props * update types * fix pipeline * fix test env check * nicer casting * fix booleans * update deeplink url handling and make it more robust * add socket error handler * Add COSE signature verification tests (#788) * Update ESLint config and lock prettier config * Refine ESLint config and fix lint issues * save wip updates * eslint updates * eslint fixes * Add COSE signature verification tests * fix tests * SEL-553: Show NFC Progress (#764) * feat: add haptics * fix: BAC FAILED error event * update lock file --------- Co-authored-by: Justin Hernandez <[email protected]> * SEL-566: Navigate Home based on document validity (#768) * feat: navigate home if atleast one valid document is present * update comments * Review: Remove unnecessary continue statement * feat: add tracking * SEL-544: Generate Mock DSC on mock-passport flow (#772) * feat: Generate mock DSC on mock-passport flow * Remove console log * yarn format * revert to mock dsc generation * SEL-570: Display user ID in prove screen (#790) * Display user ID on prove screen * Add user ID formatting util and tests * Clarify user ID formatting * fix nice * add tests and save toggle wip * update tests based on feedback * say connected wallet when wallet * fix: Add localhost validation to prevent invalid endpoint usage in QR Code SDK (#794) * Feat/mobile deployment automation (#759) * feat: add version management system with build number tracking - Add version.json to track iOS/Android build numbers separately - Create version.cjs script for build number management - Add Fastlane version_manager.rb helper - Keep npm version for semver, version.json for build tracking * feat: integrate version.json with Fastlane deployment process ## What Changed - Updated iOS and Android Fastlane lanes to use version.json for build number management - Added automatic build number increment on deployment - Added deployment timestamp tracking ## How It Works ### iOS Deployment 1. Reads current build number from version.json 2. Increments iOS build number (e.g., 148 → 149) 3. Updates Xcode project with new build number via increment_build_number 4. Proceeds with TestFlight deployment 5. Updates lastDeployed timestamp on successful upload ### Android Deployment 1. Reads current build number from version.json 2. Increments Android build number (e.g., 82 → 83) 3. Updates build.gradle with new version code via increment_version_code 4. Proceeds with Play Store deployment 5. Updates lastDeployed timestamp on successful upload ## Why This Change - Eliminates manual version/build number entry - Prevents version conflicts between deployments - Provides single source of truth for build numbers - Enables automatic deployments without human intervention - Tracks deployment history with timestamps ## Dependencies - Requires version.json file (already created in previous commit) - Uses existing Fastlane plugins: - increment_build_number (iOS - built-in) - increment_version_code (Android - from plugin) - Version numbers still managed by npm version command * feat: enhance deploy confirmation with version.json info * fix: use ENV variable directly in increment_build_number to avoid secret masking * fix: correct xcodeproj path for GitHub Actions workflow * feat: add test mode to workflow for safe testing - Skip store uploads when test_mode is true - Test version bumps and builds without deployment - Prevent accidental pushes to TestFlight/Play Store * fix: use gradle_file_path instead of gradle_file for increment_version_code * fix: use gsub to remove ../ prefix for CI compatibility * chore: remove accidentally committed files - Remove .cursor/mcp.json - Remove .cursorignore - Remove deployment-automation-summary.md - Remove deployment-meeting-questions.md - Remove pipeline.md * feat: auto-commit version.json after successful deployment - Commits version.json changes back to repository - Only runs when test_mode is false - Uses [skip ci] to prevent infinite loops - Checks for actual changes before committing * feat : update package.json in build step using npm version * feat: add comprehensive caching to mobile deployment workflow - Add caching for Yarn dependencies, Ruby gems, CocoaPods, Gradle, and Android NDK - Implement cache versioning strategy for easy cache invalidation - Fix cache order: caches now restored after checkout but before dependency installation - Update mobile-setup action to skip installs when dependencies are cached - Add cache size monitoring to track usage against GitHub's 10GB limit - Fix Slack notification bug: skip notifications in test_mode - Add detailed logging for package.json version updates (show from/to versions) Expected performance improvement: ~50% faster builds (from ~15min to ~7-10min) * fix: move bundler config after Ruby setup in mobile-setup action * fix: rename cache env vars to avoid Yarn conflicts Yarn was interpreting YARN_CACHE_VERSION as its own config setting. Prefixed all cache version env vars with GH_ to avoid conflicts. * fix: remove bundler deployment mode to allow Gemfile updates The deployment mode was causing bundler to fail when Gemfile changed (nokogiri was removed). CI should be able to update the lockfile as needed. * feat: implement strict lock file enforcement (Option 1) - Re-enable bundler deployment mode for strict Gemfile.lock checking - Use yarn install --immutable for strict yarn.lock checking - Add clear error messages when lock files are out of date - Add pre-checks to verify lock files exist - This ensures reproducible builds and makes caching maximally effective When developers change dependencies, they must now: 1. Run yarn install or bundle install locally 2. Commit the updated lock files 3. CI will fail with helpful instructions if they forget * fix: update Gemfile.lock for CI environment Remove nokogiri from Gemfile.lock since it's excluded in CI environments (GITHUB_ACTIONS=true). This allows the strict lock file checks to pass in CI. * fix: correct yarn.lock path for monorepo workspace The project uses Yarn workspaces with yarn.lock at the repository root, not in the app directory. Updated paths to check for yarn.lock at workspace root and use it for cache keys. * fix: handle both boolean and string test_mode parameter The test_mode parameter was only checking for string 'true' but could be passed as boolean true from command line. Now handles both cases to ensure test mode works correctly for iOS and Android. * fix: address code review feedback for mobile deployment workflow - Replace jq with Node.js for version extraction (jq not available on macOS runners) - Fix concurrent commit race condition by creating separate update-version job - Add platform validation to version_manager.rb and version.cjs scripts - Use POSIX-compatible single = for shell string comparisons - Ensure single atomic commit when deploying to both platforms * fix: formatting and linting issues - Remove trailing spaces from workflow YAML file - Fix prettier formatting in JavaScript files - Add -y flag to yarn version command for non-interactive mode - Address all lint warnings from CI --------- Co-authored-by: Jayaditya Gupta <[email protected]> * fix: increment iOS build number * fix: bump app version to 2.6.3 for iOS release * App/deeplink callback (#789) * add deepllinkCallback support * bump package version * yarn nice * fix background countdown * cast the URL to prevent malicious code introduction * fix: use cleanDocumentNumber (#784) * increment iOS bundle version * Feat/push to dev main (#767) * feat: add version management system with build number tracking - Add version.json to track iOS/Android build numbers separately - Create version.cjs script for build number management - Add Fastlane version_manager.rb helper - Keep npm version for semver, version.json for build tracking * feat: integrate version.json with Fastlane deployment process ## What Changed - Updated iOS and Android Fastlane lanes to use version.json for build number management - Added automatic build number increment on deployment - Added deployment timestamp tracking ## How It Works ### iOS Deployment 1. Reads current build number from version.json 2. Increments iOS build number (e.g., 148 → 149) 3. Updates Xcode project with new build number via increment_build_number 4. Proceeds with TestFlight deployment 5. Updates lastDeployed timestamp on successful upload ### Android Deployment 1. Reads current build number from version.json 2. Increments Android build number (e.g., 82 → 83) 3. Updates build.gradle with new version code via increment_version_code 4. Proceeds with Play Store deployment 5. Updates lastDeployed timestamp on successful upload ## Why This Change - Eliminates manual version/build number entry - Prevents version conflicts between deployments - Provides single source of truth for build numbers - Enables automatic deployments without human intervention - Tracks deployment history with timestamps ## Dependencies - Requires version.json file (already created in previous commit) - Uses existing Fastlane plugins: - increment_build_number (iOS - built-in) - increment_version_code (Android - from plugin) - Version numbers still managed by npm version command * feat: enhance deploy confirmation with version.json info * fix: use ENV variable directly in increment_build_number to avoid secret masking * fix: correct xcodeproj path for GitHub Actions workflow * feat: add test mode to workflow for safe testing - Skip store uploads when test_mode is true - Test version bumps and builds without deployment - Prevent accidental pushes to TestFlight/Play Store * fix: use gradle_file_path instead of gradle_file for increment_version_code * fix: use gsub to remove ../ prefix for CI compatibility * chore: remove accidentally committed files - Remove .cursor/mcp.json - Remove .cursorignore - Remove deployment-automation-summary.md - Remove deployment-meeting-questions.md - Remove pipeline.md * feat: auto-commit version.json after successful deployment - Commits version.json changes back to repository - Only runs when test_mode is false - Uses [skip ci] to prevent infinite loops - Checks for actual changes before committing * feat : update package.json in build step using npm version * feat: add comprehensive caching to mobile deployment workflow - Add caching for Yarn dependencies, Ruby gems, CocoaPods, Gradle, and Android NDK - Implement cache versioning strategy for easy cache invalidation - Fix cache order: caches now restored after checkout but before dependency installation - Update mobile-setup action to skip installs when dependencies are cached - Add cache size monitoring to track usage against GitHub's 10GB limit - Fix Slack notification bug: skip notifications in test_mode - Add detailed logging for package.json version updates (show from/to versions) Expected performance improvement: ~50% faster builds (from ~15min to ~7-10min) * fix: move bundler config after Ruby setup in mobile-setup action * fix: rename cache env vars to avoid Yarn conflicts Yarn was interpreting YARN_CACHE_VERSION as its own config setting. Prefixed all cache version env vars with GH_ to avoid conflicts. * fix: remove bundler deployment mode to allow Gemfile updates The deployment mode was causing bundler to fail when Gemfile changed (nokogiri was removed). CI should be able to update the lockfile as needed. * feat: implement strict lock file enforcement (Option 1) - Re-enable bundler deployment mode for strict Gemfile.lock checking - Use yarn install --immutable for strict yarn.lock checking - Add clear error messages when lock files are out of date - Add pre-checks to verify lock files exist - This ensures reproducible builds and makes caching maximally effective When developers change dependencies, they must now: 1. Run yarn install or bundle install locally 2. Commit the updated lock files 3. CI will fail with helpful instructions if they forget * fix: update Gemfile.lock for CI environment Remove nokogiri from Gemfile.lock since it's excluded in CI environments (GITHUB_ACTIONS=true). This allows the strict lock file checks to pass in CI. * fix: correct yarn.lock path for monorepo workspace The project uses Yarn workspaces with yarn.lock at the repository root, not in the app directory. Updated paths to check for yarn.lock at workspace root and use it for cache keys. * fix: handle both boolean and string test_mode parameter The test_mode parameter was only checking for string 'true' but could be passed as boolean true from command line. Now handles both cases to ensure test mode works correctly for iOS and Android. * fix: address code review feedback for mobile deployment workflow - Replace jq with Node.js for version extraction (jq not available on macOS runners) - Fix concurrent commit race condition by creating separate update-version job - Add platform validation to version_manager.rb and version.cjs scripts - Use POSIX-compatible single = for shell string comparisons - Ensure single atomic commit when deploying to both platforms * fix: formatting and linting issues - Remove trailing spaces from workflow YAML file - Fix prettier formatting in JavaScript files - Add -y flag to yarn version command for non-interactive mode - Address all lint warnings from CI * feat: implement automated branch-based mobile deployments - Add mobile-deploy-auto.yml workflow that triggers on PR merges to dev/main - Update mobile-deploy.yml to support workflow_call for reusability - Add deployment_track, version_bump, and auto_deploy parameters - Create new Fastlane lanes (deploy_auto) for iOS and Android - Implement smart version bumping based on PR labels (major/minor/patch) - Add graceful error handling for Play Store permission issues - Enhance Slack notifications with deployment track information This enables automatic deployments when PRs are merged: - dev branch → internal testing track - main branch → production track - Skip deployment with [skip-deploy] in PR or no-deploy label * feat: add automated git tagging and release system - Add automatic git tagging for production deployments (v2.5.5, platform-specific tags) - Create GitHub releases with changelogs for production deployments - Add manual release script (yarn release) for version bumping and tagging - Implement simple changelog generation from git history - Add comprehensive deployment documentation in .github/MOBILE_DEPLOYMENT.md - Update app/README.md with deployment commands and workflows This completes the release automation system requested in the ticket for manual tagging and versioning with automated changelogs and release notes. --------- Co-authored-by: Jayaditya Gupta <[email protected]> * Implement basic code splitting * cm feedback * update lock * yarn nice * add typing to crypto loader * fix type. more opportunities * lint suggestions * build dependencies before linting * fix build command * save updated imports * update build checks * fix import * fix imports and test * fix install commands * Update Gemfile.lock to exclude nokogiri in CI environments - Regenerated Gemfile.lock with GITHUB_ACTIONS=true to match the conditional nokogiri exclusion in the Ge…
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores
Tests