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>
📝 WalkthroughWalkthroughThis PR updates the factory and paymaster contracts for clone-based deployment and initializer wiring, adds generated ABI/package tooling and deployment scripts, and adds CI/lint workflows while removing the ChangesContract refactor and test coverage
Package tooling, scripts, and generated exports
CI, linting, and repository cleanup
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (5)
.github/workflows/release.yml (1)
34-40: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache the npm cache, not
~/node_modules.
npm ciinstalls into the workspacenode_modulesand deletes it on each run, so the current cache never warms this job. Switchingactions/setup-nodetocache: npm(or caching~/.npm) will make the release pipeline faster and more predictable.Also applies to: 47-52
🤖 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/release.yml around lines 34 - 40, The current cache step is targeting ~/node_modules, but npm ci recreates workspace node_modules on every run so this cache will not be reused effectively. Update the release workflow cache setup around the node module caching step to cache the npm download cache instead, preferably by using actions/setup-node with cache: npm or by caching ~/.npm, and make the same change in the other matching cache block so both release jobs benefit consistently.scripts/deployFactory.ts (2)
57-57: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd timeout to
waitForTransactionReceipt.Without a timeout, this call can hang indefinitely if the RPC drops the transaction or network partitions.
- const receipt = await publicClient.waitForTransactionReceipt({ hash: txHash }); + const receipt = await publicClient.waitForTransactionReceipt({ hash: txHash, timeout: 120_000 });🤖 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 `@scripts/deployFactory.ts` at line 57, The `waitForTransactionReceipt` call in `deployFactory.ts` can hang forever if the transaction is dropped or the network stalls. Update the `publicClient.waitForTransactionReceipt` invocation to include a timeout (and any related retry/confirmation options if available) so the deployment flow fails fast instead of waiting indefinitely.
61-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding contract verification step.
After deployment, consider verifying the contract on Etherscan/Sourcify so the source is available for the factory address consumers will interact with.
#!/bin/bash # Example: add to package.json scripts or CI # npx hardhat verify --network sepolia <factoryAddress> <tdocDeployer> <paymasterImpl>🤖 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 `@scripts/deployFactory.ts` around lines 61 - 70, Add a post-deployment contract verification step to the deploy output in deployFactory.ts so users can verify the factory contract on Etherscan/Sourcify after deployment. Update the console guidance near the existing “Next steps” section and reference the deployed factoryAddress along with the constructor args used by the deployment flow (tdocDeployer and paymasterImpl), so consumers know how to run verification for the PlatformAccountFactory.scripts/deployPlatformPaymaster.ts (2)
31-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
FACTORY_ADDRESSvalidation.The env var is checked at module level (lines 31-32) and again in
main()(line 43). SinceFACTORY_ADDRESSis a const evaluated at module load, themain()check is unreachable if the module-level check passes. Remove the duplicate or keep only the module-level check.- if (!FACTORY_ADDRESS) throw new Error("FACTORY_ADDRESS not set");🤖 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 `@scripts/deployPlatformPaymaster.ts` around lines 31 - 43, The `FACTORY_ADDRESS` check in `main()` is redundant because `deployPlatformPaymaster.ts` already validates `process.env.FACTORY_ADDRESS` at module load and assigns `FACTORY_ADDRESS` before `main()` runs. Remove the duplicate guard inside `main()` (or move the validation there and eliminate the top-level check) so the environment validation happens only once and stays centered around the `FACTORY_ADDRESS` constant.
81-83: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd timeout to
waitForTransactionReceipt.Same concern as deployFactory.ts - this can hang indefinitely without a timeout.
const receipt = await publicClient.waitForTransactionReceipt({ hash: txHash, + timeout: 120_000, });🤖 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 `@scripts/deployPlatformPaymaster.ts` around lines 81 - 83, The transaction receipt wait in deployPlatformPaymaster can hang indefinitely because publicClient.waitForTransactionReceipt is called without a timeout. Update the receipt wait in the deployPlatformPaymaster flow to pass an explicit timeout (consistent with deployFactory.ts) so the script fails fast instead of blocking forever. Keep the change localized to the waitForTransactionReceipt call and ensure the timeout value is clearly set there.
🤖 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 @.eslintrc.json:
- Around line 17-22: The ESLint config currently enables
import/no-extraneous-dependencies without registering the import plugin, which
causes lint config loading to fail. Update .eslintrc.json to either add the
eslint-plugin-import plugin configuration alongside the existing rules or remove
the import/no-extraneous-dependencies rule if the plugin is not intended to be
used. Make sure the fix is applied in the top-level ESLint config so npm run
lint can load successfully.
In @.github/workflows/linters.yml:
- Around line 25-28: The dependency installation in the linter workflow
currently uses npm install, which can drift from the pinned CI dependency set
and run scripts too early. Update the Install Commit Lint Dependencies step in
linters.yml to use npm ci with --ignore-scripts so it matches the lockfile-based
install used elsewhere and avoids lifecycle script execution while checkout
credentials are present.
- Around line 21-28: The Commit Lint job currently includes
JulienKode/pull-request-name-linter-action in the reusable linters workflow, but
that action only works on pull_request events and can break release runs
triggered from release.yml on push to main. Move the PR title check out of the
linters job in linters.yml into the pull_requests workflow, or add an event
guard on the existing step/job so it only runs when github.event_name is
pull_request.
In @.github/workflows/pull_requests.yml:
- Around line 2-3: The pull request workflow trigger is missing the edited
event, so PR-title validation will not rerun when only the title changes. Update
the pull_request.types configuration in the workflow definition to include
edited alongside opened, reopened, and synchronize so the linter stack
revalidates titles after edits.
- Around line 19-30: The eslint-review job only checks out the repository before
running reviewdog/action-eslint, so the local eslint binary and TypeScript
ESLint packages are missing. Update the workflow to add actions/setup-node and
run npm ci before the reviewdog step, using the eslint-review job as the place
to insert the dependency installation steps.
In `@contracts/Factory.sol`:
- Around line 16-21: The Factory configuration paths can currently accept
non-contract addresses, which later breaks clone deployment and
PlatformPaymaster initialization. Update the Factory constructor and the config
entry points around the existing _tdocDeployer and _paymasterImplementation
assignments to mirror the nonzero checks and also require that both addresses
contain contract code, using the existing validation flow in Factory and related
deploy/setup functions. Keep the checks consistent wherever these addresses are
set so invalid deployer or implementation values are rejected before any clone
or paymaster logic runs.
- Around line 36-48: The deterministic clone salt in deployPlatformPaymaster and
computePaymasterAddress is currently only `salt`, so any caller can reuse it for
a different platform. Update Factory to derive the clone salt from
`platformAddress` (or require platform authorization) and use the same derived
value in both `Clones.cloneDeterministic` and
`Clones.predictDeterministicAddress`, keeping the deployment and prediction
logic consistent.
In `@contracts/mocks/MockRegistry.sol`:
- Around line 17-24: The MockRegistry access-control stub is too permissive and
hides real failures in tests. Update the IAccessControl methods in MockRegistry
so grantRole and renounceRole behave like AccessControl: only the role admin can
grant a role, and renounceRole must only clear the role for the account passed
in and require that account to be msg.sender. Use the existing _roles storage in
MockRegistry to enforce these checks so mintDocument/registry tests fail when
PlatformPaymaster is configured with the wrong admin or principal.
In `@contracts/mocks/MockTdocDeployer.sol`:
- Around line 10-15: The MockTdocDeployer.deploy mock is ignoring the forwarded
implementation address, so it cannot catch bad values passed across the
paymaster → deployer boundary. Update deploy() to accept and use the
implementation argument when constructing the deployed registry path, and keep
MockRegistry creation aligned with the forwarded implementation plus the decoded
admin from params. This will ensure deployRegistry failures from address(0) or
stale implementations surface in the test suite.
In `@contracts/PlatformPaymaster.sol`:
- Around line 256-260: Update PlatformPaymaster validation so mint sponsorship
is checked the same way as other protected calls: in the validation path around
the mintDocument handling, reuse the same authorizedCallers/owner authorization
and dailyLimit/dailySpend logic currently used before sponsor approval. If the
sender is not authorized or the projected spend would exceed the limit, emit the
same rejection event and return a failed validation result instead of accepting
the UserOp for execution.
- Around line 95-104: The PlatformPaymaster.initialize function only validates
_owner, so an invalid _tdocDeployer can be permanently stored and break
deployRegistry with no recovery path. Add an initializer guard in initialize to
reject a zero address and any address that is not a contract before assigning
tdocDeployer, keeping the existing ownership and dailyLimit setup intact.
- Around line 171-185: Add a caller authorization check before the mint flow in
mintDocument so only approved requesters can use the paymaster to mint;
currently the code only validates authorizedRegistries[registry] and then
proceeds to mint and auto-authorize beneficiary and holder. Update the
mintDocument logic in PlatformPaymaster to gate access with an appropriate
authorization mapping or role check before calling
ITradeTrustToken(registry).mint, while keeping the existing registry validation
and post-mint authorization updates for titleEscrow, authorizedCallers,
authorizedTitleEscrows, and documentsMinted.
In `@package.json`:
- Around line 32-41: The npm scripts in package.json are using POSIX-only shell
syntax, which breaks on Windows. Update the build:sol, build:abis, and test
scripts to set TS_NODE_TRANSPILE_ONLY via a cross-platform mechanism such as
cross-env, and replace the clean script’s rm -rf with a cross-platform delete
approach like rimraf or a Node-based cleanup helper. Keep the existing script
names and wiring in place so build, test, and clean continue to work
consistently across shells.
In `@scripts/deployFactory.ts`:
- Around line 31-33: The PRIVATE_KEY handling in deployFactory should be
consistent with the trimmed account/env inputs used elsewhere. Trim the
process.env.PRIVATE_KEY value before passing it into privateKeyToAccount in the
deployer setup, and make sure the deployer initialization path uses the
sanitized string rather than the raw env var so whitespace does not break
account creation.
In `@scripts/deployImplementation.ts`:
- Around line 21-29: The deployment flow is using a default EntryPoint in
`deployImplementation` that can diverge from the one used by
`stakePlatformPaymaster`, causing the paymaster to be deployed/staked against
different EntryPoints. Centralize the EntryPoint selection in a shared constant
or config used by both scripts, or require `ENTRY_POINT` explicitly in both
`main` functions so `DEFAULT_ENTRY_POINT` cannot silently drift from the
paymaster staking path.
In `@scripts/generate-abis.ts`:
- Around line 47-60: The main flow in main() deletes OUT_DIR before verifying
all CONTRACTS_TO_EXPORT artifacts exist, which can leave src/index.ts pointing
at a missing ./abis folder if validation fails. Move the artifact existence
checks for each artifactPath ahead of the fs.rmSync/fs.mkdirSync cleanup, or
generate into a temporary directory and only replace OUT_DIR after all artifacts
are confirmed and generation succeeds. Use main, CONTRACTS_TO_EXPORT, OUT_DIR,
and artifactPath to locate the fix.
In `@test/PlatformPaymaster.ts`:
- Around line 285-339: The mintDocument tests only verify mapping updates and
event side effects, so add coverage for the actual sponsorship gate in
_validatePaymasterUserOp. Extend the existing mintDocument suite in
PlatformPaymaster to assert that a user and holder authorized by mintDocument
are accepted by the paymaster validation path, and that an unrelated caller is
still rejected. Use the existing paymaster, paymasterAsOther, authorizedCallers,
and _validatePaymasterUserOp flow so the test proves the new sponsorship check
works end to end.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 34-40: The current cache step is targeting ~/node_modules, but npm
ci recreates workspace node_modules on every run so this cache will not be
reused effectively. Update the release workflow cache setup around the node
module caching step to cache the npm download cache instead, preferably by using
actions/setup-node with cache: npm or by caching ~/.npm, and make the same
change in the other matching cache block so both release jobs benefit
consistently.
In `@scripts/deployFactory.ts`:
- Line 57: The `waitForTransactionReceipt` call in `deployFactory.ts` can hang
forever if the transaction is dropped or the network stalls. Update the
`publicClient.waitForTransactionReceipt` invocation to include a timeout (and
any related retry/confirmation options if available) so the deployment flow
fails fast instead of waiting indefinitely.
- Around line 61-70: Add a post-deployment contract verification step to the
deploy output in deployFactory.ts so users can verify the factory contract on
Etherscan/Sourcify after deployment. Update the console guidance near the
existing “Next steps” section and reference the deployed factoryAddress along
with the constructor args used by the deployment flow (tdocDeployer and
paymasterImpl), so consumers know how to run verification for the
PlatformAccountFactory.
In `@scripts/deployPlatformPaymaster.ts`:
- Around line 31-43: The `FACTORY_ADDRESS` check in `main()` is redundant
because `deployPlatformPaymaster.ts` already validates
`process.env.FACTORY_ADDRESS` at module load and assigns `FACTORY_ADDRESS`
before `main()` runs. Remove the duplicate guard inside `main()` (or move the
validation there and eliminate the top-level check) so the environment
validation happens only once and stays centered around the `FACTORY_ADDRESS`
constant.
- Around line 81-83: The transaction receipt wait in deployPlatformPaymaster can
hang indefinitely because publicClient.waitForTransactionReceipt is called
without a timeout. Update the receipt wait in the deployPlatformPaymaster flow
to pass an explicit timeout (consistent with deployFactory.ts) so the script
fails fast instead of blocking forever. Keep the change localized to the
waitForTransactionReceipt call and ensure the timeout value is clearly set
there.
🪄 Autofix (Beta)
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
Run ID: 348a6507-544f-4435-86d6-d623b7aad175
⛔ 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 (70)
.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.tscommitlint.config.jscontracts/Factory.solcontracts/Lock.solcontracts/PlatformPaymaster.solcontracts/mocks/MockEntryPoint.solcontracts/mocks/MockRegistry.solcontracts/mocks/MockTdocDeployer.solignition/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/deployFactory.tsscripts/deployImplementation.tsscripts/deployPlatformPaymaster.tsscripts/generate-abis.tsscripts/mintDocumentGasless.tsscripts/stakeOwnerOnEP9.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 (40)
- 7702Frontend/tailwind.config.js
- 7702Frontend/package.json
- interactionContrats/base/RegistryAccess.sol
- interactionContrats/base/TradeTrustTokenMintable.sol
- interactionContrats/base/TradeTrustTokenBaseURI.sol
- 7702Frontend/tsconfig.json
- ignition/modules/Lock.ts
- contracts/Lock.sol
- interactionContrats/base/TradeTrustTokenBase.sol
- interactionContrats/base/TradeTrustTokenBurnable.sol
- interactionContrats/utils/SigHelper.sol
- 7702Frontend/src/lib/pimlico.ts
- interactionContrats/base/SBTUpgradeable.sol
- interactionContrats/TradeTrustToken.sol
- 7702Frontend/src/components/DelegationPanel.tsx
- 7702Frontend/index.html
- 7702Frontend/src/vite-env.d.ts
- 7702Frontend/postcss.config.js
- 7702Frontend/src/components/TrustVCPanel.tsx
- 7702Frontend/vite.config.ts
- interactionContrats/TitleEscrowFactory.sol
- 7702Frontend/src/index.css
- 7702Frontend/src/App.tsx
- 7702Frontend/src/components/SignerPanel.tsx
- test/Lock.ts
- 7702Frontend/src/lib/abis.ts
- scripts/stakeOwnerOnEP9.ts
- 7702Frontend/src/components/TitleEscrowPanel.tsx
- 7702Frontend/src/lib/constants.ts
- 7702Frontend/src/components/PaymasterPanel.tsx
- interactionContrats/utils/TDocDeployer.sol
- 7702Frontend/src/components/PaymasterAdminPanel.tsx
- 7702Frontend/src/main.tsx
- interactionContrats/base/TradeTrustTokenRestorable.sol
- 7702Frontend/src/components/StoragePanel.tsx
- 7702Frontend/src/components/WalletConnect.tsx
- 7702Frontend/.env.example
- interactionContrats/base/TradeTrustSBT.sol
- interactionContrats/TitleEscrow.sol
- 7702Frontend/src/components/RegistryPanel.tsx
| "rules": { | ||
| "no-console": "off", | ||
| "@typescript-eslint/no-var-requires": "off", | ||
| "@typescript-eslint/no-explicit-any": "warn", | ||
| "@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_" }], | ||
| "import/no-extraneous-dependencies": "off" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files of interest:\n'
git ls-files .eslintrc.json package.json .github/workflows/linters.yml
printf '\n--- .eslintrc.json ---\n'
cat -n .eslintrc.json
printf '\n--- package.json (relevant lines) ---\n'
cat -n package.json | sed -n '1,220p'
printf '\n--- workflow excerpt ---\n'
cat -n .github/workflows/linters.yml | sed -n '1,220p'
printf '\n--- search for import plugin / extends ---\n'
rg -n '"eslint-plugin-import"|"\bimport/no-extraneous-dependencies\b"|extends|plugins' .eslintrc.json package.json .github/workflows/linters.ymlRepository: TrustVC/7702SmartAccount
Length of output: 5812
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'noop\n'Repository: TrustVC/7702SmartAccount
Length of output: 167
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "ROOT=$(pwd)"
echo "--- .eslintrc.json ---"
cat -n .eslintrc.json
echo "--- package.json plugin/import search ---"
rg -n '"eslint-plugin-import"|"import/no-extraneous-dependencies"|extends|plugins' package.json .eslintrc.jsonRepository: TrustVC/7702SmartAccount
Length of output: 1859
Add eslint-plugin-import or remove this rule. .eslintrc.json:8-22 references import/no-extraneous-dependencies without registering the import plugin, so npm run lint fails on config load.
🤖 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 @.eslintrc.json around lines 17 - 22, The ESLint config currently enables
import/no-extraneous-dependencies without registering the import plugin, which
causes lint config loading to fail. Update .eslintrc.json to either add the
eslint-plugin-import plugin configuration alongside the existing rules or remove
the import/no-extraneous-dependencies rule if the plugin is not intended to be
used. Make sure the fix is applied in the top-level ESLint config so npm run
lint can load successfully.
| commit-lint: | ||
| name: Commit Lint | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Install Commit Lint Dependencies | ||
| run: npm install @commitlint/config-conventional | ||
| - uses: JulienKode/pull-request-name-linter-action@v0.5.0 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== workflows files ==\n'
git ls-files .github/workflows
printf '\n== linters.yml (numbered) ==\n'
cat -n .github/workflows/linters.yml
printf '\n== release.yml (numbered) ==\n'
cat -n .github/workflows/release.ymlRepository: TrustVC/7702SmartAccount
Length of output: 2824
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== lines mentioning workflow_call / pull_request / push ==\n'
rg -n "workflow_call|pull_request|push|pull_request_target|workflow_dispatch|commit-lint|pull-request-name-linter-action|npm install `@commitlint/config-conventional`" .github/workflowsRepository: TrustVC/7702SmartAccount
Length of output: 647
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for path in [Path('.github/workflows/linters.yml'), Path('.github/workflows/release.yml')]:
print(f"\n== {path} ==")
text = path.read_text()
for i, line in enumerate(text.splitlines(), 1):
if 'commit-lint' in line or 'pull-request-name-linter-action' in line or 'workflow_call' in line or 'pull_request' in line or 'push:' in line or 'main' in line:
print(f"{i:4}: {line}")
PYRepository: TrustVC/7702SmartAccount
Length of output: 383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== pull_requests.yml (numbered) ==\n'
cat -n .github/workflows/pull_requests.yml
printf '\n== references to pull-request-name-linter-action across repo ==\n'
rg -n "pull-request-name-linter-action|commit-lint|Commit Lint|pr title|pull request name" .Repository: TrustVC/7702SmartAccount
Length of output: 1063
🌐 Web query:
JulienKode/pull-request-name-linter-action requires pull_request event title github context
💡 Result:
The JulienKode/pull-request-name-linter-action specifically requires a pull_request event to function because it relies on the GitHub Actions context to retrieve the pull request title [1]. The action retrieves the title internally from the GitHub context payload using github.context.payload.pull_request.title [1]. Because of this, it must be triggered by a workflow configured with the pull_request event [2][3]. To ensure the action correctly receives the pull request title, your workflow must include the appropriate configuration in the on: section [2][3]: on: pull_request: types: ['opened', 'edited', 'reopened', 'synchronize'] If a workflow is triggered by an event that does not contain pull request metadata (such as a push event), the action will not have access to the title [4][5]. Furthermore, users have noted that if you manually re-run a workflow, the action may use the cached title from the original event payload rather than the updated PR title [6]. If you find the action is not detecting title changes, ensure that your workflow includes the edited activity type [2][7].
Citations:
- 1: https://github.com/JulienKode/pull-request-name-linter-action/blob/master/AGENT.md
- 2: https://github.com/JulienKode/pull-request-name-linter-action/blob/master/README.md
- 3: https://github.com/marketplace/actions/pull-request-linter-action
- 4: https://www.baeldung.com/ops/github-pr-title-get
- 5: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts
- 6: github.context.payload.pull_request.title always remains the same when rerunning action. actions/toolkit#1204
- 7: https://github.com/marketplace/actions/pull-request-linter
Move the PR-title lint out of the reusable workflow. JulienKode/pull-request-name-linter-action only works on pull_request events, but .github/workflows/release.yml reuses linters.yml on push to main, so this job can fail in release runs. Move it to .github/workflows/pull_requests.yml or guard it with if: github.event_name == 'pull_request'.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 25-25: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 27-27: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
🤖 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/linters.yml around lines 21 - 28, The Commit Lint job
currently includes JulienKode/pull-request-name-linter-action in the reusable
linters workflow, but that action only works on pull_request events and can
break release runs triggered from release.yml on push to main. Move the PR title
check out of the linters job in linters.yml into the pull_requests workflow, or
add an event guard on the existing step/job so it only runs when
github.event_name is pull_request.
| pull_request: | ||
| types: [opened, reopened, synchronize] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Re-run this workflow when the PR title is edited.
The linter stack now includes PR-title validation, but pull_request.types excludes edited. A title fixed after a failure will not be revalidated until someone pushes another commit or reopens the PR.
Suggested fix
pull_request:
- types: [opened, reopened, synchronize]
+ types: [opened, reopened, synchronize, edited]📝 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.
| pull_request: | |
| types: [opened, reopened, synchronize] | |
| pull_request: | |
| types: [opened, reopened, synchronize, edited] |
🤖 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 2 - 3, The pull request
workflow trigger is missing the edited event, so PR-title validation will not
rerun when only the title changes. Update the pull_request.types configuration
in the workflow definition to include edited alongside opened, reopened, and
synchronize so the linter stack revalidates titles after edits.
| "build:sol": "TS_NODE_TRANSPILE_ONLY=1 hardhat compile", | ||
| "build:abis": "TS_NODE_TRANSPILE_ONLY=1 ts-node scripts/generate-abis.ts", | ||
| "build:js": "tsup", | ||
| "build": "npm run build:sol && npm run build:abis && npm run build:js", | ||
| "clean": "rm -rf dist cache artifacts src/abis", | ||
| "lint": "eslint src scripts test --ext .ts", | ||
| "lint:fix": "npm run lint -- --fix", | ||
| "prepublishOnly": "npm run build", | ||
| "semantic-release": "semantic-release", | ||
| "test": "TS_NODE_TRANSPILE_ONLY=1 hardhat test" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n package.json | sed -n '1,140p'
echo
printf '%s\n' '--- cross-env search ---'
rg -n '"cross-env"|cross-env' package.json package-lock.json pnpm-lock.yaml yarn.lock . 2>/dev/null | sed -n '1,120p'
echo
printf '%s\n' '--- os/docs search ---'
rg -n 'Windows|cross-platform|POSIX|npm run build|npm test|CI' README* docs .github package.json 2>/dev/null | sed -n '1,160p'Repository: TrustVC/7702SmartAccount
Length of output: 3238
🌐 Web query:
npm scripts environment variable assignment Windows default shell TS_NODE_TRANSPILE_ONLY=1 cross-env documentation
💡 Result:
NPM scripts run in platform-dependent shells: /bin/sh on POSIX-compliant systems and cmd.exe on Windows [1][2]. Because these shells handle environment variable syntax differently (e.g., POSIX uses VAR=val command, while cmd.exe requires set VAR=val && command), hardcoding variables directly in your package.json scripts often leads to cross-platform compatibility issues [3][4][5]. To ensure your scripts work reliably across both Windows and POSIX environments, you should use the cross-env package [3][6][4]. cross-env Documentation and Usage cross-env allows you to set environment variables using POSIX syntax (e.g., NODE_ENV=production) in your npm scripts, and it automatically handles the necessary transformations to make them work on Windows [3][6][5]. To use it, add cross-env as a development dependency and prefix your command [5]: "scripts": { "start": "cross-env TS_NODE_TRANSPILE_ONLY=1 ts-node server.ts" } - cross-env and cross-env-shell: cross-env uses cross-spawn to execute commands, while cross-env-shell uses the shell option from Node's spawn [3][7]. Use cross-env-shell if you need to run multiple commands in series or require shell features like command substitution or signal handling [3][6][7]. TS_NODE_TRANSPILE_ONLY=1 The environment variable TS_NODE_TRANSPILE_ONLY is a recognized configuration for ts-node that enables fast execution by skipping type checking [8][9][10]. While you can set this variable directly in your environment, using it within an npm script requires cross-platform care. Using it with cross-env ensures that your script remains portable across different operating systems [3][5]. Recommended Alternative For better maintainability and to avoid platform-specific script syntax, ts-node recommends defining options directly in your tsconfig.json file [8][9][10]: { "ts-node": { "transpileOnly": true } } This approach is generally preferred over environment variables or command-line flags, as it allows you to configure behavior centrally without shell-dependent syntax [8][10].
Citations:
- 1: https://docs.npmjs.com/cli/v11/commands/npm-run/
- 2: https://docs.npmjs.com/cli/v9/commands/npm-run-script/?v=true
- 3: https://github.com/kentcdodds/cross-env/blob/master/README.md
- 4: https://www.npmjs.com/package/cross-env
- 5: https://razvanvancea.ro/blog/2025/06/10/using-cross-env-in-your-node-js-projects-a-practical-guide/
- 6: https://github.com/kentcdodds/cross-env
- 7: https://cdn.jsdelivr.net/npm/cross-env@10.1.0/README.md
- 8: https://www.npmjs.com/package/ts-node
- 9: https://github.com/TypeStrong/ts-node/blob/master/README.md
- 10: https://github.com/TypeStrong/ts-node/blob/v10.4.0/README.md
Make the npm scripts cross-platform
TS_NODE_TRANSPILE_ONLY=1 ... and rm -rf are POSIX-only, so npm run build, npm run test, and npm run clean fail in the default Windows shell. Use cross-env and rimraf or a small Node wrapper instead.
🤖 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 32 - 41, The npm scripts in package.json are using
POSIX-only shell syntax, which breaks on Windows. Update the build:sol,
build:abis, and test scripts to set TS_NODE_TRANSPILE_ONLY via a cross-platform
mechanism such as cross-env, and replace the clean script’s rm -rf with a
cross-platform delete approach like rimraf or a Node-based cleanup helper. Keep
the existing script names and wiring in place so build, test, and clean continue
to work consistently across shells.
| const deployer = privateKeyToAccount( | ||
| process.env.PRIVATE_KEY as `0x${string}`, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Trim PRIVATE_KEY before use.
tdocDeployer and paymasterImpl are trimmed, but PRIVATE_KEY is not. Leading/trailing whitespace in the private key env var will cause privateKeyToAccount to fail with an opaque error.
const deployer = privateKeyToAccount(
- process.env.PRIVATE_KEY as `0x${string}`,
+ process.env.PRIVATE_KEY.trim() as `0x${string}`,
);📝 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.
| const deployer = privateKeyToAccount( | |
| process.env.PRIVATE_KEY as `0x${string}`, | |
| ); | |
| const deployer = privateKeyToAccount( | |
| process.env.PRIVATE_KEY.trim() as `0x${string}`, | |
| ); |
🤖 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 `@scripts/deployFactory.ts` around lines 31 - 33, The PRIVATE_KEY handling in
deployFactory should be consistent with the trimmed account/env inputs used
elsewhere. Trim the process.env.PRIVATE_KEY value before passing it into
privateKeyToAccount in the deployer setup, and make sure the deployer
initialization path uses the sanitized string rather than the raw env var so
whitespace does not break account creation.
| const DEFAULT_ENTRY_POINT = | ||
| "0x4337084D9E255Ff0702461CF8895CE9E3b5Ff108" as `0x${string}`; | ||
|
|
||
| async function main() { | ||
| if (!process.env.PRIVATE_KEY) throw new Error("PRIVATE_KEY not set"); | ||
| if (!process.env.SEPOLIA_RPC_URL) throw new Error("SEPOLIA_RPC_URL not set"); | ||
|
|
||
| const entryPoint = (process.env.ENTRY_POINT ?? | ||
| DEFAULT_ENTRY_POINT) as `0x${string}`; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Unify the EntryPoint source across the deployment scripts.
This implementation defaults to the Sepolia v0.8 EntryPoint, but the repo still has scripts/stakePlatformPaymaster.ts documented for EntryPoint v0.7. Running both flows against the same deployment will wire the paymaster to one EntryPoint and stake/fund it on another, which breaks sponsorship at runtime. Please centralize this value or require it explicitly everywhere.
🤖 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 `@scripts/deployImplementation.ts` around lines 21 - 29, The deployment flow is
using a default EntryPoint in `deployImplementation` that can diverge from the
one used by `stakePlatformPaymaster`, causing the paymaster to be
deployed/staked against different EntryPoints. Centralize the EntryPoint
selection in a shared constant or config used by both scripts, or require
`ENTRY_POINT` explicitly in both `main` functions so `DEFAULT_ENTRY_POINT`
cannot silently drift from the paymaster staking path.
| function main() { | ||
| if (fs.existsSync(OUT_DIR)) fs.rmSync(OUT_DIR, { recursive: true }); | ||
| fs.mkdirSync(OUT_DIR, { recursive: true }); | ||
|
|
||
| const indexLines: string[] = ["// Auto-generated by scripts/generate-abis.ts — do not edit manually\n"]; | ||
|
|
||
| for (const { sol, contract, exportName, typeName } of CONTRACTS_TO_EXPORT) { | ||
| const artifactPath = path.join(ARTIFACTS_DIR, sol, `${contract}.json`); | ||
|
|
||
| if (!fs.existsSync(artifactPath)) { | ||
| console.error(`[generate-abis] Missing artifact: ${artifactPath}`); | ||
| console.error(" → Run `npm run build:sol` first."); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Validate artifacts before deleting src/abis.
This removes the generated sources first and only then checks whether each artifact exists. If one artifact is missing, src/index.ts still re-exports ./abis, so the repo is left in a broken state until generation is rerun. Validate all artifact paths up front, or write into a temp directory and swap on success.
🤖 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 `@scripts/generate-abis.ts` around lines 47 - 60, The main flow in main()
deletes OUT_DIR before verifying all CONTRACTS_TO_EXPORT artifacts exist, which
can leave src/index.ts pointing at a missing ./abis folder if validation fails.
Move the artifact existence checks for each artifactPath ahead of the
fs.rmSync/fs.mkdirSync cleanup, or generate into a temporary directory and only
replace OUT_DIR after all artifacts are confirmed and generation succeeds. Use
main, CONTRACTS_TO_EXPORT, OUT_DIR, and artifactPath to locate the fix.
| describe("mintDocument", function () { | ||
| it("mints a document, authorizes beneficiary+holder+escrow, increments counter", async function () { | ||
| const { paymaster, paymasterAsOther, mockRegistry, other, user } = | ||
| await loadFixture(deployFixture); | ||
|
|
||
| // Authorize the registry | ||
| await paymaster.write.addRegistry([mockRegistry.address]); | ||
|
|
||
| await paymasterAsOther.write.mintDocument([ | ||
| mockRegistry.address, | ||
| user.account.address, // beneficiary | ||
| other.account.address, // holder | ||
| 1n, | ||
| "0x" as `0x${string}`, | ||
| ]); | ||
|
|
||
| expect(await paymaster.read.documentsMinted([other.account.address])).to.equal(1n); | ||
| expect(await paymaster.read.authorizedCallers([user.account.address])).to.be.true; | ||
| expect(await paymaster.read.authorizedCallers([other.account.address])).to.be.true; | ||
|
|
||
| // TitleEscrowLinked event tells us the escrow address | ||
| const events = await paymaster.getEvents.TitleEscrowLinked(); | ||
| expect(events).to.have.lengthOf(1); | ||
| const escrow = events[0].args.titleEscrow as `0x${string}`; | ||
| expect(await paymaster.read.authorizedTitleEscrows([escrow])).to.be.true; | ||
| }); | ||
|
|
||
| it("reverts for an unauthorized registry", async function () { | ||
| const { paymasterAsOther, mockRegistry, other, user } = await loadFixture(deployFixture); | ||
| await expect( | ||
| paymasterAsOther.write.mintDocument([ | ||
| mockRegistry.address, // not yet authorized | ||
| user.account.address, | ||
| other.account.address, | ||
| 1n, | ||
| "0x" as `0x${string}`, | ||
| ]), | ||
| ).to.be.rejectedWith("registry not authorized"); | ||
| }); | ||
|
|
||
| it("documentsMinted increments per caller", async function () { | ||
| const { paymaster, paymasterAsOther, mockRegistry, other, user } = | ||
| await loadFixture(deployFixture); | ||
| await paymaster.write.addRegistry([mockRegistry.address]); | ||
|
|
||
| await paymasterAsOther.write.mintDocument([ | ||
| mockRegistry.address, user.account.address, other.account.address, 1n, "0x" as `0x${string}`, | ||
| ]); | ||
| await paymasterAsOther.write.mintDocument([ | ||
| mockRegistry.address, user.account.address, other.account.address, 2n, "0x" as `0x${string}`, | ||
| ]); | ||
|
|
||
| expect(await paymaster.read.documentsMinted([other.account.address])).to.equal(2n); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add coverage for the actual sponsorship gate, not just the mapping side effects.
These tests prove that mintDocument toggles authorizedCallers, but they never prove that _validatePaymasterUserOp now accepts those callers and rejects everyone else. A bug in the new sponsorship check would still leave this suite green.
🤖 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/PlatformPaymaster.ts` around lines 285 - 339, The mintDocument tests
only verify mapping updates and event side effects, so add coverage for the
actual sponsorship gate in _validatePaymasterUserOp. Extend the existing
mintDocument suite in PlatformPaymaster to assert that a user and holder
authorized by mintDocument are accepted by the paymaster validation path, and
that an unrelated caller is still rejected. Use the existing paymaster,
paymasterAsOther, authorizedCallers, and _validatePaymasterUserOp flow so the
test proves the new sponsorship check works end to end.
- semantic-release: add dev branch as beta prerelease channel - release.yml: trigger workflow on dev pushes in addition to main - README: full project documentation
There was a problem hiding this comment.
🧹 Nitpick comments (3)
README.md (3)
96-99: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a security warning about
.envand private keys.The setup instructions tell users to copy
.env.exampleto.envand fill inPRIVATE_KEY, but there is no warning that.envmust be git-ignored and that private keys should never be committed. This is a common foot-gun for developers new to the project.cp .env.example .env # fill in PRIVATE_KEY, SEPOLIA_RPC_URL, PIMLICO_API_KEY + +# ⚠️ Never commit .env — it contains your private key!🤖 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 96 - 99, Add a clear security warning in the setup instructions near the `.env` setup step: explicitly state that `.env` must remain git-ignored and that `PRIVATE_KEY` (and any other secrets) should never be committed or shared. Update the README guidance around the `cp .env.example .env` step so users know to keep secrets local and verify `.env` is excluded from version control.
133-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify prerequisites for gasless minting.
The "Mint a document gaslessly" section only provides a command with no context on what environment variables or prior steps are required. Users need at least
PAYMASTER_ADDRESS,EIP7702_IMPL_ADDRESS, and likely the Pimlico API key already configured. Consider adding a brief note or sub-bullet listing the required env vars for this step.🤖 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 133 - 137, The “Mint a document gaslessly” section only shows the hardhat command and needs the missing setup context. Update the README section for this step to mention the required prerequisites, including the environment variables needed by scripts/mintDocumentGasless.ts such as PAYMASTER_ADDRESS, EIP7702_IMPL_ADDRESS, and the Pimlico API key, and note any prior configuration steps users must complete before running the command.
139-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument additional environment variables used by deployment scripts.
The environment variables table is missing several variables that the deployment scripts consume, which will cause runtime errors if users follow only the documented table:
PLATFORM_ADDRESS— defaults to deployer address indeployPlatformPaymaster.tsDAILY_LIMIT_ETH— defaults to"0"(unlimited) indeployPlatformPaymaster.tsDEPLOY_SALT— auto-generated if omitted indeployPlatformPaymaster.tsSTAKE_AMOUNT_ETH— defaults to"0.01"instakePlatformPaymaster.tsDEPOSIT_AMOUNT_ETH— defaults to"0.05"instakePlatformPaymaster.tsUNSTAKE_DELAY_SEC— defaults to"86400"instakePlatformPaymaster.tsAdd these to the table with their descriptions and default values so users know what to configure.
🤖 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, The environment variables table in README is missing several values used by the deployment scripts, so update the table to include PLATFORM_ADDRESS, DAILY_LIMIT_ETH, DEPLOY_SALT, STAKE_AMOUNT_ETH, DEPOSIT_AMOUNT_ETH, and UNSTAKE_DELAY_SEC. Refer to the deployment logic in deployPlatformPaymaster.ts and stakePlatformPaymaster.ts when adding each entry, and make sure the descriptions include the documented defaults or behavior (for example, deployer address fallback, “0” unlimited, auto-generated salt, and the stake/deposit/delay defaults).
🤖 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.
Nitpick comments:
In `@README.md`:
- Around line 96-99: Add a clear security warning in the setup instructions near
the `.env` setup step: explicitly state that `.env` must remain git-ignored and
that `PRIVATE_KEY` (and any other secrets) should never be committed or shared.
Update the README guidance around the `cp .env.example .env` step so users know
to keep secrets local and verify `.env` is excluded from version control.
- Around line 133-137: The “Mint a document gaslessly” section only shows the
hardhat command and needs the missing setup context. Update the README section
for this step to mention the required prerequisites, including the environment
variables needed by scripts/mintDocumentGasless.ts such as PAYMASTER_ADDRESS,
EIP7702_IMPL_ADDRESS, and the Pimlico API key, and note any prior configuration
steps users must complete before running the command.
- Around line 139-151: The environment variables table in README is missing
several values used by the deployment scripts, so update the table to include
PLATFORM_ADDRESS, DAILY_LIMIT_ETH, DEPLOY_SALT, STAKE_AMOUNT_ETH,
DEPOSIT_AMOUNT_ETH, and UNSTAKE_DELAY_SEC. Refer to the deployment logic in
deployPlatformPaymaster.ts and stakePlatformPaymaster.ts when adding each entry,
and make sure the descriptions include the documented defaults or behavior (for
example, deployer address fallback, “0” unlimited, auto-generated salt, and the
stake/deposit/delay defaults).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8424238-bbd5-4125-b5a4-4a81c3934e0b
📒 Files selected for processing (4)
.github/workflows/release.yml.gitignoreREADME.mdpackage.json
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/release.yml
- package.json
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
30-48: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winHarden the release install step
npm ciwill run install hooks from locked dependencies (esbuild,keccak,secp256k1, etc.) inside a job with write permissions. Match the other workflows and usenpm ci --ignore-scriptshere.🤖 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/release.yml around lines 30 - 48, The release workflow’s install step currently runs dependency hooks during npm ci, which is inconsistent with the other workflows and increases risk in a write-enabled job. Update the install command in the release job to use npm ci --ignore-scripts, keeping the existing setup around actions/checkout, actions/cache, and actions/setup-node unchanged.
🧹 Nitpick comments (5)
.env.example (1)
29-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueQuote string value containing spaces.
REMARK=optional remark textshould be quoted to prevent parsing ambiguities with certain dotenv parsers or special characters.-REMARK=optional remark text +REMARK="optional remark text"🤖 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 at line 29, The REMARK example in the .env.example snippet is an unquoted string with spaces, which can be parsed inconsistently by dotenv tools. Update the REMARK entry to use a quoted value in the example so it is unambiguous and matches the expected formatting for spaced strings..github/workflows/release.yml (1)
35-42: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache npm’s package store instead of
node_modules.npm ciinstalls into the workspace and clearsnode_modules, so this cache won’t help;setup-nodewithcache: npmis the better fit.🤖 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/release.yml around lines 35 - 42, The caching step in the release workflow is targeting node_modules, but this does not help with npm ci because it recreates the workspace install and clears that directory. Update the cache strategy in the release workflow to use npm’s package store via setup-node with cache: npm, and remove the actions/cache@v4 node_modules cache so the workflow uses the built-in npm cache instead.scripts/lib/network.ts (2)
38-43: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd address-format validation to
getEnvconsumers.Callers cast the raw string straight to
`0x${string}`with no validation. A malformed.envvalue fails with an opaque viem error deep in the script rather than a clear message here.♻️ Suggested addition
+import { isAddress } from "viem"; + export function getEnv(suffix: string, name: string, required = true): string { const key = `${name}_${suffix}`; const val = process.env[key]?.trim(); if (!val && required) throw new Error(`${key} is not set in .env`); return val ?? ""; } + +/** Read and validate a network-specific address env var. */ +export function getAddressEnv(suffix: string, name: string): `0x${string}` { + const val = getEnv(suffix, name); + if (!isAddress(val)) throw new Error(`${name}_${suffix} is not a valid address: "${val}"`); + return val as `0x${string}`; +}🤖 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 `@scripts/lib/network.ts` around lines 38 - 43, The raw env lookup in getEnv is being cast directly to an address type by its callers without validation, so add explicit address-format checking at the consumer sites before the `0x${string}` cast. Update the scripts that call getEnv to validate the returned value with the existing address parsing/validation flow and throw a clear local error when the value is malformed, rather than letting viem fail later.
1-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider centralizing the duplicated EIP-7702 delegation/bootstrap flow.
The "check current delegate → redelegate if needed →
to7702SimpleSmartAccount→createSmartAccountClient" block is repeated almost identically indeployRegistryGasless.ts,mintDocumentGasless.ts, andtrFunctions/_setup.ts'sbuildClient(). Since this file already centralizes network/env resolution, it's a reasonable home for a shared bootstrap helper to reduce triplicated logic.🤖 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 `@scripts/lib/network.ts` around lines 1 - 43, The EIP-7702 bootstrap flow is duplicated across deployRegistryGasless.ts, mintDocumentGasless.ts, and trFunctions/_setup.ts buildClient(), so move the shared “check current delegate → redelegate if needed → to7702SimpleSmartAccount → createSmartAccountClient” logic into a reusable helper in scripts/lib/network.ts or a nearby shared module. Expose a small function that accepts the network config and account/client inputs, returns the configured smart account client, and have the three call sites use it instead of inlining the sequence. Keep network/env resolution in getNetworkConfig and getEnv, but centralize the repeated delegation setup behind one named helper to avoid drift.scripts/trFunctions/_setup.ts (1)
5-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument
TITLE_ESCROW_ADDRESS_<NETWORK>env var.
getTRContract(suffix)(Line 119) requiresTITLE_ESCROW_ADDRESS_<suffix>viagetEnv, but the header env-var list omits it, unlikePAYMASTER_ADDRESS_<NETWORK>which is documented.📝 Proposed doc fix
// PAYMASTER_ADDRESS_<NETWORK> — deployed PlatformPaymaster (v0.8 EntryPoint) +// TITLE_ESCROW_ADDRESS_<NETWORK> — target TitleEscrow contract for tr* scripts // PRIVATE_KEY — funded wallet (pays gas for delegation tx if needed)🤖 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 `@scripts/trFunctions/_setup.ts` around lines 5 - 10, The env-var header in _setup.ts is missing TITLE_ESCROW_ADDRESS_<NETWORK>, even though getTRContract(suffix) depends on TITLE_ESCROW_ADDRESS_<suffix> via getEnv. Update the documented environment variable list near the top of the file to include TITLE_ESCROW_ADDRESS_<NETWORK>, matching the style used for PAYMASTER_ADDRESS_<NETWORK>, so users can configure the required escrow address for each network.
🤖 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:
- Line 71: Remove the duplicated FACTORY_ADDRESS_AMOY entry from the “Filled in
after running gasless scripts” section in .env.example so it is defined only
once in the deployments section, matching the Sepolia pattern; keep only
REGISTRY_ADDRESS_AMOY and TITLE_ESCROW_ADDRESS_AMOY there, and preserve the
existing FACTORY_ADDRESS_AMOY value defined earlier.
In @.github/workflows/release.yml:
- Line 50: The release workflow is running semantic-release on a branch that is
not configured for releases. Update the semantic-release configuration to
include the dev branch, or change the release job in the workflow so the
semantic-release step only runs on branches already configured for release; use
the semantic-release invocation in the release workflow to locate the change.
---
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 30-48: The release workflow’s install step currently runs
dependency hooks during npm ci, which is inconsistent with the other workflows
and increases risk in a write-enabled job. Update the install command in the
release job to use npm ci --ignore-scripts, keeping the existing setup around
actions/checkout, actions/cache, and actions/setup-node unchanged.
---
Nitpick comments:
In @.env.example:
- Line 29: The REMARK example in the .env.example snippet is an unquoted string
with spaces, which can be parsed inconsistently by dotenv tools. Update the
REMARK entry to use a quoted value in the example so it is unambiguous and
matches the expected formatting for spaced strings.
In @.github/workflows/release.yml:
- Around line 35-42: The caching step in the release workflow is targeting
node_modules, but this does not help with npm ci because it recreates the
workspace install and clears that directory. Update the cache strategy in the
release workflow to use npm’s package store via setup-node with cache: npm, and
remove the actions/cache@v4 node_modules cache so the workflow uses the built-in
npm cache instead.
In `@scripts/lib/network.ts`:
- Around line 38-43: The raw env lookup in getEnv is being cast directly to an
address type by its callers without validation, so add explicit address-format
checking at the consumer sites before the `0x${string}` cast. Update the scripts
that call getEnv to validate the returned value with the existing address
parsing/validation flow and throw a clear local error when the value is
malformed, rather than letting viem fail later.
- Around line 1-43: The EIP-7702 bootstrap flow is duplicated across
deployRegistryGasless.ts, mintDocumentGasless.ts, and trFunctions/_setup.ts
buildClient(), so move the shared “check current delegate → redelegate if needed
→ to7702SimpleSmartAccount → createSmartAccountClient” logic into a reusable
helper in scripts/lib/network.ts or a nearby shared module. Expose a small
function that accepts the network config and account/client inputs, returns the
configured smart account client, and have the three call sites use it instead of
inlining the sequence. Keep network/env resolution in getNetworkConfig and
getEnv, but centralize the repeated delegation setup behind one named helper to
avoid drift.
In `@scripts/trFunctions/_setup.ts`:
- Around line 5-10: The env-var header in _setup.ts is missing
TITLE_ESCROW_ADDRESS_<NETWORK>, even though getTRContract(suffix) depends on
TITLE_ESCROW_ADDRESS_<suffix> via getEnv. Update the documented environment
variable list near the top of the file to include
TITLE_ESCROW_ADDRESS_<NETWORK>, matching the style used for
PAYMASTER_ADDRESS_<NETWORK>, so users can configure the required escrow address
for each network.
🪄 Autofix (Beta)
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
Run ID: 8b9e8174-9775-41a2-86cc-f416d71d459d
📒 Files selected for processing (34)
.env.example.github/workflows/linters.yml.github/workflows/release.yml.github/workflows/tests.ymlEIP7702_METAMASK_ARCHITECTURE.mdPIMLICO_EIP7702_DOCS.mdcontracts/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.solhardhat.config.tsscripts/deployEIP7702.tsscripts/deployFactory.tsscripts/deployImplementation.tsscripts/deployPlatformPaymaster.tsscripts/deployRegistryGasless.tsscripts/lib/network.tsscripts/mintDocumentGasless.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.ts
💤 Files with no reviewable changes (12)
- contracts/Etherspot/interfaces/PackedUserOperation.sol
- contracts/Etherspot/interfaces/IAggregator.sol
- contracts/Etherspot/interfaces/IPaymaster.sol
- contracts/Etherspot/interfaces/INonceManager.sol
- contracts/Etherspot/interfaces/IStakeManager.sol
- PIMLICO_EIP7702_DOCS.md
- contracts/Etherspot/EtherspotPaymaster.sol
- contracts/Etherspot/core/UserOperationLib.sol
- contracts/Etherspot/BasePaymaster.sol
- contracts/Etherspot/core/Helpers.sol
- contracts/Etherspot/interfaces/IEntryPoint.sol
- EIP7702_METAMASK_ARCHITECTURE.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/tests.yml
- .github/workflows/linters.yml
- scripts/deployImplementation.ts
- scripts/deployFactory.ts
| # Filled in after running gasless scripts with NETWORK=amoy | ||
| REGISTRY_ADDRESS_AMOY=0x_filled_after_deployRegistry | ||
| TITLE_ESCROW_ADDRESS_AMOY=0x_filled_after_mintDocument | ||
| FACTORY_ADDRESS_AMOY=0x_filled_after_deployFactory |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Remove duplicated FACTORY_ADDRESS_AMOY key.
Line 61 already defines FACTORY_ADDRESS_AMOY=0x_filled_by_deployFactory. The duplicate at line 71 with a different placeholder value will overwrite the first definition. Following the Sepolia pattern (lines 43–54), the factory address should only appear once in the deployments section; the "Filled in after running gasless scripts" section should contain only REGISTRY_ADDRESS_AMOY and TITLE_ESCROW_ADDRESS_AMOY.
-# Filled in after running gasless scripts with NETWORK=amoy
-REGISTRY_ADDRESS_AMOY=0x_filled_after_deployRegistry
-TITLE_ESCROW_ADDRESS_AMOY=0x_filled_after_mintDocument
-FACTORY_ADDRESS_AMOY=0x_filled_after_deployFactory
+# Filled in after running gasless scripts with NETWORK=amoy
+REGISTRY_ADDRESS_AMOY=0x_filled_after_deployRegistry
+TITLE_ESCROW_ADDRESS_AMOY=0x_filled_after_mintDocument📝 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.
| FACTORY_ADDRESS_AMOY=0x_filled_after_deployFactory | |
| # Filled in after running gasless scripts with NETWORK=amoy | |
| REGISTRY_ADDRESS_AMOY=0x_filled_after_deployRegistry | |
| TITLE_ESCROW_ADDRESS_AMOY=0x_filled_after_mintDocument |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 71-71: [DuplicatedKey] The FACTORY_ADDRESS_AMOY key is duplicated
(DuplicatedKey)
[warning] 71-71: [UnorderedKey] The FACTORY_ADDRESS_AMOY key should go before the REGISTRY_ADDRESS_AMOY key
(UnorderedKey)
🤖 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 at line 71, Remove the duplicated FACTORY_ADDRESS_AMOY entry
from the “Filled in after running gasless scripts” section in .env.example so it
is defined only once in the deployments section, matching the Sepolia pattern;
keep only REGISTRY_ADDRESS_AMOY and TITLE_ESCROW_ADDRESS_AMOY there, and
preserve the existing FACTORY_ADDRESS_AMOY value defined earlier.
|
|
||
| - run: npm ci | ||
| - run: npm run build | ||
| - run: npm run semantic-release |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n --glob 'package.json' --glob '.releaserc*' --glob 'release.config.*' 'semantic-release|branches|dev|main'Repository: TrustVC/7702SmartAccount
Length of output: 162
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== top-level files =="
git ls-files | sed -n '1,200p'
echo
echo "== semantic-release related files =="
git ls-files | rg '(^|/)(package\.json|\.releaserc(\.[^/]+)?|release\.config\.[^/]+|.*release.*\.yml|.*release.*\.yaml)$'
echo
echo "== search for release config terms =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' 'semantic-release|release\.branches|branches\s*:' .
echo
echo "== workflow snippet =="
sed -n '1,140p' .github/workflows/release.ymlRepository: TrustVC/7702SmartAccount
Length of output: 8449
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== semantic-release related files =="
git ls-files | rg '(^|/)(package\.json|\.releaserc(\.[^/]+)?|release\.config\.[^/]+|.*release.*\.yml|.*release.*\.yaml)$' || true
echo
echo "== search for release config terms =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' 'semantic-release|release\.branches|branches\s*:' . || true
echo
echo "== workflow snippet =="
sed -n '1,140p' .github/workflows/release.ymlRepository: TrustVC/7702SmartAccount
Length of output: 6610
Restrict semantic-release to configured branches
.github/workflows/release.yml triggers on dev, but there’s no repo-level semantic-release branches config for it. Either add dev to the release config or stop running this job on that branch.
🤖 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/release.yml at line 50, The release workflow is running
semantic-release on a branch that is not configured for releases. Update the
semantic-release configuration to include the dev branch, or change the release
job in the workflow so the semantic-release step only runs on branches already
configured for release; use the semantic-release invocation in the release
workflow to locate the change.
1 participant
Summary by CodeRabbit
mainanddev.7702Frontendapp and multiple legacy interaction/Etherspot contract components.