-
Notifications
You must be signed in to change notification settings - Fork 12
Add PillarDAO app #423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Add PillarDAO app #423
Conversation
WalkthroughAdds a new PillarDAO sub-app (manifest, entry, panels, styles, components, tests), WalletConnect onboarding and session management, Snapshot voting integration with pagination and fallback, membership on-chain reads with localStorage caching, Polygon chain support, build/deploy configs (nixpacks, wrangler), and a README note about ReOwn WalletConnect Project ID. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant OnboardingPanel
participant WalletConnect
participant SessionStore as "WC Sessions"
User->>OnboardingPanel: Paste wc: URI and click Connect
OnboardingPanel->>WalletConnect: connect(wcUri)
WalletConnect-->>SessionStore: create session
WalletConnect-->>OnboardingPanel: success / error
OnboardingPanel-->>User: display sessions or show error
sequenceDiagram
autonumber
actor User
participant VotingPanel
participant SnapshotAPI as "Snapshot GraphQL"
User->>VotingPanel: View latest proposals
VotingPanel->>SnapshotAPI: query(space: pillar-dao, first: N, skip: 0)
SnapshotAPI-->>VotingPanel: proposals[]
VotingPanel-->>User: render list
User->>VotingPanel: View more
VotingPanel->>SnapshotAPI: query(skip: next)
SnapshotAPI-->>VotingPanel: proposals[] | empty
VotingPanel-->>User: append or disable "more"
sequenceDiagram
autonumber
participant MembershipPanel
participant LocalStorage
participant PolygonRPC as "Polygon RPC"
MembershipPanel->>LocalStorage: read cache(chainId,address,nft)
alt connected
MembershipPanel->>PolygonRPC: balanceOf(address)
PolygonRPC-->>MembershipPanel: balance
opt has NFT
MembershipPanel->>PolygonRPC: tokenOfOwnerByIndex(address,0)
MembershipPanel->>PolygonRPC: viewDepositTimestamp(tokenId)
PolygonRPC-->>MembershipPanel: tokenId, timestamp
end
end
MembershipPanel->>LocalStorage: write updated cache
MembershipPanel-->>User: display membership info
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/apps/pillardao/index.test.tsx (1)
85-181: Reset global spies between testsSince each scenario spies on
global.fetchandwindow.open, we should callvi.restoreAllMocks()(or restore those specific spies) in anafterEachto return the globals to their original implementations. That keeps later tests or other suites from inheriting mocked behaviour and makes the suite safer to extend.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
src/apps/pillardao/icon.pngis excluded by!**/*.pngsrc/apps/pillardao/images/pillar-dao-image.pngis excluded by!**/*.pngsrc/apps/pillardao/images/pillar-dao-member-nft.pngis excluded by!**/*.pngsrc/apps/pillardao/images/wallet-connect-example.pngis excluded by!**/*.pngsrc/apps/pillardao/images/wallet-connect-sign-in-example.pngis excluded by!**/*.png
📒 Files selected for processing (13)
README.md(1 hunks)nixpacks.toml(1 hunks)src/apps/pillardao/components/AnimatedTitle.tsx(1 hunks)src/apps/pillardao/components/CopyHelp.tsx(1 hunks)src/apps/pillardao/components/MembershipPanel.tsx(1 hunks)src/apps/pillardao/components/OnboardingPanel.tsx(1 hunks)src/apps/pillardao/components/Styles.tsx(1 hunks)src/apps/pillardao/components/VotingPanel.tsx(1 hunks)src/apps/pillardao/index.test.tsx(1 hunks)src/apps/pillardao/index.tsx(1 hunks)src/apps/pillardao/manifest.json(1 hunks)src/containers/Main.tsx(3 hunks)wrangler.jsonc(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/apps/pillardao/components/CopyHelp.tsx (1)
src/components/LandingPage/Img/index.jsx (1)
Img(27-43)
src/apps/pillardao/components/MembershipPanel.tsx (1)
src/apps/pillardao/components/Styles.tsx (4)
Section(3-6)MembershipRow(29-37)NftBox(39-50)Row(8-10)
src/apps/pillardao/components/OnboardingPanel.tsx (2)
src/services/walletConnect.ts (1)
useWalletConnect(57-1297)src/apps/pillardao/components/Styles.tsx (10)
Section(3-6)Row(8-10)SmallNote(12-16)ConnectLayout(52-58)ConnectAside(69-73)ConnectInline(115-124)RightAddon(93-99)ConnectError(81-85)WalletInfo(101-113)SessionItem(184-196)
src/apps/pillardao/components/VotingPanel.tsx (1)
src/apps/pillardao/components/Styles.tsx (6)
Section(3-6)Row(8-10)ProposalsBox(18-27)ProposalItem(126-140)ProposalBody(148-160)CollapseButton(162-175)
src/apps/pillardao/index.test.tsx (1)
src/theme/index.ts (1)
defaultTheme(83-163)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/apps/pillardao/components/MembershipPanel.tsx (2)
51-59: Consider extracting ABI definitions to a constants file.The inline ABI definitions work correctly but could be moved to a shared constants file (e.g.,
abis.tsorconstants.ts) to improve maintainability and reusability.
167-172: Consider extracting date formatting to a helper function.The IIFE for date formatting could be extracted to improve readability and reusability.
Example refactor:
const formatMemberSince = (timestamp: number): string => { const d = new Date(timestamp * 1000); const date = d.toLocaleDateString(); const time = d.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' }); return `${date} ${time}`; };Then use:
- {effectiveDepositTs > 0 - ? (() => { - const d = new Date(effectiveDepositTs * 1000); - const date = d.toLocaleDateString(); - const time = d.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' }); - return `${date} ${time}`; - })() - : '—'} + {effectiveDepositTs > 0 ? formatMemberSince(effectiveDepositTs) : '—'}src/apps/pillarx-app/components/TokensWithMarketDataTile/test/TokensWithMarketDataTile.test.tsx (1)
207-207: Fix typo in test description.The test description has a typo: "renders the right number of rowsd" should be "renders the right number of rows".
- it('renders the right number of rowsd', () => { + it('renders the right number of rows', () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
src/apps/pillarx-app/components/EditorialTile/test/__snapshots__/EditorialTile.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokenMarketDataRow/tests/__snapshots__/LeftColumnTokenMarketDataRow.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokensWithMarketDataTile/test/__snapshots__/TokensWithMarketDataTile.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/CardsSwap/test/__snapshots__/CardSwap.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
src/apps/pillardao/components/CopyHelp.tsx(1 hunks)src/apps/pillardao/components/MembershipPanel.tsx(1 hunks)src/apps/pillarx-app/components/TokenMarketDataRow/tests/LeftColumnTokenMarketDataRow.test.tsx(1 hunks)src/apps/pillarx-app/components/TokensWithMarketDataTile/test/TokensWithMarketDataTile.test.tsx(2 hunks)wrangler.jsonc(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- wrangler.jsonc
- src/apps/pillardao/components/CopyHelp.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/apps/pillardao/components/MembershipPanel.tsx (1)
src/apps/pillardao/components/Styles.tsx (4)
Section(3-6)MembershipRow(29-37)NftBox(39-50)Row(8-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit-tests
- GitHub Check: lint
🔇 Additional comments (8)
src/apps/pillardao/components/MembershipPanel.tsx (6)
1-49: LGTM!The component setup, types, and cache utilities are well-structured. The localStorage operations are properly guarded with try-catch blocks, and the cache loading effect has correct dependencies.
61-88: LGTM! Previous review concern has been addressed.The zero-balance check in
effectiveMembershipId(lines 82-83) properly handles the case when NFT balance drops to zero, preventing stale cached token IDs from being displayed. This addresses the concern raised in the previous review.
90-103: LGTM!The deposit timestamp logic correctly prioritizes on-chain data over cached data, with a sensible fallback to zero.
105-110: LGTM!The truncate utility is straightforward and handles edge cases appropriately.
112-127: Verify the eslint-disable justification.The
exhaustive-depsdisable on line 127 appears intentional sincecachedis deliberately omitted to avoid infinite loops (the effect writes cache → updates state → triggers effect). However, this pattern can be fragile. Consider adding a comment explaining whycachedis intentionally excluded from the dependency array.
129-149: LGTM!The not-connected state UI is clear and appropriate.
src/apps/pillarx-app/components/TokensWithMarketDataTile/test/TokensWithMarketDataTile.test.tsx (1)
110-111: LGTM!Proper cleanup with
vi.useRealTimers()inafterEachensures fake timers don't leak between tests.src/apps/pillarx-app/components/TokenMarketDataRow/tests/LeftColumnTokenMarketDataRow.test.tsx (1)
36-44: Verifyviis imported or globally configured.The fake timer setup and cleanup follow the same correct pattern as the other test file in this PR, ensuring deterministic relative time calculations. However,
viis referenced without a visible import statement.The verification script from the first file (
TokensWithMarketDataTile.test.tsx) will also confirm whetherviis properly available in this test file. Ifglobals: trueis configured invitest.config.ts, no import is needed. Otherwise, add:import { vi } from 'vitest';
src/apps/pillarx-app/components/TokensWithMarketDataTile/test/TokensWithMarketDataTile.test.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/apps/pillardao/components/MembershipPanel.tsx (1)
186-214: LGTM! Security improvements applied.The
window.opencalls on lines 194 and 208 now include'noopener'in the features string, addressing the security concern from the previous review.The duplicate URL-building logic for Polygonscan and Blockscout could still be extracted into helper functions as suggested in the previous review, but this remains an optional refactor.
🧹 Nitpick comments (3)
src/apps/pillardao/components/MembershipPanel.tsx (3)
51-59: Consider extracting ABI definitions to a shared constants file.The inline ABI definitions for ERC721 and membership contract methods could be extracted to a shared constants file (e.g.,
src/apps/pillardao/constants/abis.ts) if they're used in multiple components across the PillarDAO app. This would improve maintainability and reduce duplication.Example structure:
// src/apps/pillardao/constants/abis.ts export const ERC721_BALANCE_ABI = [ { inputs: [{ name: 'owner', type: 'address' }], name: 'balanceOf', outputs: [{ type: 'uint256' }], stateMutability: 'view', type: 'function' }, ] as const; export const ERC721_ENUMERABLE_ABI = [ { inputs: [{ name: 'owner', type: 'address' }, { name: 'index', type: 'uint256' }], name: 'tokenOfOwnerByIndex', outputs: [{ type: 'uint256' }], stateMutability: 'view', type: 'function' }, ] as const; export const MEMBERSHIP_TIMESTAMP_ABI = [ { inputs: [{ name: 'member', type: 'address' }], name: 'viewDepositTimestamp', outputs: [{ type: 'uint256' }], stateMutability: 'view', type: 'function' }, ] as const;
112-127: Consider validating balance before writing tokenId to cache.When the NFT balance becomes zero, the
nftFirstTokenReadquery is disabled (line 77), but itsdatafield may retain the previous token ID value. The cache writing effect uses this potentially stale value on line 114 without checking the current balance. While the UI correctly handles this case (due to the balance check ineffectiveMembershipId), the cache could store stale membership data.Apply this diff to ensure cache integrity:
useEffect(() => { if (!isConnected) return; + const balance = nftBalanceRead?.data as bigint | undefined; const tokenId = nftFirstTokenRead?.data as bigint | undefined; const depositTs = Number(depositTsRead?.data || BigInt(0)); - const hasUseful = (tokenId && tokenId > BigInt(0)) || depositTs > 0; + // Only consider tokenId valid if balance > 0 + const validTokenId = balance && balance > BigInt(0) && tokenId && tokenId > BigInt(0) ? tokenId : undefined; + const hasUseful = validTokenId !== undefined || depositTs > 0; if (!hasUseful) return; writeMembershipCache({ address: resolvedAddress, chainId, nftContract, - tokenId: tokenId && tokenId > BigInt(0) ? String(tokenId) : cached?.tokenId, + tokenId: validTokenId ? String(validTokenId) : undefined, depositTimestamp: depositTs > 0 ? depositTs : (cached?.depositTimestamp || 0), updatedAt: Date.now(), }); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isConnected, chainId, resolvedAddress, nftContract, nftFirstTokenRead?.data, depositTsRead?.data]); + }, [isConnected, chainId, resolvedAddress, nftContract, nftBalanceRead?.data, nftFirstTokenRead?.data, depositTsRead?.data]);
167-172: Consider extracting date formatting to a helper function.The inline IIFE for date formatting could be extracted to a helper function for improved readability and potential reuse.
const formatMembershipDate = (timestamp: number): string => { const d = new Date(timestamp * 1000); const date = d.toLocaleDateString(); const time = d.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' }); return `${date} ${time}`; }; // Usage: <div className="value"> {effectiveDepositTs > 0 ? formatMembershipDate(effectiveDepositTs) : '—'} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/apps/pillardao/components/MembershipPanel.tsx(1 hunks)src/apps/pillarx-app/components/TokenMarketDataRow/tests/LeftColumnTokenMarketDataRow.test.tsx(2 hunks)src/apps/pillarx-app/components/TokensWithMarketDataTile/test/TokensWithMarketDataTile.test.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/apps/pillarx-app/components/TokensWithMarketDataTile/test/TokensWithMarketDataTile.test.tsx
- src/apps/pillarx-app/components/TokenMarketDataRow/tests/LeftColumnTokenMarketDataRow.test.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/apps/pillardao/components/MembershipPanel.tsx (1)
src/apps/pillardao/components/Styles.tsx (4)
Section(3-6)MembershipRow(29-37)NftBox(39-50)Row(8-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit-tests
- GitHub Check: lint
🔇 Additional comments (3)
src/apps/pillardao/components/MembershipPanel.tsx (3)
61-97: LGTM! Contract reads are well-structured.The contract read hooks are properly configured:
- Balance read is always enabled when connected
- Token read is conditionally enabled only when balance > 0 (good optimization)
- Timestamp read is enabled when connected
The query enabling logic correctly prevents unnecessary on-chain calls when the wallet has no NFTs.
81-88: LGTM! Zero-balance handling is correct.The balance check on line 82-83 correctly prevents using stale token IDs from cache or disabled queries when the wallet's NFT balance is zero. This fix (from previous review) ensures the UI accurately reflects membership status.
179-179: LGTM! Alt text has been added.The NFT image now includes descriptive alt text ("PillarDAO Member NFT"), which improves accessibility for screen readers. This addresses the previous review comment.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores