feat: add /accounts/:accountId/convert endpoint by Imod7 · Pull Request #1007 · paritytech/substrate-api-sidecar

Imod7 · 2022-08-03T06:47:37Z

Description

This endpoint will receive as an input an Account ID or a Public key (hex) that is given by the user as the path parameter and convert it into an SS58 address. The conversion will depend on the values of the 3 query parameters that are mentioned below.

This implementation was motivated by this comment while making changes in the validate endpoint.

From SS58 Address to Account ID also ?
Does this endpoint also implements the reverse conversion, meaning from SS58 Address to Account ID ? No because this functionality is already covered by the /accounts/:address/validate endpoint after this PR was merged.

Query Parameters

This endpoint can take the following 3 query parameters :

scheme
prefix
publicKey

Possible values

The possible values for these parameters are the following :

scheme can take one of those 3 values {sr25519, ed25519, ecdsa}
prefix can be one of the prefices found here
publicKey is a boolean value and can be true or false

Default values

The default values for these query params are the following :

scheme = sr25519
prefix = 42
publicKey = true

Examples

So, for example if the query parameters that are given are the following :
- prefix=2
- scheme=ed25519
- publicKey=false
then that means that the endpoint will give as an output :
- a Kusama formatted SS58 address (based on the ss58-registry the prefix=2 corresponds to the Kusama network)
- based on the ed25519 algorithm (default value)
- assuming that the given path parameter is an Account ID (and not a Public Key (hex))
If no query parameters are specified then the endpoint will take their default values (as defined above) and will output :
- a Substrate formatted SS58 address (default value of prefix = 42 which corresponds to substrate)
- based on the sr25519 scheme (default value) and
- assuming that the given path parameter is a Public Key (hex)

Special Case / Use of query param `publicKey`

In most of the cases we can output a valid SS58 address based only on the scheme and prefix since Public Key (hex) is identical to the Account ID (so specifying the query param publicKey does not make any difference).

However, there is a specific case for which we also need to know if the path parameter that is given is :

a Public Key (hex) or
an Account ID

hence we need the query parameter publicKey to be specified as true or false.

This case is when we have scheme=ecdsa.
When we have ecdsa as scheme then Public key is different than the Account ID so we first need to hash the public key with blake2 and then get the SS58 address from that.

*** This is also explained in this substrate PR (subkey: display SS58 encoding of public key)

Sample Response

Sample request :
/accounts/0x002b5bab9258f5aff3117fce47854ef4c29c0eeae3faf855a2d6a9b67861c598/validate?prefix=0

Sample response :

{
  "ss58Prefix": "42",
  "network": "substrate",
  "address": "5C4vjiCiAiVYcx5fZ9g9FftViRqRwFRNpSDVrh6dzE35x2d8",
  "accountId": "0x002b5bab9258f5aff3117fce47854ef4c29c0eeae3faf855a2d6a9b67861c598",
  "scheme": "sr25519",
  "publicKey": true
}

chains-config

It was also added in chains-config -> Controllers of :

Polkadot
Kusama
Westend

Todos

Double check if the conversion (from accountId to address) that happens in the accountConvert method is correct
Make use of the query param public (because currently is not used)
Update Docs
Add Tests

- Controller, service, response type

jsdw · 2022-08-03T16:33:46Z

Dumb question probably (I read the comments this came from and didn't quite understand); what does the public query param do? Does it apply to every scheme?

- Using the query param `publicKey` to output the SS58 address if the input/parameter that is given by the user is a Public Key (hex) and not an accountID.

Imod7 · 2022-08-16T06:02:12Z

Dumb question probably (I read the comments this came from and didn't quite understand); what does the public query param do? Does it apply to every scheme?

@jsdw Thank you so much for your question and it is a quite logical question to have since when I created this PR I was actually not even using the query param public (now renamed to publicKey). After a clarification from @IkerAlus it was also clear to me the purpose of this parameter and now I added some code that actually uses it.
Since this is one very important point/specific case of the implementation of this endpoint, I tried to explain it in the Description of the PR under the section "Special Case / Use of query param publicKey". I hope this explanation and the substrate PR (subkey: display SS58 encoding of public key) that I cite answers your question.

- Added specific types that the `RequestHandler` can accept. - Cleaned the code that validates the query params (and sets default values) in the Controller. - Added tests with different valid or invalid endpoints to check.

- Updated the docs with the `convert` endpoint functionality & path & query params.

Imod7 · 2022-08-24T11:26:20Z

I added the tests + docs in this PR.

In the docs I populated some "keys" at my own discretion since I am not sure if we follow a pattern/rule to populate those. These are the following :
- the schema at the "200" response
```
schema:
   $ref: '#/components/schemas/AccountConvert'
```
- the operationId
- Added 3 responses codes (200, 400, 404)
Another point is that in src/chains-configI only added the AccountConvert controller in Polkadot, Kusama and Westend. Should I add it in all the chains ?

TarikGul · 2022-08-25T13:45:25Z

Nice job, sounds good.

Another point is that in src/chains-configI only added the AccountConvert controller in Polkadot, Kusama and Westend. Should I add it in all the chains ?

No need to add it to the others, they can always push up a PR if they would like it.

TarikGul

Overall very solid job, just some nits, and a few questions above.

- Keeping the "AccountId" written with the same format. - Other minor changes in formatting.

TarikGul

Great Job, LGTM 👍 Just need to resolve the merge conflict

TarikGul

New commits look great! Lets get this merged in soon.

jsdw · 2022-09-21T09:39:15Z

+			!(
+				cryptoScheme === 'ed25519' ||
+				cryptoScheme === 'sr25519' ||
+				cryptoScheme === 'ecdsa'
+			)


Totally happy with this!

When there are lots of conditions you can also use an approach like if (!['ed25519', 'sr25519', 'ecdsa'].includes(cryptoScheme)) { ... }, but your way is more efficient :)

jsdw · 2022-09-21T09:46:10Z

+		let network = null;
+		for (const networkParams of allNetworks) {
+			if (networkParams['prefix'] === ss58Prefix) {
+				network = networkParams['network'];
+				break;
+			}
+		}


Assuming allNetworks is an array, you can also do:

let network = allNetworks.find(n => n['prefix'] === ss58Prefix); if (!network) { throw new BadRequest(...) }

Not a big deal though :)

jsdw

lgtm, nice work!

Added the convert endpoint

7a9e165

- Controller, service, response type

Imod7 requested a review from a team August 3, 2022 06:47

Removed getPair

0d600b0

TarikGul reviewed Aug 4, 2022

View reviewed changes

Comment thread src/controllers/accounts/AccountsConvertController.ts Outdated

TarikGul reviewed Aug 4, 2022

View reviewed changes

Comment thread src/controllers/accounts/AccountsConvertController.ts Outdated

SS58 Address from Public key (hex)

cd54947

- Using the query param `publicKey` to output the SS58 address if the input/parameter that is given by the user is a Public Key (hex) and not an accountID.

Imod7 added 2 commits August 24, 2022 07:19

Changes in validation code & Adding tests

5df7378

- Added specific types that the `RequestHandler` can accept. - Cleaned the code that validates the query params (and sets default values) in the Controller. - Added tests with different valid or invalid endpoints to check.

Updated the Docs

72755fc

- Updated the docs with the `convert` endpoint functionality & path & query params.

Imod7 requested a review from TarikGul August 24, 2022 11:16

TarikGul assigned Imod7 Aug 30, 2022