finalUserOp replaced with userOp for overrides proper usage #239
Conversation
| @@ -113,7 +113,7 @@ export abstract class SmartAccount implements ISmartAccount { | |||
| if (!this.provider) throw new Error('Provider is not present for making rpc calls') | |||
| // if no bundler url is provided run offchain logic to assign following values of UserOp | |||
| // maxFeePerGas, maxPriorityFeePerGas, verificationGasLimit, callGasLimit, preVerificationGas | |||
| finalUserOp = await this.calculateUserOpGasValues(finalUserOp) | |||
| finalUserOp = await this.calculateUserOpGasValues(userOp) | |||
There was a problem hiding this comment.
If userOp already contains some keys here, calculateUserOpGasValues will skip local estimation and use provided key (aka override) here?
There was a problem hiding this comment.
so essentially someone would have to pass overrides and skipBundlerEstimation true both in order for it to act as an override.
There was a problem hiding this comment.
should we make two override flags?
bundlerEstimationOverrides and overrides
There was a problem hiding this comment.
if overrides is given then assume skipBundlerEstimation to be true?
There was a problem hiding this comment.
We can reinstate the previous logic. If all gas attributes are provided in buildUserOp, there is no need to use off-chain logic, and we can also avoid calling the bundler for estimation.
There was a problem hiding this comment.
yeah then overrides should be limited to only callGasLimit and verificationGasLimit.
preVerificationGas, maxFeePerGas, maxPriorityFeePerGas would need to still come from the bundler
There was a problem hiding this comment.
and if we'd need to make bundler call and use some partial values provided in overrides, these overrides directly get applied to bundler call which might mess with estimated limits again..
There was a problem hiding this comment.
there are two kinds of overrides which now requires discussion with @tomarsachin2271 and @arcticfloyd1984
There was a problem hiding this comment.
I would merge this PR and then handle this differently.
current communication would be
- send overrides and skipBundler flag true to just do the overrides
- send overrides which will only be used for bundler estimation (for debugging some cases it helps)
- no overrides and skip flag true which will be used to calculate values locally in the sdk
- one can always replace keys in userop before it gets signed by paymaster (if involved) and the user
finalUserOp passed to calculateUserOpGasValues function is replaced with userOp that has overrides gas attributes