feat: jit spend permissions #1611
Merged
+2,545
−223
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Heimdall Review Status
|
stephancill
force-pushed
the
feat/jit-spend-permissions
branch
from
c2348c8 to
df64701
Compare
stephancill
commented
Apr 24, 2025
stephancill
force-pushed
the
feat/jit-spend-permissions
branch
from
faeda99 to
a193064
Compare
stephancill
force-pushed
the
feat/jit-spend-permissions
branch
2 times, most recently
from
d48cc89 to
2375f21
Compare
stephancill
force-pushed
the
feat/jit-spend-permissions
branch
from
2375f21 to
8bf75cf
Compare
stephancill
commented
Apr 24, 2025
Comment on lines
355
to
367
| export function createWalletSendCallsRequest({ | ||
| to, | ||
| data, | ||
| value, | ||
| from, | ||
| chainId, | ||
| }: { | ||
| to: Address; | ||
| data: Hex; | ||
| value: Hex; | ||
| from: Address; | ||
| chainId: Hex; | ||
| }) { |
There was a problem hiding this comment.
This has been changed to accept an array of calls instead of a single call in #1612
* feat: paymaster urls in config * fix: capabilties arg in createWalletSendCallsRequest * fix: createWalletSendCallsRequest chainId type
cb-jake
reviewed
Apr 25, 2025
| const { request: subAccountRequest } = await createSubAccountSigner({ | ||
| address: subAccount.address, | ||
| owner: ownerAccount.account, | ||
| client: client, | ||
| factory: subAccount.factory, | ||
| factoryData: subAccount.factoryData, | ||
| parentAddress: globalAccountAddress, |
There was a problem hiding this comment.
nit: we should standardize naming for the global account. No strong preference global, parent, universal are all good with me
| capabilities: Record<string, unknown> | ||
| ) { | ||
| // Modify request to include auto sub account capabilities | ||
| const modifiedRequest = { ...request }; | ||
|
|
||
| if (capabilities && ['wallet_sendCalls', 'wallet_connect'].includes(request.method)) { | ||
| if (capabilities && request.method.startsWith('wallet_')) { |
There was a problem hiding this comment.
would still prefer tighter restriction here, non blocker for this PR thoooo
There was a problem hiding this comment.
actually leaning towards removing this conditional from here entirely because it it's kinda outside of the scope of the function
| }, | ||
| { | ||
| isRed: false, | ||
| info: 'Continue in Popup', |
There was a problem hiding this comment.
non blocker: have we finalized on the "continue in popup" messaging
cb-jake
previously approved these changes
Apr 25, 2025
fan-zhang-sv
previously approved these changes
Apr 25, 2025
* fix: account ordering for wallet_addSubAccount * feat: signer selector on /auto-sub-account page * fix: convert errors thrown by viem * fix: handle errors from other versions of viem
* feat: sub accounts data suffix support * fix: capabilities type * fix: tests
fan-zhang-sv
approved these changes
Apr 25, 2025
cb-jake
approved these changes
Apr 25, 2025
|
Review Error for cb-jake @ 2025-04-25 21:18:20 UTC |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Implements behavior to request spend permissions from the global wallet at transaction time
dynamicSpendLimitsconfig in SDK constructorSCWSignerwithhandleInsufficientBalanceErrorparentAddressarg tocreateSubAccountSignerHow did you test your changes?
/auto-sub-accounts