Demonstrate SafeECDSAPlugin in in-page wallet #97
Conversation
| @@ -16,7 +16,7 @@ function getRemappings() { | |||
|
|
|||
| const config: HardhatUserConfig = { | |||
| solidity: { | |||
| version: "0.8.12", | |||
| version: "0.8.19", | |||
There was a problem hiding this comment.
This is probably fine, but bumping solidity versions should be a considered step. If we're using a library that was audited for a particular version of compiler, we should keep that version of compiler in the list, and add any later additional versions (like we did in blswallet for blslibs).
(Related: #94)
| short: 'Deploy account', | ||
| desc: [ | ||
| 'The current 4337 account type requires a separate deployment. Please', | ||
| 'enter a private key or seed phraise with funds to do this.', |
| "account-abstraction": "github:eth-infinitism/account-abstraction#0.6.0", | ||
| "hardhat": "^2.17.3", | ||
| "openzeppelin-contracts": "github:openzeppelin/openzeppelin-contracts#v4.9.3", | ||
| "safe-contracts": "github:safe-global/safe-contracts#v1.4.1" |
There was a problem hiding this comment.
More of a curiosity, is there a preference to import from github vs a team's node release?
There was a problem hiding this comment.
Yeah there seems to be a small benefit - it allows you to use the same import paths as you do by default when using foundry. This makes compatibility easier because solidity doesn't have package management (as far as I know) so when your dependency has a dependency you need to install that yourself.
| import {EntryPoint} from "@account-abstraction/contracts/core/EntryPoint.sol"; | ||
| import {SimpleAccountFactory} from "@account-abstraction/contracts/samples/SimpleAccountFactory.sol"; | ||
| import {EntryPoint} from "account-abstraction/contracts/core/EntryPoint.sol"; | ||
| import {SimpleAccountFactory} from "account-abstraction/contracts/samples/SimpleAccountFactory.sol"; |
There was a problem hiding this comment.
@ not required to indicate from a node module?
There was a problem hiding this comment.
Nope. The @ is just a standard prefix for npm namespaces, eg @account-abstraction/contracts, but many npm packages don't use a namespace, eg ethers. When using hardhat, you're allowed to use any path beginning from node_modules.
| ): Promise<string> { | ||
| const ownerWallet = new ethers.Wallet(this.privateKey); | ||
|
|
||
| return await ownerWallet.signMessage(ethers.getBytes(userOpHash)); |
There was a problem hiding this comment.
Should verify _userOp corresponds to signed userOpHash?
There was a problem hiding this comment.
Hmm. I wrote it this way because it's an internal tool and userOpHash is already available at the call site, so it's easier to just provide it rather than recalculating it.
Having said that, the call site looks like this:
let userOpHash = await contracts.entryPoint.getUserOpHash(userOp);
userOp.signature = await account.sign(userOp, userOpHash);This means that the RPC node could get us to sign anything by lying about the userOpHash. We generally do put a lot of trust in the RPC node, but calculating the userOpHash locally is probably a good idea.
|
@blakecduncan A bit about squashing vs merging |
Resolves #74
SafeECDSAFactoryfor creating a safe with the ecdsa plugininitCodewith just usingSafeECDSAFactory(since you need to create 2 contracts anyway and 4337 won't let you do that withinitCodeso you need a separate creation step anyway)sendEthhardhat task and includes instructions in the readme to make setup easier and more clearEthereumApiso this happens via a common interface (IAccount)IAccountto supportSafeECDSAAccount(SafeECDSAAccountWrapper)