feat: add governance action modal and integrate queue/execute functions#1784
feat: add governance action modal and integrate queue/execute functions#1784
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81b161426a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <Button | ||
| className="hidden lg:flex" | ||
| onClick={() => setIsQueueModalOpen(true)} | ||
| > | ||
| Queue Proposal |
There was a problem hiding this comment.
Restrict governance action buttons to valid states
These CTAs are rendered unconditionally, so users can open queue/execute flows for proposals that are not actionable (for example ongoing or defeated) and even while disconnected. In those cases the modal either fails on-chain with a revert or sits in a waiting state because handleAction exits early when no wallet/account is available, which creates a broken primary flow for proposal pages. Please gate these buttons by both wallet connection and the expected status transitions (succeeded for queue, pending_execution/queued for execute).
Useful? React with 👍 / 👎.
| abi: GovernorQueueAbi, | ||
| address, | ||
| functionName: "queue", | ||
| args: contractArgs, |
There was a problem hiding this comment.
Add DAO-specific action routing before queue calls
This utility always submits OpenZeppelin-style queue/execute(targets,values,calldatas,descriptionHash) calls for every DAO, but at least SHU uses Azorius governance (its ABI exposes executeProposal rather than OZ queue/execute). As a result, governance actions for SHU will fail during simulation/send even when the proposal is otherwise executable. This needs DAO-specific dispatch similar to the vote handler selection logic, or explicit exclusion of unsupported DAOs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68fe14f863
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | Address | ||
| | undefined; | ||
| const chain = daoConfig?.daoOverview?.chain; | ||
| const { data: walletClient } = useWalletClient({ chainId: chain?.id }); |
There was a problem hiding this comment.
Handle wrong-chain wallets before opening delegation flow
Using useWalletClient({ chainId }) returns no wallet client when the user is connected on a different network; with this change, the modal now enters the waiting state but never progresses because both the effect and action handler require a truthy walletClient. In that scenario users see an indefinite "confirm in wallet" step with no network-switch prompt or error, so delegation is effectively blocked unless they manually discover the chain mismatch.
Useful? React with 👍 / 👎.
| account: Address, | ||
| ): ActionArgs => { | ||
| return { | ||
| targets: proposalTargets.map((t) => (t ?? "0x") as `0x${string}`), |
There was a problem hiding this comment.
Validate null targets instead of coercing to invalid address
This conversion turns missing targets into "0x", which is not a valid address, so any proposal payload containing a null target (the function explicitly accepts (string | null)[]) will fail during simulateContract before transaction submission. That makes queue/execute unusable for those records and hides the underlying data problem behind a low-level address error; input validation with a clear message is safer than placeholder coercion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93f0ac28b7
ℹ️ 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".
apps/dashboard/features/governance/utils/submitGovernanceAction.ts
Outdated
Show resolved
Hide resolved
| }, [address, walletClient, step, action, proposal, daoId, onClose, chain]); | ||
|
|
||
| useEffect(() => { | ||
| if (!isOpen || !walletClient || step !== "waiting-signature") return; |
There was a problem hiding this comment.
Surface wrong-chain state before auto-submitting action
This effect exits when walletClient is undefined, but the modal still renders the active "confirm in wallet" step; with useWalletClient({ chainId }), that undefined case happens whenever the user is connected on the wrong network. Because the header buttons are gated by address (not chain), users can open queue/execute and get stuck in an indefinite waiting state with no network-switch prompt or error.
Useful? React with 👍 / 👎.
| import { mainnet } from "wagmi/chains"; | ||
| import { mainnet, optimism, scroll } from "wagmi/chains"; | ||
|
|
||
| const alchemyApiKey = process.env.NEXT_PUBLIC_ALCHEMY_KEY; |
There was a problem hiding this comment.
this will leak the alchemy api key
There was a problem hiding this comment.
The key was already exposed in the client-side bundle. Not introduced here. But I did a research here and looks like exposing alchemy key is normal because on alchemy dashboard we relate the key with a specific domain. So, no problems exposing it.
apps/dashboard/features/governance/components/proposal-overview/ProposalSection.tsx
Outdated
Show resolved
Hide resolved
| /> | ||
| {address && | ||
| proposalStatus === "succeeded" && | ||
| daoId.toUpperCase() !== DaoIdEnum.SHU && ( |
There was a problem hiding this comment.
SHU uses Azorius governance which has no queue step, proposals go directly to execute
apps/dashboard/features/governance/components/modals/GovernanceActionModal.tsx
Outdated
Show resolved
Hide resolved
| proposalTargets: (string | null)[], | ||
| proposalValues: (string | null)[], | ||
| proposalCalldatas: (string | null)[], |
There was a problem hiding this comment.
graphql schema returns (Maybe< String >[]). thats why
| proposalTargets: (string | null)[], | ||
| proposalValues: (string | null)[], | ||
| proposalCalldatas: (string | null)[], |
There was a problem hiding this comment.
why would you have this as null?
There was a problem hiding this comment.
graphql schema returns (Maybe< String >[]). thats why
apps/dashboard/features/governance/utils/submitGovernanceAction.ts
Outdated
Show resolved
Hide resolved
apps/dashboard/features/governance/utils/submitGovernanceAction.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bca9397988
ℹ️ 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".
| proposal.targets.filter((t) => t !== null), | ||
| proposal.values.filter((v) => v !== null), | ||
| (proposal.calldatas ?? []).filter((c) => c !== null), |
There was a problem hiding this comment.
Keep governance payload arrays index-aligned
handleAction removes nulls from targets, values, and calldatas independently, which can desynchronize operations whenever one list has a null at a different index. In that scenario, queue/execute can revert on array-length mismatch or submit mismatched target/value/calldata tuples. Because the proposal fields are typed as nullable lists, this is a reachable production path for malformed records; these arrays should be validated and transformed in lockstep (or rejected) instead of filtered separately.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5bbbae4c2
ℹ️ 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".
| case DaoIdEnum.UNISWAP: | ||
| case DaoIdEnum.NOUNS: { |
There was a problem hiding this comment.
Route Lil Nouns actions through GovernorBravo path
This switch only special-cases UNI and NOUNS, so LIL_NOUNS falls into the default OpenZeppelin 4-arg queue/execute path. Fresh evidence: apps/api/src/lib/client.ts maps DaoIdEnum.LIL_NOUNS to NounsClient, and Nouns-style governors expose proposal-id-based queue/execute calls; using the 4-arg call shape here will fail simulation/submission and block governance actions for Lil Nouns proposals.
Useful? React with 👍 / 👎.
| <Button | ||
| className="hidden lg:flex" | ||
| onClick={() => setIsQueueModalOpen(true)} |
There was a problem hiding this comment.
Expose queue/execute actions on mobile proposal pages
The new queue/execute CTA is desktop-only (hidden lg:flex), and this change does not add any mobile trigger for the new modals (the mobile bottom bar still only wires voting). On mobile viewports, users cannot open queue/execute flows for succeeded/queued proposals, so governance lifecycle actions are unavailable on phones/tablets.
Useful? React with 👍 / 👎.
No description provided.