Skip to content
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

fix: fetch correct cctp history for contract wallets #2058

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

brtkx
Copy link
Contributor

@brtkx brtkx commented Nov 5, 2024

Closes FS-969

Issue

User withdraws USDC via CCTP from L2 Safe to L1. Then they connect to the destination account and don't see the transaction in the history.

Current setup and why it doesn't work:

Currently we use cctpTypeToFetch to determine what type of transfers we fetch: deposits, withdrawals or all. In case of EOA we fetch all (and it's fine because EOA's address is the same on L1 and L2). However, when using SCW we fetch either deposits or withdrawals. The current flow for SCW is as follows:

  1. Fetch deposits if connected to the parent chain.
  2. Fetch withdrawals if connected to the child chain.

Now if user is connected to the parent chain, we only fetch deposits. Which means they won't see withdrawals to their account and won't be able to claim from this account.

Solution

To fix this, we need to fetch all CCTP transfers for SCW as well, but we need to keep in mind that L1 and L2 addresses may not be the same. To cover this we filter out transactions that aren't user's transactions. We use the following logic for this:

I. Fetching deposits

  1. Connected to L1
    Show if sender === walletAddress
  2. Connected to L2
    Show if receiver === walletAddress

II. Fetching withdrawals

  1. Connected to L1
    1. Show if receiver === walletAddress
  2. Connected to L2
    1. Show if sender === walletAddress

@cla-bot cla-bot bot added the cla-signed label Nov 5, 2024
Copy link

vercel bot commented Nov 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
arbitrum-token-bridge ✅ Ready (Inspect) Visit Preview Nov 8, 2024 0:31am

@fionnachan fionnachan self-requested a review November 5, 2024 14:13
@brtkx brtkx requested a review from spsjvc November 5, 2024 14:14
Copy link
Member

@fionnachan fionnachan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested claiming on Sepolia using an SCW, it works well

export const useCCTPDeposits = ({
walletAddress,
l1ChainId,
pageNumber,
pageSize,
enabled
}: fetchCctpParams) => {
const { isSmartContractWallet } = useAccountType()
const chainId = useChainId()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we want to avoid using useChainId() because it returns Ethereum for chains not in wagmi config?

@brtkx brtkx marked this pull request as draft November 5, 2024 18:30
@brtkx brtkx marked this pull request as ready for review November 6, 2024 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants