Skip to content

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