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

Create API address derivation path type. #2174

Closed
4 tasks done
jonathanknowles opened this issue Sep 24, 2020 · 6 comments
Closed
4 tasks done

Create API address derivation path type. #2174

jonathanknowles opened this issue Sep 24, 2020 · 6 comments
Assignees

Comments

@jonathanknowles
Copy link
Member

jonathanknowles commented Sep 24, 2020

Context

We need to be able to represent address derivation paths in the API.

Decision

Create type ApiAddressDerivationPath that can encode an address derivation path.

A derivation path should be serialized to JSON as a list.

For example, the path 1852H/1815H/0H/0/0 should have the following JSON representation:

[
  {
    "derivation_index": 1852,
    "derivation_type": "hardened"
  },
  {
    "derivation_index": 1815,
    "derivation_type": "hardened"
  },
  {
    "derivation_index": 0,
    "derivation_type": "hardened"
  },
  {
    "derivation_index": 0,
    "derivation_type": "soft"
  },
  {
    "derivation_index": 0,
    "derivation_type": "soft"
  }
]

Each entry in the list should have the following pair of fields:

Field name Type Domain
derivation_index natural number [ 0 .. 2^31 − 1 ]
derivation_type enumeration [ hardened | soft ]

Acceptance Criteria

  • There must be an ApiAddressDerivation type.
  • Values of the ApiAddressDerivation type must be serializable to JSON and deserializable from JSON.
  • Deserialization of a previously serialized value must yield the original value.
  • The JSON representation of an ApiAddressDerivation value should be a list.

Development

QA

See #2177, which includes:

  • JSON roundtrip encoding/decoding tests for all types.
  • JSON golden tests for all types.
  • negative tests for the ApiRelativeAddressIndex type.
@paweljakubas
Copy link
Contributor

paweljakubas commented Sep 24, 2020

I wonder if first three paths, ie., are not superfluous 1852H/1815H/0H. the first two for sure if we are dealing with ada. Last one depicting account maybe also... On the other hand, it is only presentation layer so maybe the full information does not hurt.

Another thing : we want to use relative indexes (counting from 0) also for hardened index?

@rvl
Copy link
Contributor

rvl commented Sep 25, 2020

Was there anything wrong with just using a string to represent derivation path?
It's pretty simple to parse.

@jonathanknowles
Copy link
Member Author

Was there anything wrong with just using a string to represent derivation path?
It's pretty simple to parse.

Hi @rvl.

We could indeed have used a string, and you're right, it would not be very hard to parse.

The use of a JSON list was requested here: https://input-output-rnd.slack.com/archives/GBT05825V/p1600859749018000?thread_ts=1600843830.004300&cid=GBT05825V

@jonathanknowles
Copy link
Member Author

@paweljakubas

Another thing : we want to use relative indexes (counting from 0) also for hardened index?

Good point! I've updated the task description to specify that we will use relative indices, ranging from 0 to 2^31 - 1.

iohk-bors bot added a commit that referenced this issue Sep 25, 2020
2177: Create API address derivation path type. r=jonathanknowles a=jonathanknowles

# Issue Number

#2174 

# Overview

This PR adds a new type `ApiAddressDerivationPath`. A value of this type serializes to a JSON **list**.

For example, the path `1852H/1815H/0H/0/0` has the following JSON representation:

```json
[
  {
    "derivation_index": 1852,
    "derivation_type": "hardened"
  },
  {
    "derivation_index": 1815,
    "derivation_type": "hardened"
  },
  {
    "derivation_index": 0,
    "derivation_type": "hardened"
  },
  {
    "derivation_index": 0,
    "derivation_type": "soft"
  },
  {
    "derivation_index": 0,
    "derivation_type": "soft"
  }
]
```

Each entry in the list has the following pair of fields:

| Field name | Type | Domain |
| -- | -- | -- |
| `derivation_index` | natural number | `[ 0 .. 2^31 − 1 ]` |
| `derivation_type` | enumeration | `[ hardened \| soft ]` | 

# Details

This PR also:

- Adds JSON roundtrip tests for `ApiAddressDerivationPath`.
- Adds a missing JSON roundtrip test for the `ApiAddress` type.
- Fixes the upper bound of `addressIndex` to be `2^32 - 1` rather than `2^32`.
- Adds a matching swagger definition for `ApiAddressDerivationPath`.

Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>
@jonathanknowles jonathanknowles added this to Backlog in Adrestia via automation Sep 25, 2020
@jonathanknowles jonathanknowles moved this from Backlog to In Progress in Adrestia Sep 25, 2020
@rvl
Copy link
Contributor

rvl commented Sep 28, 2020

In that slack thread, a JSON list of strings was requested, not a list of objects. I fail to see how a list of objects like this will help the user.

@KtorZ KtorZ moved this from In Progress to QA in Adrestia Nov 18, 2020
@piotr-iohk
Copy link
Contributor

lgtm

Adrestia automation moved this from QA to Closed Nov 18, 2020
@CharlesMorgan-IOHK CharlesMorgan-IOHK removed this from Closed in Adrestia Jun 21, 2023
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

No branches or pull requests

4 participants