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 MultiAddress #128

Closed
wants to merge 1 commit into from

Conversation

vgeddes
Copy link
Contributor

@vgeddes vgeddes commented Jan 15, 2021

MultiAddress is the new default address type in Substrate master (See paritytech/substrate#7380).

Tested against the latest node-template from Substrate master using the following:

go test -v -timeout 30s -run ^TestChain_SubmitExtrinsic$ github.com/centrifuge/go-substrate-rpc-client/v2/teste2e

Leaving this PR in draft mode as a bunch of tests still need to be updated 🙂

@vgeddes vgeddes marked this pull request as draft January 15, 2021 17:01
@@ -25,7 +25,7 @@ type ExtrinsicSignatureV3 struct {
}

type ExtrinsicSignatureV4 struct {
Copy link
Contributor Author

@vgeddes vgeddes Jan 15, 2021

Choose a reason for hiding this comment

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

For consistency with polkadot-js/types, I've not added a ExtrinsicSignatureV5 and have just updated the V4 struct.

@@ -25,7 +25,7 @@ type ExtrinsicSignatureV3 struct {
}

type ExtrinsicSignatureV4 struct {
Signer Address
Copy link
Contributor

Choose a reason for hiding this comment

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

this will break for all the chains without the multiaddress.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, there are a couple of chains also using substrate master who for reasons of simplicity prefer not to use MultiAddress.

We could this solve this by making ExtrinsicSignatureV4.Signer an interface type:

type ExtrinsicSignatureV4 {
    Signer interface{}
    ...
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could abstract out the Signerwith Address and MultiAddress as default implementations. This wouldn't break the compatibility

@Gauthamastro
Copy link

Guys, we are working on ChainBridge, and this PR needs to be merged since this issue a blocker for us. Can you please merge the PR if it is ready?
@vedhavyas @vgeddes

@vgeddes
Copy link
Contributor Author

vgeddes commented Mar 8, 2021

Guys, we are working on ChainBridge, and this PR needs to be merged since this issue a blocker for us. Can you please merge the PR if it is ready?
@vedhavyas @vgeddes

Hey @Gauthamastro we're kinda using our own fork of GSRPC now as MultiAddress support is only one of about a half-dozen changes required to support Substrate 3.0 fully (at least for our specific needs):

https://github.com/Snowfork/go-substrate-rpc-client

We don't have an ETA for merging this work upstream, as it is lower priority for us right now, given our schedule.

@Gauthamastro
Copy link

Guys, we are working on ChainBridge, and this PR needs to be merged since this issue a blocker for us. Can you please merge the PR if it is ready?
@vedhavyas @vgeddes

Hey @Gauthamastro we're kinda using our own fork of GSRPC now as MultiAddress support is only one of about a half-dozen changes required to support Substrate 3.0 fully (at least for our specific needs):

https://github.com/Snowfork/go-substrate-rpc-client

We don't have an ETA for merging this work upstream, as it is lower priority for us right now, given our schedule.

Ohh okay thanks for the fork. We will use that

@vedhavyas
Copy link
Contributor

@vgeddes we started working on v3 changes. I have pulled your work on Multi address from this PR. Closing this in favor of that(#143).

@vedhavyas vedhavyas closed this Apr 7, 2021
@vgeddes
Copy link
Contributor Author

vgeddes commented Apr 7, 2021

@vedhavyas cool cool. There's a bunch of other V3 changes in our fork if you want to pick up those too: https://github.com/Snowfork/go-substrate-rpc-client. They also include RPC APIs for offchain storage.

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.

3 participants