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

Set ----> OrderSet #1057

Closed
wants to merge 1 commit into from
Closed

Set ----> OrderSet #1057

wants to merge 1 commit into from

Conversation

xumoyan
Copy link

@xumoyan xumoyan commented Aug 25, 2023

Wallets sometimes encounter a situation where authorizations do not receive a response when interacting with certain dApps. Upon investigation, it has been found that dApps retrieve the first element from the authorization array, and using a Set can alter the order of authorizations. The recommended approach is to use an OrderSet, which not only removes duplicates but also maintains the original order of authorizations.

@flypaper0
Copy link
Contributor

I think it's not safe to rely on elements order. More safe approach can be to iterate by set.

@flypaper0 flypaper0 requested a review from llbartekll September 4, 2023 14:28
@flypaper0 flypaper0 added the question Further information is requested label Sep 4, 2023
@flypaper0
Copy link
Contributor

Why dapps taking first element? It seems like vary optimistic implementation.

@xumoyan
Copy link
Author

xumoyan commented Sep 5, 2023

Get the first item from the list obtained from https://curve.fi/#/ethereum/swap. Not sure if there are any other dapps to handle it. OrderSet both maintains the order and removes duplicate elements, which is more reasonable

@llbartekll
Copy link
Contributor

@xumoyan can you give a specific example?

@xumoyan
Copy link
Author

xumoyan commented Sep 6, 2023

Wallet: TP iOS Version
DApp: Curve

After connecting to Wallet Connect, Curve is likely unable to display the connection status. However, switching to a different network reveals that the connection is working properly.

The issue with Curve arises from accessing the first element of the array it retrieves. However, the first element is not necessarily the chain ID for Ethereum.

Testing on Android and web platforms has been successful. Only iOS is affected due to the use of a Set, which changes the order and causes this problem.

@xumoyan
Copy link
Author

xumoyan commented Sep 14, 2023

@llbartekll Has this problem not recurred?

@llbartekll
Copy link
Contributor

no one has ever reported similar issue, so I am trying to understand.
It may also be just an incorrect integration.

can you share all the namespaces used in the session, exact requests and errors?

@flypaper0
Copy link
Contributor

I'm closing it due to inactivity. Looks like incorrect SDK integration from Curve side

@flypaper0 flypaper0 closed this Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants