-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
V2.34 release #4888
V2.34 release #4888
Conversation
now I need to go through the all variables and states to check if something is missing
Fixing bugs and adding new features
Fix modal interaction for email verification flow
Fixing statuses for unverified email
Fixing notification condition
Fixing text translation for vouched users
Fix button boost behaviour
Feat/security headers
…e_form Removing email verification from project verification form
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces updates to localization files for Catalan, English, and Spanish, adding new entries for "Q/acc" and "Q/acc News." Additionally, it includes a new JSON file defining the ABI for a USDT smart contract. Other changes involve modifications to various components and hooks, enhancing functionality related to token handling, error management, and user interface elements. Notably, the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (24)
✅ Files skipped from review due to trivial changes (1)
🔇 Additional comments (23)src/components/views/userProfile/donationsTab/recurringTab/StreamRow.tsx (3)
The addition of
The additional null check for
The code assumes 18 decimals for formatting the monthly flow rate. This might not work correctly for all tokens. Let's verify the decimals of configured tokens: Consider using the token's actual decimals: - limitFraction(formatUnits(BigInt(monthlyFlowRate), 18)),
+ limitFraction(formatUnits(BigInt(monthlyFlowRate), superToken?.decimals || 18)), src/components/views/donate/Recurring/RecurringDonationCard.tsx (1)
When the selected token has 6 decimals, both Please verify if this logic is intentional. If it is, consider adding a comment to explain the reasoning behind resetting the amounts to zero to improve code readability. src/components/views/donate/Recurring/RecurringDonationModal/Item.tsx (2)
The code now uses Please verify that
When calculating the USD value, ensure that the correct decimals are used for precision. Double-check that src/lib/transaction.ts (1)
Casting Ensure that Optionally, consider adding validation logic: if (!/^0x[0-9a-fA-F]{64}$/.test(txHash)) {
throw new Error('Invalid transaction hash format');
} src/components/views/donate/Recurring/SelectTokenModal/StreamInfo.tsx (1)
The display of the token symbol and name now uses Verify that src/lib/constants/links.ts (1)
The new constants follow the established pattern and maintain consistent formatting with the rest of the file. src/components/views/donate/Recurring/ModifySuperToken/ModifySuperTokenModal.tsx (1)
The union type src/components/views/givfarm/GIVfarmBottom.tsx (2)
The change simplifies the PoolRow rendering while maintaining the existing pool filtering logic. Line range hint The renderPools function should handle the case when chainId is undefined more explicitly. Consider adding explicit error handling: const renderPools = (chainId?: number, showArchivedPools?: boolean) => {
+ if (chainId === undefined) {
+ console.warn('Chain ID is undefined, falling back to default pools');
+ }
const pools = getPoolsInfoByChainID(chainId);
// ... rest of the function
}; src/components/views/donate/Recurring/ModifySuperToken/WithDrawSuperToken.tsx (1)
The prop change from ✅ Verification successfulProp change is compatible with Item component's expectations The change from 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check Item component's prop type definition
ast-grep --pattern 'interface $Props {
$$$
token: $Type
$$$
}'
Length of output: 70 Script: #!/bin/bash
# Let's try a different approach to find the Item component and its props
# First, let's find files containing "Item" component
fd "Item" --type f --exec rg -l "export.*Item"
# Then, let's look for token prop usage in these files
rg "token.*:" -A 5 -B 5
Length of output: 67859 Script: #!/bin/bash
# Let's look for the Item component specifically in the file containing the WithDrawSuperToken component
rg -l "WithDrawSuperToken" --type ts --type tsx
# Then let's examine the Item component's usage and definition in the same directory
fd "Item" --type f --exec rg "interface.*Props|token.*:" -C 5 src/components/views/donate/Recurring/ModifySuperToken/
Length of output: 70970 Script: #!/bin/bash
# Let's check the Item component definition in the RecurringDonationModal directory
rg -A 10 "interface IItemProps" src/components/views/donate/Recurring/RecurringDonationModal/Item.tsx
Length of output: 270 src/hooks/useCreateEvmDonation.tsx (1)
The changes enhance type safety by:
Also applies to: 33-33 src/hooks/usePassport.ts (1)
The type assertion src/components/views/donate/Recurring/RecurringDonationModal/RecurringDonationModal.tsx (2)
The code correctly handles tokens with 6 decimals (like USDT) by converting amounts to 18 decimals for compatibility with the system. The conversion uses
The flow rate calculations now use the converted amounts ( Also applies to: 305-307 src/artifacts/usdtMainnetABI.json (1)
The ABI correctly defines all necessary functions and events for interacting with the USDT mainnet contract, including:
Verify the ABI source and version. Ensure this ABI matches the official USDT mainnet contract to prevent potential security issues. ✅ Verification successfulUSDT ABI verification successful - ABI matches official USDT contract The ABI is being used correctly in the codebase for USDT token transfers, specifically for the mainnet USDT contract address (0xdac17f958d2ee523a2206206994597c13d831ec7). The implementation includes:
The code in helpers.ts shows a thoughtful implementation that specifically handles USDT's unique requirements by using this custom ABI when interacting with the USDT mainnet contract address. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify the ABI matches the official USDT contract on Etherscan
# Note: Using gh api to avoid rate limits
# Get verified contract ABI from Etherscan via GitHub API
gh api graphql -f query='
{
repository(owner: "tether", name: "contracts") {
object(expression: "master") {
... on Commit {
tree {
entries {
name
object {
... on Blob {
text
}
}
}
}
}
}
}
}'
Length of output: 602 Script: #!/bin/bash
# Let's try to find any references or imports of this ABI in the codebase
# to understand its usage and verify its source
# Search for references to this ABI file
rg -l "usdtMainnetABI" --type ts --type js
# Search for any USDT contract addresses that might help verify the source
rg -i "usdt.*contract.*address|tether.*contract.*address" --type ts --type js
# Look for any documentation or comments about USDT integration
rg -l "USDT|Tether" README.md docs/ 2>/dev/null
Length of output: 179 Script: #!/bin/bash
# Let's check the usage context in helpers.ts to understand how this ABI is used
cat src/lib/helpers.ts
# Also let's verify the ABI structure matches standard USDT interface
# by checking for required ERC20 methods and USDT-specific methods
jq '[.[] | select(.type=="function") | .name] | sort' src/artifacts/usdtMainnetABI.json
Length of output: 19637 src/lib/helpers.ts (3)
The import is correctly added to handle USDT mainnet contract interactions.
The comments clearly explain why a custom ABI is needed for USDT on mainnet. The conditional selection of ABI based on the contract address is a good solution.
The code correctly handles the case where decimals is returned as a bigint by converting it to a number. Also applies to: 391-398 src/config/production.tsx (1)
The token configurations are well structured with all necessary fields:
Also applies to: 541-554, 558-568 lang/en.json (1)
The new translations for "Q/acc" and "Q/acc News" are properly added to the English language file. 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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (14)
src/components/views/userProfile/donationsTab/recurringTab/StreamRow.tsx (2)
49-49
: Remove commented out codeThe commented line
// const { symbol } = tokenStream[0].token;
should be removed as it's no longer needed.
Line range hint
1-143
: Consider component decomposition for better maintainabilityThe StreamRow component handles multiple responsibilities including:
- Token balance display
- Flow rate calculations
- Chain switching
- Modal management
Consider breaking these into smaller, focused components for better maintainability and testing.
src/wagmiConfigs.ts (1)
41-51
: Refactor HTTP Transport Configuration for Multiple ChainsThe current implementation handles custom HTTP transport only for the
mainnet
chain using Infura. This hard-coded approach limits scalability and may lead to inconsistent behavior across different chains.Consider refactoring the transport logic to dynamically configure HTTP transports for all chains. You could map each chain ID to its respective RPC endpoint, possibly using a configuration object. This will enhance maintainability and make it easier to add support for additional chains in the future.
src/components/views/donate/Recurring/RecurringDonationCard.tsx (3)
153-156
: Avoid Hardcoded Scaling FactorsUsing a hardcoded
scaleFactor
of10000n
for tokens with 6 decimals may reduce flexibility and clarity.Consider calculating the
scaleFactor
based on the token's decimals to accommodate tokens with varying decimal places. For example:- const scaleFactor = - selectedRecurringToken?.token.decimals === 6 ? 10000n : 1n; + const scaleFactor = 10n ** BigInt(18 - selectedRecurringToken.token.decimals);This approach dynamically adjusts the
scaleFactor
based on the token's decimals, enhancing maintainability.
163-164
: Ensure Consistent Casing of Token IDsConverting token IDs to lowercase when accessing
tokenStreams
suggests inconsistent casing in token IDs, which could lead to potential bugs.Ensure that token IDs are stored and compared in a consistent case throughout the application. It might be better to normalize token IDs to lowercase when they are first stored or to enforce a consistent casing convention.
Also applies to: 783-785
573-581
: Refactor Nested Function Calls for ReadabilityThe deeply nested function calls used to format the donation amount can impact code readability and maintainability.
Consider breaking down the nested calls into intermediate variables:
const totalAmount = totalStreamPerSec * ONE_MONTH_SECONDS; const formattedUnits = formatUnits( totalAmount, selectedRecurringToken?.token.decimals || 18, ); const limitedFraction = limitFraction(formattedUnits); const donationAmount = formatDonation(limitedFraction);This refactoring enhances clarity and makes the code easier to understand.
src/components/views/donate/Recurring/SelectTokenModal/StreamInfo.tsx (1)
81-81
: Display Correct Token SymbolEnsure that the token symbol displayed alongside the balance is correct.
Confirm that
superToken.symbol
accurately represents the token's symbol in this context. If the underlying token's symbol should be displayed instead, adjust the code accordingly.src/components/views/donate/Recurring/ModifySuperToken/ModifySuperTokenModal.tsx (1)
Line range hint
93-108
: Consider refactoring for better readability.The nested ternary with optional chaining could be simplified for better maintainability.
Consider this more readable approach:
-const [token, superToken] = useMemo( - () => - props.selectedToken.isSuperToken - ? [ - props.selectedToken.underlyingToken || - config.OPTIMISM_CONFIG.SUPER_FLUID_TOKENS.find( - token => token.id === props.selectedToken.id, - )?.underlyingToken, - props.selectedToken as ISuperToken, - ] - : [ - props.selectedToken, - findSuperTokenByTokenAddress(props.selectedToken.id), - ], - [props.selectedToken], -); +const [token, superToken] = useMemo(() => { + if (props.selectedToken.isSuperToken) { + const selectedSuperToken = props.selectedToken as ISuperToken; + const underlyingToken = selectedSuperToken.underlyingToken || + config.OPTIMISM_CONFIG.SUPER_FLUID_TOKENS.find( + token => token.id === selectedSuperToken.id + )?.underlyingToken; + return [underlyingToken, selectedSuperToken]; + } + return [ + props.selectedToken, + findSuperTokenByTokenAddress(props.selectedToken.id), + ]; +}, [props.selectedToken]);src/components/views/givfarm/GIVfarmBottom.tsx (1)
Line range hint
37-115
: Consider refactoring pool configuration logic for better maintainability.The current implementation has duplicated code across chain ID cases and complex spreading of configurations.
Consider extracting common pool configurations and using a more maintainable approach:
+const BASE_POOLS = [ + ...(config.GNOSIS_CONFIG.pools || []), + ...(config.GNOSIS_CONFIG.regenPools || []), + ...(config.MAINNET_CONFIG.pools || []), + ...(config.MAINNET_CONFIG.regenPools || []), +]; + +const ALL_GIVPOWER = [ + ...(config.ZKEVM_CONFIG.GIVPOWER ? [config.ZKEVM_CONFIG.GIVPOWER] : []), + ...(config.GNOSIS_CONFIG.GIVPOWER ? [config.GNOSIS_CONFIG.GIVPOWER] : []), + ...(config.OPTIMISM_CONFIG.GIVPOWER ? [config.OPTIMISM_CONFIG.GIVPOWER] : []), +]; + const getPoolsInfoByChainID = ( chainId?: number, ): Array<SimplePoolStakingConfig | BalancerPoolStakingConfig | ICHIPoolStakingConfig> => { - switch (chainId) { - case config.GNOSIS_NETWORK_NUMBER: - return [/* ... */]; - case config.OPTIMISM_NETWORK_NUMBER: - return [/* ... */]; - case config.ZKEVM_NETWORK_NUMBER: - return [/* ... */]; - default: - return [/* ... */]; - } + return [...ALL_GIVPOWER, ...BASE_POOLS]; };src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx (1)
131-133
: Consider using a dedicated utility for case-insensitive comparisonWhile the case-insensitive comparison is good for consistency, consider extracting it into a utility function for reuse.
+const compareTokenIds = (id1: string, id2: string) => id1.toLowerCase() === id2.toLowerCase(); + const token = superTokens.find( - token => - token.id.toLowerCase() === tokenId.toLowerCase(), + token => compareTokenIds(token.id, tokenId), ) as IToken;src/components/Footer/Footer.tsx (1)
244-244
: Consider maintaining consistent spacingThe gap property was changed from 72px to 0px which might affect the visual hierarchy and spacing consistency.
Consider if this change aligns with the design system's spacing guidelines.
src/components/views/donate/OneTime/SelectTokenModal/SelectTokenModal.tsx (1)
151-157
: Remove redundant variable declarationThe introduction of
sQuery
is unnecessary assearchQuery
could be used directly in the filter conditions.Apply this diff to simplify the code:
- const sQuery: string = searchQuery; const filtered = tokens.filter( token => token.symbol .toLowerCase() - .includes(sQuery.toLowerCase()) || - token.name.toLowerCase().includes(sQuery.toLowerCase()), + .includes(searchQuery.toLowerCase()) || + token.name.toLowerCase().includes(searchQuery.toLowerCase()), );lang/es.json (1)
727-728
: Consider translating the new entriesThe additions match the changes mentioned in the summary. However, consider whether "Q/acc" and "Q/acc News" should be translated to Spanish for consistency with the rest of the localization file, unless they are proper nouns or brand names that should remain in English.
- "component.title.qacc": "Q/acc", - "component.title.qacc_news": "Q/acc News", + "component.title.qacc": "Q/acc", + "component.title.qacc_news": "Noticias Q/acc",lang/ca.json (1)
101-102
: The new entries need to be translated to CatalanThe new entries "Q/acc" and "Q/acc News" are currently in English. Consider translating them to Catalan to maintain consistency with the rest of the localization file.
- "component.title.qacc": "Q/acc", - "component.title.qacc_news": "Q/acc News", + "component.title.qacc": "Q/acc", + "component.title.qacc_news": "Notícies Q/acc",
🛑 Comments failed to post (7)
src/components/views/userProfile/donationsTab/recurringTab/StreamRow.tsx (1)
22-28:
⚠️ Potential issueAdd null check for tokenStream access
The current implementation assumes
tokenStream[0]
exists, which could lead to runtime errors iftokenStream
is empty.Consider adding a null check:
const superToken = useMemo( () => + tokenStream[0]?.token?.id ? config.OPTIMISM_CONFIG.SUPER_FLUID_TOKENS.find( s => s.id === tokenStream[0].token.id, - ), + ) : undefined, [tokenStream], );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const superToken = useMemo( () => tokenStream[0]?.token?.id ? config.OPTIMISM_CONFIG.SUPER_FLUID_TOKENS.find( s => s.id === tokenStream[0].token.id, ) : undefined, [tokenStream], );
src/wagmiConfigs.ts (1)
44-44:
⚠️ Potential issuePotential Exposure of Infura API Key
The usage of
NEXT_PUBLIC_INFURA_API_KEY
exposes the Infura API key to the client-side, as environment variables prefixed withNEXT_PUBLIC_
are accessible in the browser. This could lead to unauthorized usage of your Infura account.Apply this diff to secure the API key:
- const infuraKey = process.env.NEXT_PUBLIC_INFURA_API_KEY; + const infuraKey = process.env.INFURA_API_KEY;Ensure that
INFURA_API_KEY
is defined in your server-side environment variables and not exposed to the client.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const infuraKey = process.env.INFURA_API_KEY;
src/components/views/donate/Recurring/ModifySuperToken/DepositSuperToken.tsx (2)
144-145:
⚠️ Potential issueRemove sensitive logging statements
Console logging of token details could expose sensitive information in production.
Remove these debug logs:
-console.log('token', token); -console.log('supertoken', superToken);
137-143:
⚠️ Potential issueReview decimal conversion logic for potential precision loss
The conversion of token amounts with 6 decimals involves division operations that could lead to precision loss. Consider using a more precise conversion method.
Consider this safer approach:
-const currentAmount = Number(amount) / Number(divisor); -newAmount = ethers.utils - .parseUnits(currentAmount.toString(), 18) - .toBigInt(); +newAmount = (amount * BigInt(10 ** 12));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (token && token.decimals === 6) { const divisor = BigInt(10 ** token.decimals); newAmount = (amount * BigInt(10 ** 12)); }
src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx (2)
88-90:
⚠️ Potential issueReview token filtering logic
The current filtering condition might be counterintuitive. It includes tokens with zero balance and excludes tokens with positive balance.
Consider inverting the condition for clarity:
-return !(newBalances[token.underlyingToken.symbol] > 0n); +return newBalances[token.underlyingToken.symbol] <= 0n;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const filteredTokens = superTokens.filter(token => { return newBalances[token.underlyingToken.symbol] <= 0n; });
191-222: 🛠️ Refactor suggestion
Add key prop to mapped Fragment and simplify nested structure
The current implementation has two issues:
- Missing key prop in the mapped Fragment
- Deeply nested ternary operators affecting readability
Consider this refactoring:
-{underlyingTokens.length > 0 ? ( - underlyingTokens.map(token => ( - <> +{underlyingTokens.length > 0 ? ( + underlyingTokens.map(token => ( + <Fragment key={token.underlyingToken?.symbol}> <TokenInfo - key={token.underlyingToken?.symbol} token={token.underlyingToken} balance={balances[token.underlyingToken.symbol]} disable={ balances[token.underlyingToken.symbol] === undefined || balances[token.underlyingToken.symbol] === 0n } onClick={() => { setSelectedRecurringToken({ token: token.underlyingToken, balance: balances[token.underlyingToken.symbol], }); setShowModal(false); }} /> - </> + </Fragment> )) ) : ( <Caption> {formatMessage({ id: 'label.you_have_super_token_for_all_tokens', })} </Caption> )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.{underlyingTokens.length > 0 ? ( underlyingTokens.map(token => ( <Fragment key={token.underlyingToken?.symbol}> <TokenInfo token={token.underlyingToken} balance={ balances[ token.underlyingToken.symbol ] } disable={ balances[ token.underlyingToken.symbol ] === undefined || balances[ token.underlyingToken.symbol ] === 0n } onClick={() => { setSelectedRecurringToken({ token: token.underlyingToken, balance: balances[ token.underlyingToken .symbol ], }); setShowModal(false); }} /> </Fragment> )) ) : ( <Caption> {formatMessage({ id: 'label.you_have_super_token_for_all_tokens', })} </Caption> )}
src/components/Footer/Footer.tsx (1)
145-160:
⚠️ Potential issueAdd security attributes and accessibility improvements to external links
The new links are missing security attributes and accessibility indicators.
Apply this diff to enhance security and accessibility:
- <a href={links.QACC} target='_blank'> + <a href={links.QACC} target='_blank' rel="noreferrer noopener" aria-label="Q/acc (opens in new tab)"> <LinkItem color={textColor}> {formatMessage({ id: 'component.title.qacc', })} </LinkItem> </a> - <a href={links.QACC_NEWS} target='_blank'> + <a href={links.QACC_NEWS} target='_blank' rel="noreferrer noopener" aria-label="Q/acc News (opens in new tab)"> <LinkItem color={textColor}> {formatMessage({ id: 'component.title.qacc_news', })} </LinkItem> </a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<LinkColumn> <a href={links.QACC} target='_blank' rel="noreferrer noopener" aria-label="Q/acc (opens in new tab)"> <LinkItem color={textColor}> {formatMessage({ id: 'component.title.qacc', })} </LinkItem> </a> <a href={links.QACC_NEWS} target='_blank' rel="noreferrer noopener" aria-label="Q/acc News (opens in new tab)"> <LinkItem color={textColor}> {formatMessage({ id: 'component.title.qacc_news', })} </LinkItem> </a> </LinkColumn>
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.
BIG ONE, but looks good to me :)
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!
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores