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

Remove the safeSingletonContract property from FractalContracts and BaseContracts interfaces #1660

Merged
merged 1 commit into from
May 8, 2024
Merged

Remove the safeSingletonContract property from FractalContracts and BaseContracts interfaces #1660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 8 additions & 9 deletions src/components/pages/DaoSettings/components/Signers/hooks/useAddSigner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useCallback } from 'react';
import { useTranslation } from 'react-i18next';
import { getAddress, isHex } from 'viem';
import { encodeFunctionData, getAddress } from 'viem';
import GnosisSafeL2Abi from '../../../../../../assets/abi/GnosisSafeL2';
import useSubmitProposal from '../../../../../../hooks/DAO/proposal/useSubmitProposal';
import { useFractal } from '../../../../../../providers/App/AppProvider';
import { ProposalExecuteData } from '../../../../../../types';
Expand All @@ -26,16 +27,14 @@ const useAddSigner = () => {
if (!baseContracts || !daoAddress) {
return;
}
const { safeSingletonContract } = baseContracts;
const description = 'Add Signer';

const encodedAddOwner = safeSingletonContract.asSigner.interface.encodeFunctionData(
'addOwnerWithThreshold',
[newSigner, BigInt(threshold)],
);
if (!isHex(encodedAddOwner)) {
return;
}
const encodedAddOwner = encodeFunctionData({
abi: GnosisSafeL2Abi,
functionName: 'addOwnerWithThreshold',
args: [getAddress(newSigner), BigInt(threshold)],
});

const calldatas = [encodedAddOwner];

const proposalData: ProposalExecuteData = {
Expand Down
17 changes: 7 additions & 10 deletions src/components/pages/DaoSettings/components/Signers/hooks/useRemoveSigner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useCallback } from 'react';
import { useTranslation } from 'react-i18next';
import { isHex, getAddress } from 'viem';
import { getAddress, encodeFunctionData } from 'viem';
import GnosisSafeL2Abi from '../../../../../../assets/abi/GnosisSafeL2';
import useSubmitProposal from '../../../../../../hooks/DAO/proposal/useSubmitProposal';
import { useFractal } from '../../../../../../providers/App/AppProvider';
import { ProposalExecuteData } from '../../../../../../types';
Expand All @@ -26,17 +27,13 @@ const useRemoveSigner = ({
if (!baseContracts || !daoAddress) {
return;
}
const { safeSingletonContract } = baseContracts;
const description = 'Remove Signers';

const encodedRemoveOwner = safeSingletonContract.asProvider.interface.encodeFunctionData(
'removeOwner',
[prevSigner, signerToRemove, BigInt(threshold)],
);

if (!isHex(encodedRemoveOwner)) {
return;
}
const encodedRemoveOwner = encodeFunctionData({
abi: GnosisSafeL2Abi,
functionName: 'removeOwner',
args: [getAddress(prevSigner), getAddress(signerToRemove), BigInt(threshold)],
});

const calldatas = [encodedRemoveOwner];

Expand Down
5 changes: 3 additions & 2 deletions src/hooks/DAO/useBuildDAOTx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const useBuildDAOTx = () => {
keyValuePairs,
fractalRegistry,
safeFactory,
safe: safeSingleton,
zodiacModuleProxyFactory,
},
} = useNetworkConfig();
Expand All @@ -51,7 +52,6 @@ const useBuildDAOTx = () => {
}
const {
multiSendContract,
safeSingletonContract,
linearVotingMasterCopyContract,
linearVotingERC721MasterCopyContract,
fractalAzoriusMasterCopyContract,
Expand Down Expand Up @@ -94,7 +94,6 @@ const useBuildDAOTx = () => {

const buildrerBaseContracts: BaseContracts = {
fractalModuleMasterCopyContract: fractalModuleMasterCopyContract.asSigner,
safeSingletonContract: safeSingletonContract.asSigner,
multisigFreezeGuardMasterCopyContract: multisigFreezeGuardMasterCopyContract.asSigner,
multiSendContract: multiSendContract.asSigner,
freezeERC20VotingMasterCopyContract: freezeERC20VotingMasterCopyContract.asSigner,
Expand All @@ -114,6 +113,7 @@ const useBuildDAOTx = () => {
keyValuePairs,
fractalRegistry,
safeFactory,
safeSingleton,
zodiacModuleProxyFactory,
parentAddress,
parentTokenAddress,
Expand Down Expand Up @@ -167,6 +167,7 @@ const useBuildDAOTx = () => {
keyValuePairs,
fractalRegistry,
safeFactory,
safeSingleton,
zodiacModuleProxyFactory,
],
);
Expand Down
22 changes: 11 additions & 11 deletions src/hooks/DAO/useDeployAzorius.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { useCallback } from 'react';
import { useTranslation } from 'react-i18next';
import { useNavigate } from 'react-router-dom';
import { getAddress, isHex } from 'viem';
import { encodeFunctionData, getAddress, isHex } from 'viem';
import { usePublicClient } from 'wagmi';
import GnosisSafeL2Abi from '../../assets/abi/GnosisSafeL2';
import { DAO_ROUTES } from '../../constants/routes';
import { TxBuilderFactory } from '../../models/TxBuilderFactory';
import { useFractal } from '../../providers/App/AppProvider';
Expand All @@ -29,6 +30,7 @@ const useDeployAzorius = () => {
keyValuePairs,
fractalRegistry,
safeFactory,
safe: safeSingleton,
zodiacModuleProxyFactory,
},
addressPrefix,
Expand All @@ -54,7 +56,6 @@ const useDeployAzorius = () => {
}
const {
multiSendContract,
safeSingletonContract,
linearVotingMasterCopyContract,
linearVotingERC721MasterCopyContract,
fractalAzoriusMasterCopyContract,
Expand All @@ -78,7 +79,6 @@ const useDeployAzorius = () => {

const builderBaseContracts: BaseContracts = {
fractalModuleMasterCopyContract: fractalModuleMasterCopyContract.asProvider,
safeSingletonContract: safeSingletonContract.asProvider,
multisigFreezeGuardMasterCopyContract: multisigFreezeGuardMasterCopyContract.asProvider,
multiSendContract: multiSendContract.asProvider,
freezeERC20VotingMasterCopyContract: freezeERC20VotingMasterCopyContract.asProvider,
Expand All @@ -98,6 +98,7 @@ const useDeployAzorius = () => {
keyValuePairs,
fractalRegistry,
safeFactory,
safeSingleton,
zodiacModuleProxyFactory,
undefined,
undefined,
Expand All @@ -110,14 +111,12 @@ const useDeployAzorius = () => {
owners: safe.owners,
});

const encodedAddOwnerWithThreshold =
safeSingletonContract.asProvider.interface.encodeFunctionData('addOwnerWithThreshold', [
multiSendContract.asProvider.address,
1,
]);
if (!isHex(encodedAddOwnerWithThreshold)) {
return;
}
Comment on lines -118 to -120
Copy link
Contributor

Choose a reason for hiding this comment

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

the isHex function seems to be off and on used in conjunction with the encodeFunctionData. is there a reason its being used in some places and not others?

Copy link
Member Author

@adamgall adamgall May 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

viem's encodeFunctionData() will always return a "Hex" string, so no need to use the isHex function on data returned from that function.

It was needed when using ethers interface.encodeFunctionData(), because that function just returns a plain string, and typescript doesn't allow plain strings to be passed into viem functions that are expecting Hex.

As we progress through this refactor, we'll be removing all of these isHex checks, because we won't be using ethers any longer!

It's jsut needed temporarily for now as we have both ethers and viem in the codebase.

const encodedAddOwnerWithThreshold = encodeFunctionData({
abi: GnosisSafeL2Abi,
functionName: 'addOwnerWithThreshold',
args: [getAddress(multiSendContract.asProvider.address), 1n],
});

const encodedMultisend = multiSendContract.asProvider.interface.encodeFunctionData(
'multiSend',
[safeTx],
Expand Down Expand Up @@ -162,6 +161,7 @@ const useDeployAzorius = () => {
keyValuePairs,
fractalRegistry,
safeFactory,
safeSingleton,
zodiacModuleProxyFactory,
],
);
Expand Down
11 changes: 10 additions & 1 deletion src/models/TxBuilderFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ethers } from 'ethers';
import { PublicClient, getAddress, getContract } from 'viem';
import GnosisSafeL2Abi from '../assets/abi/GnosisSafeL2';
import GnosisSafeProxyFactoryAbi from '../assets/abi/GnosisSafeProxyFactory';
import { GnosisSafeL2 } from '../assets/typechain-types/usul/@gnosis.pm/safe-contracts/contracts';
import { GnosisSafeL2__factory } from '../assets/typechain-types/usul/factories/@gnosis.pm/safe-contracts/contracts';
Expand Down Expand Up @@ -35,6 +36,7 @@ export class TxBuilderFactory extends BaseTxBuilder {
private keyValuePairsAddress: string;
private fractalRegistryAddress: string;
private gnosisSafeProxyFactoryAddress: string;
private gnosisSafeSingletonAddress: string;
private moduleProxyFactoryAddress: string;

constructor(
Expand All @@ -49,6 +51,7 @@ export class TxBuilderFactory extends BaseTxBuilder {
keyValuePairsAddress: string,
fractalRegistryAddress: string,
gnosisSafeProxyFactoryAddress: string,
gnosisSafeSingletonAddress: string,
moduleProxyFactoryAddress: string,
parentAddress?: string,
parentTokenAddress?: string,
Expand All @@ -70,6 +73,7 @@ export class TxBuilderFactory extends BaseTxBuilder {
this.keyValuePairsAddress = keyValuePairsAddress;
this.fractalRegistryAddress = fractalRegistryAddress;
this.gnosisSafeProxyFactoryAddress = gnosisSafeProxyFactoryAddress;
this.gnosisSafeSingletonAddress = gnosisSafeSingletonAddress;
this.moduleProxyFactoryAddress = moduleProxyFactoryAddress;
}

Expand All @@ -84,10 +88,15 @@ export class TxBuilderFactory extends BaseTxBuilder {
address: getAddress(this.gnosisSafeProxyFactoryAddress),
client: this.publicClient,
});
const safeSingletonContract = getContract({
abi: GnosisSafeL2Abi,
address: getAddress(this.gnosisSafeSingletonAddress),
client: this.publicClient,
});
const { predictedSafeAddress, createSafeTx } = await safeData(
this.baseContracts.multiSendContract,
safeProxyFactoryContract,
this.baseContracts.safeSingletonContract,
safeSingletonContract,
this.daoData as SafeMultisigDAO,
this.saltNum,
this.fallbackHandler,
Expand Down
10 changes: 3 additions & 7 deletions src/models/helpers/safeData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,13 @@ import {
import GnosisSafeL2Abi from '../../assets/abi/GnosisSafeL2';
import GnosisSafeProxyFactoryAbi from '../../assets/abi/GnosisSafeProxyFactory';
import { MultiSend } from '../../assets/typechain-types/usul';
import { GnosisSafeL2 } from '../../assets/typechain-types/usul/@gnosis.pm/safe-contracts/contracts';
import { buildContractCallViem } from '../../helpers/crypto';
import { SafeMultisigDAO } from '../../types';

export const safeData = async (
multiSendContract: MultiSend,
safeFactoryContract: GetContractReturnType<typeof GnosisSafeProxyFactoryAbi, PublicClient>,
safeSingletonContract: GnosisSafeL2,
safeSingletonContract: GetContractReturnType<typeof GnosisSafeL2Abi, PublicClient>,
daoData: SafeMultisigDAO,
saltNum: bigint,
fallbackHandler: string,
Expand Down Expand Up @@ -52,7 +51,7 @@ export const safeData = async (
}

const predictedSafeAddress = getCreate2Address({
from: getAddress(safeFactoryContract.address),
from: safeFactoryContract.address,
salt: keccak256(
encodePacked(
['bytes', 'uint256'],
Expand All @@ -62,10 +61,7 @@ export const safeData = async (
bytecodeHash: keccak256(
encodePacked(
['bytes', 'uint256'],
[
safeFactoryContractProxyCreationCode,
hexToBigInt(getAddress(safeSingletonContract.address)),
],
Comment on lines 61 to -68
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the removal of the getAddress function here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Presumably because the safeSingletonContract type went from an ethers contract object to a viem contract object.

ethers contract.address is a string
viem contract.address is an Address

[safeFactoryContractProxyCreationCode, hexToBigInt(safeSingletonContract.address)],
),
),
});
Expand Down
2 changes: 0 additions & 2 deletions src/types/contract.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
ERC721FreezeVoting,
} from '@fractal-framework/fractal-contracts';
import { MultiSend } from '../assets/typechain-types/usul';
import { GnosisSafeL2 } from '../assets/typechain-types/usul/@gnosis.pm/safe-contracts/contracts';

export interface ContractEvent {
blockTimestamp: number;
Expand All @@ -19,7 +18,6 @@ export type ContractConnection<T> = {

export interface BaseContracts {
fractalModuleMasterCopyContract: FractalModule;
safeSingletonContract: GnosisSafeL2;
multisigFreezeGuardMasterCopyContract: MultisigFreezeGuard;
multiSendContract: MultiSend;
freezeERC20VotingMasterCopyContract: ERC20FreezeVoting;
Expand Down
2 changes: 0 additions & 2 deletions src/types/fractal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
import { Dispatch } from 'react';
import { Address } from 'viem';
import { MultiSend } from '../assets/typechain-types/usul';
import { GnosisSafeL2 } from '../assets/typechain-types/usul/@gnosis.pm/safe-contracts/contracts';
import { FractalGovernanceActions } from '../providers/App/governance/action';
import { GovernanceContractActions } from '../providers/App/governanceContracts/action';
import { FractalGuardActions } from '../providers/App/guard/action';
Expand Down Expand Up @@ -332,7 +331,6 @@ export interface FractalContracts {
fractalAzoriusMasterCopyContract: ContractConnection<Azorius>;
linearVotingMasterCopyContract: ContractConnection<LinearERC20Voting>;
linearVotingERC721MasterCopyContract: ContractConnection<LinearERC721Voting>;
safeSingletonContract: ContractConnection<GnosisSafeL2>;
fractalModuleMasterCopyContract: ContractConnection<FractalModule>;
multisigFreezeGuardMasterCopyContract: ContractConnection<MultisigFreezeGuard>;
azoriusFreezeGuardMasterCopyContract: ContractConnection<AzoriusFreezeGuard>;
Expand Down