Update VotesERC20Wrapper #1638
Update VotesERC20Wrapper #1638
Conversation
… deployments artifact
…ine because they both have the same function that was being used from this contract instance
… and FractalContracts type
…ject from useSafeContracts
✅ Deploy Preview for fractal-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -1135,4 +1135,6 @@ export default [ | |||
| stateMutability: 'payable', | |||
| type: 'receive', | |||
| }, | |||
| ]; | |||
| ] as const; | |||
There was a problem hiding this comment.
Adding as const here is necessary for viem's typescript support.
https://viem.sh/docs/typescript#type-inference
This small change resulted in some type cleanup in the one place where this ABI was being used!
| signers.map(signer => getAddress(signer)), | ||
| 1n, // Threshold | ||
| zeroAddress, | ||
| zeroHash, | ||
| fallbackHandler, | ||
| getAddress(fallbackHandler), | ||
| zeroAddress, | ||
| 0, | ||
| 0n, |
There was a problem hiding this comment.
As mentioned in the previous comment, these needed to be updated because due to type errors, after adding as const to the GnosisSafeL2 ABI! So nice
| @@ -44,7 +44,7 @@ export function DelegateModal({ close }: { close: Function }) { | |||
| validAddress = getAddress(await signer.resolveName(values.address)); | |||
| } | |||
| const votingTokenContract = | |||
| baseContracts.votesERC20WrapperMasterCopyContract.asSigner.attach(votesTokenContractAddress); | |||
| baseContracts.votesTokenMasterCopyContract.asSigner.attach(votesTokenContractAddress); | |||
There was a problem hiding this comment.
This is a funny little update, worth narrowing in on.
Basically, why were we previously using the votesERC20WrapperMasterCopyContract object/type here?
I don't think it matters which contract object we use, because we're doing an .attach (which forces a new object instance of the same type but at a new address), and then the only contract function being called from this new votingTokenContract object is a function that exists on both the VotesToken and the VotesERC20Wrapper ABIs.
So, I don't really think it mattered which existing "baseContract" object we started from (between the two in this changeset), but I do think it makes more intentional sense to use the one that I changed to here.
| const contractCallViem = useCallback( | ||
| (params: ContractCallParamsViem) => { | ||
| if (!publicClient) return; | ||
|
|
||
| let toastId: React.ReactText; | ||
| toastId = toast(params.pendingMessage, { | ||
| autoClose: false, | ||
| closeOnClick: false, | ||
| draggable: false, | ||
| closeButton: false, | ||
| progress: 1, | ||
| }); | ||
| setPending(true); | ||
| params | ||
| .contractFn() | ||
| .then(txReceipt => { | ||
| return Promise.all([ | ||
| publicClient.waitForTransactionReceipt({ hash: txReceipt }), | ||
| toastId, | ||
| ]); | ||
| }) | ||
| .then(([txReceipt, toastID]) => { | ||
| toast.dismiss(toastID); | ||
| if (txReceipt.status === 'reverted') { | ||
| toast.error(params.failedMessage); | ||
| if (params.failedCallback) params.failedCallback(); | ||
| } else if (txReceipt.status === 'success') { | ||
| toast(params.successMessage); | ||
| if (params.successCallback) params.successCallback(txReceipt); | ||
| } else { | ||
| toast.error(t('errorTransactionUnknown')); | ||
| if (params.failedCallback) params.failedCallback(); | ||
| } | ||
| if (params.completedCallback) params.completedCallback(); | ||
|
|
||
| // Give the block event emitter a couple seconds to play the latest | ||
| // block on the app state, before informing app that the transaction | ||
| // is completed. | ||
| setTimeout(() => { | ||
| setPending(false); | ||
| }, 2000); | ||
| }) | ||
| .catch((error: ProviderRpcError) => { | ||
| logError(error); | ||
| toast.dismiss(toastId); | ||
| setPending(false); | ||
|
|
||
| if (error.code === 4001) { | ||
| toast.error(t('errorUserDeniedTransaction')); | ||
| return; | ||
| } | ||
|
|
||
| toast.error(t('errorGeneral', { ns: 'common' })); | ||
| }); | ||
| }, | ||
| [t, publicClient], | ||
| ); |
There was a problem hiding this comment.
Added a new function here for the generic contractCall, but supporting viem-style contract calls. Was too messy to try to combine both ethers- and viem-style calls into one function. Eventually, the other function will be removed after we get through all of the PRs that this one is kicking off.
The code here does exactly the same thing as the other function, just with some different object types (viem stuff instead of ethers stuff)
| const { address: account } = useAccount(); | ||
| const baseContracts = useSafeContracts(); | ||
| const { loadERC20TokenAccountData } = useERC20LinearToken({ onMount: false }); | ||
|
|
||
| const [contractCall, pending] = useTransaction(); | ||
| const [, pending, contractCallViem] = useTransaction(); |
There was a problem hiding this comment.
Here is an instance where we're using the new contractCallViem function instead of the original one, because we have a new viem-style contract object.
| @@ -32,7 +33,7 @@ export function WrapToken({ close }: { close: () => void }) { | |||
| const baseContracts = useSafeContracts(); | |||
|
|
|||
| const { loadERC20TokenAccountData } = useERC20LinearToken({ onMount: false }); | |||
| const [contractCall, pending] = useTransaction(); | |||
| const [, pending, contractCallViem] = useTransaction(); | |||
There was a problem hiding this comment.
Here's another place where the new function is used.
Main thing that this PR does is:
VotesERC20WrapperTypeChain imports fromfractal-contractsin the codebase.getContract().Additionally, this PR starts to do some refactoring cleanup, removing some layers of abstraction:
votesERC20WrapperMasterCopyContract"asSigner / asProvider" object fromuseSafeContracts.votesERC20WrapperMasterCopyContractfromAzoriusContractstype andFractalContractstype.There was some other light cleanup, which I'll document in some inline PR review notes.