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

Integrate skip widget #1943

Merged
merged 3 commits into from
Dec 12, 2024
Merged

Integrate skip widget #1943

merged 3 commits into from
Dec 12, 2024

Conversation

grod220
Copy link
Contributor

@grod220 grod220 commented Dec 11, 2024

Closes #1925

Adds a new layout and userflow to shield page:

compressed.mov

Copy link

changeset-bot bot commented Dec 11, 2024

🦋 Changeset detected

Latest commit: 6d59eac

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@penumbra-zone/ui-deprecated Patch
minifront Minor
node-status Patch
@repo/tailwind-config Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -39,17 +39,19 @@
"@radix-ui/react-navigation-menu": "^1.1.4",
"@radix-ui/react-portal": "^1.0.4",
"@remix-run/router": "^1.16.1",
"@tanstack/react-query": "4.36.1",
"@skip-go/widget": "^3.0.22",
"@tanstack/react-query": "5.62.7",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The react-query dep bump was required for the skip widget to work. Unfortunately, the osmo-query hooks only work on v4. Had to do some refactoring as a consequence, but it got simpler as a result.

Comment on lines +22 to +35
const useCosmosChainBalance = () => {
const { address, getRpcEndpoint, chain } = useChainConnector();

const rpcEndpointQuery = useRpcEndpoint({
getter: getRpcEndpoint,
options: {
enabled: !!address,
staleTime: Infinity,
queryKey: ['rpc endpoint', address, chain.chain_name],
// Needed for osmo-query's internal caching
queryKeyHashFn: queryKey => {
return JSON.stringify([...queryKey, chain.chain_name]);
},
return useQuery({
queryKey: ['rpc endpoint', address, chain.chain_name],
queryFn: async () => {
if (!address) {
throw new Error('missing address');
}
const endpoint = await getRpcEndpoint();
const client = await StargateClient.connect(endpoint);
return client.getAllBalances(address);
},
}) as UseQueryResult<string>;

const rpcClientQuery = useRpcClient({
rpcEndpoint: rpcEndpointQuery.data ?? '',
options: {
enabled: !!address && !!rpcEndpointQuery.data,
staleTime: Infinity,
queryKey: ['rpc client', address, rpcEndpointQuery.data, chain.chain_name],
// Needed for osmo-query's internal caching
queryKeyHashFn: queryKey => {
return JSON.stringify([...queryKey, chain.chain_name]);
},
},
}) as UseQueryResult<ProtobufRpcClient>;

const { cosmos: cosmosQuery, osmosis: osmosisQuery } = createRpcQueryHooks({
rpc: rpcClientQuery.data,
enabled: !!address,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Far simpler than what the former osmo-query helpers were providing

Copy link
Contributor

Choose a reason for hiding this comment

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

using standard useQuery over osmo-query specific hooks also has the benefit of reducing reliance on third-party libs, which minimizes another potential source of breakage.

Comment on lines 4 to 5
srcChainId: 'osmosis-1',
srcAssetDenom: 'ibc/498A0751C798A0D9A389AA3691123DADA57DAA4FE165D5C75894505B876BA6E4', // $USDC on osmosis
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be thoughtful about the default chain+asset here

Copy link
Member

Choose a reason for hiding this comment

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

Since there are issues with USDC transfers, can we use USDY instead?

Comment on lines +53 to +58
const { data: receiverView } = useQuery({
queryKey: ['receiverView', txInfo?.toJson({ typeRegistry }), option],
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- TODO: justify
queryFn: () => fetchReceiverView(txInfo!),
enabled: option === TxDetailsTab.RECEIVER && !!txInfo,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New v5 react-query requires object param style

@@ -83,7 +84,7 @@ export const SegmentedPicker = <ValueType extends { toString: () => string }>({
size === 'md' && 'text-sm px-3 h-8',
size === 'lg' && 'px-4 h-10',
value !== option.value && 'text-light-grey',
grow && 'grow',
grow && 'flex-1', // Use 'flex-1' for equal growth
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed to add this as the tabs would be determined by text width and not be even

@@ -93,7 +94,7 @@ export const SegmentedPicker = <ValueType extends { toString: () => string }>({
/>
)}

<div className='absolute inset-0 z-20 flex items-center justify-center'>
<div className='absolute inset-0 z-10 flex items-center justify-center'>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Found a bug where the tabs were overlaying the modal, this fixes that

@grod220 grod220 force-pushed the skip-integration branch 2 times, most recently from e58f381 to 6806478 Compare December 11, 2024 13:44
@@ -5,7 +5,7 @@
"license": "(MIT OR Apache-2.0)",
"type": "module",
"scripts": {
"build": "vite build",
"build": "cross-env NODE_OPTIONS=--max-old-space-size=8192 vite build",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Requires amping up memory to build app now 🏋️

@grod220 grod220 requested a review from a team December 11, 2024 14:04
Comment on lines 6 to 7
{ title: 'Deposit (skip)', href: PagePath.DEPOSIT_SKIP, enabled: true },
{ title: 'Deposit (manual)', href: PagePath.DEPOSIT_MANUAL, enabled: true },
Copy link
Member

Choose a reason for hiding this comment

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

nit: can these be Deposit (Skip) and Deposit (Manual)?

@hdevalence
Copy link
Member

This looks great! The deposit flow is way better and easier to understand.

Copy link
Contributor

@TalDerei TalDerei left a comment

Choose a reason for hiding this comment

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

tested the ibc flows 👍

can you add a changeset?

Comment on lines +22 to +35
const useCosmosChainBalance = () => {
const { address, getRpcEndpoint, chain } = useChainConnector();

const rpcEndpointQuery = useRpcEndpoint({
getter: getRpcEndpoint,
options: {
enabled: !!address,
staleTime: Infinity,
queryKey: ['rpc endpoint', address, chain.chain_name],
// Needed for osmo-query's internal caching
queryKeyHashFn: queryKey => {
return JSON.stringify([...queryKey, chain.chain_name]);
},
return useQuery({
queryKey: ['rpc endpoint', address, chain.chain_name],
queryFn: async () => {
if (!address) {
throw new Error('missing address');
}
const endpoint = await getRpcEndpoint();
const client = await StargateClient.connect(endpoint);
return client.getAllBalances(address);
},
}) as UseQueryResult<string>;

const rpcClientQuery = useRpcClient({
rpcEndpoint: rpcEndpointQuery.data ?? '',
options: {
enabled: !!address && !!rpcEndpointQuery.data,
staleTime: Infinity,
queryKey: ['rpc client', address, rpcEndpointQuery.data, chain.chain_name],
// Needed for osmo-query's internal caching
queryKeyHashFn: queryKey => {
return JSON.stringify([...queryKey, chain.chain_name]);
},
},
}) as UseQueryResult<ProtobufRpcClient>;

const { cosmos: cosmosQuery, osmosis: osmosisQuery } = createRpcQueryHooks({
rpc: rpcClientQuery.data,
enabled: !!address,
Copy link
Contributor

Choose a reason for hiding this comment

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

using standard useQuery over osmo-query specific hooks also has the benefit of reducing reliance on third-party libs, which minimizes another potential source of breakage.


return {
data: ibcAddrs,
isLoading: registryIsLoading || ibcAddrsLoading,
error: registryErr || ibcAddrssErr,
error: registryErr ?? ibcAddrssErr,
Copy link
Contributor

Choose a reason for hiding this comment

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

worth considering a separate effort to better capture IBC error codes and provide more user-friendly shielding / unshielding error messages

Screenshot 2024-12-11 at 12 39 43 PM Screenshot 2024-12-11 at 12 39 13 PM

@hdevalence
Copy link
Member

I tested this and was able to successfully withdraw UM to Osmosis and deposit UM on Penumbra. The Skip widget didn't work on the preview domain but the Skip team has offered to allowlist the domains in the Prax Registry so it should work when published.

@grod220 grod220 merged commit 39a9fd9 into main Dec 12, 2024
6 checks passed
@grod220 grod220 deleted the skip-integration branch December 12, 2024 07:49
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.

Improve IBC transfer page for minifront by using the Skip Widget
3 participants