Skip to content

NGRAVE ZERO brand import list integration#2831

Merged
vvvvvv1vvvvvv merged 10 commits intoRabbyHub:developfrom
ngraveio:ngrave-integration
Apr 9, 2025
Merged

NGRAVE ZERO brand import list integration#2831
vvvvvv1vvvvvv merged 10 commits intoRabbyHub:developfrom
ngraveio:ngrave-integration

Conversation

@caiquecruz
Copy link
Contributor

NGRAVE ZERO brand import list integration

Note:

  • Since Keystone already uses the bc-ur package and NGRAVE ZERO shares the same device type QR Hardware Wallet Device, explicit checks have been added to ensure proper handling.

Copy link
Contributor

@he1m4n6a he1m4n6a left a comment

Choose a reason for hiding this comment

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

Reviewed, no security risk.

.find((item) => item.type === keyring);
const alias = generateAliasName({
brandName: brandName,
brandName: isNgraveZero ? 'NGRAVE ZERO' : brandName,
Copy link
Contributor

Choose a reason for hiding this comment

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

It is best to use brandName directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@heisenberg-2077 Without this explicit check, it would display as 'Keystone' because it shares the same keyring type: 'QR Hardware Wallet Device', can we do this for this specific case ?

Copy link
Member

Choose a reason for hiding this comment

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

We need to modify the code to ensure that other brands that also use QRCode keyring will not be displayed as keystone. Please wait for us to modify it again to ensure that you do not need to do isNgraveZero recognition anywhere separately.

@vvvvvv1vvvvvv vvvvvv1vvvvvv added the keep For keep PR not auto-close by stale action label Apr 2, 2025
@heisenberg-2077
Copy link
Contributor

Please sync the latest code from the develop branch, add the NGRAVE ZERO as follows

https://github.com/RabbyHub/Rabby/blob/develop/src/ui/views/NewUserImport/ImportList.tsx#L68

        {
          type: KEYRING_CLASS.HARDWARE.KEYSTONE,
          logo: WALLET_BRAND_CONTENT[WALLET_BRAND_TYPES.NGRAVEZERO].icon,
          brand: WALLET_BRAND_CONTENT[WALLET_BRAND_TYPES.NGRAVEZERO].brand,
        },

@caiquecruz
Copy link
Contributor Author

caiquecruz commented Apr 8, 2025

Please sync the latest code from the develop branch, add the NGRAVE ZERO as follows

https://github.com/RabbyHub/Rabby/blob/develop/src/ui/views/NewUserImport/ImportList.tsx#L68

        {
          type: KEYRING_CLASS.HARDWARE.KEYSTONE,
          logo: WALLET_BRAND_CONTENT[WALLET_BRAND_TYPES.NGRAVEZERO].icon,
          brand: WALLET_BRAND_CONTENT[WALLET_BRAND_TYPES.NGRAVEZERO].brand,
        },

@heisenberg-2077 should I update it ? And also remove the isNgraveZero checks ?

@heisenberg-2077
Copy link
Contributor

Please sync the latest code from the develop branch, add the NGRAVE ZERO as follows
develop/src/ui/views/NewUserImport/ImportList.tsx#L68

        {
          type: KEYRING_CLASS.HARDWARE.KEYSTONE,
          logo: WALLET_BRAND_CONTENT[WALLET_BRAND_TYPES.NGRAVEZERO].icon,
          brand: WALLET_BRAND_CONTENT[WALLET_BRAND_TYPES.NGRAVEZERO].brand,
        },

@heisenberg-2077 should I update it ? And also remove the isNgraveZero checks ?

Yes, just add the wallet brand to the tipList array and test to see if it works as expected.

@caiquecruz
Copy link
Contributor Author

Please sync the latest code from the develop branch, add the NGRAVE ZERO as follows
develop/src/ui/views/NewUserImport/ImportList.tsx#L68

        {
          type: KEYRING_CLASS.HARDWARE.KEYSTONE,
          logo: WALLET_BRAND_CONTENT[WALLET_BRAND_TYPES.NGRAVEZERO].icon,
          brand: WALLET_BRAND_CONTENT[WALLET_BRAND_TYPES.NGRAVEZERO].brand,
        },

@heisenberg-2077 should I update it ? And also remove the isNgraveZero checks ?

Yes, just add the wallet brand to the tipList array and test to see if it works as expected.

@heisenberg-2077 worked perfectly, thank you! Also reverted the explicit checks and removed the unused i18n keys.

@vvvvvv1vvvvvv vvvvvv1vvvvvv merged commit 406d296 into RabbyHub:develop Apr 9, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

keep For keep PR not auto-close by stale action

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants