Support batch import of addresses through the API

[KtorZ]

Context

Exchanges typically use our wallet as a giant bucket of addresses, using a single account containing many addresses. Usually, they keep a mapping of 1 address per user, such that, they can advertise a "deposit address" to their users.
This means that, when they create a new random address from the API, and show it to their users, this address should remain available and known until used. If they were to restart their wallet with an empty DB, they'll loose all unused address and there's no guarantee they'll ever generate those again.

Decision

In order to allow exchange to migrate from versions prior to 1.4 to 1.4+ using the new data-layer, we need to provide them with an extra endpoint to give them the ability to import addresses into an existing wallet / account.

Acceptance Criterias

1. The API must support an extra endpoint to perform batch import of addresses
2. The API handler mustn't fail if one or several addresses can't be imported (but rather, go through with the rest and report the failures back, if any).
3. The API handler mustn't override existing addresses in the DB
4. The API may disallow batch import during a restoration (unless we can correctly manage the fact that addresses may be imported prior to being discovered during restoration)
5. The API may define a hard-limit for the batch's size

---

Development Plan

• I intend to add an extra endpoint POST /api/v1/wallets/{wid}/accounts/{accix}/addresses to define batch import
• I intend to respond with a number of success and, a list of addresses that has failed to import (if any)
• I intend to implement the logic "naively", using existing handles to create one address and iterate over each address in the batch to add them one by one.
• I intend to add integration tests to verify various scenarios of address import (used address, unused address, during restoration...)

PR

Number	Base
#259	develop
#268	develop
input-output-hk/cardano-sl#4041	release/2.0.2
input-output-hk/cardano-sl#4040	develop
#286	develop
#308	develop
#309	develop
#319	develop
#320	develop
input-output-hk/cardano-sl#4069	develop
input-output-hk/cardano-sl#4070	release/2.0.2
#327	develop
input-output-hk/cardano-sl#4084	develop

QA

Note, this has also been backported to develop

Criteria	Coverage
1.	A new endpoint has landed in the API here
2.	The response type for the endpoint makes it obvious, and its behavior is illustrated here where, failures and successes are part of the response
3.	This is a DB invariant that holds because the implementation re-use existing DB functions to create addresses. Also, this is illustrated by the integration scenario here
4.	Instrumenting automated testing on that is hard, and we here rely on the fact that we re-use available functions to talk to the DB so in theory, it shouldn't matter much. The restoration code is already capable of handling collisions (since addresses may be "discovered" multiple times during restoration). So having an address already in the DB shouldn't matter. Ideally, that's something we ought to test manually (but haven't so far)
5.	This endpoint is likely going to be used once in order to help exchanges to migrate to newer versions. Investing time and efforts in optimizing this is not a priority. Hence, this may "break" or become horribly slow if an exchange tries to import 10K addresses at a time. I've added a small disclaimer alongside a little walkthrough in the API documentation (under the "Common Use-Cases" section)

---

Retrospective

[piotr-iohk]

LGTM.

With slight exception of point 4. If I'm not mistaken that could be fairly easy tested using https://github.com/input-output-hk/cardano-wallet/wiki/Building-Running-and-Testing-a-wallet-node

1. start node connected to mainnet
2. start restoration of a wallet that existed on mainnet
3. while it is restoring perform the batch import of addresses

Also It is not clear to me whether we disallow batch import during restoration or not. (Acceptance criteria says may... unless) Point 4 in QA section rather implies we allow it (?)

[KtorZ]

You're 100% right. The point 4 is a note because, I am not actually sure of what will happen if we do import some addresses during restoration (especially some used ones). I shouldn't matter and I can't see anything obviously wrong with that but you know, testing it is always better.

I suggest we do it before closing the ticket 👍

Would you, or would you rather leave that to me?

[piotr-iohk]

I'll go for it. Sure.

[piotr-iohk]

I did some manual testing and documented scenarios under #308.

[KtorZ]

@piotr-iohk Last PR about displaying a more meaningful error message has been backported to develop. So I think we're good to go with this ticket 👍

[piotr-iohk]

lgtm 👍