feat(protocol-kit): Add deployment functionality to Safe class + Viem #948
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
* Add `SafeWebAuthnSignerFactory` contract * Add tests for passkey flow in `isOwner` function * Add tests for signTransaction + createAddOwnerTx (wip) * Add webauthnShim to emulate the Web Authentication API implemented in browsers Copied from [safe-modules repo](https://github.com/safe-global/safe-modules/blob/main/modules/passkey/test/utils/webauthnShim.ts) * Use webauthnShim for passkey mocks * Deploy `FCLP256Verifier` in local hardhat environment and use it as as WebAuthnContract * Run passkey tests only for contract versions 1.3.0 + 1.4.1 --------- Co-authored-by: Daniel Somoza <[email protected]>
…f a hardcoded address
Passkey 4337 support
…#855) * Add test on the provider level
This reverts commit c60c3a5.
dasanra
changed the base branch from
added-deployment-safe-class
to
development
dasanra
changed the base branch from
development
to
added-deployment-safe-class
dasanra
changed the base branch from
added-deployment-safe-class
to
development
Pull Request Test Coverage Report for Build 10815229465Details
💛 - Coveralls |
dasanra
changed the title
dasanra
changed the title
yagopv
reviewed
Sep 12, 2024
| const saltNonce = config.DEPLOY_SAFE.SALT_NONCE | ||
|
|
||
| // Create SDK instance | ||
| const safeSDK = await Safe.init({ |
There was a problem hiding this comment.
Suggested change
| const safeSDK = await Safe.init({ | |
| let protocolKit = await Safe.init({ |
yagopv
reviewed
Sep 12, 2024
| saltNonce, | ||
| callback | ||
| }) | ||
| const deployedSafeSDK = await safeSDK.deploy() |
There was a problem hiding this comment.
Suggested change
| const deployedSafeSDK = await safeSDK.deploy() | |
| protocolKit = await safeSDK.deploy() |
yagopv
reviewed
Sep 12, 2024
| }) | ||
| const deployedSafeSDK = await safeSDK.deploy() | ||
|
|
||
| console.log('Deployed Safe address:', await deployedSafeSDK.getAddress()) |
There was a problem hiding this comment.
Suggested change
| console.log('Deployed Safe address:', await deployedSafeSDK.getAddress()) | |
| console.log('Deployed Safe address:', await protocolKit.getAddress()) |
yagopv
reviewed
Sep 12, 2024
|
|
||
| console.log('Deployed Safe:', await safe.getAddress()) | ||
| // check if its deployed | ||
| console.log('Safe Account deployed: ', await deployedSafeSDK.isSafeDeployed()) |
There was a problem hiding this comment.
Suggested change
| console.log('Safe Account deployed: ', await deployedSafeSDK.isSafeDeployed()) | |
| console.log('Safe Account deployed: ', await protocolKit.isSafeDeployed()) |
yagopv
reviewed
Sep 12, 2024
| @@ -264,13 +268,6 @@ class Safe { | |||
| */ | |||
| async getAddress(): Promise<string> { | |||
| if (this.#predictedSafe) { | |||
| const safeVersion = await this.getContractVersion() | |||
There was a problem hiding this comment.
This is removed in several places but not sure why
|
About the open questions:
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
What it solves
Resolves #446
SafeFactoryclassSafeclass:#addPreValidatedSignature: Adds a PreValidatedSignature to the transaction if the threshold is not reached.#isReadyToExecute: Checks if the transaction has enough signatures to be executed.deployfunction in theSafeclass.getSafeAddressFromDeploymentTxutil function: Returns the address of a SafeProxy from the deployment tx receipt.Open Questions
Callback Parameter Removal:
callbackparameter from the new deploy() function that was present in the oldSafeFactoryclass. We can reintroduce it if the team prefers.Function Naming:
deploy()instead ofdeployAndSend(). I am open to changing it if there are any objections or suggestions for a better name.Return Type of
deploy():deploy()method returns the new instance of theprotocol-kitwith the Safe already deployed. However, we can change this to return the transaction hash (tx-hash) or another value if that is preferable.Please review and provide your feedback on these points.
Previous PR reference
#905