feat: display Stark name in Sidebar #184

irisdv · 2023-12-08T15:22:38Z

Close #183

This PR displays the user Stark name in the Sidebar component.

To test it, you can use the python script buy_domain from this repo : https://github.com/irisdv/test_scripts

stanleyyconsensys

LGTM, thanks for the work

IMHO, i think there is no reason to create another snap method to get the name individually

instead the logic should bundle together in current recoverAccounts

stanleyyconsensys · 2024-01-05T11:25:17Z

packages/wallet-ui/src/utils/utils.ts

+  if (!domain) return '';
+  const ellipsis = '...';
+
+  if (domain.length <= maxLength) {


does the domain always in eng? if not we may consider to verify the length by [...domain].length

For now yes, but it could change in the future so I updated this part

stanleyyconsensys · 2024-01-05T11:26:05Z

packages/starknet-snap/src/utils/starknetUtils.ts

+export const getStarkNameUtil = async (network: Network, userAddress: string) => {
+  const provider = getProvider(network);
+  return provider.getStarkName(userAddress);
+};


Add EOS

Suggested change

};

};

so it's weird because there is one, but it's not showing

stanleyyconsensys · 2024-01-05T11:26:25Z

packages/starknet-snap/src/types/snapApi.ts

+
+export interface GetStarkNameRequestParam extends BaseRequestParams {
+  userAddress: string;
+}


Add EOS

Suggested change

}

}

stanleyyconsensys · 2024-01-05T11:33:42Z

packages/starknet-snap/src/utils/starknetUtils.ts

+
+export const getStarkNameUtil = async (network: Network, userAddress: string) => {
+  const provider = getProvider(network);
+  return provider.getStarkName(userAddress);


please check if the StarknetIdContract in sepolia
if yes, please pass in the sepolia StarknetIdContract when using sepolia

the current starknet.js does not contains sepolia contract in getStarknetIdContract

It's not yet deployed, but it will be in the coming days. We plan on updating starknet.js with it as well

is any update @irisdv

I made a PR to starknet.js adding the Sepolia contracts but it won't be deployed in production before a few weeks. I can add them in this PR, what do you think ?

irisdv · 2024-01-05T14:18:37Z

LGTM, thanks for the work

IMHO, i think there is no reason to create another snap method to get the name individually

instead the logic should bundle together in current recoverAccounts

I updated the code to get the stark name in recoverAccounts. I'm just not entirely sure of the best way to store it to use it in the ui, for now I created a state.starkNames but maybe storing it in state.accounts would make more sense ?

stanleyyconsensys · 2024-01-08T06:50:36Z

LGTM, thanks for the work
IMHO, i think there is no reason to create another snap method to get the name individually
instead the logic should bundle together in current recoverAccounts

I updated the code to get the stark name in recoverAccounts. I'm just not entirely sure of the best way to store it to use it in the ui, for now I created a state.starkNames but maybe storing it in state.accounts would make more sense ?

Thank you very much for the work

For SNAP
in order to bundle with recoverAccounts, you may have also need to update the methods of upsertAccount in snapUtils

For UI
you may need to change the structure of setAccounts , instead of storing a string address, but a object {address, name}
you may need to change the all files that need to using the address from state

in such big changes, may be it is better to rollback to use your original implementation, a single snap api to get the starkname

sorry for causing you the inconvenient

irisdv · 2024-01-08T08:49:29Z

LGTM, thanks for the work
IMHO, i think there is no reason to create another snap method to get the name individually
instead the logic should bundle together in current recoverAccounts

I updated the code to get the stark name in recoverAccounts. I'm just not entirely sure of the best way to store it to use it in the ui, for now I created a state.starkNames but maybe storing it in state.accounts would make more sense ?

Thank you very much for the work

For SNAP in order to bundle with recoverAccounts, you may have also need to update the methods of upsertAccount in snapUtils

For UI you may need to change the structure of setAccounts , instead of storing a string address, but a object {address, name} you may need to change the all files that need to using the address from state

in such big changes, may be it is better to rollback to use your original implementation, a single snap api to get the starkname

sorry for causing you the inconvenient

No worries, I rolled back to the previous version !

stanleyyconsensys · 2024-01-08T09:27:06Z

LGTM, thanks for the work
IMHO, i think there is no reason to create another snap method to get the name individually
instead the logic should bundle together in current recoverAccounts

I updated the code to get the stark name in recoverAccounts. I'm just not entirely sure of the best way to store it to use it in the ui, for now I created a state.starkNames but maybe storing it in state.accounts would make more sense ?

Thank you very much for the work
For SNAP in order to bundle with recoverAccounts, you may have also need to update the methods of upsertAccount in snapUtils
For UI you may need to change the structure of setAccounts , instead of storing a string address, but a object {address, name} you may need to change the all files that need to using the address from state
in such big changes, may be it is better to rollback to use your original implementation, a single snap api to get the starkname
sorry for causing you the inconvenient

No worries, I rolled back to the previous version !

just keep the domain.length checking where you did
thank you very much

irisdv · 2024-01-08T10:17:19Z

LGTM, thanks for the work
IMHO, i think there is no reason to create another snap method to get the name individually
instead the logic should bundle together in current recoverAccounts

I updated the code to get the stark name in recoverAccounts. I'm just not entirely sure of the best way to store it to use it in the ui, for now I created a state.starkNames but maybe storing it in state.accounts would make more sense ?

Thank you very much for the work
For SNAP in order to bundle with recoverAccounts, you may have also need to update the methods of upsertAccount in snapUtils
For UI you may need to change the structure of setAccounts , instead of storing a string address, but a object {address, name} you may need to change the all files that need to using the address from state
in such big changes, may be it is better to rollback to use your original implementation, a single snap api to get the starkname
sorry for causing you the inconvenient

No worries, I rolled back to the previous version !

just keep the domain.length checking where you did thank you very much

done

stanleyyconsensys · 2024-01-08T10:33:15Z

LGTM

irisdv added 3 commits December 8, 2023 16:04

feat: display Stark name in sidebar

e03f9f7

feat: display name in AccountDetails

cd92c8c

fix: only show in Sidebar

b9c9b46

irisdv requested a review from a team as a code owner December 8, 2023 15:22

irisdv requested review from Akaryatrh and jonesho December 8, 2023 15:22

irisdv mentioned this pull request Dec 8, 2023

feat: allow users to transfer money to a .stark name #185

Open

irisdv added 4 commits December 8, 2023 16:49

fix: result schema type for starkNet_getStarkName

4703ba4

fix: starkNet_getStarkName description

e754463

fix: conflicts with branch main

a9e9fac

test: add getStarkName test

4703026

stanleyyconsensys requested changes Jan 5, 2024

View reviewed changes

irisdv requested a review from stanleyyconsensys January 5, 2024 14:14

irisdv force-pushed the feat/display_starkname_sidebar branch from d99b384 to 4703026 Compare January 8, 2024 08:44

irisdv force-pushed the feat/display_starkname_sidebar branch from 2a29be5 to 4703026 Compare January 8, 2024 10:16

fricoben mentioned this pull request Jan 11, 2024

Metamask snap first sending integration lfglabs-dev/app.starknet.id#541

Closed

5 tasks

stanleyyconsensys merged commit 60f37da into Consensys:main Feb 21, 2024
3 checks passed

github-actions bot mentioned this pull request Feb 21, 2024

chore: release main #199

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: display Stark name in Sidebar #184

feat: display Stark name in Sidebar #184

irisdv commented Dec 8, 2023

stanleyyconsensys left a comment

stanleyyconsensys Jan 5, 2024

irisdv Jan 5, 2024

stanleyyconsensys Jan 5, 2024

irisdv Jan 5, 2024

stanleyyconsensys Jan 5, 2024

stanleyyconsensys Jan 5, 2024

irisdv Jan 5, 2024

stanleyyconsensys Jan 23, 2024

irisdv Jan 24, 2024

irisdv commented Jan 5, 2024

stanleyyconsensys commented Jan 8, 2024

irisdv commented Jan 8, 2024

stanleyyconsensys commented Jan 8, 2024

irisdv commented Jan 8, 2024

stanleyyconsensys commented Jan 8, 2024

feat: display Stark name in Sidebar #184

feat: display Stark name in Sidebar #184

Conversation

irisdv commented Dec 8, 2023

stanleyyconsensys left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

irisdv commented Jan 5, 2024

stanleyyconsensys commented Jan 8, 2024

irisdv commented Jan 8, 2024

stanleyyconsensys commented Jan 8, 2024

irisdv commented Jan 8, 2024

stanleyyconsensys commented Jan 8, 2024