Feat/add cbbtc recurring #4955
Feat/add cbbtc recurring #4955
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request focuses on modifying the handling of tokens with 8 decimal places across multiple components. The changes primarily involve adjusting decimal place calculations, scaling factors, and input validation for tokens with 8 decimals. The modifications ensure more consistent and precise processing of token amounts, particularly for tokens that have 8 decimal places, by updating the logic in deposit, input, and display components. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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: 0
🔭 Outside diff range comments (1)
src/components/views/donate/Recurring/ModifySuperToken/DepositSuperToken.tsx (1)
Line range hint
138-144: Avoid potential precision loss in token amount conversion.The current implementation has several concerns:
- Converting BigInt to Number using
Number(amount)could lead to precision loss for large amounts.- The decimal handling is limited to only 6 and 8 decimals, which might not cover all token types.
- There's no validation for the
decimalsvalue.Consider this safer implementation:
- if (token && (token.decimals === 6 || token.decimals === 8)) { - const divisor = BigInt(10 ** token.decimals); - const currentAmount = Number(amount) / Number(divisor); - newAmount = ethers.utils - .parseUnits(currentAmount.toString(), 18) - .toBigInt(); + if (token?.decimals != null) { + // Validate decimals + if (token.decimals < 0 || token.decimals > 18) { + throw new Error('Invalid token decimals'); + } + + // Convert from token decimals to 18 decimals while preserving precision + if (token.decimals < 18) { + newAmount = amount * BigInt(10 ** (18 - token.decimals)); + } else if (token.decimals > 18) { + newAmount = amount / BigInt(10 ** (token.decimals - 18)); + } + }This implementation:
- Preserves precision by using BigInt arithmetic
- Supports any valid number of decimals
- Validates the decimals value
- Handles both scaling up and down to 18 decimals
🧹 Nitpick comments (1)
src/components/views/donate/Recurring/ModifySuperToken/DepositSuperToken.tsx (1)
Line range hint
145-157: Consider adding error details to improve debugging.The error handling could be more informative to help diagnose issues in production.
const upgradeOperation = await superTokenAsset.upgrade({ amount: newAmount.toString(), }); const tx = await upgradeOperation.exec(signer); const res = await tx.wait(); if (!res.status) { - throw new Error('Deposit failed'); + throw new Error(`Deposit failed: Transaction reverted. Hash: ${tx.hash}`); } refreshBalance(); setStep(EModifySuperTokenSteps.DEPOSIT_CONFIRMED); } catch (error: any) { if (error?.code === 'ACTION_REJECTED') { setStep(EModifySuperTokenSteps.MODIFY); } else { + console.error('Deposit error:', { + error, + token: token?.symbol, + decimals: token?.decimals, + amount: amount.toString(), + newAmount: newAmount.toString(), + }); showToastError(error); setStep(EModifySuperTokenSteps.DEPOSIT); } - console.error('error', error); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/views/donate/Recurring/ModifySuperToken/DepositSuperToken.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
src/components/views/donate/Recurring/ModifySuperToken/DepositSuperToken.tsx (1)
Line range hint
138-144: Verify consistent decimal handling across the codebase.Let's check if there are other components that need similar updates for handling different token decimals.
✅ Verification successful
Decimal handling implementation is consistent with codebase patterns
The implementation follows the established patterns for handling tokens with 6 and 8 decimals, matching similar implementations in RecurringDonationCard, RecurringDonationModal, and AmountInput components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar token decimal handling patterns echo "Searching for token decimal comparisons..." rg "decimals === 6" --type ts rg "decimals === 8" --type ts rg "token\.decimals" -B 2 -A 2 --type ts echo "Searching for potential test files..." fd "test|spec" -e ts -e tsx | grep -i "token\|deposit"Length of output: 13692
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/AmountInput/AmountInput.tsx (1)
83-85: Fix outdated comment.The comment states "Limit to 8 decimals" but the code actually limits to 6 decimals.
- if (_decimals?.length > 6) return; // Limit to 8 decimals + if (_decimals?.length > 6) return; // Limit to 6 decimals
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/AmountInput/AmountInput.tsx(2 hunks)src/components/views/donate/Recurring/RecurringDonationCard.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
src/components/AmountInput/AmountInput.tsx (1)
55-55: Verify the impact of reducing decimal precision for 8-decimal tokens.The change reduces the maximum allowed decimals from 8 to 6 for tokens with 8 decimal places. While this might improve UI readability, it could affect precision in certain scenarios.
✅ Verification successful
Display precision change for 8-decimal tokens is aligned with industry standards
The reduction from 8 to 6 decimal places only affects the display/input formatting and not the underlying token precision. This change:
- Improves readability while maintaining sufficient precision for all practical use cases
- Matches the standard Bitcoin display format of 6 decimal places
- Only impacts one token (cbBTC) in the entire codebase
- Does not affect actual transaction precision
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any token declarations or configurations with 8 decimals # to understand the impact scope rg -A 5 'decimals.*8' .Length of output: 23939
src/components/views/donate/Recurring/RecurringDonationCard.tsx (2)
363-377: LGTM! Consistent decimal handling across components.The balance display logic now aligns with the decimal precision changes in AmountInput.tsx, ensuring consistent display of token amounts throughout the application.
160-160:⚠️ Potential issueVerify the scale factor adjustment for 8-decimal tokens.
The scale factor for 8-decimal tokens has been significantly increased from 100n to 1000000n. This change affects flow rate calculations and should be carefully validated.
Please verify:
- The impact on existing streams
- The alignment with Superfluid's flow rate calculations
- The relationship with the decimal precision changes in AmountInput.tsx
✅ Verification successful
Scale factor change for 8-decimal tokens is correct
The change from 100n to 1000000n for 8-decimal tokens is a proper fix that:
- Maintains proportional scaling with 6-decimal tokens (10000n vs 1000000n)
- Aligns with the 6-decimal place precision limit in AmountInput.tsx
- Preserves calculation accuracy as scale factors cancel out in final flow rates
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for flow rate calculations to understand the impact rg -A 5 'flowRate|scaleFactor' .Length of output: 34582
Script:
#!/bin/bash # Search for decimal-specific token handling and scale factors rg -A 3 "decimals.*[68]|scaleFactor" .Length of output: 18988
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/views/userProfile/donationsTab/recurringTab/EndStreamModal.tsx (2)
120-123: Consider explicitly handling missing contracts for this network.If
finddoesn’t return a matching contract (e.g., no anchorContracts for this network), the code falls back to an empty string at line 126. This may cause errors if the flow requires a valid address. Consider raising an error or providing a more informative fallback.You might use something like:
const matchingContract = donation.project.anchorContracts.find( contract => contract.networkId === recurringNetworkId, ); +if (!matchingContract) { + throw new Error(`No anchor contract found for network ${recurringNetworkId}.`); +}
126-126: Double-check fallback to empty string as receiver.If no matching address is found, passing
''as a receiver may produce unintended behavior. Consider using a zero address (0x000…) or throwing an error instead of using an empty string to avoid potentially ambiguous scenarios in your flow logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/views/userProfile/donationsTab/recurringTab/EndStreamModal.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
Summary by CodeRabbit
New Features
Bug Fixes
Performance