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

add support for common good parachains #130

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ShankarWarang
Copy link

@ShankarWarang ShankarWarang commented Aug 23, 2024

Added common good chains:

  • Polkadot AssetHub
  • Polkadot BridgeHub
  • Polkadot Collectives
  • Polkadot Coretime
  • Polkadot People
  • Kusama AssetHub
  • Kusama BridgeHub
  • Kusama Coretime
  • Encointer Network
  • Kusama People

🔗 zboto Link

brenzi added a commit to brenzi/ledger-substrate-js that referenced this pull request Sep 5, 2024
@emmanuelm41
Copy link
Member

Hi @ShankarWarang! Just to confirm, these chains are not levering the new feature in polkadot for the generic, right? Besides, can you please share with us sources you used to set slip0044 and the ss85_addr_type?

@ShankarWarang
Copy link
Author

Hi @ShankarWarang! Just to confirm, these chains are not levering the new feature in polkadot for the generic, right? Besides, can you please share with us sources you used to set slip0044 and the ss85_addr_type?

Gm @emmanuelm41 👋
As far as I know, all above common good chains have implemented the necessary changes required for the generic polkadot app, thanks to @bkchr.
Reference: polkadot-fellows/runtimes#337

These common-good parachains share the same slip0044 (same native token) and the ss85_addr_type as their corresponding relaychains (Polkadot & Kusama). I guess this might cause conflict as per the current implementation?

@emmanuelm41
Copy link
Member

Gm @ShankarWarang. If they have implemented the feature for the generic polkadot app, then there is no need to add them as done on this PR. What you did is meant for apps that are chain-specific, as before the CheckMetadataHash feature exists. We could merge it anyway, but it is not really necessary.

Supported_apps is just used here...

export function newSubstrateApp(transport: Transport, chainName: string) {
const requestedApp = supportedApps.find((app: SubstrateAppParams) => {
return app.name.toLowerCase() === chainName.toLowerCase()
})
if (requestedApp != null) {
return new SubstrateApp(transport, requestedApp.cla, requestedApp.slip0044)
}
throw new Error(`Error: ${chainName} not supported`)
}

While this is the class for the generic app...

constructor(transport: Transport, txMetadataChainId?: string, txMetadataSrvUrl?: string) {
super(transport, PolkadotGenericApp._params)
this.txMetadataSrvUrl = txMetadataSrvUrl
this.txMetadataChainId = txMetadataChainId
if (!this.transport) {
throw new Error('Transport has not been defined')
}
}

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.

2 participants