-
Notifications
You must be signed in to change notification settings - Fork 647
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.
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
merge v0.5 to main #230
Merged
Merged
merge v0.5 to main #230
Conversation
This file contains 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
use with "yarn gas-checker" currently deploys batches of 1,2,20,21, and display the gas diff
* inital code import Gnosis code as-is. probably can remove all non-essential contracts (e.g. test, samples) or better, import as external library. * removed unused contracts (not used,fail compilation) * initial Gnosis-Safe Proxy account * refactor: - use @gnosis.pm/safe-contracts package - separate contracts into separate files. * cleanup, single owner * cleanup contracts simpler fallback handler * added tests failure cases counterfactual creation * change to "Manager" - manager is not a module, only fallback, entrypoint - replaceManager now works * ignore from coverage (fails to compile for coverage) * fix dangling test * Fix lint * Set expected code lenght to be 324 Co-authored-by: Alex Forshtat <[email protected]>
* A-32-enforce-checkin-gas-reports * use geth for local gas checks * changed "big" to 10 (avoid getting close to 128k tx limit)
* refactor handleOps * remove paymasterMode use "paymaster!=address(0)" saves 2 accesses to calldata member, and 460 gas per userOp (moved the requiredPerfund logic into ep.getPaymentInfo) * read UserOp static fields into MemoryUserOp Co-authored-by: Alex Forshtat <[email protected]>
* update the EIP to support aggregated signatures * support creation of aggregated wallet (simulateValidation) * simulateValidation with param offChainSigCheck if false, calls aggregator.validateUserOpSignature if true, returns also offChainSigInfo to be used by off-chain code to validate the signature * hash pubkey into requestId Vitalik Buterin [01/08/2022 10:40]: Basically, if one account has a pubkey P, someone can make an evil account with key Q - P, where they know q (the privkey of Q), and then they pass off a signature with q as being an "aggregate" signature of the same message signed by both K1 = P and K2 = Q-P (because K1 + K2 = Q) The fix to this is to hash the pubkey into the msghash, so you never get two different keys signing the same message. And I think this has to be enforced at the BLS aggregate verifier layer Co-authored-by: Alex Forshtat <[email protected]>
initCode as deployer+data, instead of constructor code - supports any deployer contract - initCode doesn't have to include entire CREATE2 constructor code (it is only a method call to the deployer contract)
extract IEntryPoint, IStakeManager all core interfaces in "interfaces" all core contracts in "core"
rename callGas to callGasLimit rename verificationGas to verificationGasLimit to reflect the fact these are limits, and the user pays for actual gas used. Note that "preVerificationGas" is always paid in full.
https://github.com/opengsn/account-abstraction/tree/main/contcts is an invalid address, may need to be changed to: https://github.com/eth-infinitism/account-abstraction/tree/main/contracts
* update npm package solidity: ``` import "@account-abstraction/contracts/core/BaseWallet.sol"; import "@account-abstraction/contracts/interfaces/IPaymaster.sol"; ``` typescript: ``` import { IEntryPoint, UserOperationStruct } from "@account-abstraction/contracts" ``` ABI: ``` import { abi as entryPointAbi } from '@account-abstraction/contracts/artifacts/IEntryPoint.json' ```
- initCode to be used through CreateSender - EIP wording.
Mitigation suggested by @Agusx1211
* Allow _validateSignature to be non-view
Currently, when a wallet already exists, it's not clear if `entryPoint.getSenderAddress()` will revert or not, since that depends on the internal logic of the wallet factory. I'm updating the spec to clarify that the wallet factory should try to return the address if the wallet already exists, since that's more friendly to clients who don't want to keep track of deployed wallet addresses.
`validateUserOp()`, `validatePaymaterUserOp()` may return the "deadline" time this operation is valid. They may not use the block. timestamp opcode, but the EntryPoint does validate the return value. bundlers may also use this value (returned also from `simulateValidation()`) to drop UserOps that are "just about" to expire.
* SimpleWalletDeployer: return address even for an existing wallet.
There is no need to compile the entire project (as it is far slower) saves ~1200gas per op.
Fix a vulnerability where any withdrawn deposits are not actually reflected in the deposit info storage. The vulnerability allows any wallet or paymaster to withdraw all funds deposited in an Entrypoint contract.
- simulateValidation() always reverts - successful result is error `SimulationResult` - (no need to call from address(0), but always need to catch revert reason) - returns also aggregator address, and never calls it. - bundler should either reject UserOP or validate the signature using `aggregator.validateUserOpSignature()` (or an equivalent native library code)
- stake-value and unstake delay are not managed by entryPoint. - `simulateValidation()` does return them (in its `SimulationResult`, so that the bundler can validate the paymaster stake is valid (and reject the UserOp if not) - stake values for a signature aggregator are also returned (in SimulationResultWithAggregation`, ) if the wallet uses an aggregator.
allow a wallet to access wallet-specific storage in other contracts (based on its own address)
wallet is expected to silently ignore signature failure in case the request came from:address(0) this way, we can implement eth_callUserOp without explicit use acceptance.
* rename IWallet to IAccount (and all other contracts, e.g. SimpleWallet to SimpleAccount, etc..)
* Add rpc methods definitions better definition of the required RPC methods: * eth_sendUserOperation * eth_estimateUserOperationGas * eth_getUserOperationReceipt * eth_supportedEntryPoints * fixed error codes, error examples, removed "mined"
* Changing validFrom, validUntil to uint64
* Calling _setPublicKey when initializing BLSAccount.
…202) * Indexing aggregator in SignatureAggregatorChanged event
* AA-139 N-08: Inconsistent ordering [core and samples] --------- Co-authored-by: shahafn <[email protected]>
* AA-140 N-09: Stake size inconsistency [core] --------- Co-authored-by: shahafn <[email protected]>
These are notes to developers using these samples. Co-authored-by: shahafn <[email protected]>
* N-07 rename vars and internal methods
Co-authored-by: shahafn <[email protected]>
* Removing unused imports --------- Co-authored-by: Dror Tirosh <[email protected]>
Co-authored-by: Dror Tirosh <[email protected]>
…les] (#210) Co-authored-by: Dror Tirosh <[email protected]>
* validateUserOp no longer receives an "aggregator" parameter. * instead, validateUserOp returns "validationData" which includes aggregator, validAfter, validFrom * helpers packValidationData, unpackValidationData can be used to pack and unpack this structure. * removed IAggregatedAccount interface, as getAggregator is no longer needed. * moved all validateSignatures calls to the beginning of the handleAggregatedOps, before any mutable calls of wallets or paymasters
The "paymaster" parameter is not needed in FailedOp: - reason code starts with "AA3x" to signify this is an error about a paymaster - the paymaster address can be extracted from the UserOperation itself. - FailedOp can be caused by other entities too, such as factory (for errors starting with "AA1x" or account itself ("AA2x")
* fixing VerifyingPaymaster
Fit nonce and owner into the same slot, to save unneeded SLOAD during validation (They used to be in the same slot, and got "unaligned" when added a proxy, since using "Initializeable" uses the first 2 bytes in the storage.
shahafn
approved these changes
Mar 2, 2023
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.
No description provided.