Add comarketing #4891
Add comarketing #4891
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request includes updates to localization files for Catalan, English, and Spanish languages, adding new keys related to "Q/acc" and "Q/acc News" while enhancing existing translations for clarity. Additionally, a new redirect rule is introduced in the Next.js configuration, and the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (25)
🔇 Additional comments (32)src/components/views/donate/Recurring/RecurringDonationModal/Item.tsx (2)
The code shows inconsistent patterns for accessing token properties:
This inconsistency suggests that either:
Let's verify the token structure usage across the codebase: Also applies to: 34-34
The price calculation uses the same token decimals for formatting. Ensure this is correct, as some tokens (like USDT) might have different decimal places (6) compared to the standard 18. Let's verify the price calculation decimal handling: ✅ Verification successfulToken decimals are properly handled across the codebase The verification shows that the codebase correctly handles different token decimals:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check price calculation decimal handling for different tokens
# Expected: Find evidence of proper decimal handling for various token types
# Search for price calculation patterns
echo "=== Price calculation patterns ==="
ast-grep --pattern 'formatUnits($_, token.decimals)'
# Search for token definitions to verify decimal values
echo "=== Token decimal definitions ==="
rg -A 2 "decimals.*[0-9]+"
Length of output: 15071 src/components/views/donate/Recurring/RecurringDonationCard.tsx (3)
The scale factor is properly applied in the monthly stream calculation, ensuring accurate amounts for tokens with different decimal places.
The code resets both ✅ Verification successfulResetting amounts for 6 decimal tokens is consistent with token conversion logic The reset of amounts in
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Find other instances where 6 decimal tokens are handled specially
rg -l "decimals === 6"
Length of output: 253 Script: #!/bin/bash
# Check how other components handle 6 decimal tokens
rg "decimals === 6" -A 5 -B 2
Length of output: 3580
The token ID is converted to lowercase here. This is good practice, but we should verify that this is handled consistently everywhere to prevent case-sensitivity issues. ✅ Verification successfulToken ID case handling is consistently implemented across the codebase The codebase shows a consistent pattern of converting token IDs to lowercase when used as keys or in comparisons. This is evident in:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Find all token ID comparisons in the codebase
ast-grep --pattern 'token.id $$$'
Length of output: 35 Script: #!/bin/bash
# Search for token.id usage with ripgrep for better pattern matching
rg "token\.id" -t typescript -t javascript --no-heading -n
# Also search for tokenId pattern as it might be used alternatively
rg "tokenId" -t typescript -t javascript --no-heading -n
Length of output: 187 Script: #!/bin/bash
# Search for token.id usage without file type restrictions
rg "token\.id" --no-heading -n
# Also search for tokenId pattern
rg "tokenId" --no-heading -n
# Search for toLowerCase() calls near token patterns
rg "token.*toLowerCase\(\)" -A 2 -B 2 --no-heading -n
Length of output: 20251 src/components/views/donate/Recurring/SelectTokenModal/StreamInfo.tsx (2)
The addition of the
The component now uses Also applies to: 81-81 ✅ Verification successfulToken symbol access is properly safeguarded The
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check for potential undefined token symbol access
ast-grep --pattern 'superToken.symbol'
Length of output: 530 Script: #!/bin/bash
# Let's check how superToken is defined and passed to these components
ast-grep --pattern 'const superToken = $_'
# Also check for type definitions
ast-grep --pattern 'type $_Props = {
$$$
superToken: $_
$$$
}'
# And check for prop destructuring
ast-grep --pattern 'function $_({ $$$, superToken, $$$ }: $_)'
Length of output: 2145 Script: #!/bin/bash
# Let's check the type definition of SUPER_FLUID_TOKENS and findSuperTokenByTokenAddress
ast-grep --pattern 'SUPER_FLUID_TOKENS: $_'
# Also check for null checks before accessing superToken.symbol
ast-grep --pattern 'if (!superToken) {
$$$
}'
# And check the implementation of findSuperTokenByTokenAddress
ast-grep --pattern 'function findSuperTokenByTokenAddress($_) {
$$$
}'
# Let's also see where StreamInfo component is defined
ast-grep --pattern 'function StreamInfo($_) {
$$$
}'
Length of output: 857 Script: #!/bin/bash
# Let's check how StreamInfo is used and how superToken is passed to it
rg "StreamInfo" -A 2
# And check the actual file content where superToken is used
cat src/components/views/donate/Recurring/SelectTokenModal/StreamInfo.tsx
Length of output: 9153 src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx (1)
The case-insensitive token ID comparison makes the matching more robust. src/components/views/donate/OneTime/SelectTokenModal/SelectTokenModal.tsx (1)
The introduction of src/wagmiConfigs.ts (1)
The implementation correctly uses an environment variable for the Infura API key and provides a fallback to the default transport when needed. src/lib/transaction.ts (1)
The type assertion package.json (1)
The version updates for viem and wagmi are minor/patch versions, which should be backward compatible. However, let's verify there are no breaking changes that could affect the recent modifications to transaction handling. Also applies to: 75-76 ✅ Verification successfulBased on the changelog outputs, I can generate the final response since I have enough information to verify the dependency compatibility. Dependencies are compatible with recent transaction handling changes The version updates for viem (2.21.16 -> 2.21.19) and wagmi (2.12.16 -> 2.12.17) contain only patch changes with no breaking changes:
The modifications are safe to proceed with as they maintain backward compatibility and don't affect transaction handling functionality. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for breaking changes in the updated dependencies
# Check viem changelog between versions
gh api repos/wagmi-dev/viem/releases | jq -r '.[] | select(.tag_name | contains("2.21.")) | .body'
# Check wagmi changelog between versions
gh api repos/wagmi-dev/wagmi/releases | jq -r '.[] | select(.tag_name | contains("2.12.")) | .body'
Length of output: 32950 src/components/views/userProfile/donationsTab/recurringTab/StreamRow.tsx (3)
Good addition of useMemo for superToken computation. This prevents unnecessary lookups on each render.
The changes improve token handling by:
Also applies to: 67-67, 71-72
Good addition of the superToken existence check before rendering ModifySuperTokenModal. This prevents potential runtime errors. src/lib/constants/links.ts (1)
The QACC links have been added correctly to the constants. However, we should verify these endpoints are accessible and properly configured. ✅ Verification successfulQACC links are accessible and properly configured Both endpoints return HTTP 200 status codes with proper headers, confirming they are accessible and correctly set up:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check if QACC endpoints are accessible
curl -I https://qacc.giveth.io/
curl -I https://qacc.giveth.io/news
Length of output: 2718 src/components/views/donate/Recurring/ModifySuperToken/ModifySuperTokenModal.tsx (2)
Good improvement to support both ISuperToken and IToken types. This makes the component more flexible. Line range hint The useMemo logic for handling different token types looks good, but we should verify all code paths are properly tested. Consider adding type guards or assertions to make the token type handling more explicit: - props.selectedToken.isSuperToken
+ 'isSuperToken' in props.selectedTokensrc/components/views/givfarm/GIVfarmBottom.tsx (1)
The removal of the unnecessary next.config.js (1)
The redirect rule is properly structured, but we should verify that the destination PDF file exists in the assets directory. ✅ Verification successfulPDF file exists at the expected location The co-marketing PDF file is present at 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check if the PDF file exists in the assets directory
fd -t f "Giveth-Co-Marketing-Slides.pdf" public/assets/
Length of output: 100 src/components/views/donate/Recurring/ModifySuperToken/DepositSuperToken.tsx (2)
The ethers import is necessary for the new token decimal conversion functionality.
The current implementation specifically handles 6-decimal tokens. We should verify this works correctly for all token decimals used in the system. ✅ Verification successfulLet me analyze the code further to understand how token decimals are handled. Let me check one more aspect to understand the token decimal handling in the codebase. Based on the analysis of the codebase, I can now provide the final response: The amount conversion is properly handled for all token decimals The implementation correctly handles tokens with different decimal places:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Find all token definitions to verify their decimal places
echo "Searching for token decimal definitions..."
rg -A 5 "decimals.*=.*[0-9]+"
Length of output: 6597 Script: #!/bin/bash
# Let's check how the amount is handled in the ModifySuperToken component and related files
ast-grep --pattern 'parseUnits($$$)'
# Also check for any token decimal conversions
ast-grep --pattern 'decimals.*===.*6'
Length of output: 513 Script: #!/bin/bash
# Let's check the token conversion logic in the component
ast-grep --pattern 'if ($token.decimals === 6) {
$$$
}'
# Also check for any default decimal handling
rg "decimals.*\|\|.*18" -A 3
Length of output: 4960 src/components/Footer/Footer.tsx (1)
The gap property was changed from ✅ Verification successfulLet me gather more context about the gap property usage in the Footer component and related files. Gap property change appears intentional and consistent with responsive design Looking at the context, the 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check if this gap is consistent with other footer implementations
rg -A 2 'gap:\s*0\s+0px;'
Length of output: 146 Script: #!/bin/bash
# Check for all gap property usages in Footer.tsx and related files
rg -A 2 'gap:' src/components/Footer/
# Also check for any other Footer components or similar patterns
fd Footer src/components/ --exec rg -l 'gap:'
Length of output: 1112 src/hooks/useCreateEvmDonation.tsx (1)
The changes improve type safety by:
Also applies to: 33-33 src/hooks/usePassport.ts (1)
The change ensures type safety by explicitly casting the address parameter to the expected format required by scoreUserAddress function. src/components/views/donate/Recurring/RecurringDonationModal/RecurringDonationModal.tsx (2)
The conversion logic for 6-decimal tokens to 18 decimals is correct, but there are a few considerations: Let's verify if there are other 6-decimal tokens that need this handling: Consider extracting the decimal conversion logic into a utility function for reusability and better maintainability: +const convertTo18Decimals = (amount: bigint, decimals: number): bigint => {
+ const divisor = BigInt(10 ** decimals);
+ const currentAmount = Number(amount) / Number(divisor);
+ return ethers.utils.parseUnits(currentAmount.toString(), 18).toBigInt();
+};
-if (selectedRecurringToken.token.decimals === 6) {
- const divisor = BigInt(10 ** selectedRecurringToken.token.decimals);
- const currentAmount = Number(amount) / Number(divisor);
- newAmount = ethers.utils.parseUnits(currentAmount.toString(), 18).toBigInt();
-
- const currentPerMonth = Number(perMonthAmount) / Number(divisor);
- newPerMonthAmount = ethers.utils.parseUnits(currentPerMonth.toString(), 18).toBigInt();
+if (selectedRecurringToken.token.decimals === 6) {
+ newAmount = convertTo18Decimals(amount, selectedRecurringToken.token.decimals);
+ newPerMonthAmount = convertTo18Decimals(perMonthAmount, selectedRecurringToken.token.decimals);
}✅ Verification successfulDecimal handling for 6-decimal tokens is consistently implemented The search results confirm that the 6-decimal token handling is already consistently implemented across the codebase:
The current implementation is correct and aligns with other parts of the codebase. While the refactoring suggestion for better maintainability is valid, it's not a critical concern as the conversion logic is consistently handled. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for token declarations with 6 decimals
ast-grep --pattern 'decimals = 6'
rg -A 2 'decimals.*6'
Length of output: 1784
The flow rate calculations are using the converted amounts correctly, but we should ensure the precision is maintained throughout the calculations. Let's verify if there are any existing flow rate calculations that might need similar handling: Also applies to: 305-307 ✅ Verification successfulFlow rate calculations are correctly handling precision The flow rate calculations in the RecurringDonationModal are correctly handling precision for both 6-decimal and 18-decimal tokens. The code first adjusts the 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for other flow rate calculations
rg -A 3 'flowRate.*=.*perMonthAmount'
Length of output: 39 Script: #!/bin/bash
# Let's try a broader search for flow rate calculations
rg -i "flowrate|flow.*rate" -A 3
# Also search for specific variable name
rg "_flowRate" -A 3
Length of output: 66484 Script: #!/bin/bash
# Let's search for flow rate calculation patterns specifically in the RecurringDonationModal
ast-grep --pattern 'let _flowRate = $$$'
Length of output: 522 src/config/production.tsx (2)
The configuration for the bridged USDC super token (USDC.e) follows the standard structure and naming conventions.
Several token contract addresses have been updated:
Also applies to: 558-558, 564-564 ✅ Verification successfulLet me gather more context about these token configurations to ensure there are no inconsistencies. Based on the gathered information, I can now generate the final response: Token contract addresses have been verified The updated token addresses are consistently defined in the configuration with their correct corresponding names and symbols:
The tokens are properly integrated into the codebase's token handling system, with appropriate references in donation, balance checking, and token management functionalities. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify the token contracts are deployed and have the expected properties
# Check if the contracts exist and verify their properties
for address in "0x35adeb0638eb192755b6e52544650603fe65a006" "0x4f604735c1cf31399c6e711d5962b2b3e0225ad3" "0x9f41d0aa24e599fd8d0c180ee3c0f609dc41c622"; do
echo "Checking contract at $address"
# Look for contract creation transactions
rg -A 2 "creation of.*$address"
# Look for any references to these addresses in the codebase
rg -A 2 "$address"
done
Length of output: 1776 Script: #!/bin/bash
# Search for any previous/old token addresses that might have been replaced
rg -B 2 -A 2 "Super USD Coin|Glo Dollar|Super Glo Dollar" src/config/
# Look for any token-related configuration or validation logic
ast-grep --pattern 'const tokens = {
$$$
}'
# Check for any token address validation or verification logic
rg -A 2 "validateToken|verifyToken|tokenAddress"
Length of output: 16067 lang/en.json (1)
Added localization strings for Q/acc section titles. The entries follow the existing naming convention and structure. lang/es.json (1)
The new keys for Q/acc follow proper localization practices by:
lang/ca.json (1)
The additions follow the established naming conventions and appropriately preserve the English terms "Q/acc" and "Q/acc News" as proper nouns. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (11)
src/components/views/donate/Recurring/ModifySuperToken/WithDrawSuperToken.tsx (3)
Line range hint
83-84: Fix error message typo.The error message "address not found1" appears to have an unintentional "1" suffix.
- throw new Error('address not found1'); + throw new Error('Address not found');
Line range hint
144-152: Align comment with implementation regarding buffer time.The comment suggests a 40-second buffer, but the code implements a 60-second buffer. This inconsistency could lead to confusion in future maintenance.
Update the comment to match the implementation:
- /// this one needs some thought - we should allow for a buffer of 40 seconds (or more) in the balance we show to the user + // We allow for a buffer of 60 seconds in the balance shown to the user
Line range hint
153-153: Remove debug console.log statement.Debug logging should not be present in production code.
- console.log('SuperTokenBalance', SuperTokenBalance);src/components/views/donate/Recurring/RecurringDonationCard.tsx (2)
153-156: Document the scale factor calculation and extract the magic numberThe scale factor calculation uses a magic number
10000n. This should be extracted as a named constant with documentation explaining its purpose and calculation.+// The scale factor of 10000 is used to maintain precision when handling 6 decimal tokens +const SCALE_FACTOR_6_DECIMALS = 10000n; + const scaleFactor = - selectedRecurringToken?.token.decimals === 6 ? 10000n : 1n; + selectedRecurringToken?.token.decimals === 6 ? SCALE_FACTOR_6_DECIMALS : 1n;
783-785: Consider simplifying the token stream accessThe token stream access can be simplified using optional chaining and nullish coalescing.
-tokenStreams[ - selectedRecurringToken?.token.id.toLowerCase() || '' -] +tokenStreams[selectedRecurringToken?.token.id?.toLowerCase() ?? '']src/wagmiConfigs.ts (1)
41-43: Consider creating a tracking issue for the TODOThe comment indicates this is a temporary hotfix. We should track this technical debt to ensure proper transport management for all chains is implemented.
Would you like me to create a GitHub issue to track this improvement task?
src/components/views/donate/Recurring/ModifySuperToken/DepositSuperToken.tsx (2)
129-143: Consider extracting token decimal conversion logic.The token decimal conversion logic could be moved to a separate utility function for better reusability and maintainability. Also, consider adding validation for other decimal values.
+ // In src/lib/helpers/token.ts + export const normalizeTokenAmount = (amount: bigint, tokenDecimals: number): bigint => { + if (tokenDecimals === 6) { + const divisor = BigInt(10 ** tokenDecimals); + const currentAmount = Number(amount) / Number(divisor); + return ethers.utils.parseUnits(currentAmount.toString(), 18).toBigInt(); + } + return amount; + } - let newAmount = amount; - if (token && token.decimals === 6) { - const divisor = BigInt(10 ** token.decimals); - const currentAmount = Number(amount) / Number(divisor); - newAmount = ethers.utils - .parseUnits(currentAmount.toString(), 18) - .toBigInt(); - } + const newAmount = token ? normalizeTokenAmount(amount, token.decimals) : amount;
144-145: Consider using structured logging.Replace console.log with structured logging for better debugging capabilities.
- console.log('token', token); - console.log('supertoken', superToken); + console.log('Upgrading token to superToken:', { + token, + superToken, + amount: newAmount.toString() + });src/artifacts/usdtMainnetABI.json (1)
70-85: Review security implications of admin functionsThe USDT ABI includes sensitive admin functions like
addBlackListanddestroyBlackFunds. While these are part of the official USDT contract, it's important to note their implications:
- These functions can only be called by the contract owner
- They represent centralized control capabilities of USDT
Consider adding comments in the code to document these administrative capabilities and their potential impact on transactions.
Also applies to: 538-553
src/lib/helpers.ts (1)
396-398: Ensure robust type handling for decimalsThe conversion from bigint to number is correct but could be more explicit.
Consider adding a type guard:
-if (typeof decimals === 'bigint') { +if (typeof decimals === 'bigint') { + if (decimals > BigInt(Number.MAX_SAFE_INTEGER)) { + throw new Error('Decimal value too large'); + } decimals = Number(decimals.toString()); }src/config/production.tsx (1)
Line range hint
529-564: Consider the impact on existing streams and integrationsThe changes to token IDs and addition of USDC.e could affect:
- Existing payment streams using these tokens
- Frontend components that reference these tokens
- Integration points with SuperFluid
Consider:
- Adding a migration plan for existing streams
- Updating frontend components to handle both old and new token IDs during transition
- Testing SuperFluid integration with the new token configurations
🛑 Comments failed to post (5)
src/components/views/donate/Recurring/ModifySuperToken/WithDrawSuperToken.tsx (1)
190-190:
⚠️ Potential issueRemove non-null assertion and add proper type safety.
The non-null assertion (
!) onsuperTokencould lead to runtime errors if the prop is undefined. Consider adding proper validation and error handling.Here's a safer approach:
-token={superToken!} +token={superToken ?? undefined}Also, consider adding prop validation at the component level:
if (!superToken) { return ( <Wrapper> <div>Error: SuperToken is required for withdrawal</div> </Wrapper> ); }src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx (2)
197-208: 🛠️ Refactor suggestion
Add null checks for nested token properties
The deep nested access of token properties could lead to runtime errors if any intermediate value is undefined.
Add null checks or use optional chaining:
- balances[token.underlyingToken.symbol] + balances[token.underlyingToken?.symbol ?? '']📝 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.balance={ balances[ token.underlyingToken?.symbol ?? '' ] } disable={ balances[ token.underlyingToken?.symbol ?? '' ] === undefined || balances[ token.underlyingToken?.symbol ?? '' ] === 0n
193-222:
⚠️ Potential issueRemove unnecessary Fragment and add key to mapped elements
The Fragment (
<>) is unnecessary here and thekeyprop should be moved to the outer element of the map.Apply this fix:
- underlyingTokens.map(token => ( - <> - <TokenInfo - key={token.underlyingToken?.symbol} - token={token.underlyingToken} - ... - /> - </> - )) + underlyingTokens.map(token => ( + <TokenInfo + key={token.underlyingToken?.symbol} + token={token.underlyingToken} + ... + /> + ))Committable suggestion skipped: line range outside the PR's diff.
src/components/Footer/Footer.tsx (1)
146-159:
⚠️ Potential issueAdd security attributes and accessibility improvements to external links
The new QACC links open in new tabs but are missing security attributes and accessibility improvements.
Apply these changes:
- <a href={links.QACC} target='_blank'> + <a href={links.QACC} target='_blank' rel='noreferrer noopener' aria-label='QACC (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='QACC 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.<a href={links.QACC} target='_blank' rel='noreferrer noopener' aria-label='QACC (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='QACC News (opens in new tab)'> <LinkItem color={textColor}> {formatMessage({ id: 'component.title.qacc_news', })} </LinkItem> </a>src/lib/helpers.ts (1)
379-385: 🛠️ Refactor suggestion
Consider network-agnostic USDT handling
The USDT contract address is hardcoded for mainnet. This might cause issues when dealing with other networks.
Consider making the USDT address configurable per network:
+const USDT_ADDRESSES = { + 1: '0xdac17f958d2ee523a2206206994597c13d831ec7', // mainnet + 10: '0x94b008aa00579c1307b0ef2c499ad98a8ce58e58', // optimism + // Add other network addresses +}; -contractAddress === '0xdac17f958d2ee523a2206206994597c13d831ec7' +USDT_ADDRESSES[chainId] === contractAddressCommittable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Chores