Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Feat: enable Safe version 1.4.1 #3209

Merged
merged 5 commits into from
Feb 14, 2024
Merged

Feat: enable Safe version 1.4.1 #3209

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0b40793
Feat: enable Safe version 1.4.1
katspaugh Feb 6, 2024
cfa8f08
Pass safe version to protocol-kit
katspaugh Feb 6, 2024
efe33dd
Safe version type
katspaugh Feb 6, 2024
a5a5c45
Merge with dev
katspaugh Feb 14, 2024
dc8eb67
Revert message signature changes
katspaugh Feb 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions src/components/new-safe/create/logic/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { showNotification } from '@/store/notificationsSlice'
import { SafeFactory } from '@safe-global/protocol-kit'
import type Safe from '@safe-global/protocol-kit'
import type { DeploySafeProps } from '@safe-global/protocol-kit'
import { createEthersAdapter } from '@/hooks/coreSDK/safeCoreSDK'
import { createEthersAdapter, isValidSafeVersion } from '@/hooks/coreSDK/safeCoreSDK'

import { backOff } from 'exponential-backoff'
import { LATEST_SAFE_VERSION } from '@/config/constants'
Expand Down Expand Up @@ -56,13 +56,20 @@ export const getSafeDeployProps = async (
}
}

const getSafeFactory = async (ethersProvider: BrowserProvider): Promise<SafeFactory> => {
if (!isValidSafeVersion(LATEST_SAFE_VERSION)) {
throw new Error('Invalid Safe version')
}
const ethAdapter = await createEthersAdapter(ethersProvider)
const safeFactory = await SafeFactory.create({ ethAdapter, safeVersion: LATEST_SAFE_VERSION })
return safeFactory
}

/**
* Create a Safe creation transaction via Core SDK and submits it to the wallet
*/
export const createNewSafe = async (ethersProvider: BrowserProvider, props: DeploySafeProps): Promise<Safe> => {
const ethAdapter = await createEthersAdapter(ethersProvider)

const safeFactory = await SafeFactory.create({ ethAdapter })
const safeFactory = await getSafeFactory(ethersProvider)
return safeFactory.deploySafe(props)
}

Expand All @@ -73,9 +80,7 @@ export const computeNewSafeAddress = async (
ethersProvider: BrowserProvider,
props: DeploySafeProps,
): Promise<string> => {
const ethAdapter = await createEthersAdapter(ethersProvider)

const safeFactory = await SafeFactory.create({ ethAdapter })
const safeFactory = await getSafeFactory(ethersProvider)
return safeFactory.predictSafeAddress(props.safeAccountConfig, props.saltNonce)
}

Expand Down
2 changes: 1 addition & 1 deletion src/hooks/coreSDK/safeCoreSDK.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const isLegacyVersion = (safeVersion: string): boolean => {
}

export const isValidSafeVersion = (safeVersion?: SafeInfo['version']): safeVersion is SafeVersion => {
const SAFE_VERSIONS: SafeVersion[] = ['1.3.0', '1.2.0', '1.1.1', '1.0.0']
const SAFE_VERSIONS: SafeVersion[] = ['1.4.1', '1.3.0', '1.2.0', '1.1.1', '1.0.0']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a config constant? Why don't we keep it in config/constants.ts next to the LATEST_SAFE_VERSION? Maybe changing it to:

export const LATEST_SAFE_VERSION = process.env.NEXT_PUBLIC_SAFE_VERSION || '1.3.0'
export SUPPORTED_SAFE_VERSION = ['1.4.1', '1.3.0', '1.2.0', '1.1.1', '1.0.0']

This way both const will live next to each other and supporting 1.5 one day would be just changing config/constants.ts file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this altogether because ultimately it just repeats the same check in the protocol-kit. We can instead look up the version via protocol-kit or safe-deployments.

return !!safeVersion && SAFE_VERSIONS.some((version) => semverSatisfies(safeVersion, version))
}

Expand Down
8 changes: 5 additions & 3 deletions src/services/tx/tx-sender/sdk.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,13 @@ export const getSafeSDKWithSigner = async (onboard: OnboardAPI, chainId: SafeInf
}

export const getSupportedSigningMethods = (safeVersion: SafeInfo['version']): SigningMethod[] => {
if (!hasSafeFeature(SAFE_FEATURES.ETH_SIGN, safeVersion)) {
return [SigningMethod.ETH_SIGN_TYPED_DATA]
const methods = [SigningMethod.ETH_SIGN_TYPED_DATA]

if (hasSafeFeature(SAFE_FEATURES.ETH_SIGN, safeVersion)) {
methods.push(SigningMethod.ETH_SIGN)
}

return [SigningMethod.ETH_SIGN_TYPED_DATA, SigningMethod.ETH_SIGN]
return methods
}

export const tryOffChainTxSigning = async (
Expand Down
Loading