HydraDX -> Hydration rebrand

[jak-pan]

Companion polkadot-js/apps#10669

[jak-pan]

What is the key in the map taken from? Is it rpc.chain? If so should we add new chain to support the transition when we change this?

[TarikGul]

Test needs to be resolved. I am confused about your question can you link it to a specific line in the code.

[jak-pan]

I am talking all of the lines including hydradx as a key to something

common/packages/hw-ledger/src/defaults.ts

Line 21 in 37fa211

hydradx: 'HydraDX',

common/packages/networks/src/defaults/ledger.ts

Line 25 in 37fa211

hydradx: 0x00000162,

common/packages/networks/src/defaults/genesis.ts

Line 68 in 37fa211

hydradx: [

Is this all mapped first from the genesis hash to the hydradx key and then used throughout other apps? I know that Polkadot Vault uses //hydradx derivation path by default when creating a new wallet.

I was wondering if it would be better to keep the hydradx as well as hydration for some transition period but then I don't know how all the consumers would handle the conflicts.

Can you advise the best route forward here? I would like to get at least display names correct with breaking the least possible amount of things.

[jak-pan]

Test needs to be resolved.

What do you suggest here? It seems that there is a legacy ledger-substrate app which was never released for HydraDX but somehow the @polkadot/dev-test is looking for deps there in some cryptyc way.

[TarikGul]

Hey sorry for the delay, things have been hectic and finally cooled down.

In terms of the new LedgerGeneric wrapper that support the new Polkadot app the naming of your chain only has the affect to see if that chain is supported or not. It checks to see if ledgerApps has the chain name -

common/packages/hw-ledger/src/defaults.ts

Line 6 in 7ef452b

export const ledgerApps: Record<string, string> = {

That being said - For the old Legacy Ledger wrapper it did the same check via the chain name, but also connected to the ledger app via the chain name. This could cause potential issues for older accounts trying to use the legacy app.

common/packages/hw-ledger/src/Ledger.ts

Line 134 in 7ef452b

this.#app = newSubstrateApp(transport as any, this.#ledgerName);

Is this all mapped first from the genesis hash to the hydradx key and then used throughout other apps

For the extension - yes. For other apps I don't know.

[TarikGul]

Can you advise the best route forward here? I would like to get at least display names correct with breaking the least possible amount of things.

Question: What is the chain name that the hydra is now returning?

I think on a branding sense in apps, we can totally change it to the new brand. I don't think that will have much affect. Most of the internals in apps, and the extension especially with ledger will use what is received by the chain or what is within these config files to confirm the integrity on connection.

[TarikGul]

For the test, i believe you can adjust the naming here:

https://github.com/paritytech/ss58-registry/blob/f54e1ea030a48404a20dca48db3dd5f671ba2cc1/ss58-registry.json#L545-L551

It's a little annoying because it's another repo so apologies, but that will fix the tests.

I would also test this by adjusting the values in the node_modules for the @substrate/ss58-registry package to ensure there are no mistakes and the tests pass before getting the PR merged in the other repo.

[jak-pan]

https://github.com/paritytech/ss58-registry/blob/f54e1ea030a48404a20dca48db3dd5f671ba2cc1/ss58-registry.json#L545-L551

Maybe I should rename display name here and duplicate this entry with "hydration" network key to be sure we don't break something?

[TarikGul]

Maybe I should rename display name here and duplicate this entry with "hydration" network key to be sure we don't break something?

Makes sense to me! Just to be clear as well, the naming in the default files that contain info like the ledger slip44 or the genesis should reflect the chain name that is being returned by the chain itself.

Little tip as well: Also if you would like to actually see what the effects would look like in apps or the extension etc - you can run yarn polkadot-dev-copy-to <repo_name> Just make sure the other repo shares the same parent directory.

[TarikGul]

Also no need to adjust the ss58 json file in /test for this PR, once we have the ss58-registry package updated this repo will automatically update that file to reflect the changes.

[jak-pan]

@TarikGul can you please rerun the tests? The ss58 has been updated

[TarikGul]

@jak-pan Thanks for pinging me, in order for the tests to pass you will need to pull in master. I updated the ss58-registry for you.

[jak-pan]

@jak-pan Thanks for pinging me, in order for the tests to pass you will need to pull in master. I updated the ss58-registry for you.

Looks like it still failed but https://github.com/paritytech/ss58-registry/blob/0e88f87b185cdee829f624b88d3cfc47eaaf2161/ss58-registry.json#L547 this is updated in master

[TarikGul]

@jak-pan Thanks for pinging me, in order for the tests to pass you will need to pull in master. I updated the ss58-registry for you.

Looks like it still failed but https://github.com/paritytech/ss58-registry/blob/0e88f87b185cdee829f624b88d3cfc47eaaf2161/ss58-registry.json#L547 this is updated in master

Yea looks like we just need to patch that test since it is resolving the the value of the key hydradx: 'hydraDX' for ledger supported apps, but the value is actually 'hydration', just one moment, and I'll post a code snippet that you can add to the test that should solve this

[TarikGul]

We should actually revert this to HydraDX, and write a comment above it saying this has been rebranded to Hydration but because Ledger Supported Apps still has HydraDx as the name, the value will remain as HydraDX. I think this is okay because the following has been updated in the ss58-registry with the correct naming.

[jak-pan]

Our Ledger app was never released. Or is this something else?

[jak-pan]

If this is anything released it should be also renamed in ledger.

[TarikGul]

https://github.com/Zondax/ledger-substrate-js/blob/main/src/supported_apps.ts#L204-L207

[TarikGul]

Honestly I don't know how much that actually matters for the generic app, I would assume nothing at all actually.

[TarikGul]

But if you want to cross all your T's and dot all your i's it might be worth updating it in the ledger app as well. In the meantime I think it would be totally okay to write an exception in the test that caters to "Hydration"

[jak-pan]

Yeah ok this looks like the support repo for the old "app" that was never released. I think this can safely be ignored. I'll update the test to pass.

[TarikGul]

@jak-pan Comment above which I think should solve this!

[jak-pan]

@jak-pan Comment above which I think should solve this!

Worked :)

[TarikGul]

Okay so now that the ss58-registry is updated - we can move focus onto Apps now.

[polkadot-js-bot]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.