feat: @towns-protocol/smart-account package#4792
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
miguel-nascimento
force-pushed
the
miguel/smart-account-package
branch
from
b9fbe5a to
f56730f
Compare
miguel-nascimento
force-pushed
the
miguel/smart-account-package
branch
from
f56730f to
563a82b
Compare
miguel-nascimento
requested review from
Tee-S,
sergekh2,
shuhuiluo and
texuf
as code owners
PR SummaryAdds ERC-4337 support end-to-end and a new SDK for modular smart accounts.
Written by Cursor Bugbot for commit a06a572. This will update automatically on new commits. Configure here. |
📝 WalkthroughWalkthroughAdds a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In `@core/justfile`:
- Around line 204-214: The anvil-base-with-alto recipe's while loop that checks
EntryPoint deployment can hang indefinitely; add a timeout mechanism by
introducing a max-wait variable and a counter (or use the timeout command)
inside the anvil-base-with-alto recipe around the existing while ! cast code ...
loop, increment the counter each iteration, and if the max is exceeded print a
clear error ("EntryPoint not deployed after Xs") and exit non-zero instead of
looping forever; ensure the change touches the anvil-base-with-alto recipe and
the specific while ! cast 0x0000000071727De22E5E9d8BAf0edAc6f37da032 ... loop so
deployment failure cleanly aborts the script.
In `@packages/anvil-docker/Dockerfile`:
- Around line 35-36: Remove the global Alto bundler installation from the
builder stage by deleting the RUN bun install -g `@pimlico/alto`@0.0.20
instruction in the Dockerfile builder stage; ensure the runtime stage still
installs or retains `@pimlico/alto`@0.0.20 so Alto is available at runtime but not
during build, and verify no other setup scripts (e.g., setup.sh) depend on the
builder having the bundler.
In `@packages/bot/src/bot.test.ts`:
- Around line 2490-2522: The UserOperation sent via
bundlerClient.sendUserOperation for the installApp call must include explicit
gas limit fields; update the userOp in the bot can be installed in DM channel
test (the bundlerClient.sendUserOperation invocation that targets
appRegistryDapp.installer.address and functionName 'installApp' with
bobSmartAccount) to add explicit gas values such as gasLimit,
verificationGasLimit and preVerificationGas (match the pattern used by other
UserOps in this file) so the modular account UserOp always supplies required gas
parameters to the bundler.
In `@packages/smart-account/AGENTS.md`:
- Around line 1-6: Add an "Agents" section to AGENTS.md (or explicitly state "No
agents defined") that clearly documents whether this package defines any agents
and, if it does, lists each agent's responsibilities and behavior; for each
agent include its name, triggers/entry points, expected inputs and outputs, side
effects (e.g., on-chain interactions via viem), and failure/retry semantics so
consumers know how to use them. If there are no agents, add a short paragraph
"No agents defined for this package" and a note explaining that agent-like
behavior is handled by the modular account/plugin architecture (referencing
Modular Accounts / EntryPoint v0.7). Keep the section brief and consistent with
the existing README tone.
In `@packages/smart-account/src/abis/modularFactoryAbi.ts`:
- Around line 387-392: The ABI entry for the function renounceOwnership
incorrectly sets stateMutability to 'view'; update the ABI object for the
function named "renounceOwnership" so its stateMutability is "nonpayable" (leave
name, inputs, and outputs unchanged) to allow state modification per the Ownable
pattern.
In `@packages/smart-account/src/create2/nativeSMASigner.ts`:
- Around line 31-38: In pack1271Signature, add a uint32 bounds check for the
entityId parameter before calling toHex: verify it's an integer and 0 <=
entityId <= 0xFFFFFFFF (4294967295) and throw a clear error (e.g., RangeError)
if it fails; this prevents toHex from silently producing oversized output and
ensures the returned signature is exactly 4 bytes for the entityId field.
In `@packages/smart-account/src/create2/toModularSmartAccount.ts`:
- Around line 173-201: The comment block contains a real-looking hardcoded
private key string (PRIVATE_KEY = '0x1ffeb42d...') which trips secret scanners;
remove or redact that exact literal from the comments in
toModularSmartAccount.ts and replace it with a non-secret placeholder (e.g.,
PRIVATE_KEY = '<redacted>' or PRIVATE_KEY = '0x...') and add a brief note to
rotate the key if it was ever used; ensure references to getSenderAddress and
getInitData remain intact but contain no real secrets.
- Around line 310-312: isAccountDeployed incorrectly coerces the getCode result
with !! which treats '0x' (no-code marker) as truthy; change isAccountDeployed
to first await the code via getAction(getCode,'getCode') into a local variable
(e.g., code) and then return a precise check that code is neither undefined nor
the empty-bytecode string (e.g., code !== undefined && code !== '0x'), mirroring
the check used in src/id/discoverAccount.ts; update the async isAccountDeployed
method to use that explicit comparison so addresses with no deployed bytecode
return false.
In `@packages/smart-account/src/create2/toSimpleSmartAccount.ts`:
- Around line 105-121: In encodeCalls (the function that builds calldata using
simpleAccountAbi and chooses between 'execute' and 'executeBatch'), add
validation for batch requests: when calls.length > 1, scan calls for any
call.value that is non-zero (e.g., not 0n or > 0) and throw a clear error (e.g.,
Error or TypeError) stating that executeBatch does not support value transfers;
only proceed to encodeFunctionData for 'executeBatch' if all call.value are
zero. Ensure the check runs before constructing the args for executeBatch so no
value-bearing batch can be encoded.
In `@packages/smart-account/src/create2/toTownsSmartAccount.ts`:
- Around line 35-45: The forced address branch in toTownsSmartAccount currently
defaults preferredType to 'modular' and may choose the wrong implementation;
update the function to validate when config.address (forceAddress) is present
that config.preferredType is explicitly provided and is one of the allowed
values (e.g., 'modular' or 'simple'), and throw a clear error if it's
missing/invalid instead of falling back to 'modular'; adjust the early logic
around the destructured variables ({ client, owner, preferredType, address:
forceAddress }), and ensure the code routes to toModularSmartAccount or
toSimpleSmartAccount only after this validation (you can still call
discoverAccount only when forceAddress is absent).
In `@packages/smart-account/src/id/discoverAccount.ts`:
- Around line 18-40: The discovery uses the factory call getAddressSemiModular
but then returns type 'modular', causing a mismatch; update the factory read in
discoverAccount.ts (the client.readContract call using getAddressSemiModular) to
call the correct factory method for full modular accounts (use getAddress with
args [owner, 0n, 0n]) if you intend to detect modular accounts, or alternatively
keep getAddressSemiModular and change the returned type to 'semiModular'
wherever discoverAccount returns { type: ... } (refer to getAddressSemiModular,
getAddress, modularAddr and the return blocks) so the computed address and
reported type are consistent.
In `@packages/smart-account/test/userop.test.ts`:
- Around line 22-30: The test currently casts
process.env.BASE_ADDRESSES_SPACE_FACTORY to Address (SPACE_FACTORY_ADDRESS)
which hides missing envs and causes opaque failures later; update the
initialization to explicitly read the env into a variable and immediately fail
fast if it's undefined or empty (throw an Error or call process.exit with a
clear message) before continuing, referencing the constant SPACE_FACTORY_ADDRESS
so callers still use the same symbol but now get a clear error when
BASE_ADDRESSES_SPACE_FACTORY is not set; ensure the failure message includes the
env name for quick debugging.
🧹 Nitpick comments (6)
packages/smart-account/src/constants.ts (1)
1-16: Consider per-chain overrides for hardcoded addresses.
If these are not universal across all supported networks, add a chain map or override hook to avoid misrouting deployments.core/justfile (1)
151-198: Consider extracting shared constants to reduce duplication.
ANVIL_PRIVATE_KEY,ENTRYPOINT_V06, andENTRYPOINT_V07are duplicated between_alto-startandalto-run. Extract these to variables at the top of the file or a shared location.Note: The static analysis flagging the private key is a false positive — this is Anvil's well-known first test key, which is public by design.
♻️ Suggested refactor
Add near the top of the file (after line 13):
# Anvil default test private key (public, for local dev only) ANVIL_PRIVATE_KEY := "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" ENTRYPOINT_V06 := "0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789" ENTRYPOINT_V07 := "0x0000000071727De22E5E9d8BAf0edAc6f37da032"Then reference these in the recipes.
packages/anvil-docker/scripts/run.sh (1)
73-80: Cleanup trap passes wrong exit code on normal exit.When the script exits normally via
wait $BASE_PIDcompleting, the trap'cleanup $?'passes the exit status ofwait, which could mask signal-based exits. Consider usingcleanup 0for EXIT trap specifically.♻️ Suggested improvement
-trap 'cleanup $?' SIGINT SIGTERM EXIT +trap 'cleanup 130' SIGINT +trap 'cleanup 143' SIGTERM +trap 'cleanup 0' EXITscripts/deploy-base-contracts.sh (1)
98-115: Add error handling for factory owner retrieval.If
cast callfails or returns an unexpected value, subsequent commands may behave unexpectedly. The static analysis flag is a false positive (well-known Anvil test key).🛠️ Suggested improvement
ANVIL_PRIVATE_KEY=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80 FACTORY_OWNER=$(cast call $MODULAR_ACCOUNT_FACTORY_ADDRESS "owner()(address)" --rpc-url $BASE_ANVIL_RPC_URL) +if [ -z "$FACTORY_OWNER" ] || [ "$FACTORY_OWNER" = "0x0000000000000000000000000000000000000000" ]; then + echo "Error: Failed to get factory owner or owner is zero address" + exit 1 +fi echo "Factory owner: $FACTORY_OWNER"packages/smart-account/src/id/detectAccountType.ts (1)
18-20: Consider handling zero address explicitly.When the storage slot contains the zero address (uninitialized proxy or no implementation),
implAddresswill be0x0000000000000000000000000000000000000000. This passes the!impl || impl === '0x'check but won't match any known implementation, correctly returningnull. However, an explicit check improves readability.Optional improvement
if (!impl || impl === '0x') return null const implAddress = ('0x' + impl.slice(-40)) as Address + if (implAddress === '0x0000000000000000000000000000000000000000') return nullpackages/smart-account/src/create2/toModularSmartAccount.ts (1)
141-148: Make chainId memoization robust to falsy values.Use an undefined check to avoid re-fetching if chainId were ever 0.
♻️ Suggested tweak
- let chainId: number + let chainId: number | undefined const getMemoizedChainId = async () => { - if (chainId) return chainId + if (chainId !== undefined) return chainId chainId = client.chain ? client.chain.id : await getAction(client, getChainId, 'getChainId')({})
| async isAccountDeployed() { | ||
| return !!(await getAction(client, getCode, 'getCode')({ address: accountAddress })) | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
viem getCode return value empty bytecode
💡 Result:
Common reasons viem's getCode returns empty (undefined or "0x") and how to check/fix:
- Address is an EOA (not a contract) — getCode returns undefined/0x for EOAs. Check transactions/create txs to confirm. [1][2]
- Wrong chain / network mismatch — you queried a different chain than where the contract is deployed. Verify client.chain or provider and the contract's chain. [1]
- Contract not yet deployed or deployed at different address — confirm deployment receipt and use the exact deployed address.
- Contract was self-destructed — a destroyed contract returns no bytecode. Check historical block (use blockNumber/blockTag) to read code at an earlier block. [1]
- ENS/name resolution or address formatting issue — ensure address is a checksummed 0x... address and not an unresolved ENS string. Resolve ENS first if needed.
- Using the wrong blockTag / non-archive node — reading past-state bytecode requires specifying blockNumber and the node must support archive queries. [1]
- RPC/provider limitations or rate-limiting — some RPCs may return empty on errors; try another provider or check RPC error logs.
Quick debug commands (viem publicClient):
- Read latest code: await publicClient.getCode({ address: '0x...' }) — returns Hex | undefined. [1]
- Read at specific block: await publicClient.getCode({ address: '0x...', blockNumber: 12345678n }) if you expect code at a past block. [1]
If you want, paste the address + chain (and blockTag if relevant) and I’ll check and suggest the exact cause.
Sources: viem docs (getCode) and Ethereum discussion on empty getCode for EOAs/causes. [1][2]
References
[1] viem getCode docs — returns bytecode or undefined; supports blockNumber/blockTag; JSON-RPC eth_getCode.
[2] Ethereum answers: getCode returns 0x for EOAs / explanation of causes.
🌐 Web query:
viem getCode function return type "0x" undefined hex
💡 Result:
Short answer: viem's getCode can return a Hex string or undefined. In practice RPC eth_getCode returns "0x" for an EOA (no contract code); viem may represent "no bytecode" as either the literal "0x" hex or undefined depending on provider/response parsing. (viem.sh)
What to check and how to handle it
- Confirm you're calling publicClient.getCode({ address }) (viem). (viem.sh)
- Confirm the provider/node is fully synced and the address is valid (fast-sync or wrong blockTag can return empty/undefined). (ethereum.stackexchange.com)
- Normalize both cases in your code:
Example (JS/TS):
const code = await publicClient.getCode({ address });
// treat either '0x' or undefined as "no contract"
const hasContract = code && code !== '0x';
If you want, paste the exact call, provider, and the returned value and I’ll help debug further.
Citations:
- 1: https://viem.sh/docs/contract/getCode?utm_source=openai
- 2: https://viem.sh/docs/contract/getCode?utm_source=openai
- 3: https://ethereum.stackexchange.com/questions/66890/eth-getcode-always-returns-0x?utm_source=openai
🏁 Script executed:
cd packages/smart-account && rg "getCode" -A 2 -B 2Repository: towns-protocol/towns
Length of output: 1141
🏁 Script executed:
cd packages/smart-account && sed -n '1,100p' src/id/discoverAccount.ts | cat -nRepository: towns-protocol/towns
Length of output: 1844
isAccountDeployed returns true even when account has no code.
getCode returns '0x' (or undefined) for addresses with no deployed bytecode, both of which are falsy but don't evaluate to false with !!. The proposed fix only checks for '0x' and misses the undefined case. Match the existing pattern in src/id/discoverAccount.ts:
🐛 Fix
async isAccountDeployed() {
- return !!(await getAction(client, getCode, 'getCode')({ address: accountAddress }))
+ const code = await getAction(client, getCode, 'getCode')({ address: accountAddress })
+ return !!(code && code !== '0x')
},🤖 Prompt for AI Agents
In `@packages/smart-account/src/create2/toModularSmartAccount.ts` around lines 310
- 312, isAccountDeployed incorrectly coerces the getCode result with !! which
treats '0x' (no-code marker) as truthy; change isAccountDeployed to first await
the code via getAction(getCode,'getCode') into a local variable (e.g., code) and
then return a precise check that code is neither undefined nor the
empty-bytecode string (e.g., code !== undefined && code !== '0x'), mirroring the
check used in src/id/discoverAccount.ts; update the async isAccountDeployed
method to use that explicit comparison so addresses with no deployed bytecode
return false.
| client.readContract({ | ||
| address: MODULAR_ACCOUNT_FACTORY, | ||
| abi: modularFactoryAbi, | ||
| functionName: 'getAddressSemiModular', | ||
| args: [owner, 0n], | ||
| }), | ||
| ]) | ||
|
|
||
| const [simpleCode, modularCode] = await Promise.all([ | ||
| client.getCode({ address: simpleAddr }), | ||
| client.getCode({ address: modularAddr }), | ||
| ]) | ||
|
|
||
| if (modularCode && modularCode !== '0x') { | ||
| return { address: modularAddr, type: 'modular', deployed: true } | ||
| } | ||
| if (simpleCode && simpleCode !== '0x') { | ||
| return { address: simpleAddr, type: 'simple', deployed: true } | ||
| } | ||
|
|
||
| return { | ||
| address: preferredType === 'modular' ? modularAddr : simpleAddr, | ||
| type: preferredType, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the discoverAccount.ts file
fd "discoverAccount.ts" --type fRepository: towns-protocol/towns
Length of output: 112
🏁 Script executed:
# Search for factory contract definitions
fd ".*[Ff]actory.*" packages/contracts --type f -name "*.sol"Repository: towns-protocol/towns
Length of output: 294
🏁 Script executed:
# Search for getAddressSemiModular and getAddress method definitions
rg -n "function\s+(getAddressSemiModular|getAddress)" --type sol -A 3Repository: towns-protocol/towns
Length of output: 91
🏁 Script executed:
# Read the discoverAccount.ts file
cat -n packages/smart-account/src/id/discoverAccount.tsRepository: towns-protocol/towns
Length of output: 1844
🏁 Script executed:
# Search for factory contracts
fd "Factory" packages/contracts --type f | head -20Repository: towns-protocol/towns
Length of output: 998
🏁 Script executed:
# Search for getAddressSemiModular and getAddress in sol files
rg "getAddressSemiModular|getAddress" packages/contracts --type-add 'sol:*.sol' -t sol -A 2Repository: towns-protocol/towns
Length of output: 46
🏁 Script executed:
# Search for SimpleAccountFactory and ModularAccountFactory
fd "SimpleAccountFactory|ModularAccountFactory" packages/contracts --type fRepository: towns-protocol/towns
Length of output: 46
🏁 Script executed:
# Search for getAddressSemiModular more broadly
rg "getAddressSemiModular" packages/contractsRepository: towns-protocol/towns
Length of output: 46
🏁 Script executed:
# Search for account factory implementations
rg "function getAddress" packages/contracts --context 2Repository: towns-protocol/towns
Length of output: 46
🏁 Script executed:
# Find where the constants are defined
rg "SIMPLE_ACCOUNT_FACTORY|MODULAR_ACCOUNT_FACTORY" packages/smart-account --type tsRepository: towns-protocol/towns
Length of output: 1083
🏁 Script executed:
# Find the ABI files
fd "modularFactoryAbi|simpleFactoryAbi" packages/smart-account --type fRepository: towns-protocol/towns
Length of output: 168
🏁 Script executed:
# Check what's in the abis directory
fd "abi" packages/smart-account/src --type dRepository: towns-protocol/towns
Length of output: 96
🏁 Script executed:
# Read the modular factory ABI to see available methods
cat -n packages/smart-account/src/abis/modularFactoryAbi.tsRepository: towns-protocol/towns
Length of output: 19980
🏁 Script executed:
# Read the simple factory ABI to see available methods
cat -n packages/smart-account/src/abis/simpleFactoryAbi.tsRepository: towns-protocol/towns
Length of output: 1755
Use getAddress with entityId to discover modular accounts.
The function calls getAddressSemiModular to compute the address but returns type 'modular'. These are different account types: getAddressSemiModular computes SemiModularAccountBytecode addresses (semi-modular), while getAddress(owner, salt, entityId) computes ModularAccount addresses (modular). This mismatch will cause incorrect account type detection.
Replace line 21 with:
functionName: 'getAddress',
args: [owner, 0n, 0n],
Or return type 'semiModular' if semi-modular discovery is intended.
🤖 Prompt for AI Agents
In `@packages/smart-account/src/id/discoverAccount.ts` around lines 18 - 40, The
discovery uses the factory call getAddressSemiModular but then returns type
'modular', causing a mismatch; update the factory read in discoverAccount.ts
(the client.readContract call using getAddressSemiModular) to call the correct
factory method for full modular accounts (use getAddress with args [owner, 0n,
0n]) if you intend to detect modular accounts, or alternatively keep
getAddressSemiModular and change the returned type to 'semiModular' wherever
discoverAccount returns { type: ... } (refer to getAddressSemiModular,
getAddress, modularAddr and the return blocks) so the computed address and
reported type are consistent.
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> chore: migrating userops package to use smart account package 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> fix build
miguel-nascimento
force-pushed
the
miguel/smart-account-package
branch
from
1ae7d3f to
a06a572
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@core/justfile`:
- Around line 179-198: The justfile target alto-run embeds secret/private-key
and entrypoint literals (ANVIL_PRIVATE_KEY, ENTRYPOINT_V06, ENTRYPOINT_V07)
which triggers gitleaks; change it to read those values from environment
variables (falling back to the same names used by _alto-start or error if
missing) and/or reference an allowlist variable instead of hardcoding the
literals, updating the bunx invocation to use the env vars (e.g., use
${ANVIL_PRIVATE_KEY}, ${ENTRYPOINT_V06}, ${ENTRYPOINT_V07} sourced from the
environment) so the key is not committed.
- Around line 151-173: The embedded private key in the _alto-start justfile
target (ANVIL_PRIVATE_KEY) must be removed; update the target to read the key
from an environment variable (e.g., ANVIL_PRIVATE_KEY) and fail fast if it's not
set before launching bunx so the flags --executor-private-keys and
--utility-private-key use the env value; ensure you no longer commit any literal
key and add a short runtime check that exits with a clear error when the env var
is missing (or document an allowlist alternative).
In `@packages/anvil-docker/scripts/run.sh`:
- Around line 35-62: The readiness loops that wait on nc -z for ports 8545 and
4337 can hang indefinitely; add a MAX_WAIT (seconds) and a counter (e.g.,
WAITED=0) in both loops that increments each pass, and if WAITED >= MAX_WAIT
echo a clear error, kill the background process (use BASE_PID for the Anvil loop
and ALTO_PID for the Alto loop) and exit 1; keep the existing nc checks, sleep
1, and success echo paths unchanged so the script still reports readiness when
successful.
- Around line 8-14: Remove the hardcoded ANVIL_PRIVATE_KEY value in run.sh and
instead load it from the environment (use the ANVIL_PRIVATE_KEY env var) and
fail fast if it's not provided; update the script's ANVIL_PRIVATE_KEY reference
to read from the env and add an explicit check that exits with a clear error
message when empty, leaving the ENTRYPOINT_* variables unchanged.
In `@scripts/deploy-base-contracts.sh`:
- Around line 98-106: The script currently embeds a literal Anvil private key in
the ANVIL_PRIVATE_KEY variable (used when funding FACTORY_OWNER and calling cast
send); remove the hardcoded secret and instead load ANVIL_PRIVATE_KEY from an
environment file (e.g., .env.localhost) or an exported env var, validate it at
script start and exit with a clear error if missing, and update references
(ANVIL_PRIVATE_KEY and the cast send invocation) to use the env value;
alternatively, if you must keep the default key for local CI, add it to your
secret scanner allowlist rather than committing the literal.
♻️ Duplicate comments (8)
packages/smart-account/AGENTS.md (1)
1-6: Document agent responsibilities (or explicitly state none).AGENTS.md currently reads as package docs only. Add a brief section stating whether this package defines any agents (or note "No agents defined for this package" if applicable). Based on learnings, document agent behavior/responsibilities in AGENTS.md.
packages/bot/src/bot.test.ts (1)
2490-2522: Add explicit gas limits to the DM install UserOp.This UserOp lacks explicit gas limits unlike other operations in the file. The Alto bundler requires them for modular accounts.
Suggested fix
const userOpHash = await bundlerClient.sendUserOperation({ account: bobSmartAccount, calls: [ { to: appRegistryDapp.installer.address as `0x${string}`, abi: IAppInstallerAbi, functionName: 'installApp', args: [appAddress, bobSmartAccount.address, '0x'], value: parseEther('0.02'), }, ], + callGasLimit: 300000n, + verificationGasLimit: 500000n, + preVerificationGas: 100000n, })packages/smart-account/test/userop.test.ts (1)
22-24: Fail fast if BASE_ADDRESSES_SPACE_FACTORY is missing.
Castingundefinedto Address hides config errors.✅ Guard the env
-const SPACE_FACTORY_ADDRESS = process.env.BASE_ADDRESSES_SPACE_FACTORY as Address +const SPACE_FACTORY_ENV = process.env.BASE_ADDRESSES_SPACE_FACTORY +if (!SPACE_FACTORY_ENV) { + throw new Error('BASE_ADDRESSES_SPACE_FACTORY is required for UserOp tests') +} +const SPACE_FACTORY_ADDRESS = SPACE_FACTORY_ENV as AddressAs per coding guidelines, validate required inputs before use.
packages/smart-account/src/create2/toTownsSmartAccount.ts (1)
35-40: Require preferredType when forcing address.
Defaulting tomodularcan route a simple account to the wrong implementation.✅ Require explicit preferredType when address is provided
- const { client, owner, preferredType = 'modular', address: forceAddress } = config + const { client, owner, preferredType, address: forceAddress } = config + const resolvedType = preferredType ?? 'modular' + if (forceAddress && preferredType === undefined) { + throw new Error('preferredType is required when address is provided') + } // Discover or use forced address const discovered = forceAddress - ? { address: forceAddress, type: preferredType, deployed: false } - : await discoverAccount(client, owner.address, preferredType) + ? { address: forceAddress, type: resolvedType, deployed: false } + : await discoverAccount(client, owner.address, resolvedType)As per coding guidelines, validate configuration inputs.
packages/smart-account/src/create2/nativeSMASigner.ts (1)
31-35: ValidateentityIdis uint32 beforetoHex.
Out-of-range values yield invalid signatures.✅ Add uint32 guard
export function pack1271Signature(validationSignature: Hex, entityId: number): Hex { + if (!Number.isInteger(entityId) || entityId < 0 || entityId > 0xffffffff) { + throw new RangeError('entityId must be a uint32') + } return concat([ '0x00', toHex(entityId, { size: 4 }),As per coding guidelines, validate inputs. Please verify viem’s
toHexbehavior for oversize values.packages/smart-account/src/create2/toModularSmartAccount.ts (2)
173-179: Remove the hardcoded private key from comments.
This trips secret scanners even in comments.🧹 Redact the key
- // const PRIVATE_KEY = '0x1ffeb42d5991715177ec8bf1f06aef3a62e558d3dc6a686cc9fe23a8ab652bb2' + // const PRIVATE_KEY = '<redacted>'
310-312:isAccountDeployedtreats'0x'as deployed.
!!makes'0x'truthy; match the explicit check used elsewhere.✅ Match discoverAccount-style check
async isAccountDeployed() { - return !!(await getAction(client, getCode, 'getCode')({ address: accountAddress })) + const code = await getAction(client, getCode, 'getCode')({ address: accountAddress }) + return !!(code && code !== '0x') },Please verify viem
getCodereturn semantics on your RPC.core/justfile (1)
204-212: Timeout still missing on EntryPoint wait loop.
Same concern as prior review; add a max wait to avoid infinite hangs.
🧹 Nitpick comments (2)
packages/smart-account/AGENTS.md (1)
270-292: Add language specifier to fenced code block.The file structure code block should have a language identifier for consistency.
Suggested fix
-``` +```text /packages/smart-account/ ├── src/scripts/deploy-base-contracts.sh (1)
33-97: Fail fast if fetched bytecode is empty.
cast codereturns0xwhen the address isn’t deployed or the RPC is wrong, andanvil_setCodewill silently install empty code. Add a guard (or helper) before setting code.✅ Suggested guard (apply to each contract)
ENTRYPOINT_V07_BYTECODE=$(cast code $ENTRYPOINT_V07_ADDRESS --rpc-url $BASE_RPC_URL) +if [ "$ENTRYPOINT_V07_BYTECODE" = "0x" ]; then + echo "Missing bytecode for EntryPoint v0.7 at $ENTRYPOINT_V07_ADDRESS" + exit 1 +fi cast rpc anvil_setCode $ENTRYPOINT_V07_ADDRESS $ENTRYPOINT_V07_BYTECODE --rpc-url $BASE_ANVIL_RPC_URL
1 participant
This PR adds smart account package. It allow us to test Bot DM feature.
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com