feat: add DAO proposal linking#14
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds DAOhaus v3 proposal linking: DB migration and schema updates, a server sync engine matching on-chain BAAL proposals with DAOhaus subgraph data, query APIs for proposal activity, classification/view wiring, admin sync integration, and member/admin UI pages to view proposal-linked transfers. ChangesProposal Linking System
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin User
participant SyncForm as Sync Form
participant SyncAction as syncQuarterTransactions
participant SyncEngine as syncDaoProposalsForPeriod
participant GnosisRPC as Gnosis RPC
participant DAOhaus as DAOhaus Subgraph
participant DB as Database
participant Audit as Audit Log
Admin->>SyncForm: Click Sync Transactions
SyncForm->>SyncAction: Submit quarterly sync
par Treasury Sync
SyncAction->>DB: Sync treasury_transactions
and Proposal Sync
SyncAction->>SyncEngine: syncDaoProposalsForPeriod(period)
SyncEngine->>DB: Select treasury_transactions in period
SyncEngine->>GnosisRPC: Fetch candidate transactions / decode processProposal
SyncEngine->>DAOhaus: Introspect & fetch proposals
SyncEngine->>SyncEngine: Match proposals by execution tx hash
SyncEngine->>DB: Upsert dao_proposals
SyncEngine->>DB: Update treasury_transactions.daoProposalId
SyncEngine-->>SyncAction: DaoProposalSyncResult (counts/errors)
end
SyncAction->>Audit: Write audit event (treasury + proposal outcomes)
SyncAction->>DB: Revalidate /proposals cache
SyncAction-->>SyncForm: Redirect with proposal match count
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds DAOhaus proposal syncing and linking into the quarter transaction import/review workflow, persisting proposals in a new dao_proposals table and exposing linked proposal context in both the admin transaction review UI and a new member-facing proposal activity page.
Changes:
- Add
dao_proposalstable +treasury_transactions.dao_proposal_idforeign key and indexes to persist proposal metadata and link treasury transactions to proposals. - Extend the quarter sync server action to sync proposals for the quarter period, audit the outcome, and refresh the new
/proposalspage. - Update UI to show proposal links inline/contextually in admin transaction review and add a new
/proposalsactivity page for members.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/transaction-classification.ts | Extends transfer classification view/query to include optional linked DAO proposal data. |
| src/lib/proposal-activity.ts | Adds server-side query/mapping for proposal-linked transfer activity rows. |
| src/lib/dao-proposals.ts | Implements proposal discovery (subgraph + onchain), upsert into dao_proposals, and linking to treasury transactions. |
| src/db/schema.ts | Introduces daoProposals table and adds daoProposalId reference on treasuryTransactions. |
| src/app/proposals/page.tsx | Adds member-facing proposal activity page UI. |
| src/app/page.tsx | Adds “Proposals” navigation entry for members with access. |
| src/app/admin/quarters/[id]/transactions/transaction-review-toast.tsx | Extends sync toast messaging to include proposal link counts. |
| src/app/admin/quarters/[id]/transactions/sync-transactions-form.tsx | Enhances sync button UX with step-based progress labels. |
| src/app/admin/quarters/[id]/transactions/page.tsx | Displays linked proposal context inline and in transfer details; passes proposal count to toast. |
| src/app/admin/quarters/[id]/transactions/actions.ts | Runs proposal sync during quarter sync, logs audit metadata, and revalidates proposals page. |
| drizzle/meta/0009_snapshot.json | Records updated schema snapshot including new table/column/indexes. |
| drizzle/meta/_journal.json | Adds migration journal entry for migration 0009. |
| drizzle/0009_wakeful_freak.sql | Adds dao_proposals table, treasury_transactions.dao_proposal_id, indexes, FK, and trigger. |
| .env.example | Documents new DAOhaus syncing configuration environment variables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
src/lib/dao-proposals.ts (2)
130-141: ⚡ Quick winConsider consistent error handling across configuration getters.
getGnosisClient()throws ifGNOSIS_RPC_URLis missing, whilegetDaoAddress()returnsnullifDAO_CONTRACT_ADDRESSis missing. This inconsistency could cause confusing errors: if the DAO address is missing, sync gracefully returnsskipped: true, but if the RPC URL is missing later, it throws an exception ingetOnchainProposalMatches().Consider either throwing early for both missing configs or handling both as optional with appropriate skip logic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/dao-proposals.ts` around lines 130 - 141, getGnosisClient() currently throws when GNOSIS_RPC_URL is missing while getDaoAddress() returns null, causing inconsistent behavior between getOnchainProposalMatches() and the DAO sync flow; make them consistent by choosing one approach and implementing it across the helpers: either (A) make getDaoAddress() throw the same Error as getGnosisClient() so missing config fails fast, or (B) make getGnosisClient() return null (or undefined) instead of throwing and update getOnchainProposalMatches() to detect a null client or address and return { skipped: true } early. Update all callers of getGnosisClient, getDaoAddress, and getOnchainProposalMatches to follow the chosen pattern so missing GNOSIS_RPC_URL or DAO_CONTRACT_ADDRESS are handled identically.
629-679: ⚡ Quick winUpsert fallback logic is indirect and could be clearer.
The function attempts
insert().onConflictDoNothing()which can conflict on either unique index ((chain_id, dao_address, proposal_id)or(chain_id, execution_tx_hash)), then falls back toupdate()byexecution_tx_hash. While this works, it's indirect and makes the intent unclear.Consider explicit conflict handling
+ const [row] = await db + .insert(daoProposals) + .values(values) + .onConflictDoUpdate({ + target: [daoProposals.chainId, daoProposals.executionTxHash], + set: values, + }) + .returning(); + - const [row] = await db - .insert(daoProposals) - .values(values) - .onConflictDoNothing() - .returning(); - const proposalRow = - row ?? - ( - await db - .update(daoProposals) - .set(values) - .where( - and( - eq(daoProposals.chainId, gnosis.id), - sql`lower(${daoProposals.executionTxHash}) = ${proposal.executionTxHash}`, - ), - ) - .returning() - )[0]; + const proposalRow = row;This makes the intent explicit: upsert by execution hash, which is the unique key for linking to transactions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/dao-proposals.ts` around lines 629 - 679, The upsert flow in upsertAndLinkProposal is indirect (insert().onConflictDoNothing() + separate update by execution_tx_hash); change it to a single explicit upsert by the execution hash unique key: perform insert(daoProposals).values(values).onConflictDoUpdate(...) targeting the unique constraint/key for (chainId, executionTxHash) (use the same lower-case/normalized expression used elsewhere for executionTxHash) and set the columns to the incoming values, then use the returned row as proposalRow; keep the subsequent update to treasuryTransactions as-is (update(treasuryTransactions).set({ daoProposalId: proposalRow.id }) ...). Ensure you preserve the normalization (lower()) when matching executionTxHash so the onConflict target and the where used for matching are consistent.src/app/page.tsx (1)
137-139: 💤 Low valueUse optional chaining for permission checks inside the nav block.
Line 137 directly accesses
session.permissions.canAccesswithout optional chaining. Although the parent condition at lines 130-132 guaranteessession.permissionsexists when this code runs, using optional chaining (session.permissions?.canAccess) improves defensive coding and makes the code more resilient to future refactoring. The same pattern applies to lines 140, 143, and 146.♻️ Suggested pattern for all permission checks
- {session.permissions.canAccess ? ( + {session.permissions?.canAccess ? ( <AppNavLink href="/proposals">Proposals</AppNavLink> ) : null} - {session.permissions.canWriteRaidAccounting ? ( + {session.permissions?.canWriteRaidAccounting ? ( <AppNavLink href="/raids">Raids</AppNavLink> ) : null} - {session.permissions.canAdmin ? ( + {session.permissions?.canAdmin ? ( <AppNavLink href="/admin/providers">Providers</AppNavLink> ) : null} - {session.permissions.canAdmin ? ( + {session.permissions?.canAdmin ? (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/page.tsx` around lines 137 - 139, The nav block directly accesses session.permissions.canAccess (rendering <AppNavLink href="/proposals">) — update these checks to use optional chaining (e.g., session.permissions?.canAccess) to guard against undefined permissions; apply the same pattern to the other permission checks in the same nav block (the checks at/around lines referencing session.permissions for other links) so all conditionals use session.permissions?., keeping the existing rendering of AppNavLink intact.src/app/proposals/page.tsx (1)
177-179: 💤 Low valueUse optional chaining for defensive access to
session.permissions.Although line 155's early return guarantees
session.permissionsexists here, using optional chaining makes the code more resilient to refactoring and clearer about null-safety.♻️ Suggested improvement
const rows = await listProposalActivity({ - visibility: session.permissions.canAdmin ? "admin" : "member", + visibility: session.permissions?.canAdmin ? "admin" : "member", });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/proposals/page.tsx` around lines 177 - 179, The call to listProposalActivity uses session.permissions directly; change it to use optional chaining on session.permissions (e.g., session?.permissions?.canAdmin) so the visibility argument is computed defensively, e.g., fallback to "member" when permissions are undefined; update the invocation at the rows = await listProposalActivity({...}) site and ensure the ternary uses session?.permissions?.canAdmin to determine "admin" vs "member".src/lib/proposal-activity.ts (1)
86-86: 💤 Low valueRedundant WHERE clause after INNER JOIN.
Line 86 filters
isNotNull(treasuryTransactions.daoProposalId), but line 68 already inner joins on that FK — any row in the result set must have a non-nulldaoProposalId. The WHERE clause is harmless but unnecessary and may confuse readers.♻️ Simplify by removing redundant filter
.leftJoin(quarters, eq(ledgerEntries.quarterId, quarters.id)) .leftJoin(entities, eq(ledgerEntries.counterpartyEntityId, entities.id)) - .where(isNotNull(treasuryTransactions.daoProposalId)) .orderBy(asc(daoProposals.executedAt), asc(treasuryTransactionTransfers.id));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/proposal-activity.ts` at line 86, Remove the redundant WHERE filter that checks isNotNull(treasuryTransactions.daoProposalId) in the query chain inside src/lib/proposal-activity.ts: since the code already performs an INNER JOIN on the treasuryTransactions.daoProposalId FK (see the join earlier in the same query), delete the .where(isNotNull(treasuryTransactions.daoProposalId)) call so the query only relies on the INNER JOIN and is clearer; look for the query chain using treasuryTransactions.daoProposalId and remove that .where invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/admin/quarters/`[id]/transactions/actions.ts:
- Around line 214-217: The query param construction is using the wrong metric:
replace proposalResult.matchedProposals with the linked-count field
(proposalResult.linkedTransactions) when building the URLSearchParams so the
"proposals" param reflects actually linked proposals; update the params creation
(where URLSearchParams is constructed and currently references
result.importedTransfers and proposalResult.matchedProposals) to stringify
proposalResult.linkedTransactions instead, preserving the existing String(...)
conversion and other keys (imported, syncId).
In `@src/app/admin/quarters/`[id]/transactions/sync-transactions-form.tsx:
- Around line 21-31: The useEffect that advances labels while pending doesn't
reset the UI position, so add logic in the useEffect watching pending to reset
stepIndex to 0 whenever a new sync starts or finishes; specifically, inside the
same effect that currently references pending and SYNC_STEPS, call
setStepIndex(0) when pending becomes true (before starting the interval) and
also when pending becomes false (or when cleaning up) so the label returns to
the initial "Syncing treasury…" state; update the effect that contains
useEffect, pending, setStepIndex, stepIndex, and SYNC_STEPS accordingly.
In `@src/app/proposals/page.tsx`:
- Around line 34-51: In formatCurrency, invalid parsed numbers (NaN, ±Infinity)
are currently rendered as "$NaN"/"$Infinity"; update the Number.isFinite check
in the formatCurrency function so that when Number.isFinite(number) is false you
return "-" (same as the null case) instead of returning `$${value}`, preserving
the existing null check and the Intl.NumberFormat formatting path for valid
finite numbers.
In `@src/lib/dao-proposals.ts`:
- Around line 432-438: Replace the non-deterministic fallback in getProposalId
so it does not call crypto.randomUUID(); instead return undefined when no
explicit ID fields are present (i.e., only pickString from
["proposalId","proposalNumber","proposalIndex","serial","id"] and otherwise
undefined). Then update getMatchedProposal and getOnchainProposalMatch to skip
proposals with undefined IDs or, when an execution transaction hash is
available, derive a deterministic ID from that hash (use the
executionTransaction.hash + chain context) so IDs are stable across re-syncs;
ensure all checks and DB uniqueness relies on the deterministic ID or skips the
record when none exists.
- Around line 576-594: getOnchainProposalMatches performs RPC calls sequentially
by awaiting getOnchainProposalMatch inside a for loop; change it to run calls in
parallel with a concurrency limit to avoid blocking the sync workflow. Map each
transaction to a Promise that calls getOnchainProposalMatch and use Promise.all
or a controlled concurrency helper (e.g., p-map, PromisePool, or a simple
batching loop) to await results, then iterate results to set
matches.set(match.executionTxHash, match) for non-null matches; keep the
function signature and return type the same and introduce a configurable
concurrency constant (or parameter) to throttle RPC calls.
In `@src/lib/proposal-activity.ts`:
- Around line 41-43: The helper decryptNullableField currently casts any truthy
value to EncryptedField and calls decryptField, which can throw if the shape is
wrong; update decryptNullableField to first validate the object's shape
(presence and types of algorithm, keyId, iv, tag, ciphertext) or wrap the
decryptField call in a try/catch, returning null on invalid shape or decryption
error; reference the decryptNullableField and decryptField functions and the
EncryptedField shape so the checks specifically guard those properties before
casting or call decryptField inside a safe try/catch block to avoid throwing on
malformed data.
---
Nitpick comments:
In `@src/app/page.tsx`:
- Around line 137-139: The nav block directly accesses
session.permissions.canAccess (rendering <AppNavLink href="/proposals">) —
update these checks to use optional chaining (e.g.,
session.permissions?.canAccess) to guard against undefined permissions; apply
the same pattern to the other permission checks in the same nav block (the
checks at/around lines referencing session.permissions for other links) so all
conditionals use session.permissions?., keeping the existing rendering of
AppNavLink intact.
In `@src/app/proposals/page.tsx`:
- Around line 177-179: The call to listProposalActivity uses session.permissions
directly; change it to use optional chaining on session.permissions (e.g.,
session?.permissions?.canAdmin) so the visibility argument is computed
defensively, e.g., fallback to "member" when permissions are undefined; update
the invocation at the rows = await listProposalActivity({...}) site and ensure
the ternary uses session?.permissions?.canAdmin to determine "admin" vs
"member".
In `@src/lib/dao-proposals.ts`:
- Around line 130-141: getGnosisClient() currently throws when GNOSIS_RPC_URL is
missing while getDaoAddress() returns null, causing inconsistent behavior
between getOnchainProposalMatches() and the DAO sync flow; make them consistent
by choosing one approach and implementing it across the helpers: either (A) make
getDaoAddress() throw the same Error as getGnosisClient() so missing config
fails fast, or (B) make getGnosisClient() return null (or undefined) instead of
throwing and update getOnchainProposalMatches() to detect a null client or
address and return { skipped: true } early. Update all callers of
getGnosisClient, getDaoAddress, and getOnchainProposalMatches to follow the
chosen pattern so missing GNOSIS_RPC_URL or DAO_CONTRACT_ADDRESS are handled
identically.
- Around line 629-679: The upsert flow in upsertAndLinkProposal is indirect
(insert().onConflictDoNothing() + separate update by execution_tx_hash); change
it to a single explicit upsert by the execution hash unique key: perform
insert(daoProposals).values(values).onConflictDoUpdate(...) targeting the unique
constraint/key for (chainId, executionTxHash) (use the same
lower-case/normalized expression used elsewhere for executionTxHash) and set the
columns to the incoming values, then use the returned row as proposalRow; keep
the subsequent update to treasuryTransactions as-is
(update(treasuryTransactions).set({ daoProposalId: proposalRow.id }) ...).
Ensure you preserve the normalization (lower()) when matching executionTxHash so
the onConflict target and the where used for matching are consistent.
In `@src/lib/proposal-activity.ts`:
- Line 86: Remove the redundant WHERE filter that checks
isNotNull(treasuryTransactions.daoProposalId) in the query chain inside
src/lib/proposal-activity.ts: since the code already performs an INNER JOIN on
the treasuryTransactions.daoProposalId FK (see the join earlier in the same
query), delete the .where(isNotNull(treasuryTransactions.daoProposalId)) call so
the query only relies on the INNER JOIN and is clearer; look for the query chain
using treasuryTransactions.daoProposalId and remove that .where invocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a18dc988-3624-4628-830f-9db0b42ded94
📒 Files selected for processing (14)
.env.exampledrizzle/0009_wakeful_freak.sqldrizzle/meta/0009_snapshot.jsondrizzle/meta/_journal.jsonsrc/app/admin/quarters/[id]/transactions/actions.tssrc/app/admin/quarters/[id]/transactions/page.tsxsrc/app/admin/quarters/[id]/transactions/sync-transactions-form.tsxsrc/app/admin/quarters/[id]/transactions/transaction-review-toast.tsxsrc/app/page.tsxsrc/app/proposals/page.tsxsrc/db/schema.tssrc/lib/dao-proposals.tssrc/lib/proposal-activity.tssrc/lib/transaction-classification.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/proposal-activity.ts (1)
97-100: ⚡ Quick winPush the member visibility rule into the SQL query.
Line 97 applies the access check after all joined rows have already been loaded. Member requests still read admin-only proposal rows from the database, which does extra work and weakens the data-minimization boundary in this query layer. Apply the
publishedpredicate in the query itself instead of post-filtering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/proposal-activity.ts` around lines 97 - 100, The current post-query filter on rows (the rows.filter using visibility and row.quarter?.status === "published") lets non-admin requests fetch admin-only rows from the DB; instead push this predicate into the SQL that loads rows so only published quarters are returned for non-admins. Modify the query builder used to produce rows (the SQL that joins quarter) to add WHERE (visibility = 'admin' OR quarter.status = 'published') semantics — or more precisely, when visibility !== "admin" add the condition quarter.status = 'published' — so the filter isn't performed in the returned rows array.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/proposal-activity.ts`:
- Around line 97-100: The current post-query filter on rows (the rows.filter
using visibility and row.quarter?.status === "published") lets non-admin
requests fetch admin-only rows from the DB; instead push this predicate into the
SQL that loads rows so only published quarters are returned for non-admins.
Modify the query builder used to produce rows (the SQL that joins quarter) to
add WHERE (visibility = 'admin' OR quarter.status = 'published') semantics — or
more precisely, when visibility !== "admin" add the condition quarter.status =
'published' — so the filter isn't performed in the returned rows array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d1fbc3c4-d6ff-4b63-8a6c-29ae5d849497
📒 Files selected for processing (6)
src/app/admin/quarters/[id]/transactions/actions.tssrc/app/admin/quarters/[id]/transactions/sync-transactions-form.tsxsrc/app/page.tsxsrc/app/proposals/page.tsxsrc/lib/dao-proposals.tssrc/lib/proposal-activity.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/app/page.tsx
- src/app/admin/quarters/[id]/transactions/sync-transactions-form.tsx
- src/app/admin/quarters/[id]/transactions/actions.ts
- src/app/proposals/page.tsx
- src/lib/dao-proposals.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces DAO proposal syncing and linking to the admin quarter transaction workflow, allowing treasury transfers to be associated with DAOhaus proposals. It adds a new database table for proposals, updates the sync process to fetch and match proposals, and enhances the UI to display proposal information and feedback during syncs.
DAO Proposal Integration
dao_proposalstable to the database, with unique indexes and a foreign key fromtreasury_transactions, enabling transactions to be linked to DAOhaus proposals.DAOHAUS_SUBGRAPH_URL,DAOHAUS_APP_BASE_URL) in.env.exampleto configure DAOhaus proposal syncing.Sync Process Enhancements
User Interface Improvements
ProposalInline,ProposalContext, andProposalTitleLinkcomponents. (src/app/admin/quarters/[id]/transactions/page.tsxR34, src/app/admin/quarters/[id]/transactions/page.tsxR278-R344, src/app/admin/quarters/[id]/transactions/page.tsxR511, src/app/admin/quarters/[id]/transactions/page.tsxR631-R632)Navigation and Permissions
Database Migration Tracking
Summary by CodeRabbit
New Features
UX
Documentation