Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f614d9ef8a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| type: z.literal("custom"), | ||
| contractAddress: addressOrEnsSchema, | ||
| abi: z.array(z.any()), | ||
| functionName: z.string().min(1, "Required"), |
There was a problem hiding this comment.
The modal’s “Paste Calldata” path submits a custom action with abi: [] and functionName: "", but the schema unconditionally requires functionName to be non-empty. In practice, once validation runs after adding this action, formState.isValid becomes false and publishing is blocked even though the UI explicitly offers calldata mode. The custom-action schema needs a calldata-compatible branch (e.g., functionName optional when calldata is present).
Useful? React with 👍 / 👎.
| encodeFunctionData({ | ||
| abi: action.abi, | ||
| functionName: action.functionName, | ||
| args: resolvedArgs, | ||
| }), |
There was a problem hiding this comment.
Custom actions are always encoded via encodeFunctionData(...), which assumes a valid ABI/function signature and ignores the calldata field entirely. For actions created via the calldata flow (abi: [], empty function name), this path throws during publish and no transaction is sent. Add a branch that uses action.calldata directly (and action.value for ETH value) when calldata mode is used.
Useful? React with 👍 / 👎.
| const visibleTabs = useMemo(() => { | ||
| const tabs = [...PROPOSAL_TABS]; | ||
| if (isConnected) { |
There was a problem hiding this comment.
visibleTabs always starts from PROPOSAL_TABS, so connected users see an Offchain tab even for DAOs where hasOffchain is false. In that state, selecting Offchain still renders onchain data because isOnchain is forced true (activeTab === "onchain" || !hasOffchain), producing a misleading tab/content mismatch. Build tabs conditionally from hasOffchain so unavailable tabs are not shown.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d5f2e26a3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| (values.title ? 1 : 0) + | ||
| (values.discussionUrl ? 1 : 0) + | ||
| (values.body && values.body !== BODY_PLACEHOLDER ? 1 : 0); | ||
| const canPublish = | ||
| filledCount === 3 && |
There was a problem hiding this comment.
discussionUrl is optional in ProposalFormSchema, but canPublish currently requires filledCount === 3 where one of those three fields is discussionUrl. In practice, a user who fills title/body/actions and leaves the discussion URL blank can never enable the Publish button, which blocks valid proposals. This should not gate publish readiness unless discussion URL is actually a required field.
Useful? React with 👍 / 👎.
| const id = drafts.saveDraft( | ||
| { | ||
| daoId, | ||
| title: values.title, | ||
| discussionUrl: values.discussionUrl ?? "", | ||
| body: values.body, | ||
| actions: values.actions, | ||
| }, | ||
| currentDraftId, | ||
| ); | ||
| setCurrentDraftId(id); | ||
| form.reset(values, { keepValues: true, keepDirty: false }); | ||
| showCustomToast("Draft saved", "success"); | ||
| if (options?.navigateToDrafts !== false) { |
There was a problem hiding this comment.
useDrafts.saveDraft returns an empty ID when no address is present (key is null), but handleSaveDraft still shows a success toast, clears dirty state, and may navigate away. If someone opens this page without a connected wallet (or disconnects mid-session), they get a successful save UX while nothing is actually persisted, causing silent data loss.
Useful? React with 👍 / 👎.
| items={functions.map((fn) => ({ | ||
| label: fn.name, | ||
| value: fn.name, | ||
| }))} |
There was a problem hiding this comment.
The function picker uses fn.name as both label and value, so overloaded ABI entries collapse to the same option value and cannot be selected distinctly. In overloaded contracts (e.g., multiple functions with the same name), this makes users unable to target the intended signature and can produce mis-encoded calldata or reverted submissions.
Useful? React with 👍 / 👎.
1 participant
No description provided.