Creating Transacton to Migrate to L2 #387
Conversation
|
@anquetil is attempting to deploy a commit to the Nouns Builder Team on Vercel. A member of the Team first needs to authorize it. |
| where: { | ||
| dao: collectionAddress, | ||
| }, | ||
| first: 1000, |
There was a problem hiding this comment.
Ideally this should work with DAOs of any size. You can improve this by querying ownerCount before this call and if the ownerCount is > 1000 create n batches where n = ownerCount / 1000 ie 1200 members will create 2 batches, 3000 will create 3 batches then merge them together at the end of the function.
apps/web/src/modules/create-proposal/components/TransactionForm/Migration/Migration.schema.ts
Outdated
Show resolved
Hide resolved
| data: members, | ||
| error, | ||
| isValidating, | ||
| } = useSWR(isReady ? [token, chain.id] : undefined, () => |
There was a problem hiding this comment.
we only need to run this query if the DAO is on step 2 right? currently this will run every time
There was a problem hiding this comment.
Yeah, I think the issue here is that I need it in handleC2Submit, which is declared at this level, because the C2Form is rendered here. Since useSWR is a hook, it can't be inside of handleC2Submit, however. Is there a way to get this without SWR in handleC2Submit ? Or maybe I can enable the hook only if we're on Step 2?
There was a problem hiding this comment.
yea only enable the hook if we are on step 2
apps/web/src/modules/create-proposal/components/TransactionForm/Migration/Migration.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/modules/create-proposal/components/TransactionForm/Migration/Migration.tsx
Outdated
Show resolved
Hide resolved
| L2_DEPLOYMENT_ADDRESSES[targetChainID].MANAGER, | ||
| encodeFunctionData({ | ||
| abi: parseAbi([ | ||
| 'function deploy(FounderParams[] calldata _founderParams, address[] calldata _implAddresses, bytes[] calldata _implData)', |
There was a problem hiding this comment.
same thing about the ABI for these calls (mentioned this to you already but adding here so we don't forget)
apps/web/src/modules/create-proposal/components/TransactionForm/Migration/MigrationForm.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/modules/create-proposal/components/TransactionForm/Migration/MigrationForm.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/modules/create-proposal/components/TransactionForm/Migration/MigrationFormC2.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/modules/create-proposal/components/TransactionForm/Migration/MigrationFormC2.tsx
Outdated
Show resolved
Hide resolved
| onSubmit, | ||
| disabled, | ||
| }) => { | ||
| const [receivingChain, setReceivingChain] = useState<ChainType>('ZORA') |
There was a problem hiding this comment.
use formik instead of useState for form data
There was a problem hiding this comment.
in both DroposalForm and create, the dropdowns are used to set the state. should I just set onChange to be null?
There was a problem hiding this comment.
ah yea I think the dropdown component is a bit weird. continue using state hooks for now
| ] | ||
| : [zeroFounder] | ||
|
|
||
| const abiCoder = new ethers.utils.AbiCoder() |
There was a problem hiding this comment.
mentioned this to you already but remove any instances of ethers. commenting here so we dont forget
| ) | ||
|
|
||
| const propertyMetadataParamsHex = abiCoder.encode( | ||
| ['string', 'string', 'string', 'string'], |
There was a problem hiding this comment.
these params should actually be encoded as structs. pull this from the ABI instead of hardcoding here to get the right values
There was a problem hiding this comment.
these don't exist in the Manager ABI - should i be pulling it from the IMetadata ABI for example? That's the only place I see the actual structs
Description
This PR has all my work on the migration work so far, primarily things in the TransactionForm/Migration folder.
Code review
This isn't meant to be fully functional / ready on the front-end, but moreso away to get feedback on progress so far, and whether the data being used to prepare the L1Messenger transaction is working, since there's a lot of moving parts. I want to make sure to catch stuff early.
I'm not a fan of the C1/C2/C3 structure for naming the files but the sections are very linear - you have to do them in order, and so it makes sense to reflect that in the filenames?
Type of change
Checklist