-
Notifications
You must be signed in to change notification settings - Fork 116
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
feat: argent discount #1034
feat: argent discount #1034
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update adjusts multiple discount-related components and refines Argent wallet integration. The Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant AP as ArgentPage
participant LS as LocalStorage
participant WS as Wallet Checker
U->>AP: Load Argent page
AP->>LS: Check discount expiry
LS-->>AP: Return expiry status
AP->>AP: Set screen based on expiry
AP->>WS: Verify wallet connection (is Argent?)
WS-->>AP: Return wallet status
AP->>U: Render appropriate screen (prompt, discount offer, or registration)
Assessment against linked issues
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 2
🔭 Outside diff range comments (1)
components/discount/registerDiscount.tsx (1)
52-413
: Consider breaking down the large component.The component is handling multiple responsibilities including:
- Domain registration
- Sales tax calculation
- Auto-renewal
- Swiss residency
- Payment processing
Consider splitting it into smaller, focused components for better maintainability.
Example structure:
// components/discount/register/ ├── PaymentSection.tsx // Currency selection & price display ├── SalesTaxCalculator.tsx // Swiss VAT logic ├── AutoRenewalSection.tsx // Renewal checkbox & logic ├── RegistrationForm.tsx // Main form & validation └── RegisterDiscount.tsx // Main component (orchestrator)
🧹 Nitpick comments (2)
pages/argent.tsx (1)
15-16
: Consider using an enum for screen management.Using a number for screen state makes the code less maintainable. Consider using an enum to make the code more readable and type-safe.
+enum Screen { + ENDED, + OFFER, + REGISTER +} + -const [screen, setScreen] = useState<number>(1); +const [screen, setScreen] = useState<Screen>(Screen.OFFER);components/discount/registerDiscount.tsx (1)
320-328
: Extract sales tax calculation logic.The sales tax calculation logic should be moved to a separate utility function or custom hook for better reusability and testing.
+// utils/salesTax.ts +export function calculateSalesTax(price: bigint, isSwissResident: boolean) { + if (isSwissResident) { + return { + rate: swissVatRate, + amount: applyRateToBigInt(price, swissVatRate) + }; + } + return { + rate: 0, + amount: BigInt(0) + }; +} useEffect(() => { - if (isSwissResident) { - setSalesTaxRate(swissVatRate); - setSalesTaxAmount(applyRateToBigInt(price, swissVatRate)); - } else { - setSalesTaxRate(0); - setSalesTaxAmount(BigInt(0)); - } + const { rate, amount } = calculateSalesTax(price, isSwissResident); + setSalesTaxRate(rate); + setSalesTaxAmount(amount); }, [isSwissResident, price]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/discount/discountOfferScreen.tsx
(1 hunks)components/discount/registerDiscount.tsx
(4 hunks)hooks/useAllowanceCheck.tsx
(1 hunks)pages/argent.tsx
(1 hunks)styles/discount.module.css
(1 hunks)utils/discounts/argent.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/discount/discountOfferScreen.tsx
🔇 Additional comments (5)
styles/discount.module.css (2)
1-7
: New .screen class added to support updated layout.
The new CSS class is well defined with properties (min-height, position, margin-top, overflow, padding-top) that support the intended structure for the discount screen. Ensure that its usage in the component consistently matches the design specifications.
10-14
: Adjusted height for .wrapperScreen to align with revised layout spacing.
Changing the height calculation to "calc(82vh - 2rem)" appears intended to accommodate the new spacing introduced by the .screen class. Please verify that this modification produces the expected visual alignment and responsiveness across different viewport sizes.hooks/useAllowanceCheck.tsx (1)
37-38
: LGTM! Good defensive programming.The added null check for
erc20AllowanceRes
before accessing its properties prevents potential runtime errors.pages/argent.tsx (1)
29-35
: LGTM! Good wallet connection handling.The wallet connection check is well implemented with proper state management and user feedback.
components/discount/registerDiscount.tsx (1)
172-172
: LGTM! Good defensive check for metadataHash.The added check prevents constructing calls with undefined metadataHash, improving reliability.
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: 1
♻️ Duplicate comments (1)
pages/argent.tsx (1)
41-49
:⚠️ Potential issueSecurity concern: Sensitive referral data stored in plain text
The referral data—including the sponsor address—is stored without encryption, validation, or cleanup.
Consider:
- Encrypting the data before storage
- Adding validation when reading
- Implementing an expiry cleanup mechanism
const handleSetScreen = (screen: number) => { const referralData = { sponsor: argentDiscount.sponsor, expiry: new Date().getTime() + 7 * 24 * 60 * 60 * 1000, + version: '1', // Add version for future compatibility }; - localStorage.setItem("referralData", JSON.stringify(referralData)); + // Encrypt data before storage + const encryptedData = encryptData(JSON.stringify(referralData)); + localStorage.setItem("referralData", encryptedData); setScreen(screen); };
🧹 Nitpick comments (3)
pages/argent.tsx (3)
20-27
: Enhance expiry check robustness.The expiry check could be more robust by:
- Using a constant for timestamp comparison
- Adding error handling for invalid expiry dates
useEffect(() => { const currentDate = new Date(); const timestamp = currentDate.getTime(); + const MILLISECONDS_PER_SECOND = 1000; + const expiryTimestamp = Number(argentDiscount.expiry) * MILLISECONDS_PER_SECOND; - if (timestamp >= argentDiscount.expiry) { + if (!isNaN(expiryTimestamp) && timestamp >= expiryTimestamp) { setScreen(0); } }, []);
37-39
: Add bounds check to prevent invalid navigation.The
goBack
function should prevent navigation below screen 0.function goBack() { - setScreen(screen - 1); + setScreen(Math.max(0, screen - 1)); }
95-98
: Add validation for environment variable.The mailing list environment variable could be undefined, which might cause issues.
mailGroups={[ - process.env.NEXT_PUBLIC_MAILING_LIST_GROUP ?? "", + process.env.NEXT_PUBLIC_MAILING_LIST_GROUP || + throw new Error("NEXT_PUBLIC_MAILING_LIST_GROUP is not defined"), argentDiscount.discountMailGroupId, ]}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pages/argent.tsx
(1 hunks)styles/discount.module.css
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- styles/discount.module.css
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.
Some changes, you can test on sepolia with discountId=3 btw
pages/argent.tsx
Outdated
customMessage={argentDiscount.offer.customMessage} | ||
goBack={goBack} | ||
priceInEth={BigInt(argentDiscount.offer.price)} | ||
mailGroups={[ |
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.
Mail don't exist anymore you can delete everything related to that (in the discount & component also)
utils/discounts/argent.ts
Outdated
name: "Argent 1$ domain", | ||
image: "/argent/argentdiscount.webp", | ||
expiry: 1740494381000, // timestamp in ms | ||
discountMailGroupId: "124587870775149633", |
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.
same to delete
utils/discounts/argent.ts
Outdated
offer: { | ||
duration: 16, | ||
customMessage: "3 months (with Argent discount)", | ||
discountId: "0x617267656e745f6f6e655f646f6c6c6172", |
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.
Did you try this one mainnet for a year ? Maybe it works I don't remember the lenght of the discount
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
🧹 Nitpick comments (3)
pages/argent.tsx (3)
14-19
: Consider deriving initial screen state from argentDiscount expiry.Instead of setting initial screen state to 1 and then immediately checking expiry in useEffect, consider deriving it during initialization:
- const [screen, setScreen] = useState<number>(1); + const [screen, setScreen] = useState<number>(() => { + return Date.now() >= argentDiscount.expiry ? 0 : 1; + });
20-35
: Enhance expiry monitoring for real-time updates.The current implementation only checks expiry once when the component mounts. Consider implementing continuous monitoring to update the screen state if the discount expires during the session.
useEffect(() => { - const currentDate = new Date(); - const timestamp = currentDate.getTime(); - - if (timestamp >= argentDiscount.expiry) { - setScreen(0); - } - }, []); + const checkExpiry = () => { + if (Date.now() >= argentDiscount.expiry) { + setScreen(0); + } + }; + + checkExpiry(); + const interval = setInterval(checkExpiry, 60000); // Check every minute + + return () => clearInterval(interval); + }, []);
52-100
: Add loading states and error handling for wallet connection.The current implementation doesn't handle loading states or potential errors during wallet connection. Consider:
- Adding loading indicators during wallet connection
- Handling potential connection errors
- Providing feedback when switching screens
{!isArgent || !account ? ( <div className="flex flex-col items-center justify-center h-[82vh]"> <div className="mb-10"> <ArgentIcon width="150px" color="#FF875B" /> </div> <div className={styles.title}>Connect Argent wallet</div> <div className={styles.description}> To access this discount, you need to connect an Argent wallet. </div> + {connector.isLoading && <div>Connecting...</div>} + {connector.error && ( + <div className="text-red-500"> + Failed to connect: {connector.error.message} + </div> + )} {!account ? ( <div className="flex items-center mt-3"> <ConnectButton /> </div> ) : null} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/discount/registerDiscount.tsx
(4 hunks)pages/argent.tsx
(1 hunks)utils/discounts/argent.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- utils/discounts/argent.ts
- components/discount/registerDiscount.tsx
🔇 Additional comments (2)
pages/argent.tsx (2)
1-11
: Add missing type import for SearchResult.The
SearchResult
type is used in the state declaration but not imported.
41-49
: Security concern: Sensitive referral data stored in plain text.The referral data containing sponsor address is stored without encryption or validation.
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
🧹 Nitpick comments (1)
utils/discounts/argent.ts (1)
9-9
: Consider extracting the duplicated image path.The image path is duplicated. Consider extracting it to a constant to improve maintainability.
export const argentDiscount = { + image: "/argent/argentdiscount.webp", offer: { duration: 365, customMessage: "-50% (with Argent discount)", discountId: "0x617267656e745f6f6e655f646f6c6c6172", price: "288493150684927", desc: "Get your domain for half the price. Don't miss out on this exclusive opportunity only available with your Argent wallet!", title: { desc: "Mint your domain", catch: "-50% discount" }, - image: "/argent/argentdiscount.webp", + image: argentDiscount.image, }, name: "Argent -50% domain", - image: "/argent/argentdiscount.webp", expiry: 1770207266000, // timestamp in ms sponsor: "0x64d28d1d1d53a0b5de12e3678699bc9ba32c1cb19ce1c048578581ebb7f8396", };Also applies to: 12-12
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
utils/discounts/argent.ts
(1 hunks)
🔇 Additional comments (3)
utils/discounts/argent.ts (3)
3-6
: Consider moving configuration to environment variables.The discount configuration contains sensitive data like
discountId
,price
,expiry
, andsponsor
address that should be managed through environment variables rather than being hardcoded.Also applies to: 13-14
4-4
: LGTM! Clear and consistent messaging.The discount messaging is clear and consistent across all text elements, properly communicating the 50% discount offer.
Also applies to: 7-8
3-3
: Verify duration and expiry settings.Please confirm:
- Has the 365-day duration been tested on mainnet? (Previous review raised this question)
- Is the expiry date of June 2026 aligned with business requirements?
Let's check if there are any existing discounts with similar duration:
Also applies to: 13-13
❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Search for other discount configurations with duration or expiry settings rg -i "duration.*365|expiry.*2026" --type tsLength of output: 1446
Mainnet Testing and Business Requirements Confirmation
The 365-day duration configuration in
argent.ts
is consistent with similar implementations in other discount modules (e.g., in Evergreen and checkout components). However, we did not find explicit evidence within the codebase confirming that the 365-day duration has been specifically tested on mainnet. Additionally, while the expiry date (set to approximately June 2026) appears to be hardcoded as intended, it isn’t clear from the repository whether this date has been reviewed against current business requirements.
- Action Items:
- Confirm via deployment or QA logs that the 365-day discount setting has been fully validated on mainnet.
- Verify with the business or product team that the expiry date (June 2026) aligns with the intended discount lifecycle and requirements.
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.
lgtm
close: #1014
Summary by CodeRabbit
New Features
RegisterDiscount
component with improved state initialization and validation checks.Bug Fixes
Style