feat: add webauthn gas estimator #1573
Conversation
howydev
requested review from
moldy530,
rthomare,
dancoombs,
dphilipson,
linnall,
adamegyed,
Zer0dot,
jaypaik,
Blu-J,
jakehobbs,
avarobinson,
florrdv and
blakecduncan
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
How to use the Graphite Merge QueueAdd the label graphite-merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
🌿 Documentation Preview
|
| typeof uo.preVerificationGas !== "bigint" && | ||
| typeof uo.preVerificationGas !== "number") | ||
| ) { | ||
| throw new Error( |
There was a problem hiding this comment.
not sure but will type narrow this instead
|
|
||
| const entryPoint = account.getEntryPoint(); | ||
| if (entryPoint.version !== "0.7.0") { | ||
| throw new Error( |
There was a problem hiding this comment.
Why's this? Couldn't someone use this with any passkey account?
There was a problem hiding this comment.
It could, but the webauthn validation implementation can be different so i recommend we version lock the integrations. For instance:
- In v2 webauthn, we probably want to remove all instances of short circuiting, so the values returned below would change
- Another possible design is what ithaca is doing - an on-chain check for rip7212 during uo validation, in that mode we wouldn't need the gas estimator
|
|
||
| export const webauthnGasEstimator: ( | ||
| gasEstimator?: ClientMiddlewareFn | ||
| ) => ClientMiddlewareFn = |
There was a problem hiding this comment.
TODO: is it possible to get the size of authenticatorDatas and clientDataJSONs here? if so, we might be able to get a tighter bound on modifications to gas estimation
| ); | ||
| } | ||
|
|
||
| const uo = await gasEstimator_(struct, params); |
There was a problem hiding this comment.
Note: we still want to run the underlying gas estimator and modify that value instead of hardcoding PVG because the account/entity could have pre-validation hooks
| return { | ||
| ...uo, | ||
| preVerificationGas: | ||
| BigInt(typeof pvg === "string" ? hexToString(pvg) : pvg) + |
There was a problem hiding this comment.
the only possible string type here for pvg is 0x${string} so this works
There was a problem hiding this comment.
Are you sure hexToString is what you want? That's for converting UTF-8 bytes into human-readable, e.g.
hexToString("0x48656c6c6f20576f726c6421") === "Hello world!"If you're trying to convert a hex string representing a number into a bigint, you can just pass it to BigInt directly, e.g.
BigInt("0x10") === 16n
| return { | ||
| ...uo, | ||
| preVerificationGas: | ||
| BigInt(typeof pvg === "string" ? hexToString(pvg) : pvg) + |
There was a problem hiding this comment.
Are you sure hexToString is what you want? That's for converting UTF-8 bytes into human-readable, e.g.
hexToString("0x48656c6c6f20576f726c6421") === "Hello world!"If you're trying to convert a hex string representing a number into a bigint, you can just pass it to BigInt directly, e.g.
BigInt("0x10") === 16n| // perform eth call read check to see if the chain has 7212 | ||
| const { data } = await params.client.call({ data: rip7212CheckBytecode }); | ||
|
|
||
| const chainHas7212: boolean = data ? BigInt(data) === 1n : false; |
There was a problem hiding this comment.
This is fine, but seeing false in a branch of conditional feels weird to me. I might write this as !!data && BigInt(data) === 1n or data != null && BigInt(data) === 1n. (The !! is an idiom for "cast to boolean")
Pull Request Checklist
yarn test)sitefolder, and guidelines for updating/adding docs can be found in the contribution guide)feat!: breaking change)yarn lint:check) and fix any issues? (yarn lint:write)PR-Codex overview
This PR introduces a new
webauthnGasEstimatorfunction in thewebauthnGasEstimator.tsfile, which calculates gas estimates for transactions while ensuring compatibility with a specific EntryPoint version and checking for the presence of ERC-7212 on the chain.Detailed summary
rip7212CheckBytecodeconstant for ERC-7212 check.webauthnGasEstimatorfunction as a middleware.preVerificationGasand adjusts it based on ERC-7212 presence.