-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FE LiFi: Revoke approvals #2874
Conversation
Warning Rate limit exceeded@aureliusbtc has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 13 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent changes introduce a new page component Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
- Added
LifiPage
component to revoke token approvals for USDC, USDT, and WETH - Fetches and displays current allowances for these tokens
- Connect wallet button shown if user is not connected
- Revoke approval buttons displayed if allowances > 0
- Utilizes hooks for state management and existing utility functions
1 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings
const usdcAllowance = await getErc20TokenAllowance({ | ||
address: "0xbc6f5a4ed57f16af3db54da801aba8d1dc4ed675", | ||
chainId: connectedChainId, | ||
tokenAddress: usdcAddress, | ||
spender: "0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style: Consider extracting the common parameters into a variable to avoid repetition.
new Promise((resolve, reject) => { | ||
try { | ||
openConnectModal() | ||
resolve(true) | ||
} catch (e) { | ||
reject(e) | ||
} | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic: Using a promise here is redundant since openConnectModal
is already a synchronous function.
await approveToken( | ||
"0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | ||
connectedChainId, | ||
usdcAddress, | ||
0n, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style: Wrap the approveToken
call in a try/catch block to handle potential errors.
await approveToken( | ||
"0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | ||
connectedChainId, | ||
usdtAddress, | ||
0n, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style: Wrap the approveToken
call in a try/catch block to handle potential errors.
onClick={async () => { | ||
await approveToken( | ||
"0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | ||
connectedChainId, | ||
wethAddress, | ||
0n, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style: Wrap the approveToken
call in a try/catch block to handle potential errors.
Bundle ReportChanges will decrease total bundle size by 494.67kB ⬇️
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2874 +/- ##
====================================================
+ Coverage 23.57349% 90.47619% +66.90270%
====================================================
Files 679 54 -625
Lines 50841 1008 -49833
Branches 80 80
====================================================
- Hits 11985 912 -11073
+ Misses 37533 93 -37440
+ Partials 1323 3 -1320
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying sanguine with Cloudflare Pages
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/synapse-interface/pages/lifi/index.tsx (1 hunks)
Additional comments not posted (2)
packages/synapse-interface/pages/lifi/index.tsx (2)
1-12
: Review of imports: Looks clean and well-organized.The imports are specific and seem necessary for the functionality of the
LifiPage
component. No issues detected here.
176-176
: Export statement is correct.The
LifiPage
component is correctly exported as a default export.
<div className="flex justify-between"> | ||
<div> | ||
<div className="text-2xl text-white"> | ||
Revoke Li.fi Approvals | ||
</div> | ||
</div> | ||
</div> | ||
<div className="py-6"> | ||
<Grid | ||
cols={{ xs: 1 }} | ||
gap={6} | ||
className="justify-center px-2 py-16 sm:px-6 md:px-8" | ||
> | ||
<div className="pb-3 place-self-center"> | ||
<div> | ||
<div>USDC Allowance At Risk: {usdcAllowance.toString()}</div> | ||
<div>USDT Allowance At Risk: {usdtAllowance.toString()}</div> | ||
<div>WETH Allowance at Risk: {wethAllowance.toString()}</div> | ||
</div> | ||
</div> | ||
{!isConnected && ( | ||
<div className="flex flex-col justify-center h-full p-10"> | ||
<TransactionButton | ||
style={{ | ||
background: | ||
'linear-gradient(90deg, rgba(128, 0, 255, 0.2) 0%, rgba(255, 0, 191, 0.2) 100%)', | ||
border: '1px solid #9B6DD7', | ||
borderRadius: '4px', | ||
}} | ||
label="Connect wallet" | ||
pendingLabel="Connecting" | ||
onClick={() => | ||
new Promise((resolve, reject) => { | ||
try { | ||
openConnectModal() | ||
resolve(true) | ||
} catch (e) { | ||
reject(e) | ||
} | ||
}) | ||
} | ||
/> | ||
</div> | ||
)} | ||
{usdcAllowance > 0n && ( | ||
<TransactionButton | ||
className="btn btn-primary" | ||
pendingLabel="Revoking..." | ||
label="Revoke USDC Approval" | ||
onClick={async () => { | ||
await approveToken( | ||
"0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | ||
connectedChainId, | ||
usdcAddress, | ||
0n, | ||
) | ||
setUsdcAllowance(0n) | ||
}} | ||
/> | ||
)} | ||
|
||
{usdtAllowance > 0n && ( | ||
<TransactionButton | ||
className="btn btn-primary" | ||
pendingLabel="Revoking..." | ||
label="Revoke USDT Approval" | ||
onClick={async () => { | ||
await approveToken( | ||
"0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | ||
connectedChainId, | ||
usdtAddress, | ||
0n, | ||
) | ||
setUsdtAllowance(0n) | ||
}} | ||
/> | ||
)} | ||
|
||
{wethAllowance > 0n && ( | ||
<TransactionButton | ||
className="btn btn-primary" | ||
pendingLabel="Revoking..." | ||
label="Revoke WETH Approval" | ||
onClick={async () => { | ||
await approveToken( | ||
"0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | ||
connectedChainId, | ||
wethAddress, | ||
0n, | ||
) | ||
setWethAllowance(0n) | ||
}} | ||
/> | ||
)} | ||
</Grid> | ||
</div> | ||
</StandardPageContainer> | ||
</LandingPageWrapper> | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comprehensive review of the LifiPage
component.
- State Management and Hooks: The use of
useState
anduseEffect
appears appropriate. The state is managed locally and updates are triggered based on changes to relevant dependencies. - Blockchain Interactions: The component fetches token allowances and allows users to revoke approvals. It is crucial to ensure that these interactions are secure and efficient.
- Conditional Rendering: The use of conditional rendering based on the wallet connection status and token allowances is correctly implemented.
- Error Handling: Consider adding error handling for asynchronous operations, especially those interacting with the blockchain.
- Code Duplication: The code for fetching token allowances is repeated for each token. This could be refactored into a more reusable function to reduce duplication and improve maintainability.
- const usdcAllowance = await getErc20TokenAllowance({
- address: "0xbc6f5a4ed57f16af3db54da801aba8d1dc4ed675",
- chainId: connectedChainId,
- tokenAddress: usdcAddress,
- spender: "0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae",
- })
+ const fetchTokenAllowance = async (tokenAddress) => {
+ return await getErc20TokenAllowance({
+ address: "0xbc6f5a4ed57f16af3db54da801aba8d1dc4ed675",
+ chainId: connectedChainid,
+ tokenAddress,
+ spender: "0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae",
+ });
+ }
+ const usdcAllowance = await fetchTokenAllowance(usdcAddress);
- Performance Considerations: Ensure that the asynchronous functions do not block the UI or lead to performance issues.
- Security Concerns: Verify that all blockchain interactions are secure, particularly in terms of handling user addresses and chain IDs.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const LifiPage = () => { | |
const { address: currentAddress, chain, isConnected } = useAccount() | |
const [connectedChainId, setConnectedChainId] = useState(0) | |
const [address, setAddress] = useState(undefined) | |
const { openConnectModal } = useConnectModal() | |
useEffect(() => { | |
setConnectedChainId(chain?.id ?? DEFAULT_FROM_CHAIN) | |
}, [chain]) | |
useEffect(() => { | |
setAddress(currentAddress) | |
}, [currentAddress]) | |
const usdcAddress = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48' | |
const usdtAddress = '0xdac17f958d2ee523a2206206994597c13d831ec7' | |
const wethAddress = '0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2' | |
const [usdcAllowance, setUsdcAllowance] = useState<bigint>(0n) | |
const [usdtAllowance, setUsdtAllowance] = useState<bigint>(0n) | |
const [wethAllowance, setWethAllowance] = useState<bigint>(0n) | |
useEffect(() => { | |
const fetchAllowances = async () => { | |
if (address) { | |
const usdcAllowance = await getErc20TokenAllowance({ | |
address: "0xbc6f5a4ed57f16af3db54da801aba8d1dc4ed675", | |
chainId: connectedChainId, | |
tokenAddress: usdcAddress, | |
spender: "0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | |
}) | |
setUsdcAllowance(usdcAllowance) | |
const usdtAllowance = await getErc20TokenAllowance({ | |
address: "0xbc6f5a4ed57f16af3db54da801aba8d1dc4ed675", | |
chainId: connectedChainId, | |
tokenAddress: usdtAddress, | |
spender: "0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | |
}) | |
setUsdtAllowance(usdtAllowance) | |
const wethAllowance = await getErc20TokenAllowance({ | |
address: "0xbc6f5a4ed57f16af3db54da801aba8d1dc4ed675", | |
chainId: connectedChainId, | |
tokenAddress: wethAddress, | |
spender: "0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | |
}) | |
setWethAllowance(wethAllowance) | |
} | |
} | |
fetchAllowances() | |
}, [address, connectedChainId]) | |
return ( | |
<LandingPageWrapper> | |
<StandardPageContainer | |
connectedChainId={connectedChainId} | |
address={address} | |
> | |
<div className="flex justify-between"> | |
<div> | |
<div className="text-2xl text-white"> | |
Revoke Li.fi Approvals | |
</div> | |
</div> | |
</div> | |
<div className="py-6"> | |
<Grid | |
cols={{ xs: 1 }} | |
gap={6} | |
className="justify-center px-2 py-16 sm:px-6 md:px-8" | |
> | |
<div className="pb-3 place-self-center"> | |
<div> | |
<div>USDC Allowance At Risk: {usdcAllowance.toString()}</div> | |
<div>USDT Allowance At Risk: {usdtAllowance.toString()}</div> | |
<div>WETH Allowance at Risk: {wethAllowance.toString()}</div> | |
</div> | |
</div> | |
{!isConnected && ( | |
<div className="flex flex-col justify-center h-full p-10"> | |
<TransactionButton | |
style={{ | |
background: | |
'linear-gradient(90deg, rgba(128, 0, 255, 0.2) 0%, rgba(255, 0, 191, 0.2) 100%)', | |
border: '1px solid #9B6DD7', | |
borderRadius: '4px', | |
}} | |
label="Connect wallet" | |
pendingLabel="Connecting" | |
onClick={() => | |
new Promise((resolve, reject) => { | |
try { | |
openConnectModal() | |
resolve(true) | |
} catch (e) { | |
reject(e) | |
} | |
}) | |
} | |
/> | |
</div> | |
)} | |
{usdcAllowance > 0n && ( | |
<TransactionButton | |
className="btn btn-primary" | |
pendingLabel="Revoking..." | |
label="Revoke USDC Approval" | |
onClick={async () => { | |
await approveToken( | |
"0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | |
connectedChainId, | |
usdcAddress, | |
0n, | |
) | |
setUsdcAllowance(0n) | |
}} | |
/> | |
)} | |
{usdtAllowance > 0n && ( | |
<TransactionButton | |
className="btn btn-primary" | |
pendingLabel="Revoking..." | |
label="Revoke USDT Approval" | |
onClick={async () => { | |
await approveToken( | |
"0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | |
connectedChainId, | |
usdtAddress, | |
0n, | |
) | |
setUsdtAllowance(0n) | |
}} | |
/> | |
)} | |
{wethAllowance > 0n && ( | |
<TransactionButton | |
className="btn btn-primary" | |
pendingLabel="Revoking..." | |
label="Revoke WETH Approval" | |
onClick={async () => { | |
await approveToken( | |
"0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | |
connectedChainId, | |
wethAddress, | |
0n, | |
) | |
setWethAllowance(0n) | |
}} | |
/> | |
)} | |
</Grid> | |
</div> | |
</StandardPageContainer> | |
</LandingPageWrapper> | |
) | |
} | |
const LifiPage = () => { | |
const { address: currentAddress, chain, isConnected } = useAccount() | |
const [connectedChainId, setConnectedChainId] = useState(0) | |
const [address, setAddress] = useState(undefined) | |
const { openConnectModal } = useConnectModal() | |
useEffect(() => { | |
setConnectedChainId(chain?.id ?? DEFAULT_FROM_CHAIN) | |
}, [chain]) | |
useEffect(() => { | |
setAddress(currentAddress) | |
}, [currentAddress]) | |
const usdcAddress = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48' | |
const usdtAddress = '0xdac17f958d2ee523a2206206994597c13d831ec7' | |
const wethAddress = '0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2' | |
const [usdcAllowance, setUsdcAllowance] = useState<bigint>(0n) | |
const [usdtAllowance, setUsdtAllowance] = useState<bigint>(0n) | |
const [wethAllowance, setWethAllowance] = useState<bigint>(0n) | |
useEffect(() => { | |
const fetchTokenAllowance = async (tokenAddress) => { | |
return await getErc20TokenAllowance({ | |
address: "0xbc6f5a4ed57f16af3db54da801aba8d1dc4ed675", | |
chainId: connectedChainId, | |
tokenAddress, | |
spender: "0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | |
}); | |
} | |
const fetchAllowances = async () => { | |
if (address) { | |
const usdcAllowance = await fetchTokenAllowance(usdcAddress); | |
setUsdcAllowance(usdcAllowance) | |
const usdtAllowance = await fetchTokenAllowance(usdtAddress); | |
setUsdtAllowance(usdtAllowance) | |
const wethAllowance = await fetchTokenAllowance(wethAddress); | |
setWethAllowance(wethAllowance) | |
} | |
} | |
fetchAllowances() | |
}, [address, connectedChainId]) | |
return ( | |
<LandingPageWrapper> | |
<StandardPageContainer | |
connectedChainId={connectedChainId} | |
address={address} | |
> | |
<div className="flex justify-between"> | |
<div> | |
<div className="text-2xl text-white"> | |
Revoke Li.fi Approvals | |
</div> | |
</div> | |
</div> | |
<div className="py-6"> | |
<Grid | |
cols={{ xs: 1 }} | |
gap={6} | |
className="justify-center px-2 py-16 sm:px-6 md:px-8" | |
> | |
<div className="pb-3 place-self-center"> | |
<div> | |
<div>USDC Allowance At Risk: {usdcAllowance.toString()}</div> | |
<div>USDT Allowance At Risk: {usdtAllowance.toString()}</div> | |
<div>WETH Allowance at Risk: {wethAllowance.toString()}</div> | |
</div> | |
</div> | |
{!isConnected && ( | |
<div className="flex flex-col justify-center h-full p-10"> | |
<TransactionButton | |
style={{ | |
background: | |
'linear-gradient(90deg, rgba(128, 0, 255, 0.2) 0%, rgba(255, 0, 191, 0.2) 100%)', | |
border: '1px solid #9B6DD7', | |
borderRadius: '4px', | |
}} | |
label="Connect wallet" | |
pendingLabel="Connecting" | |
onClick={() => | |
new Promise((resolve, reject) => { | |
try { | |
openConnectModal() | |
resolve(true) | |
} catch (e) { | |
reject(e) | |
} | |
}) | |
} | |
/> | |
</div> | |
)} | |
{usdcAllowance > 0n && ( | |
<TransactionButton | |
className="btn btn-primary" | |
pendingLabel="Revoking..." | |
label="Revoke USDC Approval" | |
onClick={async () => { | |
await approveToken( | |
"0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | |
connectedChainId, | |
usdcAddress, | |
0n, | |
) | |
setUsdcAllowance(0n) | |
}} | |
/> | |
)} | |
{usdtAllowance > 0n && ( | |
<TransactionButton | |
className="btn btn-primary" | |
pendingLabel="Revoking..." | |
label="Revoke USDT Approval" | |
onClick={async () => { | |
await approveToken( | |
"0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | |
connectedChainId, | |
usdtAddress, | |
0n, | |
) | |
setUsdtAllowance(0n) | |
}} | |
/> | |
)} | |
{wethAllowance > 0n && ( | |
<TransactionButton | |
className="btn btn-primary" | |
pendingLabel="Revoking..." | |
label="Revoke WETH Approval" | |
onClick={async () => { | |
await approveToken( | |
"0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae", | |
connectedChainId, | |
wethAddress, | |
0n, | |
) | |
setWethAllowance(0n) | |
}} | |
/> | |
)} | |
</Grid> | |
</div> | |
</StandardPageContainer> | |
</LandingPageWrapper> | |
) | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Updated
getErc20TokenAllowance
calls to usecurrentAddress
inpackages/synapse-interface/pages/lifi/index.tsx
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/synapse-interface/pages/lifi/index.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/synapse-interface/pages/lifi/index.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Introduced new page for managing Li.fi approvals for USDC, USDT, and WETH tokens (
packages/synapse-interface/pages/lifi/index.tsx
) - Fetch token allowances and revoke approvals directly from the interface
- Conditional rendering based on wallet connection status
- Inform users about ongoing exploit and encourage revoking approvals
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Updated wallet connection button label for clarity (
packages/synapse-interface/pages/lifi/index.tsx
) - Conditional rendering based on wallet connection status (
packages/synapse-interface/pages/lifi/index.tsx
)
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- No major changes found since last review.
1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings
if (chain?.id !== chainId) { | ||
await switchNetwork({chainId: chainId}) | ||
} | ||
await approveToken(LIFI_SPENDER, chainId, tokenAddress, 0n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style: Wrap approveToken call in a try/catch block to handle potential errors.
onClick={() => new Promise((resolve) => { | ||
openConnectModal() | ||
resolve(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic: Using a promise here is redundant since openConnectModal is already a synchronous function.
Description
A clear and concise description of the features you're adding in this pull request.
Additional context
Add any other context about the problem you're solving.
Metadata
Summary by CodeRabbit
9db21f2: synapse-interface preview link
2a607fc: synapse-interface preview link
0faff57: synapse-interface preview link
19ae944: synapse-interface preview link
17c48ce: synapse-interface preview link