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 jupiter testnet #4334

Merged
merged 2 commits into from
Jan 8, 2021
Merged

add jupiter testnet #4334

merged 2 commits into from
Jan 8, 2021

Conversation

zzcwoshizz
Copy link
Contributor

hi there,

this PR is to add jupiter testnet.

greetings!

@zzcwoshizz zzcwoshizz marked this pull request as draft January 7, 2021 12:06
@zzcwoshizz zzcwoshizz marked this pull request as ready for review January 7, 2021 12:08
/* eslint-disable sort-keys */

export default {
LookupSource: 'MultiAddress'
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we also have Address: 'MultiAddress?

Copy link
Contributor

@atenjin atenjin Jan 8, 2021

Choose a reason for hiding this comment

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

Hi @jacogr
We define for type Address is same to node-template define(pub type Address = sp_runtime::MultiAddress<AccountId, ()>;). Thus there are two points related to this:

  1. node-template have not changed the define of

    types: {
      Address: 'AccountId',
      LookupSource: 'AccountId'
    }

    We think this should be changed after the pr More Extensible Multiaddress Format paritytech/substrate#7380 in substrate,

    using:

    {
      "LookupSource": "MultiAddress",
      "Address": "LookupSource"
    }

    We may create a pr to fix this if you will, or you may have some your own considerations.

  2. In our chain, using polkadot.js would use default config, thus, in node config, the default already has:

    "Address": "LookupSource"

    thus we just override

    LookupSource: 'MultiAddress'

    is ok. But you are right, override both LookupSource and Address may be better, we will change later, thanks!

add ```Address: 'MultiAddress'``` types.
@zzcwoshizz
Copy link
Contributor Author

Hi, @jacogr .

I have added the Address: 'MultiAddress'types.

Copy link
Member

@jacogr jacogr left a comment

Choose a reason for hiding this comment

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

Thank you

@jacogr jacogr merged commit 8cdcd91 into polkadot-js:master Jan 8, 2021
@polkadot-js-bot
Copy link

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.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Jun 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants