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.

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

[Production] Swap asSigner for asProvider to enforce Fractal's provider usage, logical fixes for proposal data encoding, tests #1352

Merged
merged 17 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ node_modules
src/assets/typechain-types
/docker
playwright.config.ts
vitest.config.ts
27 changes: 27 additions & 0 deletions .github/workflows/unit-tests.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: Unit Tests
on:
push:
branches:
- develop
pull_request:

jobs:
vitest:
runs-on: ubuntu-latest
environment: dev
steps:
# Checkout the repository
- name: Checkout
uses: actions/checkout@v3

# Set up the required Node.js version
- name: Set up node
uses: actions/setup-node@v3
with:
node-version-file: '.nvmrc'

- name: Install dependencies
run: npm ci

- name: Run tests
run: npm run test
3,808 changes: 3,472 additions & 336 deletions package-lock.json

Large diffs are not rendered by default.

8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
"build:windows": "for /F \"delims=\" %I in ('git rev-parse HEAD') do set \"NEXT_PUBLIC_GIT_HASH=%I\" && next build",
"graphql:build": "graphclient build",
"graphql:dev-server": "graphclient serve-dev",
"test": "npm run set:env & next test",
"test": "vitest --dir=test",
"prepare": "husky install",
"tests": "playwright test"
},
Expand All @@ -86,20 +86,24 @@
"@graphprotocol/client-cli": "^2.2.20",
"@playwright/test": "^1.32.3",
"@testing-library/jest-dom": "^5.16.5",
"@testing-library/react": "^14.2.1",
"@types/react": "^18.0.17",
"@types/react-dom": "^18.0.6",
"@typescript-eslint/eslint-plugin": "^6.19.0",
"@typescript-eslint/parser": "^6.19.0",
"@vitejs/plugin-react": "^4.2.1",
"encoding": "^0.1.13",
"eslint": "^8.22.0",
"eslint-config-airbnb-typescript": "^17.1.0",
"eslint-config-next": "^13.2.4",
"eslint-plugin-import": "^2.26.0",
"eslint-plugin-prettier": "^4.2.1",
"husky": "^8.0.0",
"jsdom": "^24.0.0",
"playwright": "^1.32.3",
"prettier": "^2.7.1",
"typescript": "^5.0.4",
"url": "^0.11.0"
"url": "^0.11.0",
"vitest": "^1.2.2"
}
}
2 changes: 1 addition & 1 deletion src/components/Proposals/ProposalSummary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export default function ProposalSummary({
useEffect(() => {
async function loadProposalVotingWeight() {
if (tokenContract && address) {
const pastVotingWeight = await tokenContract.asSigner.getPastVotes(address, startBlock);
const pastVotingWeight = await tokenContract.asProvider.getPastVotes(address, startBlock);
setProposalsERC20VotingWeight(
pastVotingWeight.div(votesTokenDecimalsDenominator).toString()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export function VoteContextProvider({
} else if (type === GovernanceType.AZORIUS_ERC20) {
newCanVote =
(
await ozLinearVotingContract!.asSigner.getVotingWeight(
await ozLinearVotingContract!.asProvider.getVotingWeight(
user.address,
proposal.proposalId
)
Expand Down
2 changes: 1 addition & 1 deletion src/components/pages/DaoDashboard/Info/InfoGovernance.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export function InfoGovernance() {
};
if (freezeGuardType == FreezeGuardType.MULTISIG) {
if (freezeGuardContract) {
const freezeGuard = freezeGuardContract.asSigner as MultisigFreezeGuard;
const freezeGuard = freezeGuardContract.asProvider as MultisigFreezeGuard;
setTimelockPeriod(await formatBlocks(await freezeGuard.timelockPeriod()));
setExecutionPeriod(await formatBlocks(await freezeGuard.executionPeriod()));
}
Expand Down
6 changes: 3 additions & 3 deletions src/components/pages/DaoHierarchy/useFetchNodes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function useFetchNodes(address?: string) {
async (safeInfo?: Partial<SafeInfoResponseWithGuard>) => {
if (safeInfo && safeInfo.guard && multisigFreezeGuardMasterCopyContract) {
if (safeInfo.guard !== ethers.constants.AddressZero) {
const guard = multisigFreezeGuardMasterCopyContract.asSigner.attach(safeInfo.guard);
const guard = multisigFreezeGuardMasterCopyContract.asProvider.attach(safeInfo.guard);
const guardOwner = await guard.owner();
if (guardOwner !== safeInfo.address) {
return guardOwner;
Expand All @@ -53,13 +53,13 @@ export function useFetchNodes(address?: string) {
azoriusFreezeGuardMasterCopyContract &&
fractalAzoriusMasterCopyContract
) {
const azoriusContract = fractalAzoriusMasterCopyContract?.asSigner.attach(
const azoriusContract = fractalAzoriusMasterCopyContract?.asProvider.attach(
azoriusModule.moduleAddress
);
const azoriusGuardAddress = await azoriusContract.getGuard();
if (azoriusGuardAddress !== ethers.constants.AddressZero) {
const guard =
azoriusFreezeGuardMasterCopyContract.asSigner.attach(azoriusGuardAddress);
azoriusFreezeGuardMasterCopyContract.asProvider.attach(azoriusGuardAddress);
const guardOwner = await guard.owner();
if (guardOwner !== safeInfo.address) {
return guardOwner;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const useRemoveSigner = ({
const description = 'Remove Signers';

const calldatas = [
safeSingletonContract.asSigner.interface.encodeFunctionData('removeOwner', [
safeSingletonContract.asProvider.interface.encodeFunctionData('removeOwner', [
prevSigner,
signerToRemove,
BigNumber.from(threshold),
Expand All @@ -54,7 +54,7 @@ const useRemoveSigner = ({
failedToastMessage: t('removeSignerFailureToastMessage'),
});
}, [
safeSingletonContract.asSigner.interface,
safeSingletonContract.asProvider.interface,
prevSigner,
signerToRemove,
threshold,
Expand Down
9 changes: 2 additions & 7 deletions src/components/ui/menus/ManageDAO/ManageDAOMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import useSubmitProposal from '../../../../hooks/DAO/proposal/useSubmitProposal'
import useUserERC721VotingTokens from '../../../../hooks/DAO/proposal/useUserERC721VotingTokens';
import useClawBack from '../../../../hooks/DAO/useClawBack';
import useBlockTimestamp from '../../../../hooks/utils/useBlockTimestamp';
import { useEthersProvider } from '../../../../hooks/utils/useEthersProvider';
import { useFractal } from '../../../../providers/App/AppProvider';
import {
FractalGuardContracts,
Expand Down Expand Up @@ -70,9 +69,6 @@ export function ManageDAOMenu({
} = useFractal();
const currentTime = BigNumber.from(useBlockTimestamp());
const { push } = useRouter();
const {
network: { chainId },
} = useEthersProvider();
const safeAddress = fractalNode?.daoAddress;

const { getCanUserCreateProposal } = useSubmitProposal();
Expand Down Expand Up @@ -111,12 +107,12 @@ export function ManageDAOMenu({
azoriusModule.moduleAddress
),
};
const votingContractAddress = await getEventRPC<Azorius>(azoriusContract, chainId)
const votingContractAddress = await getEventRPC<Azorius>(azoriusContract)
.queryFilter((azoriusModule.moduleContract as Azorius).filters.EnabledStrategy())
.then(strategiesEnabled => {
return strategiesEnabled[0].args.strategy;
});
const rpc = getEventRPC<ModuleProxyFactory>(zodiacModuleProxyFactoryContract, chainId);
const rpc = getEventRPC<ModuleProxyFactory>(zodiacModuleProxyFactoryContract);
const filter = rpc.filters.ModuleProxyCreation(votingContractAddress, null);
const votingContractMasterCopyAddress = await rpc
.queryFilter(filter)
Expand All @@ -143,7 +139,6 @@ export function ManageDAOMenu({

loadGovernanceType();
}, [
chainId,
fractalAzoriusMasterCopyContract,
linearVotingERC721MasterCopyContract,
linearVotingMasterCopyContract,
Expand Down
4 changes: 2 additions & 2 deletions src/components/ui/proposal/useProposalCountdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ export function useProposalCountdown(proposal: FractalProposal) {
async function getCountdown() {
const freezeGuard =
freezeGuardType === FreezeGuardType.MULTISIG
? (freezeGuardContract?.asSigner as MultisigFreezeGuard)
? (freezeGuardContract?.asProvider as MultisigFreezeGuard)
: freezeGuardType === FreezeGuardType.AZORIUS
? (freezeGuardContract?.asSigner as AzoriusFreezeGuard)
? (freezeGuardContract?.asProvider as AzoriusFreezeGuard)
: undefined;

const isSafeGuard = freezeGuardType === FreezeGuardType.MULTISIG;
Expand Down
29 changes: 4 additions & 25 deletions src/helpers/crypto.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { TypedDataSigner } from '@ethersproject/abstract-signer';
import { BigNumber, Contract, constants, utils, BigNumberish, Signer } from 'ethers';
import { sepolia, mainnet, polygon } from 'wagmi/chains';
import { sepolia, mainnet } from 'wagmi/chains';
import { ContractConnection } from '../types';
import { MetaTransaction, SafePostTransaction, SafeTransaction } from '../types/transaction';

Expand Down Expand Up @@ -183,31 +183,10 @@ export const encodeMultiSend = (txs: MetaTransaction[]): string => {
};

/**
* Explained by our future overlord, ChatGPT:
*
* On networks like Polygon where block times are very short, events cannot be
* looked up without specifying a starting block number. If a user connects their
* Metamask wallet to Polygon as a public provider, attempting to load events in
* this way would cause it to fail, resulting in reduced performance and a high
* probability of failure.
*
* While power users can swap out the remote procedure call (RPC) in their
* Metamask for a custom one, this option is not feasible for most users. As a
* solution, the contract can be used as a provider with our own keys, allowing
* us to use our own rate limits.
*
* This gave rise to the idea of updating the useSafeContracts hook to enable
* connections as signers (using a connected wallet) or using our own keys as
* providers. The asSigner option is essentially connecting as a signer or provider,
* making it the ideal choice for most normal use cases.
* TODO: Remove getEventRPC usage as whole
*/
export function getEventRPC<T>(connection: ContractConnection<T>, chainId: number): T {
switch (chainId) {
case polygon.id:
return connection.asProvider;
default:
return connection.asSigner;
}
export function getEventRPC<T>(connection: ContractConnection<T>): T {
return connection.asProvider;
}

export function supportsENS(chainId: number): boolean {
Expand Down
39 changes: 16 additions & 23 deletions src/hooks/DAO/loaders/governance/useAzoriusProposals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ export const useAzoriusProposals = () => {
}
}, [ozLinearVotingContract, erc721LinearVotingContract]);
const provider = useEthersProvider();
const {
network: { chainId },
} = provider;
const decode = useSafeDecoder();
const decodeTransactions = useCallback(
async (transactions: MetaTransaction[]) => {
Expand All @@ -58,14 +55,13 @@ export const useAzoriusProposals = () => {
) {
return [];
}
const rpc = getEventRPC<Azorius>(azoriusContract, chainId);
const rpc = getEventRPC<Azorius>(azoriusContract);
const proposalCreatedFilter = rpc.filters.ProposalCreated();

const proposalCreatedEvents = await rpc.queryFilter(proposalCreatedFilter);

const strategyContract = getEventRPC<LinearERC20Voting | LinearERC721Voting>(
ozLinearVotingContract ?? erc721LinearVotingContract!,
chainId
ozLinearVotingContract ?? erc721LinearVotingContract!
);

const proposals = await Promise.all(
Expand Down Expand Up @@ -97,7 +93,6 @@ export const useAzoriusProposals = () => {
);
return proposals;
}, [
chainId,
decodeTransactions,
ozLinearVotingContract,
erc721LinearVotingContract,
Expand Down Expand Up @@ -132,8 +127,7 @@ export const useAzoriusProposals = () => {
};
}
const strategyContract = getEventRPC<LinearERC20Voting | LinearERC721Voting>(
ozLinearVotingContract ?? erc721LinearVotingContract!,
chainId
ozLinearVotingContract ?? erc721LinearVotingContract!
).attach(strategyAddress);
const func = async () => {
return mapProposalCreatedEventToProposal(
Expand All @@ -157,7 +151,6 @@ export const useAzoriusProposals = () => {
erc721LinearVotingContract,
azoriusContract,
provider,
chainId,
decodeTransactions,
action,
requestWithRetries,
Expand All @@ -170,7 +163,7 @@ export const useAzoriusProposals = () => {
if (!ozLinearVotingContract || !strategyType) {
return;
}
const strategyContract = getEventRPC<LinearERC20Voting>(ozLinearVotingContract, chainId);
const strategyContract = getEventRPC<LinearERC20Voting>(ozLinearVotingContract);
const votesSummary = await getProposalVotesSummary(
strategyContract,
strategyType,
Expand All @@ -188,15 +181,15 @@ export const useAzoriusProposals = () => {
},
});
},
[ozLinearVotingContract, chainId, action, strategyType]
[ozLinearVotingContract, action, strategyType]
);

const erc721ProposalVotedEventListener: TypedListener<ERC721VotedEvent> = useCallback(
async (voter, proposalId, support, tokenAddresses, tokenIds) => {
if (!erc721LinearVotingContract || !strategyType) {
return;
}
const strategyContract = getEventRPC<LinearERC721Voting>(erc721LinearVotingContract, chainId);
const strategyContract = getEventRPC<LinearERC721Voting>(erc721LinearVotingContract);
const votesSummary = await getProposalVotesSummary(
strategyContract,
strategyType,
Expand All @@ -215,38 +208,38 @@ export const useAzoriusProposals = () => {
},
});
},
[erc721LinearVotingContract, chainId, action, strategyType]
[erc721LinearVotingContract, action, strategyType]
);

useEffect(() => {
if (!azoriusContract) {
return;
}
const proposalCreatedFilter = azoriusContract.asSigner.filters.ProposalCreated();
const proposalCreatedFilter = azoriusContract.asProvider.filters.ProposalCreated();

azoriusContract.asSigner.on(proposalCreatedFilter, proposalCreatedListener);
azoriusContract.asProvider.on(proposalCreatedFilter, proposalCreatedListener);

return () => {
azoriusContract.asSigner.off(proposalCreatedFilter, proposalCreatedListener);
azoriusContract.asProvider.off(proposalCreatedFilter, proposalCreatedListener);
};
}, [azoriusContract, proposalCreatedListener]);

useEffect(() => {
if (ozLinearVotingContract) {
const votedEvent = ozLinearVotingContract.asSigner.filters.Voted();
const votedEvent = ozLinearVotingContract.asProvider.filters.Voted();

ozLinearVotingContract.asSigner.on(votedEvent, erc20ProposalVotedEventListener);
ozLinearVotingContract.asProvider.on(votedEvent, erc20ProposalVotedEventListener);

return () => {
ozLinearVotingContract.asSigner.off(votedEvent, erc20ProposalVotedEventListener);
ozLinearVotingContract.asProvider.off(votedEvent, erc20ProposalVotedEventListener);
};
} else if (erc721LinearVotingContract) {
const votedEvent = erc721LinearVotingContract.asSigner.filters.Voted();
const votedEvent = erc721LinearVotingContract.asProvider.filters.Voted();

erc721LinearVotingContract.asSigner.on(votedEvent, erc721ProposalVotedEventListener);
erc721LinearVotingContract.asProvider.on(votedEvent, erc721ProposalVotedEventListener);

return () => {
erc721LinearVotingContract.asSigner.off(votedEvent, erc721ProposalVotedEventListener);
erc721LinearVotingContract.asProvider.off(votedEvent, erc721ProposalVotedEventListener);
};
}
}, [
Expand Down
11 changes: 7 additions & 4 deletions src/hooks/DAO/loaders/governance/useERC20Claim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,21 @@ export function useERC20Claim() {
const loadTokenClaimContract = useCallback(async () => {
if (!claimingMasterCopyContract || !tokenContract) return;

const approvalFilter = tokenContract.asSigner.filters.Approval();
const approvals = await tokenContract.asSigner.queryFilter(approvalFilter);
const approvalFilter = tokenContract.asProvider.filters.Approval();
const approvals = await tokenContract.asProvider.queryFilter(approvalFilter);
if (!approvals.length) return;
const possibleTokenClaimContract = claimingMasterCopyContract.asSigner.attach(
const possibleTokenClaimContract = claimingMasterCopyContract.asProvider.attach(
approvals[0].args[1]
);
const tokenClaimFilter = possibleTokenClaimContract.filters.ERC20ClaimCreated();
const tokenClaimArray = await possibleTokenClaimContract
.queryFilter(tokenClaimFilter)
.catch(() => []);

if (!tokenClaimArray.length || tokenClaimArray[0].args[1] === tokenContract.asSigner.address) {
if (
!tokenClaimArray.length ||
tokenClaimArray[0].args[1] === tokenContract.asProvider.address
) {
return;
}
// action to governance
Expand Down
Loading
Loading