Fix/role access - #2
Conversation
- PlatformPaymaster: single-param constructor (entryPoint only); initialize() sets owner/dailyLimit/tdocDeployer on clones - Factory: Clones.cloneDeterministic; stores paymasterImplementation + tdocDeployer; onlyOwner update functions - Mocks: MockEntryPoint, MockRegistry, MockTdocDeployer for unit tests - Tests: 18 Factory + 27 PlatformPaymaster tests, all passing - Scripts: deployImplementation, deployFactory, updated deployPlatformPaymaster - src/abis: generated ABI exports for npm package (@trustvc/eip7702) - Remove: interactionContrats, Lock.sol, Lock.ts, 7702Frontend and dist added to gitignore noreply@anthropic.com>
- semantic-release: add dev branch as beta prerelease channel - release.yml: trigger workflow on dev pushes in addition to main - README: full project documentation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (98)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (11)
contracts/PlatformPaymaster.sol (2)
121-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
removeUserFromWhitelistduplicatessetUserWhitelist(user, 0).Both functions are
onlyOwnerand produce the same state and the same event. Keep the explicit function only if an external integration needs the dedicated selector.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/PlatformPaymaster.sol` around lines 121 - 127, Remove the redundant removeUserFromWhitelist function and reuse setUserWhitelist with a zero value for whitelist removal, unless an external integration explicitly requires the dedicated selector.
147-155: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUpdate the role comments.
The registry initializer grants all four roles to the paymaster. These calls grant the same roles to
msg.sender, and renouncing onlyDEFAULT_ADMIN_ROLEleaves the paymaster withMINTER_ROLE,RESTORER_ROLE, andACCEPTER_ROLE. Update lines 130–131 to document both role assignments accurately.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/PlatformPaymaster.sol` around lines 147 - 155, Update the role comments in the registry initializer near the role-grant calls to accurately document that all four roles are assigned to both the paymaster and msg.sender, while only DEFAULT_ADMIN_ROLE is renounced by the paymaster; explicitly account for the remaining MINTER_ROLE, RESTORER_ROLE, and ACCEPTER_ROLE assignments.contracts/Factory.sol (1)
28-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename or remove
setAttachedPaymaster.The function is
viewand returns a value, so thesetprefix is wrong. The public mappingattachedPaymasteralready generates the same getter. Remove the function, or rename it togetAttachedPaymaster.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/Factory.sol` around lines 28 - 33, Remove the redundant setAttachedPaymaster function from Factory, since the public attachedPaymaster mapping already provides the getter; alternatively, rename it to getAttachedPaymaster if an explicit wrapper is required.package.json (2)
34-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
cleandeletes generated ABI sources that are committed to the repository.
src/abis/*.tsis committed in this PR, butnpm run cleanremovessrc/abis. Afterclean, a developer must runbuild:abis, which needs compiled artifacts. Either stop committingsrc/abisand add it to.gitignore, or keepcleanlimited to build outputs.♻️ Option: limit `clean` to build outputs
- "clean": "rm -rf dist cache artifacts src/abis", + "clean": "rm -rf dist cache artifacts",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` around lines 34 - 45, Update the clean script so it removes only generated build outputs and preserves the committed src/abis directory. Keep the existing dist, cache, and artifacts cleanup behavior unchanged.
49-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm that
dependenciesare needed at runtime.The published
distbundle contains ABI constants and address constants only.@account-abstraction/contractsand@openzeppelin/contractsare Solidity sources. They are required only if a consumer compiles the shipped.solfiles. If that is the intent, keep them. If not, move them todevDependenciesand topeerDependenciesfor Solidity consumers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` around lines 49 - 52, Verify whether `@account-abstraction/contracts` and `@openzeppelin/contracts` are required by the published runtime bundle. If they are only needed to compile shipped Solidity files, move both from dependencies to devDependencies and peerDependencies; otherwise retain them in dependencies.tsconfig.build.json (2)
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the stale
7702Frontendexclude entry.This PR removes the frontend directory. The exclude entry no longer matches anything.
♻️ Proposed change
- "exclude": ["node_modules", "test", "scripts", "dist", "7702Frontend"] + "exclude": ["node_modules", "test", "scripts", "dist"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tsconfig.build.json` at line 11, Remove the stale "7702Frontend" entry from the exclude array in tsconfig.build.json, preserving all other excluded paths unchanged.
8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the redundant
ignoreDeprecationssetting. TypeScript 6.0.3 accepts"6.0", but this configuration uses no deprecated compiler options.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tsconfig.build.json` at line 8, Remove the redundant ignoreDeprecations compiler setting from tsconfig.build.json, leaving the remaining TypeScript configuration unchanged.README.md (2)
112-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the hardcoded test count.
The line states "runs all 45 hardhat tests". The count changes whenever a test is added, and the documentation then becomes wrong.
📝 Proposed change
-npm test # runs all 45 hardhat tests +npm test # runs the hardhat test suite🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 112, Update the npm test documentation line to describe running all Hardhat tests without specifying a fixed test count, so it remains accurate as tests change.
115-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the Amoy network.
This PR adds Amoy support to the deployment and transaction scripts. The README documents Sepolia only, in the title, the deployed address table, the prerequisites, and every
--network sepoliacommand. Add the Amoy commands and the Amoy addresses so that the documented workflow covers both networks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 115 - 137, Update the README deployment documentation to cover both Sepolia and Amoy: revise the relevant headings, prerequisites, deployed-address table, and workflow instructions, and add Amoy variants for every command currently using --network sepolia, including the gasless mint flow. Use the correct Amoy addresses and preserve the existing Sepolia instructions.src/constants/index.ts (1)
1-10: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd Amoy to the exported constants.
The deployment scripts in this PR resolve both Sepolia and Amoy.
ChainIdandcontractAddresscover Sepolia only. A consumer on Amoy getsundefinedfromcontractAddress.PlatformAccountFactory[chainId]with no compile-time error, because the index access is on a partial map.♻️ Proposed addition
export const ChainId = { Sepolia: 11155111, + Amoy: 80002, } as const; /** Deployed contract addresses indexed by chainId */ export const contractAddress = { PlatformAccountFactory: { [ChainId.Sepolia]: "0x5dcDf7fA6Ab8323F67FD66E89b6CeD4564f9F4Ff", + [ChainId.Amoy]: "0x...", }, } as const;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/constants/index.ts` around lines 1 - 10, Extend the exported ChainId constant with the Amoy chain identifier, then add the corresponding PlatformAccountFactory deployment address under contractAddress.PlatformAccountFactory using the same readonly structure. Ensure both maps provide typed entries for Sepolia and Amoy so supported chain lookups do not return undefined.src/index.ts (1)
1-2: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueNamespace re-exports limit tree-shaking for the large ABI constants.
export * as abisproduces a single namespace object. A consumer that imports only one ABI can still pull in all three, because bundlers cannot always split a namespace object. The paymaster ABI alone is about 900 lines.Consider adding subpath exports in
package.json(for example./abisand./constants), or re-export the named ABI constants directly from the root.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/index.ts` around lines 1 - 2, Replace the namespace re-exports in the root entrypoint, especially abis, with tree-shakeable named exports or add package.json subpath exports for ./abis and ./constants. Ensure consumers importing a single ABI do not receive the other ABI definitions, while preserving access to the existing ABI and constants symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.example:
- Around line 64-66: Replace the placeholder value in TDOC_IMPLEMENTATION_AMOY
with the valid deployed Amoy TDOC implementation address; if that address cannot
be committed, clearly mark the variable as a required user-provided value
instead of leaving an invalid EVM address.
- Around line 52-58: Update TDOC_IMPLEMENTATION_AMOY in .env.example to a valid
deployed Amoy TDoc implementation address, preserving the variable name so
scripts/deployRegistryGasless.ts can pass it to encodeFunctionData successfully.
In @.github/workflows/pull_requests.yml:
- Around line 22-30: Before reviewdog/action-eslint@v1 in
.github/workflows/pull_requests.yml at lines 22-30, add actions/setup-node@v4
and run npm ci --ignore-scripts so ESLint dependencies are installed from the
lockfile without lifecycle scripts. In .github/workflows/linters.yml at lines
26-28, replace the npm install `@commitlint/config-conventional` step with the
same setup-node and lockfile-based npm ci --ignore-scripts flow.
- Around line 1-3: Add top-level restrictive permissions to both
.github/workflows/pull_requests.yml (lines 1-3) and
.github/workflows/release.yml (lines 3-7), setting contents to read. Preserve
the existing job-level permissions for eslint-review and release.
In `@contracts/Factory.sol`:
- Around line 47-51: Restrict deployPlatformPaymaster to the contract owner and
bind deterministic deployment inputs to the authorized caller and
platformAddress, preventing arbitrary onboarding and salt front-running. Apply
the identical salt derivation in computePaymasterAddress so predicted addresses
remain consistent.
- Around line 20-26: Validate both constructor parameters before assigning them
in the Factory constructor: reject zero addresses for _tdocDeployer and
_paymasterImplementation using the same validation behavior as
updateTdocDeployer and updatePaymasterImplementation. Preserve the existing
assignments and ownership initialization for valid addresses.
- Around line 47-59: Update deployPlatformPaymaster to store the newly cloned
paymaster in the attachedPaymaster mapping keyed by platformAddress before
emitting PlatformOnboarded, so attachedPaymaster and setAttachedPaymaster
resolve the deployed clone.
In `@contracts/mocks/MockRegistry.sol`:
- Around line 49-58: Update MockRegistry.mint to require that msg.sender has
MINTER_ROLE before deploying MockTitleEscrow; reject unauthorized callers first,
then preserve the existing titleEscrow creation and lastTitleEscrow assignment
for authorized minters.
In `@contracts/PlatformPaymaster.sol`:
- Around line 293-298: The MINT_DOCUMENT_SEL validation path and mintDocument
remain unauthenticated. In contracts/PlatformPaymaster.sol lines 293-298,
require sender to be authorized, whitelisted, or owner and apply the same
dailyLimit check as Path A; in lines 173-186, enforce the same caller
authorization before minting or updating authorizedCallers for beneficiary and
holder.
- Around line 98-108: Update PlatformPaymaster.initialize to reject address(0)
for _tdocDeployer before assigning tdocDeployer, ensuring initialization cannot
permanently configure an unusable deployer; alternatively restore an onlyOwner
setter that provides a recovery path.
In `@README.md`:
- Around line 139-151: Update the README deployment instructions and Environment
variables table to reflect getEnv’s network-suffixed variables for both SEPOLIA
and AMOY, including FACTORY_ADDRESS_<NETWORK>, PAYMASTER_ADDRESS_<NETWORK>,
TDOC_IMPLEMENTATION_<NETWORK>, REGISTRY_ADDRESS_<NETWORK>, and
TITLE_ESCROW_ADDRESS_<NETWORK>. Add AMOY_RPC_URL and document the gasless-script
inputs OWNER_PRIVATE_KEY, TOKEN_NAME, TOKEN_SYMBOL, BENEFICIARY_ADDRESS,
HOLDER_ADDRESS, TOKEN_ID, and NETWORK.
In `@src/abis/platform-paymaster.ts`:
- Around line 260-320: Resolve the role-model mismatch between PlatformPaymaster
and its documentation: at src/abis/platform-paymaster.ts lines 260-320, if
PlatformPaymaster.sol uses AccessControl, run npm run build:abis and commit the
ABI exposing grantRole, hasRole, and MINTER_ROLE; otherwise, update README.md
line 21 to describe the actual authorizedCallers/userWhitelist gating instead of
MINTER_ROLE. Ensure both sites consistently reflect the selected contract
behavior.
In `@src/constants/index.ts`:
- Around line 6-10: Confirm the live Sepolia PlatformAccountFactory deployment,
then update the [ChainId.Sepolia] value in src/constants/index.ts (lines 6-10)
and the corresponding deployed-address table row in README.md (line 76) to use
that same address.
In `@test/Factory.ts`:
- Around line 175-183: Update Factory.sol’s deployPlatformPaymaster flow to
assign attachedPaymaster[platformAddress] before emitting PlatformOnboarded.
Extend the existing Factory test to read attachedPaymaster for
platform.account.address and assert it equals the deployed paymaster address.
---
Nitpick comments:
In `@contracts/Factory.sol`:
- Around line 28-33: Remove the redundant setAttachedPaymaster function from
Factory, since the public attachedPaymaster mapping already provides the getter;
alternatively, rename it to getAttachedPaymaster if an explicit wrapper is
required.
In `@contracts/PlatformPaymaster.sol`:
- Around line 121-127: Remove the redundant removeUserFromWhitelist function and
reuse setUserWhitelist with a zero value for whitelist removal, unless an
external integration explicitly requires the dedicated selector.
- Around line 147-155: Update the role comments in the registry initializer near
the role-grant calls to accurately document that all four roles are assigned to
both the paymaster and msg.sender, while only DEFAULT_ADMIN_ROLE is renounced by
the paymaster; explicitly account for the remaining MINTER_ROLE, RESTORER_ROLE,
and ACCEPTER_ROLE assignments.
In `@package.json`:
- Around line 34-45: Update the clean script so it removes only generated build
outputs and preserves the committed src/abis directory. Keep the existing dist,
cache, and artifacts cleanup behavior unchanged.
- Around line 49-52: Verify whether `@account-abstraction/contracts` and
`@openzeppelin/contracts` are required by the published runtime bundle. If they
are only needed to compile shipped Solidity files, move both from dependencies
to devDependencies and peerDependencies; otherwise retain them in dependencies.
In `@README.md`:
- Line 112: Update the npm test documentation line to describe running all
Hardhat tests without specifying a fixed test count, so it remains accurate as
tests change.
- Around line 115-137: Update the README deployment documentation to cover both
Sepolia and Amoy: revise the relevant headings, prerequisites, deployed-address
table, and workflow instructions, and add Amoy variants for every command
currently using --network sepolia, including the gasless mint flow. Use the
correct Amoy addresses and preserve the existing Sepolia instructions.
In `@src/constants/index.ts`:
- Around line 1-10: Extend the exported ChainId constant with the Amoy chain
identifier, then add the corresponding PlatformAccountFactory deployment address
under contractAddress.PlatformAccountFactory using the same readonly structure.
Ensure both maps provide typed entries for Sepolia and Amoy so supported chain
lookups do not return undefined.
In `@src/index.ts`:
- Around line 1-2: Replace the namespace re-exports in the root entrypoint,
especially abis, with tree-shakeable named exports or add package.json subpath
exports for ./abis and ./constants. Ensure consumers importing a single ABI do
not receive the other ABI definitions, while preserving access to the existing
ABI and constants symbols.
In `@tsconfig.build.json`:
- Line 11: Remove the stale "7702Frontend" entry from the exclude array in
tsconfig.build.json, preserving all other excluded paths unchanged.
- Line 8: Remove the redundant ignoreDeprecations compiler setting from
tsconfig.build.json, leaving the remaining TypeScript configuration unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d4807f1-29ac-4697-ad2a-5f44aa5ce31d
⛔ Files ignored due to path filters (2)
7702Frontend/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (98)
.env.example.eslintignore.eslintrc.json.github/workflows/linters.yml.github/workflows/pull_requests.yml.github/workflows/release.yml.github/workflows/tests.yml.gitignore7702Frontend/.env.example7702Frontend/index.html7702Frontend/package.json7702Frontend/postcss.config.js7702Frontend/src/App.tsx7702Frontend/src/components/DelegationPanel.tsx7702Frontend/src/components/PaymasterAdminPanel.tsx7702Frontend/src/components/PaymasterPanel.tsx7702Frontend/src/components/RegistryPanel.tsx7702Frontend/src/components/SignerPanel.tsx7702Frontend/src/components/StoragePanel.tsx7702Frontend/src/components/TitleEscrowPanel.tsx7702Frontend/src/components/TrustVCPanel.tsx7702Frontend/src/components/WalletConnect.tsx7702Frontend/src/index.css7702Frontend/src/lib/abis.ts7702Frontend/src/lib/constants.ts7702Frontend/src/lib/pimlico.ts7702Frontend/src/main.tsx7702Frontend/src/vite-env.d.ts7702Frontend/tailwind.config.js7702Frontend/tsconfig.json7702Frontend/vite.config.tsEIP7702_METAMASK_ARCHITECTURE.mdPIMLICO_EIP7702_DOCS.mdREADME.mdcommitlint.config.jscontracts/Etherspot/BasePaymaster.solcontracts/Etherspot/EtherspotPaymaster.solcontracts/Etherspot/core/Helpers.solcontracts/Etherspot/core/UserOperationLib.solcontracts/Etherspot/interfaces/IAggregator.solcontracts/Etherspot/interfaces/IEntryPoint.solcontracts/Etherspot/interfaces/INonceManager.solcontracts/Etherspot/interfaces/IPaymaster.solcontracts/Etherspot/interfaces/IStakeManager.solcontracts/Etherspot/interfaces/PackedUserOperation.solcontracts/Factory.solcontracts/Lock.solcontracts/PlatformPaymaster.solcontracts/mocks/MockEntryPoint.solcontracts/mocks/MockRegistry.solcontracts/mocks/MockTdocDeployer.solhardhat.config.tsignition/modules/Lock.tsinteractionContrats/TitleEscrow.solinteractionContrats/TitleEscrowFactory.solinteractionContrats/TradeTrustToken.solinteractionContrats/base/RegistryAccess.solinteractionContrats/base/SBTUpgradeable.solinteractionContrats/base/TradeTrustSBT.solinteractionContrats/base/TradeTrustTokenBase.solinteractionContrats/base/TradeTrustTokenBaseURI.solinteractionContrats/base/TradeTrustTokenBurnable.solinteractionContrats/base/TradeTrustTokenMintable.solinteractionContrats/base/TradeTrustTokenRestorable.solinteractionContrats/utils/SigHelper.solinteractionContrats/utils/TDocDeployer.solpackage.jsonscripts/deployEIP7702.tsscripts/deployFactory.tsscripts/deployImplementation.tsscripts/deployPlatformPaymaster.tsscripts/deployRegistryGasless.tsscripts/generate-abis.tsscripts/lib/network.tsscripts/mintDocumentGasless.tsscripts/stakeOwnerOnEP9.tsscripts/stakePlatformPaymaster.tsscripts/trFunctions/_setup.tsscripts/trFunctions/nominate.tsscripts/trFunctions/rejectTransferBeneficiary.tsscripts/trFunctions/rejectTransferHolder.tsscripts/trFunctions/rejectTransferOwners.tsscripts/trFunctions/returnToIssuer.tsscripts/trFunctions/shred.tsscripts/trFunctions/transferBeneficiary.tsscripts/trFunctions/transferOwners.tssrc/abis/eip7702-implementation.tssrc/abis/index.tssrc/abis/platform-account-factory.tssrc/abis/platform-paymaster.tssrc/constants/index.tssrc/index.tstest/Factory.tstest/Lock.tstest/PlatformPaymaster.tstsconfig.build.jsontsup.config.tsupload.json
💤 Files with no reviewable changes (52)
- 7702Frontend/tailwind.config.js
- contracts/Etherspot/interfaces/PackedUserOperation.sol
- 7702Frontend/index.html
- 7702Frontend/src/vite-env.d.ts
- 7702Frontend/src/main.tsx
- 7702Frontend/tsconfig.json
- 7702Frontend/.env.example
- 7702Frontend/src/components/RegistryPanel.tsx
- 7702Frontend/vite.config.ts
- scripts/stakeOwnerOnEP9.ts
- 7702Frontend/src/lib/constants.ts
- 7702Frontend/src/components/SignerPanel.tsx
- interactionContrats/utils/TDocDeployer.sol
- EIP7702_METAMASK_ARCHITECTURE.md
- 7702Frontend/src/components/TitleEscrowPanel.tsx
- 7702Frontend/src/components/PaymasterAdminPanel.tsx
- contracts/Etherspot/interfaces/IAggregator.sol
- 7702Frontend/src/lib/abis.ts
- 7702Frontend/src/components/TrustVCPanel.tsx
- interactionContrats/base/TradeTrustTokenBaseURI.sol
- PIMLICO_EIP7702_DOCS.md
- 7702Frontend/src/components/StoragePanel.tsx
- 7702Frontend/src/App.tsx
- 7702Frontend/src/components/WalletConnect.tsx
- contracts/Etherspot/EtherspotPaymaster.sol
- contracts/Etherspot/interfaces/INonceManager.sol
- 7702Frontend/src/index.css
- test/Lock.ts
- 7702Frontend/src/components/DelegationPanel.tsx
- interactionContrats/base/TradeTrustTokenBase.sol
- interactionContrats/utils/SigHelper.sol
- interactionContrats/base/SBTUpgradeable.sol
- interactionContrats/base/RegistryAccess.sol
- interactionContrats/base/TradeTrustTokenRestorable.sol
- contracts/Etherspot/interfaces/IStakeManager.sol
- 7702Frontend/src/components/PaymasterPanel.tsx
- contracts/Etherspot/BasePaymaster.sol
- interactionContrats/base/TradeTrustTokenBurnable.sol
- interactionContrats/TitleEscrow.sol
- interactionContrats/base/TradeTrustTokenMintable.sol
- 7702Frontend/src/lib/pimlico.ts
- interactionContrats/TitleEscrowFactory.sol
- contracts/Lock.sol
- interactionContrats/base/TradeTrustSBT.sol
- ignition/modules/Lock.ts
- contracts/Etherspot/interfaces/IEntryPoint.sol
- 7702Frontend/postcss.config.js
- contracts/Etherspot/interfaces/IPaymaster.sol
- contracts/Etherspot/core/UserOperationLib.sol
- 7702Frontend/package.json
- contracts/Etherspot/core/Helpers.sol
- interactionContrats/TradeTrustToken.sol
| # Filled in after running deployRegistryGasless.ts / mintDocumentGasless.ts | ||
| REGISTRY_ADDRESS_SEPOLIA=0x_filled_after_deployRegistry | ||
| TITLE_ESCROW_ADDRESS_SEPOLIA=0x_filled_after_mintDocument | ||
|
|
||
| # ─── Polygon Amoy Deployments ───────────────────────────────────────────────── | ||
| # Same deploy order as above but with --network amoy and NETWORK=amoy | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 '\bNETWORK\b|process\.env\.NETWORK|NETWORK=' scripts hardhat.config.tsRepository: TrustVC/7702SmartAccount
Length of output: 8682
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- target .env.example sections ---'
sed -n '40,78p' .env.example
printf '%s\n' '--- network resolution and usage ---'
rg -n -C 8 'function getNetworkConfig|const getNetworkConfig|export .*getNetworkConfig|networkName|NETWORK' scripts/trFunctions scripts/deployRegistryGasless.ts scripts/mintDocumentGasless.ts scripts -g '*.ts' | head -n 220
printf '%s\n' '--- documentation commands ---'
rg -n -C 5 'deployRegistryGasless|mintDocumentGasless|NETWORK=|NETWORK ' README.md docs .github 2>/dev/null || trueRepository: TrustVC/7702SmartAccount
Length of output: 19400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- network helper ---'
sed -n '1,180p' scripts/lib/network.ts
printf '%s\n' '--- deployment call using TDOC_IMPLEMENTATION ---'
rg -n -C 12 'tdocImpl|deployRegistry|encodeFunctionData' scripts/deployRegistryGasless.ts
printf '%s\n' '--- Amoy placeholder references ---'
rg -n -C 3 'TDOC_IMPLEMENTATION_AMOY|0x_tdoc_implementation_on_amoy' .Repository: TrustVC/7702SmartAccount
Length of output: 7040
Provide a valid Amoy TDoc implementation address.
TDOC_IMPLEMENTATION_AMOY=0x_tdoc_implementation_on_amoy is not a valid address. scripts/deployRegistryGasless.ts passes it to encodeFunctionData as the implementation argument, so Amoy registry deployment fails.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.env.example around lines 52 - 58, Update TDOC_IMPLEMENTATION_AMOY in
.env.example to a valid deployed Amoy TDoc implementation address, preserving
the variable name so scripts/deployRegistryGasless.ts can pass it to
encodeFunctionData successfully.
| # TrustVC infrastructure on Amoy (pre-deployed — do not change) | ||
| TDOC_DEPLOYER_ADDRESS_AMOY=0xfcafea839e576967b96ad1FBFB52b5CA26cd1D25 | ||
| TDOC_IMPLEMENTATION_AMOY=0x_tdoc_implementation_on_amoy |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Provide a valid Amoy TDOC implementation address.
TDOC_IMPLEMENTATION_AMOY=0x_tdoc_implementation_on_amoy is not a valid EVM address, but the surrounding comment marks this infrastructure as pre-deployed. Any Amoy script that consumes this value will fail address validation or use invalid configuration.
Replace it with the deployed address, or document it as a required value that users must populate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.env.example around lines 64 - 66, Replace the placeholder value in
TDOC_IMPLEMENTATION_AMOY with the valid deployed Amoy TDOC implementation
address; if that address cannot be committed, clearly mark the variable as a
required user-provided value instead of leaving an invalid EVM address.
| on: | ||
| pull_request: | ||
| types: [opened, reopened, synchronize] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo="$(gh repo view --json nameWithOwner --jq .nameWithOwner)"
gh api "repos/${repo}/actions/permissions/workflow" | jq .Repository: TrustVC/7702SmartAccount
Length of output: 418
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- workflow files ---'
git ls-files '.github/workflows/*'
printf '%s\n' '--- pull_requests.yml ---'
cat -n .github/workflows/pull_requests.yml
printf '%s\n' '--- release.yml ---'
cat -n .github/workflows/release.yml
printf '%s\n' '--- permission declarations and workflow/job references ---'
rg -n -C 3 '(^|[[:space:]])permissions:|uses:|^[[:space:]]{2,}jobs:|^[[:space:]]{2,}[A-Za-z0-9_-]+:' .github/workflowsRepository: TrustVC/7702SmartAccount
Length of output: 10400
🌐 Web query:
GitHub Actions permissions workflow-level job-level precedence reusable workflow caller permissions contents read documentation
💡 Result:
In GitHub Actions, GITHUB_TOKEN permissions are determined by a hierarchical precedence and, in the context of reusable workflows, are strictly capped by the caller's configuration [1][2][3]. Permission Precedence The permissions for a GITHUB_TOKEN are calculated by applying configurations in the following order of precedence, where each subsequent level can further restrict the token's access [1][4][5]: 1. Default Settings: Permissions are initially set by the enterprise, organization, or repository default settings [1][5]. 2. Workflow-Level: If a permissions key is defined at the top level of a workflow file, it overrides the default settings for all jobs in that workflow [1][4]. 3. Job-Level: If a permissions key is defined within a specific job, it overrides the workflow-level and default settings for that job only [1][6]. Reusable Workflow Caller Permissions When using reusable workflows, the caller's permissions act as a security boundary or "envelope" for the called workflow [7][2]. The following rules apply: - Capping: A reusable workflow cannot request or be granted permissions that exceed those explicitly granted by the caller (either at the workflow or job level) [2][3]. - Effective Permissions: The effective permissions for a reusable workflow are the intersection of the permissions it requests and the permissions the caller allows [2][3]. - Error Behavior: If a reusable workflow requests a permission (e.g., contents: write) that the caller has not granted (e.g., the caller only provides contents: read or the default), the workflow will fail with an error similar to: The workflow is requesting X, but is only allowed Y [2][3]. - Best Practices: To ensure successful execution and security, it is recommended to explicitly declare the minimum required permissions in both the caller (for the specific job calling the workflow) and the reusable workflow itself [2]. For example, if a reusable workflow requires contents: read, the calling job should explicitly grant that scope [8][2].
Citations:
- 1: https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions
- 2: https://latchkey.dev/learn/github-actions/reusable-workflow-permissions-not-propagated-in-ci
- 3: https://www.kenmuse.com/blog/github-actions-workflow-permissions/
- 4: https://docs.github.com/en/enterprise-cloud@latest/actions/reference/workflows-and-actions/workflow-syntax
- 5: https://github.com/github/docs/blob/main/content/actions/reference/workflows-and-actions/workflow-syntax.md
- 6: https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax?use_case=
- 7:
call-workflowgenerated caller jobs omit requiredpermissions:for reusable workflows github/gh-aw#21071 - 8: https://docs.github.com/en/actions/how-tos/reuse-automations/reuse-workflows
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
files = [
Path(".github/workflows/pull_requests.yml"),
Path(".github/workflows/release.yml"),
]
for path in files:
lines = path.read_text().splitlines()
top_permissions = [
line.strip()
for line in lines
if line and not line.startswith((" ", "\t")) and line.strip().startswith("permissions:")
]
jobs = {}
in_jobs = False
current_job = None
for line in lines:
if line == "jobs:":
in_jobs = True
continue
if not in_jobs:
continue
if line.startswith(" ") and not line.startswith(" ") and line.rstrip().endswith(":"):
current_job = line.strip()[:-1]
jobs[current_job] = {"uses": None, "permissions": []}
elif current_job and line.startswith(" uses:"):
jobs[current_job]["uses"] = line.split(":", 1)[1].strip()
elif current_job and line.startswith(" ") and line.strip().startswith(("contents:", "pull-requests:", "packages:", "id-token:")):
jobs[current_job]["permissions"].append(line.strip())
print(path)
print(" top-level permissions:", top_permissions or "absent")
for job, data in jobs.items():
if data["uses"] or data["permissions"]:
print(f" {job}: uses={data['uses'] or 'none'} permissions={data['permissions'] or 'absent'}")
PYRepository: TrustVC/7702SmartAccount
Length of output: 725
Set restrictive workflow permission defaults.
Both workflows omit top-level permissions, so reusable tests and linters jobs inherit repository or organization defaults. Add permissions: { contents: read } to both workflows. Existing job-level permissions for eslint-review and release remain in effect.
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 1-31: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
📍 Affects 2 files
.github/workflows/pull_requests.yml#L1-L3(this comment).github/workflows/release.yml#L3-L7
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pull_requests.yml around lines 1 - 3, Add top-level
restrictive permissions to both .github/workflows/pull_requests.yml (lines 1-3)
and .github/workflows/release.yml (lines 3-7), setting contents to read.
Preserve the existing job-level permissions for eslint-review and release.
Source: Linters/SAST tools
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: reviewdog/action-eslint@v1 | ||
| with: | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
| eslint_flags: "src scripts test --ext .ts" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'reviewdog/action-eslint|npm (ci|install)' \
.github/workflows/pull_requests.yml .github/workflows/linters.yml
curl -fsSL https://raw.githubusercontent.com/reviewdog/action-eslint/v1/script.sh \
| sed -n '6,20p'Repository: TrustVC/7702SmartAccount
Length of output: 2054
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- workflow files ---'
cat -n .github/workflows/pull_requests.yml
cat -n .github/workflows/linters.yml
printf '%s\n' '--- package manifests and lockfiles ---'
git ls-files | grep -E '(^|/)(package\.json|package-lock\.json|npm-shrinkwrap\.json|yarn\.lock|pnpm-lock\.yaml)$' | sort
for f in package.json package-lock.json npm-shrinkwrap.json yarn.lock pnpm-lock.yaml; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
case "$f" in
package.json) node -e 'const p=require("./package.json"); console.log(JSON.stringify({scripts:p.scripts,dependencies:p.dependencies,devDependencies:p.devDependencies},null,2))' ;;
*) sed -n '1,24p' "$f" ;;
esac
fi
done
printf '%s\n' '--- action script ---'
curl -fsSL https://raw.githubusercontent.com/reviewdog/action-eslint/v1/script.sh | cat -n | sed -n '1,80p'Repository: TrustVC/7702SmartAccount
Length of output: 6008
Block lifecycle scripts in pull-request dependency installs.
reviewdog/action-eslint@v1 runs npm install when ESLint is unavailable. This executes lifecycle scripts from pull-request-controlled package metadata while GITHUB_TOKEN has pull-requests: write.
Add actions/setup-node@v4 and npm ci --ignore-scripts before reviewdog/action-eslint@v1. Replace the npm install @commitlint/config-conventional`` step in .github/workflows/linters.yml with the same lockfile-based flow.
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 26-26: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
📍 Affects 2 files
.github/workflows/pull_requests.yml#L22-L30(this comment).github/workflows/linters.yml#L26-L28
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pull_requests.yml around lines 22 - 30, Before
reviewdog/action-eslint@v1 in .github/workflows/pull_requests.yml at lines
22-30, add actions/setup-node@v4 and run npm ci --ignore-scripts so ESLint
dependencies are installed from the lockfile without lifecycle scripts. In
.github/workflows/linters.yml at lines 26-28, replace the npm install
`@commitlint/config-conventional` step with the same setup-node and lockfile-based
npm ci --ignore-scripts flow.
Source: Linters/SAST tools
| if (innerSel == MINT_DOCUMENT_SEL) { | ||
| // credits NOT consumed; track daily spend normally | ||
| return (abi.encode(sender, maxCost, false), _packValidationData(false, 0, 0)); | ||
| // mintDocument: registry enforces MINTER_ROLE — no extra whitelist needed | ||
| return ( | ||
| abi.encode(sender, maxCost, false), | ||
| _packValidationData(false, 0, 0) | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Unauthenticated mint path in PlatformPaymaster. The PR removed the whitelist requirement from both the mint validation branch and mintDocument. Neither location now authenticates the caller, so any address can obtain sponsored gas and then self-grant authorizedCallers status for all future sponsored operations.
contracts/PlatformPaymaster.sol#L293-L298: gate theMINT_DOCUMENT_SELbranch onauthorizedCallers[sender],userWhitelist[sender] > 0, orsender == owner(), and apply thedailyLimitcheck that Path A applies.contracts/PlatformPaymaster.sol#L173-L186: restrictmintDocumentto authorized callers before it mints and before it writesauthorizedCallers[beneficiary]andauthorizedCallers[holder].
📍 Affects 1 file
contracts/PlatformPaymaster.sol#L293-L298(this comment)contracts/PlatformPaymaster.sol#L173-L186
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contracts/PlatformPaymaster.sol` around lines 293 - 298, The
MINT_DOCUMENT_SEL validation path and mintDocument remain unauthenticated. In
contracts/PlatformPaymaster.sol lines 293-298, require sender to be authorized,
whitelisted, or owner and apply the same dailyLimit check as Path A; in lines
173-186, enforce the same caller authorization before minting or updating
authorizedCallers for beneficiary and holder.
| ## Environment variables | ||
|
|
||
| | Variable | Description | | ||
| | --- | --- | | ||
| | `PRIVATE_KEY` | Deployer wallet private key | | ||
| | `SEPOLIA_RPC_URL` | Sepolia RPC endpoint | | ||
| | `PIMLICO_API_KEY` | Pimlico bundler API key | | ||
| | `TDOC_DEPLOYER_ADDRESS` | Deployed TDocDeployer address | | ||
| | `PAYMASTER_IMPLEMENTATION` | PlatformPaymaster implementation address | | ||
| | `FACTORY_ADDRESS` | PlatformAccountFactory address | | ||
| | `PAYMASTER_ADDRESS` | Deployed paymaster clone address | | ||
| | `EIP7702_IMPL_ADDRESS` | EIP7702Implementation address | | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Show the suffix values and the getEnv helper used by the scripts.
fd -t f 'network.ts' scripts --exec cat -n {}
echo "--- suffixed env reads across scripts ---"
rg -n 'getEnv\(' scriptsRepository: TrustVC/7702SmartAccount
Length of output: 3266
🏁 Script executed:
#!/bin/bash
echo "--- README command and environment sections ---"
rg -n -C 4 'OWNER_PRIVATE_KEY|TOKEN_NAME|TOKEN_SYMBOL|BENEFICIARY_ADDRESS|HOLDER_ADDRESS|TOKEN_ID|TDOC_IMPLEMENTATION|REGISTRY_ADDRESS|NETWORK|PRIVATE_KEY|FACTORY_ADDRESS|PAYMASTER_ADDRESS|TDOC_DEPLOYER_ADDRESS' README.md
echo "--- direct environment access and dotenv loading in scripts ---"
rg -n -C 2 'process\.env|dotenv|OWNER_PRIVATE_KEY|PRIVATE_KEY|TOKEN_NAME|TOKEN_SYMBOL|BENEFICIARY_ADDRESS|HOLDER_ADDRESS|TOKEN_ID|NETWORK' scripts package.json hardhat.config.ts 2>/dev/null || true
echo "--- README size and targeted lines ---"
wc -l README.md
sed -n '1,180p' README.mdRepository: TrustVC/7702SmartAccount
Length of output: 50382
🏁 Script executed:
#!/bin/bash
echo "--- environment template ---"
if [ -f .env.example ]; then cat -n .env.example; else echo ".env.example not found"; fi
echo "--- README deployment and environment sections ---"
sed -n '112,156p' README.md
echo "--- deployment output and environment writes ---"
rg -n -C 3 'Add to \.env|TDOC_DEPLOYER_ADDRESS|PAYMASTER_IMPLEMENTATION|FACTORY_ADDRESS|PAYMASTER_ADDRESS|REGISTRY_ADDRESS|TDOC_IMPLEMENTATION|EIP7702_IMPL_ADDRESS' scripts
echo "--- all environment identifiers used by scripts ---"
python3 - <<'PY'
import pathlib, re
names = {}
for p in pathlib.Path("scripts").rglob("*.ts"):
text = p.read_text()
found = set(re.findall(r'process\.env\.([A-Z][A-Z0-9_]*)', text))
found.update(re.findall(r'getEnv\([^,]+,\s*"([A-Z][A-Z0-9_]*)"', text))
for n in found:
names.setdefault(n, []).append(str(p))
for n in sorted(names):
print(f"{n}: {', '.join(names[n])}")
PYRepository: TrustVC/7702SmartAccount
Length of output: 24762
Align the README with the network-specific environment variables.
getEnv produces SEPOLIA and AMOY suffixes. Update the deployment instructions and table to use names such as FACTORY_ADDRESS_<NETWORK>, PAYMASTER_ADDRESS_<NETWORK>, TDOC_IMPLEMENTATION_<NETWORK>, REGISTRY_ADDRESS_<NETWORK>, and TITLE_ESCROW_ADDRESS_<NETWORK>. Add AMOY_RPC_URL and the gasless-script inputs, including OWNER_PRIVATE_KEY, TOKEN_NAME, TOKEN_SYMBOL, BENEFICIARY_ADDRESS, HOLDER_ADDRESS, TOKEN_ID, and NETWORK.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 139 - 151, Update the README deployment instructions
and Environment variables table to reflect getEnv’s network-suffixed variables
for both SEPOLIA and AMOY, including FACTORY_ADDRESS_<NETWORK>,
PAYMASTER_ADDRESS_<NETWORK>, TDOC_IMPLEMENTATION_<NETWORK>,
REGISTRY_ADDRESS_<NETWORK>, and TITLE_ESCROW_ADDRESS_<NETWORK>. Add AMOY_RPC_URL
and document the gasless-script inputs OWNER_PRIVATE_KEY, TOKEN_NAME,
TOKEN_SYMBOL, BENEFICIARY_ADDRESS, HOLDER_ADDRESS, TOKEN_ID, and NETWORK.
| { | ||
| "inputs": [], | ||
| "name": "acceptOwnership", | ||
| "outputs": [], | ||
| "stateMutability": "nonpayable", | ||
| "type": "function" | ||
| }, | ||
| { | ||
| "inputs": [ | ||
| { | ||
| "internalType": "address", | ||
| "name": "caller", | ||
| "type": "address" | ||
| } | ||
| ], | ||
| "name": "addAuthorizedCaller", | ||
| "outputs": [], | ||
| "stateMutability": "nonpayable", | ||
| "type": "function" | ||
| }, | ||
| { | ||
| "inputs": [ | ||
| { | ||
| "internalType": "address", | ||
| "name": "registry", | ||
| "type": "address" | ||
| } | ||
| ], | ||
| "name": "addRegistry", | ||
| "outputs": [], | ||
| "stateMutability": "nonpayable", | ||
| "type": "function" | ||
| }, | ||
| { | ||
| "inputs": [ | ||
| { | ||
| "internalType": "uint32", | ||
| "name": "unstakeDelaySec", | ||
| "type": "uint32" | ||
| } | ||
| ], | ||
| "name": "addStake", | ||
| "outputs": [], | ||
| "stateMutability": "payable", | ||
| "type": "function" | ||
| }, | ||
| { | ||
| "inputs": [ | ||
| { | ||
| "internalType": "address", | ||
| "name": "titleEscrow", | ||
| "type": "address" | ||
| } | ||
| ], | ||
| "name": "addTitleEscrow", | ||
| "outputs": [], | ||
| "stateMutability": "nonpayable", | ||
| "type": "function" | ||
| }, | ||
| { | ||
| "inputs": [ |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The paymaster role model is inconsistent between the generated ABI and the documentation. The PR objective adds minter role access, and the README describes mintDocument as MINTER_ROLE gated, but the generated ABI exposes only Ownable2Step and boolean authorization maps with no role entries. Resolve the divergence in one direction.
src/abis/platform-paymaster.ts#L260-L320: ifPlatformPaymaster.solnow uses AccessControl roles, runnpm run build:abisand commit the regenerated ABI so thatgrantRole,hasRole, andMINTER_ROLEare exported.README.md#L21-L21: if the contract keeps theauthorizedCallersanduserWhitelistmodel, replace the "MINTER_ROLE gated" wording with the actual gating mechanism.
📍 Affects 2 files
src/abis/platform-paymaster.ts#L260-L320(this comment)README.md#L21-L21
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/abis/platform-paymaster.ts` around lines 260 - 320, Resolve the
role-model mismatch between PlatformPaymaster and its documentation: at
src/abis/platform-paymaster.ts lines 260-320, if PlatformPaymaster.sol uses
AccessControl, run npm run build:abis and commit the ABI exposing grantRole,
hasRole, and MINTER_ROLE; otherwise, update README.md line 21 to describe the
actual authorizedCallers/userWhitelist gating instead of MINTER_ROLE. Ensure
both sites consistently reflect the selected contract behavior.
| export const contractAddress = { | ||
| PlatformAccountFactory: { | ||
| [ChainId.Sepolia]: "0x5dcDf7fA6Ab8323F67FD66E89b6CeD4564f9F4Ff", | ||
| }, | ||
| } as const; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The PlatformAccountFactory Sepolia address differs between the exported constant and the documentation. One deployment is recorded with two different values. Consumers that read the constant and developers that follow the README target different contracts.
src/constants/index.ts#L6-L10: confirm the live Sepolia factory deployment and set[ChainId.Sepolia]to that address.README.md#L76-L76: update the deployed address table row to the same confirmed address.
📍 Affects 2 files
src/constants/index.ts#L6-L10(this comment)README.md#L76-L76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/constants/index.ts` around lines 6 - 10, Confirm the live Sepolia
PlatformAccountFactory deployment, then update the [ChainId.Sepolia] value in
src/constants/index.ts (lines 6-10) and the corresponding deployed-address table
row in README.md (line 76) to use that same address.
| it("emits PlatformOnboarded with clone address", async function () { | ||
| const { factory, platform } = await loadFixture(deployFixture); | ||
| await deployClone(factory, platform); | ||
| const events = await factory.getEvents.PlatformOnboarded(); | ||
| expect(events).to.have.lengthOf(1); | ||
| expect(events[0].args.platformAddress).to.equal( | ||
| getAddress(platform.account.address), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Record and assert the attached paymaster.
deployPlatformPaymaster in contracts/Factory.sol does not write attachedPaymaster[platformAddress]. A successful onboarding therefore emits a clone address but leaves the lookup at the zero address.
Assign the mapping before PlatformOnboarded, then add a test that reads factory.read.attachedPaymaster([platform.account.address]).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/Factory.ts` around lines 175 - 183, Update Factory.sol’s
deployPlatformPaymaster flow to assign attachedPaymaster[platformAddress] before
emitting PlatformOnboarded. Extend the existing Factory test to read
attachedPaymaster for platform.account.address and assert it equals the deployed
paymaster address.
Add role access to minter for all the roles
Summary by CodeRabbit
New Features
Changes
Documentation
Chores