Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Deploying happychain with
|
| Latest commit: |
9b310f1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c4e619c8.happychain.pages.dev |
| Branch Preview URL: | https://aritra-eip-5792.happychain.pages.dev |
HAPPY-163 Implement `wallet_sendCalls` and capabilities
Implement EIP-5792 (EIP) (
|
There was a problem hiding this comment.
this is a placeholder, the actual component is being written in a separate PR
apps/iframe/src/requests/approved.ts
Outdated
| }) | ||
| } | ||
|
|
||
| return userOpHash |
There was a problem hiding this comment.
we could have sent the txs sequentially in the for loop but the spec mentions we should return only a singular hash
There was a problem hiding this comment.
it also says its a call identifier to check the status of the call, and not needed to be a transactionHash/userOp hash... maybe its a hash of all included userOps? just using the last one feels a bit arbitrary to me 🤔
if its an atomicBatch as mentioned before, then this would just return a single userOp anyways right? but if its not an atomicBatch, then only having the last userOp seems not ideal as a lookup key
apps/iframe/src/requests/approved.ts
Outdated
| } | ||
|
|
||
| // extract specified paymaster address from capabilities | ||
| const boopPaymasterAddress: Address | undefined = callsData.capabilities?.boopPaymaster?.address |
There was a problem hiding this comment.
in this manner, we will also allow developers to specify whether they want txs to be sent as an atomicBatch
There was a problem hiding this comment.
how does this signal about atomicBatch?
There was a problem hiding this comment.
my note above is more towards how we'll handle it in the future once we start using the deployed boop spec 🙂
There was a problem hiding this comment.
currently the only expected capability is a specified boopPaymaster, we'll support atomic batching as well
apps/iframe/src/requests/approved.ts
Outdated
| case "wallet_showCallsStatus": { | ||
| const _boopBundleId = request.payload.params?.[0] | ||
| return undefined | ||
| } |
There was a problem hiding this comment.
this request handler doesn't particularly do anything: context here
There was a problem hiding this comment.
relevant docs: https://www.eip5792.xyz/reference/showCallsStatus
| if (!hash) { | ||
| throw new InternalRpcError(new Error("Transaction hash is missing.")) | ||
| } | ||
| const smartAccountClient = (await getSmartAccountClient()) as ExtendedSmartAccountClient |
There was a problem hiding this comment.
here, the docs suggest: "the method only returns a subset of the fields that eth_getTransactionReceipt returns"
we switch this up with the return type from -
smartAccountClient.waitForUserOperationReceipt
since that's what we primarily handle here and not 1559 txs
| return <HappyUseAbi {...props} /> | ||
| case HappyMethodNames.REQUEST_SESSION_KEY: | ||
| return <HappyRequestSessionKey {...props} /> | ||
| case "wallet_sendCalls": |
There was a problem hiding this comment.
what do you mean placeholder here 🧐
| if (!user?.controllingAddress || !provider || !transport) return | ||
|
|
||
| return createWalletClient({ account: user.controllingAddress, transport }) | ||
| return createWalletClient({ account: user.controllingAddress, transport }).extend(eip5792Actions()) |
There was a problem hiding this comment.
this will be needed in the popup for when the user calls wallet_showCallsStatus
discussion context: https://happychaindevs.slack.com/archives/C084LLRJF5M/p1739780751376089?thread_ts=1739542573.757479&cid=C084LLRJF5M
demos/react/src/App.tsx
Outdated
| id: sends, | ||
| }) | ||
|
|
||
| console.log({ status, receipts }) |
There was a problem hiding this comment.
this would really benefit from the changes from #380 😄
| return <HappyUseAbi {...props} /> | ||
| case HappyMethodNames.REQUEST_SESSION_KEY: | ||
| return <HappyRequestSessionKey {...props} /> | ||
| case "wallet_sendCalls": |
There was a problem hiding this comment.
what do you mean placeholder here 🧐
| console.log("param", { method, params }) | ||
| // TODO only for testiog |
There was a problem hiding this comment.
component is only for testing, or log is? use our logger.debug :)
| wallet_watchAsset: "Watch Asset", | ||
| [HappyMethodNames.USE_ABI]: "Record ABI", | ||
| [HappyMethodNames.REQUEST_SESSION_KEY]: "Approve Session Key", | ||
| wallet_sendCalls: "Send Calls", |
There was a problem hiding this comment.
group this alphabetically with the other wallet_ calls
apps/iframe/src/requests/approved.ts
Outdated
| const callsData = request.payload.params?.[0] | ||
| if (!callsData) throw new InternalRpcError(new Error("Invalid request payload")) | ||
|
|
||
| if (user.address !== callsData.from) { |
There was a problem hiding this comment.
is all callsData.from assumed to be the same user?
| if (user.address !== callsData.from) { | |
| if (request.payload.params.some(callsData => callsData.from !== user.address)) { |
apps/iframe/src/requests/approved.ts
Outdated
| const callsData = request.payload.params?.[0] | ||
| if (!callsData) throw new InternalRpcError(new Error("Invalid request payload")) |
There was a problem hiding this comment.
there can be multiple callsData right?
this is a good point, we should validate params is actually the shape we expect, elsewhere we are just assuming it it, but its probably a good practice to validate this. maybe lets use zod?
apps/iframe/src/requests/approved.ts
Outdated
| }) | ||
| } | ||
|
|
||
| return userOpHash |
There was a problem hiding this comment.
it also says its a call identifier to check the status of the call, and not needed to be a transactionHash/userOp hash... maybe its a hash of all included userOps? just using the last one feels a bit arbitrary to me 🤔
if its an atomicBatch as mentioned before, then this would just return a single userOp anyways right? but if its not an atomicBatch, then only having the last userOp seems not ideal as a lookup key
apps/iframe/src/requests/approved.ts
Outdated
| } | ||
|
|
||
| case "wallet_showCallsStatus": { | ||
| const _boopBundleId = request.payload.params?.[0] |
There was a problem hiding this comment.
do you even need this line 🧐
There was a problem hiding this comment.
not really, just added it there to show it's presence?
I'll remove it 👍
| data: log.data as Hex, | ||
| topics: log.topics as Hex[], | ||
| })), | ||
| status: (success ? "0x1" : "0x0") as Hex, |
There was a problem hiding this comment.
follows the spec here: https://eips.ethereum.org/EIPS/eip-5792#wallet_getcallsstatus
demos/react/src/App.tsx
Outdated
| () => user?.address && createWalletClient({ account: user.address, transport: custom(provider!) }), | ||
| [user, provider], | ||
| ) | ||
| )?.extend(eip5792Actions()) |
There was a problem hiding this comment.
shouldn't this be inside useMemo on createWalletClient?
demos/react/src/App.tsx
Outdated
| id: sends, | ||
| }) | ||
|
|
||
| console.log({ status, receipts }) |
ea3cb19 to
4cf021c
Compare
4cf021c to
9b310f1
Compare
|
closing, replicated in graphite stack here |

Linked Issues
Description
docs ref
This PR implements request handlers within the iframe for the specified RPC requests that make a wallet EIP-5792 compliant.
Currently,
wallet_sendCallsonly successfully handles sending one userOp since we don't support batching yet (spec in progress)We define a new
boopPaymastercapability in which devs can specify the address of a deployed paymaster and we use that in thesendUserOphandler.Toggle Checklist
Checklist
Basics
norswap/build-system-caching).Reminder: PR review guidelines
Correctness
testnet, mainnet, standalone wallet, ...).
< INDICATE BROWSER, DEMO APP & OTHER ENV DETAILS USED FOR TESTING HERE >
< INDICATE TESTED SCENARIOS (USER INTERFACE INTERACTION, CODE FLOWS) HERE >
and have updated the code & comments accordingly.
Architecture & Documentation
(2) commenting these boundaries correctly, (3) adding inline comments for context when needed.
comments.
in a Markdown document.
pacakges/coreandpackages/react), see here for more info.