Merge Feat/update contract scripts #1211
Conversation
Tranquil-Flow
had a problem deploying
to
development
GitHub Actions
Failure
Tranquil-Flow
had a problem deploying
to
development
GitHub Actions
Failure
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on October 17
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| uint256 endOfDay = startOfDay + 1 days - 1; | ||
|
|
||
| if (currentTimestamp < startOfDay - 1 days + 1 || currentTimestamp > startOfDay + 1 days - 1) { | ||
| if (currentTimestamp < startOfDay - 1 days + 1 || currentTimestamp > endOfDay + 1 days) { |
There was a problem hiding this comment.
Bug: Date Validation Error Extends Future Range
The date validation in _performCurrentDateCheck and _performNumericCurrentDateCheck incorrectly extends the valid range. Changing the upper bound from startOfDay + 1 days - 1 to endOfDay + 1 days (where endOfDay is startOfDay + 1 days - 1) effectively allows proofs with dates one day further in the future than intended.
Additional Locations (1)
| /// @return The expiry timestamp of the commitment. | ||
| function getUidaiPubkeyExpiryTimestamp(uint256 commitment) external view virtual onlyProxy returns (uint256) { | ||
| return _uidaiPubkeyExpiryTimestamps[commitment]; | ||
| return _uidaiPubkeyCommitments[commitment]; |
There was a problem hiding this comment.
Bug: UIDAI Key Commitments Removed Expiry
This change removes time-based expiry for UIDAI pubkey commitments. The storage and all related functions now treat commitments as permanently valid once registered, rather than checking an expiry timestamp. This could be a security regression if automatic key expiration was a design requirement. The interface documentation for registerUidaiPubkeyCommitment is also outdated.
Additional Locations (3)
| const registryArtifact = artifacts.readArtifactSync("IdentityRegistryImplV1"); | ||
| const registryInterface = new ethers.Interface(registryArtifact.abi); | ||
| return registryInterface; | ||
| } |
There was a problem hiding this comment.
Bug: Incorrect Artifact for Aadhaar Registry Initialization
The getRegistryInitializeData() function reads the IdentityRegistryImplV1 artifact, but for this Aadhaar registry deployment, it should use IdentityRegistryAadhaarImplV1. This artifact mismatch means the ABI used for encoding initialization data is incorrect, which will likely cause the registry initialization to fail.
| } | ||
|
|
||
| export default buildModule("DeployNewHubAndUpgrade", (m) => { | ||
| export default buildModule("DeployNewHubAndUpgradee", (m) => { |
There was a problem hiding this comment.
Bug: Module Naming Error Causes Deployment Issues
The DeployNewHubAndUpgradee module name has a typo, with an extra 'e' at the end. This could cause confusion or issues for deployment scripts that reference the module.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUpdates span circuits config and tests, Aadhaar registry interface/implementation, hub V2 logic (configurable Aadhaar window, timestamp error change), deployment/ignition modules (new deploy/update flows), network configs, ignore rules, deployment addresses, and test adaptations to new signatures and flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant DApp
participant Hub as IdentityVerificationHubImplV2 (Proxy)
participant AadhaarReg as IdentityRegistryAadhaarImplV1 (Proxy)
rect rgb(235, 245, 255)
note over User,Hub: Configuration
User->>DApp: Set Aadhaar window (e.g., 20)
DApp->>Hub: setAadhaarRegistrationWindow(window)
Hub-->>DApp: OK
end
rect rgb(245, 235, 255)
note over DApp,AadhaarReg: Pubkey registration (no expiry)
DApp->>AadhaarReg: registerUidaiPubkeyCommitment(commitment)
AadhaarReg-->>DApp: Event: UidaiPubkeyCommitmentRegistered
end
rect rgb(235, 255, 245)
note over User,Hub: Aadhaar verification
User->>DApp: Submit proof
DApp->>Hub: registerIdentityWithAadhaar(proof, signals)
Hub->>Hub: Validate timestamps within AADHAAR_REGISTRATION_WINDOW
alt invalid timestamp
Hub-->>DApp: revert InvalidUidaiTimestamp(blockTs, proofTs)
else valid
Hub->>AadhaarReg: checkUidaiPubkey(commitment)
AadhaarReg-->>Hub: true/false
Hub-->>DApp: Success / revert accordingly
end
end
sequenceDiagram
autonumber
participant HRE as Hardhat Runtime
participant FS as FS
participant Mod as Ignition Module
participant Hub as IdentityVerificationHubImplV2
participant Reg as Aadhaar Registry
note over Mod: UpdateVerifiers / UpdateHubRegistries
HRE->>FS: Read chain-<id>/deployed_addresses.json
FS-->>HRE: deployedAddresses
Mod->>Hub: attach(hubAddress)
Mod->>Hub: batchUpdateRegisterCircuitVerifiers(...)
Mod->>Hub: batchUpdateDscCircuitVerifiers(...)
Mod->>Hub: updateVcAndDiscloseCircuit(...)
Mod->>Hub: updateRegistry(attestationId, Reg.address)
Hub-->>Mod: tx receipts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
circuits/tests/register/pubkeys.ts (1)
1-1: Fix code style issues flagged by Prettier.The pipeline reports formatting issues in this file. Run Prettier with
--writeto resolve the code style violations before merging.#!/bin/bash # Run Prettier to check and show formatting issues npx prettier --check circuits/tests/register/pubkeys.tscircuits/tests/register/register_aadhaar.test.ts (1)
1-1: Fix code style issues flagged by Prettier.The pipeline reports formatting violations. Run Prettier with
--writeto resolve the code style issues before merging.#!/bin/bash # Run Prettier to check and fix formatting issues npx prettier --check circuits/tests/register/register_aadhaar.test.tscontracts/contracts/IdentityVerificationHubImplV2.sol (1)
47-334: Initialize Aadhaar window in proxy storageIn upgradeable contracts, the inline initializer
= 20only touches the implementation’s storage. The proxy’s slot stays at zero, so_verifyRegisterProofeffectively enforces a zero-minute window and every Aadhaar registration reverts unless the owner callssetAadhaarRegistrationWindowbeforehand. Please seed the proxy storage duringinitialize()(or migrate existing deployments) so the default window is non-zero.function initialize() external reinitializer(11) { __ImplRoot_init(); // Initialize circuit version to 2 for V2 hub IdentityVerificationHubStorage storage $ = _getIdentityVerificationHubStorage(); $._circuitVersion = 2; + + if (AADHAAR_REGISTRATION_WINDOW == 0) { + AADHAAR_REGISTRATION_WINDOW = 20; + } emit HubInitializedV2(); }
♻️ Duplicate comments (4)
contracts/ignition/modules/registry/deployAadhaarRegistry.ts (1)
40-43: Fix ABI source for registry initialization
getRegistryInitializeDatastill loads theIdentityRegistryImplV1artifact, so the encodedinitializecalldata comes from the wrong ABI. When the proxy runs that call againstIdentityRegistryAadhaarImplV1the selector/arguments won’t line up and the deployment reverts. Swap in the Aadhaar artifact so we encode against the actual implementation.-function getRegistryInitializeData() { - const registryArtifact = artifacts.readArtifactSync("IdentityRegistryImplV1"); +function getRegistryInitializeData() { + const registryArtifact = artifacts.readArtifactSync("IdentityRegistryAadhaarImplV1"); const registryInterface = new ethers.Interface(registryArtifact.abi); return registryInterface; }contracts/ignition/modules/upgrade/deployNewHubAndUpgrade.ts (1)
12-12: Restore the original Ignition module nameRenaming the module to
"DeployNewHubAndUpgradee"changes the Ignition module ID, which in turn mutates the keys written todeployed_addresses.json. Downstream scripts expectingDeployNewHubAndUpgrade#…will now miss the upgraded hub artifacts, breaking automated upgrade flows. Keep the original module name.-export default buildModule("DeployNewHubAndUpgradee", (m) => { +export default buildModule("DeployNewHubAndUpgrade", (m) => {contracts/contracts/IdentityVerificationHubImplV2.sol (1)
995-1000: Revert to the original date window boundsThe new condition
currentTimestamp > endOfDay + 1 daysexpands the acceptable range by an extra day and lets future-dated proofs pass. This is the same bug called out earlier; please constrain the upper bound back to the intended day range before merging.Also applies to: 1016-1019
contracts/contracts/registry/IdentityRegistryAadhaarImplV1.sol (1)
251-256: Update documentation to reflect removal of expiry semantics.The function documentation (line 251-253) still mentions checking "if it's not expired", but the implementation no longer performs any expiry validation. The security implications of removing time-based expiry are already flagged in previous review comments.
Apply this diff to update the documentation:
- /// @notice Checks if the provided UIDAI pubkey is stored in the registry and also if it's not expired. + /// @notice Checks if the provided UIDAI pubkey is registered in the registry. /// @param pubkey The UIDAI pubkey to verify. - /// @return True if the given pubkey is stored in the registry and also if it's not expired, otherwise false. + /// @return True if the given pubkey is registered in the registry, otherwise false. function checkUidaiPubkey(uint256 pubkey) external view virtual onlyProxy returns (bool) {
🧹 Nitpick comments (4)
contracts/hardhat.config.ts (1)
70-72: Remove or implement the commented API key configuration.The commented-out per-network API key block suggests incomplete configuration work. Either uncomment and implement this approach or remove the commented code to avoid confusion.
circuits/tests/register/register_aadhaar.test.ts (1)
193-202: Consider removing or documenting the skipped test.The skipped test appears to be a development/debugging utility for logging pubkey commitments. If this is a temporary test, consider removing it or adding a comment explaining its purpose and when it should be enabled.
contracts/ignition/modules/verifiers/deployAllVerifiersNew.ts (1)
190-195: Consider using structured logging over console.log.The console.log statements will appear in deployment output. For production deployments, consider using Ignition's built-in logging mechanisms or structured logging to reduce output clutter while maintaining deployment visibility.
contracts/test/utils/deploymentV2.ts (1)
284-286: Clarify or remove commented-out updateCscaRoot calls.Three
updateCscaRootcalls are commented out (lines 284-285). If these are no longer needed, remove them entirely. If they're temporarily disabled for testing, add a comment explaining why and when they should be re-enabled to prevent confusion.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
circuits/scripts/build/build_disclose_circuits.sh(1 hunks)circuits/tests/register/pubkeys.ts(2 hunks)circuits/tests/register/register_aadhaar.test.ts(2 hunks)contracts/.gitignore(1 hunks)contracts/contracts/IdentityVerificationHubImplV2.sol(8 hunks)contracts/contracts/interfaces/IIdentityRegistryAadhaarV1.sol(1 hunks)contracts/contracts/registry/IdentityRegistryAadhaarImplV1.sol(4 hunks)contracts/contracts/tests/testUpgradedIdentityVerificationHubImplV2.sol(1 hunks)contracts/error-selectors.json(15 hunks)contracts/hardhat.config.ts(2 hunks)contracts/ignition/deployments/chain-11142220/deployed_addresses.json(1 hunks)contracts/ignition/deployments/chain-42220/deployed_addresses.json(1 hunks)contracts/ignition/modules/hub/updateRegistries.ts(1 hunks)contracts/ignition/modules/hub/updateVerifiers.ts(1 hunks)contracts/ignition/modules/registry/deployAadhaarRegistry.ts(1 hunks)contracts/ignition/modules/registry/updateRegistries.ts(1 hunks)contracts/ignition/modules/upgrade/deployNewHubAndUpgrade.ts(1 hunks)contracts/ignition/modules/verifiers/deployAllVerifiersNew.ts(1 hunks)contracts/package.json(1 hunks)contracts/test/utils/deploymentV2.ts(2 hunks)contracts/test/v2/registerAadhaar.test.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
contracts/contracts/interfaces/IIdentityRegistryAadhaarV1.solcircuits/tests/register/pubkeys.tscontracts/test/v2/registerAadhaar.test.tscontracts/test/utils/deploymentV2.tscontracts/ignition/modules/hub/updateRegistries.tscontracts/contracts/tests/testUpgradedIdentityVerificationHubImplV2.solcontracts/ignition/modules/verifiers/deployAllVerifiersNew.tscontracts/hardhat.config.tscontracts/contracts/IdentityVerificationHubImplV2.solcircuits/tests/register/register_aadhaar.test.tscontracts/contracts/registry/IdentityRegistryAadhaarImplV1.solcontracts/ignition/modules/upgrade/deployNewHubAndUpgrade.tscontracts/ignition/modules/hub/updateVerifiers.tscontracts/ignition/modules/registry/updateRegistries.tscontracts/ignition/modules/registry/deployAadhaarRegistry.ts
contracts/**
📄 CodeRabbit inference engine (AGENTS.md)
Compile Solidity contracts using
yarn workspace @selfxyz/contracts build
Files:
contracts/contracts/interfaces/IIdentityRegistryAadhaarV1.solcontracts/package.jsoncontracts/ignition/deployments/chain-11142220/deployed_addresses.jsoncontracts/ignition/deployments/chain-42220/deployed_addresses.jsoncontracts/test/v2/registerAadhaar.test.tscontracts/test/utils/deploymentV2.tscontracts/ignition/modules/hub/updateRegistries.tscontracts/contracts/tests/testUpgradedIdentityVerificationHubImplV2.solcontracts/ignition/modules/verifiers/deployAllVerifiersNew.tscontracts/hardhat.config.tscontracts/contracts/IdentityVerificationHubImplV2.solcontracts/contracts/registry/IdentityRegistryAadhaarImplV1.solcontracts/ignition/modules/upgrade/deployNewHubAndUpgrade.tscontracts/ignition/modules/hub/updateVerifiers.tscontracts/ignition/modules/registry/updateRegistries.tscontracts/error-selectors.jsoncontracts/ignition/modules/registry/deployAadhaarRegistry.ts
contracts/**/*.sol
⚙️ CodeRabbit configuration file
contracts/**/*.sol: Review Solidity smart contracts for:
- Security vulnerabilities (reentrancy, overflow, etc.)
- Gas optimization opportunities
- Access control patterns
- Event emission for important state changes
- Code documentation and NatSpec comments
Files:
contracts/contracts/interfaces/IIdentityRegistryAadhaarV1.solcontracts/contracts/tests/testUpgradedIdentityVerificationHubImplV2.solcontracts/contracts/IdentityVerificationHubImplV2.solcontracts/contracts/registry/IdentityRegistryAadhaarImplV1.sol
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use Yarn only for package management (yarn install/add/remove); do not use npm or pnpm in scripts
Files:
contracts/package.json
**/*.{test,spec}.{ts,js,tsx,jsx}
⚙️ CodeRabbit configuration file
**/*.{test,spec}.{ts,js,tsx,jsx}: Review test files for:
- Test coverage completeness
- Test case quality and edge cases
- Mock usage appropriateness
- Test readability and maintainability
Files:
contracts/test/v2/registerAadhaar.test.tscircuits/tests/register/register_aadhaar.test.ts
contracts/contracts/IdentityVerificationHubImplV2.sol
📄 CodeRabbit inference engine (.cursorrules)
Identity Verification Hub should manage multi-step verification for passports and EU ID cards, handle document attestation with zero-knowledge proofs, and implement verification paths: E-PASSPORT and EU_ID_CARD.
Files:
contracts/contracts/IdentityVerificationHubImplV2.sol
🧠 Learnings (1)
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to contracts/contracts/registry/IdentityRegistryImplV1.sol : Identity Registry Management maintains separate registries for passports and ID cards, handles DSC key commitment registration, and tracks nullifiers for duplicate prevention.
Applied to files:
contracts/error-selectors.json
🧬 Code graph analysis (3)
circuits/tests/register/register_aadhaar.test.ts (2)
circuits/tests/register/pubkeys.ts (1)
pubkeys(1-191)common/src/utils/bytes.ts (1)
splitToWords(140-153)
contracts/ignition/modules/upgrade/deployNewHubAndUpgrade.ts (2)
app/scripts/find-type-import-issues.mjs (1)
__dirname(20-20)scripts/tests/check-license-headers.test.mjs (1)
__dirname(18-18)
contracts/ignition/modules/hub/updateVerifiers.ts (1)
contracts/ignition/modules/verifiers/deployAllVerifiersNew.ts (2)
CircuitName(4-89)circuitIds(92-180)
🪛 GitHub Actions: Circuits CI
circuits/tests/register/pubkeys.ts
[warning] 1-1: Code style issues found in file. Run Prettier with --write to fix.
circuits/tests/register/register_aadhaar.test.ts
[warning] 1-1: Code style issues found in file. Run Prettier with --write to fix.
🔇 Additional comments (9)
contracts/hardhat.config.ts (2)
62-66: LGTM: Celo Sepolia network configuration added correctly.The network configuration follows the established pattern with appropriate chainId (11142220) and fallback RPC URL.
90-97: No changes needed for Blockscout verification configuration
Hardhat’s etherscan plugin supports customChains for Blockscout (including Celo Sepolia) and does not require a separate API key—your existing CELOSCAN_API_KEY can be reused.contracts/ignition/modules/verifiers/deployAllVerifiersNew.ts (1)
182-205: LGTM! Sequential deployment pattern prevents nonce conflicts.The use of the
afterdependency to enforce sequential deployment is the correct approach to avoid nonce conflicts when deploying numerous verifier contracts. This ensures deterministic deployment order and prevents race conditions.contracts/test/utils/deploymentV2.ts (1)
286-286: LGTM! Function call updated to match new interface.The call to
registerUidaiPubkeyCommitmentnow correctly uses the single-argument signature, aligning with the interface change that removed theexpiryTimestampparameter.contracts/contracts/interfaces/IIdentityRegistryAadhaarV1.sol (1)
104-104: All call sites updated to the new single-parameter signature. No further changes needed.circuits/scripts/build/build_disclose_circuits.sh (1)
20-20: Verify updated artifacts and staging verifier JSON exist.
- Ensure
circuits/build/disclose/vc_and_disclose_aadhaar/contains the.wasm,.zkey, andvc_and_disclose_aadhaar_vkey.jsonmatching the new tau=17 setup- Confirm
artifacts/contracts/verifiers/local/staging/disclose/Verifier_vc_and_disclose_aadhaar_staging.jsonis present and regenerated with the updated parameterscontracts/contracts/registry/IdentityRegistryAadhaarImplV1.sol (3)
195-200: Implementation correct for new storage model.The function correctly returns the boolean value from
_uidaiPubkeyCommitments. However, this change is part of the storage type modification flagged at lines 59-60, which violates UUPS upgrade rules.
313-319: LGTM!The use of
deletecorrectly resets the boolean mapping entry tofalse. Implementation is correct for the new storage model.
305-311: All callers updated to the new single-parameter signature
| /// @notice Mapping from UIDAI pubkey to a boolean indicating registration. | ||
| mapping(uint256 => bool) internal _uidaiPubkeyCommitments; |
There was a problem hiding this comment.
CRITICAL: Storage type change violates UUPS upgrade rules.
This change modifies the type of an existing storage variable from mapping(uint256 => uint256) to mapping(uint256 => bool), which directly violates the UUPS upgrade constraints documented in lines 14-36 of this contract. Specifically, line 18 states: "🚫 NEVER CHANGE THE TYPE of existing variables".
In a UUPS upgrade:
- The old implementation stored
uint256timestamp values in these storage slots - After upgrade, the new implementation will interpret the same slots as
boolvalues - Existing non-zero timestamps will be read as
true, but zero values becomefalse - This creates unpredictable behavior for existing commitments
Impact: Upgrading to this implementation will cause storage corruption, making existing UIDAI pubkey commitments potentially unusable or incorrectly validated.
Required fix: To safely transition to boolean semantics while preserving UUPS upgradeability, you must:
- Keep the existing
mapping(uint256 => uint256)storage variable unchanged (or rename it to_uidaiPubkeyCommitmentsDeprecated) - Add a NEW storage variable
mapping(uint256 => bool) _uidaiPubkeyCommitmentsat the END of the storage layout - Implement migration logic to populate the new mapping from the old one
- Update all functions to use the new mapping
Alternatively, consider deploying this as a completely new contract version (V2) rather than attempting to upgrade V1.
| /// @notice Updates a UIDAI pubkey commitment. | ||
| /// @dev Callable only via a proxy and restricted to the contract owner. | ||
| /// @param commitment The UIDAI pubkey commitment to update. | ||
| /// @param expiryTimestamp The new expiry timestamp of the commitment. | ||
| function updateUidaiPubkeyCommitmentExpiryTimestamp( | ||
| uint256 commitment, | ||
| uint256 expiryTimestamp | ||
| ) external onlyProxy onlyOwner { | ||
| if (expiryTimestamp < block.timestamp) revert EXPIRY_IN_PAST(); | ||
| _uidaiPubkeyExpiryTimestamps[commitment] = expiryTimestamp; | ||
| emit UidaiPubkeyCommitmentUpdated(commitment, expiryTimestamp); | ||
| function updateUidaiPubkeyCommitment(uint256 commitment) external onlyProxy onlyOwner { | ||
| _uidaiPubkeyCommitments[commitment] = true; | ||
| emit UidaiPubkeyCommitmentUpdated(commitment, block.timestamp); | ||
| } |
There was a problem hiding this comment.
Breaking change and functional redundancy.
Similar to registerUidaiPubkeyCommitment, this function's signature changed from two parameters to one, making it a breaking API change. Additionally, updateUidaiPubkeyCommitment now has identical implementation to registerUidaiPubkeyCommitment (both set the boolean to true), which makes the distinction between these functions unclear.
Consider: Document the semantic difference between "register" and "update", or consolidate these into a single function if they're truly identical.
🤖 Prompt for AI Agents
In contracts/contracts/registry/IdentityRegistryAadhaarImplV1.sol around lines
321-327, the updateUidaiPubkeyCommitment function was changed to a
single-parameter signature and now simply sets the same boolean as
registerUidaiPubkeyCommitment, creating a breaking API change and semantic
redundancy; fix this by either (A) restoring the original two-parameter
signature (matching registerUidaiPubkeyCommitment) and implementing true
"update" behavior (e.g., update commitment metadata/timestamp or other stored
fields rather than just setting boolean) and emitting the appropriate event with
both parameters, or (B) if no semantic difference is needed, remove
updateUidaiPubkeyCommitment and consolidate callers into
registerUidaiPubkeyCommitment to avoid duplicate APIs; ensure function
visibility and modifiers remain onlyProxy/onlyOwner and update tests and docs to
reflect the chosen approach.
| shouldChange: true, | ||
| nameAndDobOfac: "4183822562579010781434914867177251983368244626022840551534475857364967864437", | ||
| nameAndYobOfac: "14316795765689804800341464910235935757494922653038299433675973925727164473934", | ||
| hub: "0xe57F4773bd9c9d8b6Cd70431117d353298B9f5BF", | ||
| pubkeyCommitments: [ | ||
| "5648956411273136337349787488442520720416229937879112788241850936049694492145", | ||
| "18304035373718681408213540837772113004961405604264885188535510276454415833542", | ||
| "3099763118716361008062312602688327679110629275746483297740895929951765195538", | ||
| "5960616419594750988984019912914733527854225713611991429799390436159340745422", | ||
| "1312086597361744268424404341813751658452218312204370523713186983060138886330" | ||
| ] | ||
| }, |
There was a problem hiding this comment.
Do not hard-code the hub address
This config bakes in a single hub address, so running the module on any other chain (or after a redeploy) will push the registry to an outdated hub. That breaks verification routing and is easy to miss in prod. Instead, resolve the hub from deployed_addresses.json (or pass it in) so the update always targets the live hub.
- hub: "0xe57F4773bd9c9d8b6Cd70431117d353298B9f5BF",
+ // hub will be pulled from deployed_addresses at runtimeand inside handleRegistryDeployment:
- if (registryData.hub) {
+ const hubAddress =
+ registryData.hub ?? deployedAddresses["DeployHubV2#IdentityVerificationHub"];
+ if (hubAddress) {
const callOptions = { after: [currentOperation], id: ids() };
- currentOperation = m.call(registryContract, "updateHub", [registryData.hub], callOptions);
+ currentOperation = m.call(registryContract, "updateHub", [hubAddress], callOptions);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In contracts/ignition/modules/registry/updateRegistries.ts around lines 23 to
34, the hub address is hard-coded in the registry config which will target an
outdated hub on other chains or after redeploys; replace the literal hub value
with a lookup that resolves the current hub address from deployed_addresses.json
(or accept the hub as a parameter), then update handleRegistryDeployment to use
that resolved hub when constructing the registry payload so deployments always
target the live hub.
| "deploy:verifier:id": "npx dotenv-cli -- bash -c 'yarn hardhat ignition deploy ignition/modules/verifiers/deployAllVerifiersNew.ts --network ${NETWORK:-localhost} ${VERIFY:+--verify}'", | ||
| "deploy:verifiers": "npx dotenv-cli -- bash -c 'yarn hardhat ignition deploy ignition/modules/verifiers/deployVerifiers.ts --network ${NETWORK:-localhost} ${VERIFY:+--verify}'", | ||
| "deploy:verifiers:all": "npx dotenv-cli -- bash -c 'yarn hardhat ignition deploy ignition/modules/verifiers/deployAllVerifiers.ts --network ${NETWORK:-localhost} ${VERIFY:+--verify}'", | ||
| "deploy:verifiers:all": "npx dotenv-cli -- bash -c 'yarn hardhat ignition deploy ignition/modules/verifiers/deployAllVerifiersNew.ts --network ${NETWORK:-localhost}' --verify", |
There was a problem hiding this comment.
Restore --verify inside the Hardhat command.
Moving --verify outside the quoted bash -c string feeds it to dotenv-cli, so the Hardhat deployment never receives the flag. Today that script will throw (unknown option) or silently skip verification, breaking production deploys. Please put --verify back inside the inner command string.
-"deploy:verifiers:all": "npx dotenv-cli -- bash -c 'yarn hardhat ignition deploy ignition/modules/verifiers/deployAllVerifiersNew.ts --network ${NETWORK:-localhost}' --verify",
+"deploy:verifiers:all": "npx dotenv-cli -- bash -c 'yarn hardhat ignition deploy ignition/modules/verifiers/deployAllVerifiersNew.ts --network ${NETWORK:-localhost} --verify'",📝 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.
| "deploy:verifiers:all": "npx dotenv-cli -- bash -c 'yarn hardhat ignition deploy ignition/modules/verifiers/deployAllVerifiersNew.ts --network ${NETWORK:-localhost}' --verify", | |
| "deploy:verifiers:all": "npx dotenv-cli -- bash -c 'yarn hardhat ignition deploy ignition/modules/verifiers/deployAllVerifiersNew.ts --network ${NETWORK:-localhost} --verify'", |
🤖 Prompt for AI Agents
In contracts/package.json around line 35, the deploy script places --verify
outside the inner bash -c quoted command so dotenv-cli receives it instead of
Hardhat; move --verify back inside the single-quoted bash -c string so the
Hardhat CLI gets the flag (i.e., include --verify within the inner command after
the deploy script path) ensuring verification runs during deployment.
Summary by CodeRabbit