Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: surface descriptive session key errors #718

Merged
merged 1 commit into from
Jun 18, 2024
Merged

feat: surface descriptive session key errors #718

merged 1 commit into from
Jun 18, 2024

Conversation

dphilipson
Copy link
Contributor

@dphilipson dphilipson commented Jun 13, 2024 •
edited by pr-codex bot
Loading

The session key plugin throws opaque errors when permission checks fail. Surface better errors in the SDK after a validation failure by using state overrides to replace the session key code with a version that produces descriptive errors.

PR-Codex overview

This PR updates the sessionKeyPluginActions in the extension.ts file to improve error handling and permissions checks.

Detailed summary

  • Added executeWithSessionKey function for descriptive errors
  • Introduced fixedExecuteWithSessionKey for improved error handling
  • Included getRpcErrorMessageFromViemError function
  • Defined SessionKeyPermissionError class
  • Updated DEBUG_SESSION_KEY_BYTECODE in a separate file

The following files were skipped due to too many changes: packages/accounts/src/msca/plugins/session-key/debug-session-key-bytecode.ts

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@vercel Vercel
Copy link

vercel bot commented Jun 13, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
aa-sdk-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 17, 2024 8:26pm

@dphilipson dphilipson marked this pull request as draft June 13, 2024 08:42
@dphilipson
Copy link
Contributor Author

This is a draft version: it uses any to suppress a hairy type error and includes a testing script, testo.ts, which will be deleted when ready to publish.

dphilipson
dphilipson commented Jun 13, 2024
View reviewed changes
packages/accounts/src/msca/plugins/session-key/extension.ts Outdated
}
const data = og.encodeExecuteWithSessionKey({ args });
const sessionKeyPluginAddress =
SessionKeyPlugin.meta.addresses[client.chain.id];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with other uses of this plugin address lookup, we should probably be using a default address if the chain id is unknown.

@dphilipson dphilipson force-pushed the dp/session-key-permission-errors branch from b4b1cf9 to 484c6c8 Compare June 13, 2024 23:26
@dphilipson dphilipson force-pushed the dp/session-key-permission-errors branch from 484c6c8 to f35148f Compare June 14, 2024 20:11
@dphilipson dphilipson marked this pull request as ready for review June 14, 2024 20:12
dphilipson
dphilipson commented Jun 14, 2024
View reviewed changes
packages/accounts/src/msca/plugins/session-key/extension.ts Outdated
"./debug-session-key-bytecode.js"
);
const updatedOverrides: UserOperationOverrides<TEntryPointVersion> = {
...(overrides as UserOperationOverrides<TEntryPointVersion>),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a way to get this to work without the cast on this line.

moldy530
moldy530 previously approved these changes Jun 17, 2024
View reviewed changes
@moldy530
Copy link
Collaborator

moldy530 commented Jun 17, 2024 •
edited
Loading

oh odd.. we should update the esm build steps to use a more recent version of the module type, maybe nodenext or esnext? not sure what the best choice is. I imagine a later es version

tsc --project tsconfig.build.json --module **es2015** --outDir ./dist/esm --removeComments && echo > ./dist/esm/package.json '{"type":"module"}'

@moldy530
Copy link
Collaborator

ah also, can we change the base branch to v4.x.x

@dphilipson
feat: surface descriptive session key errors
5d1065a 
The session key plugin throws opaque errors when permission checks fail.
Surface better errors in the SDK after a validation failure by using
state overrides to replace the session key code with a version that
produces descriptive errors.
@dphilipson dphilipson force-pushed the dp/session-key-permission-errors branch from f35148f to 5d1065a Compare June 17, 2024 19:01
alchemy-bot
alchemy-bot reviewed Jun 17, 2024
View reviewed changes
account-kit/core/src/errors.ts Outdated Show resolved Hide resolved
account-kit/react/src/errors.ts Outdated Show resolved Hide resolved
account-kit/react/src/hooks/useSigner.ts Outdated Show resolved Hide resolved
@dphilipson dphilipson changed the base branch from main to v4.x.x June 17, 2024 19:04
@dphilipson
Copy link
Contributor Author

@moldy530 Done and done!

I changed the --module version to es2020, the earliest version mentioned as an alternative in the build error. Let me know if you think this will cause any problems.

@dphilipson dphilipson mentioned this pull request Jun 17, 2024
Merged
7 tasks
@dphilipson dphilipson merged commit c5465ee into v4.x.x Jun 18, 2024
5 checks passed
@dphilipson dphilipson deleted the dp/session-key-permission-errors branch June 18, 2024 17:01
moldy530 pushed a commit that referenced this pull request Jun 20, 2024
@dphilipson @moldy530
feat: surface descriptive session key errors (#718)
082d363 
The session key plugin throws opaque errors when permission checks fail.
Surface better errors in the SDK after a validation failure by using
state overrides to replace the session key code with a version that
produces descriptive errors.
moldy530 pushed a commit that referenced this pull request Jun 28, 2024
@dphilipson @moldy530
feat: surface descriptive session key errors (#718)
4c0b2f1 
The session key plugin throws opaque errors when permission checks fail.
Surface better errors in the SDK after a validation failure by using
state overrides to replace the session key code with a version that
produces descriptive errors.
moldy530 pushed a commit that referenced this pull request Jul 17, 2024
@dphilipson @moldy530
feat: surface descriptive session key errors (#718)
637bb95 
The session key plugin throws opaque errors when permission checks fail.
Surface better errors in the SDK after a validation failure by using
state overrides to replace the session key code with a version that
produces descriptive errors.
moldy530 pushed a commit that referenced this pull request Jul 30, 2024
@dphilipson @moldy530
feat: surface descriptive session key errors (#718)
c550465 
The session key plugin throws opaque errors when permission checks fail.
Surface better errors in the SDK after a validation failure by using
state overrides to replace the session key code with a version that
produces descriptive errors.
moldy530 pushed a commit that referenced this pull request Aug 13, 2024
@dphilipson @moldy530
feat: surface descriptive session key errors (#718)
45582b1 
The session key plugin throws opaque errors when permission checks fail.
Surface better errors in the SDK after a validation failure by using
state overrides to replace the session key code with a version that
produces descriptive errors.
moldy530 pushed a commit that referenced this pull request Aug 28, 2024
@dphilipson @moldy530
feat: surface descriptive session key errors (#718)
593e84c 
The session key plugin throws opaque errors when permission checks fail.
Surface better errors in the SDK after a validation failure by using
state overrides to replace the session key code with a version that
produces descriptive errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alchemy-bot alchemy-bot alchemy-bot left review comments

@moldy530 moldy530 moldy530 approved these changes

@rthomare rthomare Awaiting requested review from rthomare rthomare is a code owner

@dancoombs dancoombs Awaiting requested review from dancoombs dancoombs is a code owner

@mokok123 mokok123 Awaiting requested review from mokok123 mokok123 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dphilipson @moldy530 @alchemy-bot