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

Remove WC v1 #6214

Merged
merged 33 commits into from
Oct 25, 2024
Merged

Remove WC v1 #6214

merged 33 commits into from
Oct 25, 2024

Conversation

jinchung
Copy link
Member

@jinchung jinchung commented Oct 19, 2024

Fixes APP-1585

What changed (plus any additional context for devs)

  • Remove WalletConnect v1 (redux/walletconnect as well as entrypoints using WC v1)
  • Remove requests redux (redux/requests) which was being used by both WC v1 and v2 and replace it with a new store (state/walletConnectRequests)
  • Removed localstorage for WC requests
  • We are choosing to ignore migration from redux/localstorage to the new store
  • Follow up ticket created: alerting the user if they are using a WC v1 request

Screen recordings / screenshots

RPReplay_Final1729624897.MP4

What to test

  • Test WC flows (message signing, transactions, connections) that they work as expected
  • Test that requests still show up in the transaction list as expected and behave as expected when you interact with them

@jinchung jinchung changed the title @jin/rip wc v1 Remove WC v1 Oct 19, 2024
Copy link

linear bot commented Oct 19, 2024

@jinchung jinchung force-pushed the @jin/rip-wc-v1 branch 4 times, most recently from 381eb59 to 7d0f658 Compare October 22, 2024 16:37
@@ -5,7 +5,6 @@ import { AppRegistry, Dimensions, LogBox, StyleSheet, View } from 'react-native'
import { Toaster } from 'sonner-native';
import { MobileWalletProtocolProvider } from '@coinbase/mobile-wallet-protocol-host';
import { DeeplinkHandler } from '@/components/DeeplinkHandler';
import { AppStateChangeHandler } from '@/components/AppStateChangeHandler';
Copy link
Member Author

Choose a reason for hiding this comment

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

removing AppStateChangeHandler as it was being used only for WC v1 subscription when coming back from background


const handleAppStateChange = useCallback(
(nextAppState: AppStateStatus) => {
if (appState === 'background' && nextAppState === 'active') {
Copy link
Member Author

Choose a reason for hiding this comment

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

no longer needed as this was for loading WC v1 when coming back from background app state

import rainbowIconCircle from '@/assets/rainbow-icon-circle.png';
import { Source } from 'react-native-fast-image';

function ConnectedDapps() {
Copy link
Member Author

@jinchung jinchung Oct 22, 2024

Choose a reason for hiding this comment

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

this component was unused; was replaced with ConnectedDappsSheet a while ago


const renderItem = ({ item }) => <WalletConnectListItem {...item} />;

export default function WalletConnectList({ onLayout, ...props }) {
Copy link
Member Author

Choose a reason for hiding this comment

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

unused component


const columnStyle = padding.object(0, 10, 0, 12);

export default function WalletConnectListItem({ account, chainId, dappIcon, dappName, dappUrl }) {
Copy link
Member Author

Choose a reason for hiding this comment

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

was used for the V1 flow in ConnectedDappsSheet which is no longer needed

import { removeRequest } from '../redux/requests';
import { walletConnectSendStatus } from '../redux/walletconnect';

export default function useTransactionConfirmation() {
Copy link
Member Author

Choose a reason for hiding this comment

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

this was an unused hook

[requestId: number]: WalletconnectRequestData;
}

export interface WalletConnectRequestsState {
Copy link
Member Author

Choose a reason for hiding this comment

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

new store that replaces redux/requests and exposes the same methods for adding a request, removing a request, and getting a sorted list of the WC requests to display

@@ -752,21 +752,9 @@ export async function onSessionRequest(event: SignClientTypes.EventArguments['se
},
};

const { requests: pendingRequests } = store.getState().requests;
Copy link
Member Author

Choose a reason for hiding this comment

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

for the WC v2 flow, we used to directly update the redux/requests redux state as well as localstorage - now we use the new requests store

/**
* Display details loaded for a request.
*/
interface RequestDisplayDetails {
Copy link
Member Author

Choose a reason for hiding this comment

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

moved some of the types from the redux/requests and redux/walletconnect over to this file instead

const topic = remoteMessage?.data?.topic;

setTimeout(() => {
const requests = dispatch(requestsForTopic(topic as string));
Copy link
Member Author

Choose a reason for hiding this comment

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

not sure why it ended up like this, but this function doesn't really do anything other than fetch requests

@jinchung jinchung marked this pull request as ready for review October 22, 2024 19:23
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

Looks good on both OS's and also did a sanity check on dapp browser to make sure we're good. QA Passed 👍🏽

Copy link
Contributor

@greg-schrammel greg-schrammel left a comment

Choose a reason for hiding this comment

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

didn't test but the code is good 🫶

@jinchung jinchung merged commit 0d3f6b5 into develop Oct 25, 2024
7 of 8 checks passed
@jinchung jinchung deleted the @jin/rip-wc-v1 branch October 25, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants