[Taler] wallet exchange management

[Florian Dold]

CCing the mailing list, as Christian suggested others might be
interested in this discussion as well.

On 4/8/20 3:29 PM, Christian Grothoff wrote:
> On 4/8/20 10:56 AM, Florian Dold wrote:
>> let's discuss and design the exchange management API of the wallet
>> before I implement it in wallet-core.  The complexity of this is IMHO
>> underappreciated ;-)
> 
>> We used to have this concept of "trusting" an exchange, but I'm foggy on
>> the exact details, and we never documented them anywhere, AFAIK.  We now
>> also have additional concepts, like "ToS accepted".
> 
> ToS accepted is orthogonal.

True.

>> Thus we should first agree on the different states of an exchange in the
>> wallet and what implication they have.
>>
>> Here's the lifecycle of an exchange (that isn't already known to the
>> wallet):
>>
>> * The user enters the base URL of the exchange in some selection dialog.
> [or we may get it from a bank interaction]

Yes.

>> * The wallet-core creates a DB entry with the exchange record and
>> proceeds to download /keys and /wire.  Signatures on denominations are
>> verified in the background, this is why we create a DB entry even when
>> we won't use the exchange.
>>
>> I guess when adding a new exchange in the "exchange list editing
>> dialog", when we enter the URL the exchange info will be automatically
>> fetched (maybe even while typing), but when the user clicks "Add", the
>> exchange will be marked as "added".  Maybe "enabled" is a better word.
>> Exchanges that are not enabled be GC-ed automatically.
> 
> We may want to enable [add] only after at least /keys was downloaded and
> included a compatible version. We should probably show something like
> "checking compatibility" OR some progress spinner while the 'add' button
> is disabled and the download is ongoing. Actually validating all the
> signatures in /keys and grabbing /wire can likely wait. Upon
> incompatibilty, I'd replace the "checking compatibility" with
> "incompatible exchange" or "wrong URL" depending on the result of the
> download.

Okay, so to translate this from a UI point-of-view into an API
point-of-view:

As an optimization, the "queryExchangeInfo" API should be able to return
information about the exchange that is not 100% complete (regarding
verified denominations and wire info).  So the request would be

interface QueryExchangeInfoRequest {
  // If given, return error description if the exchange is
  // not compatible with this withdrawal operation.
  talerWithdrawUri?: string;

  // Exchange base URL to use for the query.
  exchangeBaseUrl: string;

  // If true, the query already returns a result even if
  // /wire and denomination signatures weren't processed yet
  partial: boolean;
}

Maybe Torsten can give some input about what kind of interface he would
prefer here?

>> * This new exchange is now considered "enabled" and "untrusted".  The
>> exchange might be "partially trusted" if some denominations are audited
>> by an auditor the wallet already trusts.
>>
>> Question here is:  How is this trust information modeled in the API?
>> It's a bit difficult, since there might be multiple auditors trusting
>> different subsets of denominations.
> 
> If any denomination is audited by one or more of our trusted auditors,
> I'd consider the exchange "usable".

Sure, but I guess we do need to show these details in the UI somehow, at
least in the details view?

But yes, exposing some "usable" value in the queryExchangeInfo API would
be useful.  Basically the UI can only proceed once the exchange is
usable according to the wallet-core.

>> If a withdrawal from an untrusted exchange is accepted, we *used to say*
>> that the exchange should be marked trusted ... right? 
> 
> Well, I think we used to say that we refuse to proceed *unless* the user
> checks a box to "trust this exchange without auditor".  If applicable,
> we should at this point also showing the auditor(s) and allow the user
> to "trust this auditor for $CURRENCY" instead.

Ah, so we indeed need information about the auditors in the
queryExchangeInfo request.  So a list of:

interface ExchangeAuditingState {
  auditorName: string;
  auditorBaseUrl: string;
  auditorPub: string;
  // Is the auditor already trusted by the wallet?
  trustedByWallet: boolean;
  // Does the auditor audit some reasonable set of
  // denominations of the exchange?
  // If this is false, at least some warning should be shown.
  auditedDenominationsReasonable: boolean;
}

Does this include everything we need for the UI concerning the auditing
state?  Of course this will be part of the whole response:

interface QueryExchangeInfoResponse {
  auditingState: ExchangeAuditingState;

  // Does the wallet trust the exchange directly?
  trustedDirectly: boolean;

  [... more fields ...]
}

Here's an interesting error case:  What if the exchange and wallet
disagree about the auditorBaseUrl and/or auditorName for the same
auditorPub?  Will this inconsistency be reported?  Will we just use the
auditor public key and ignore the rest?

Saying "please trust this auditor for $CURRENCY" might be offered in the
UI, but API-wise it's IMHO a different concern.  Upon clicking it, the
wallet would do some addTrustedAuditor request to wallet-core, followed
by another queryExchangeInfo.  Right?

>> But what happens
>> with partially trusted exchanges?
> 
> There is no such thing. The exchanges are not partially trusted, they
> are untrusted. The *auditor* those exchanges have results in us trusting
> (some) denominations of that exchange. The end.

Partially is the wrong word I guess.

> 
>> If we also mark them as trusted, it
>> means the next time we withdraw, we would also use unaudited
>> denominations, which of course isn't right.  So IMHO the exchange should
>> stay untrusted. 
> 
> Exactly.
> 
>> But as long as we have coins with the exchange, it can
>> be marked as "active".
> 
> Not sure what "active" is supposed to imply. "default" (per currency)
> would make more sense as a term for me.

By "active" I mean that this exchange should show up in the list of
choices when doing a withdrawal.  Otherwise, every exchange the wallet
ever used will be in this list.

>> * Now the user has withdrawn money from the exchange and maybe spent
>> all/some of it.  They decide they want to switch exchanges.  Thus they
>> set the exchange to "disabled".
> 
> Nah. During withdrawal, they MAY click on the "change exchange" button,
> pick another exchange (from list of all known exchanges or URL entry or
> QR code) and then that one becomes the "default" (and the existing
> "default" is of course lost).

Ah "default exchange", again something else that we are implicitly
mentioning, but that's not part of any API yet.

Is this really some mapping from "currency" -> "default exchange" or is
it more like "the last *compatible* exchange that was last used for
withdrawal of this currency"?

Otherwise, the exchange currently set as the "default exchange" might be
incompatible with the bank we're using.

>> So the "state information" about the exchange contains (along the boring
>> stuff)
>>  * disabled/enabled            
> 
> => for me this is default or not default

See above, I don't agree with your idea of "default" as a property of an
exchange.

But I can live with saying that we only show exchanges that are either
trusted or have been used for a withdrawal before.

Otherwise, the wallet would end up showing exchanges that we've only
queried and looked at, but never done anything else with.

>>  * directly trusted/untrusted  => 
> 
> => only trusted (without auditor) or untrusted (auditor needed)

Okay.

> 
>>  * auditor-trusted
> 
> => Not an exchange state. Per denomination.

But how does the UI care about this?  All I'm interested in for the UI
is whether the exchange is properly audited by an auditor that I trust,
or by one that I might be willing to add as trusted!

>>  * active/inactive
> 
> => Not an (explicit) exchange state.
>    Maybe implied: have/do not have coins of this exchange.

Yes, many of these properties can be computed in various ways, it
doesn't literally have to be some column in the DB ;-)

> 
>>  * ToS accepted (and which version)
> 
> => where version == ETag.

Yep.

>> When querying the exchange list for a particular withdrawal operation,
>> we also need:
>>  * bank-recommended
> 
> I'd only add 'bank-recommendations' automatically to our list of 'known
> exchanges' (untrusted), and if it is the ONLY one for that currency at
> the time, we MAY decide (UX question, but also a business and an ethical
> question!) that it gets the 'default' (then we can/would/may skip the
> 'exchange selection' dialog on withdraw).

Well, we still might want to have some symbol or short word saying that
this exchange was suggested by the bank.  Of course we can choose to
emphasize this or not.  But as a user, I would be confused if some
exchange that I don't know pops out of nowhere.

>>  * wire-compatible (so the user doesn't wonder "WTF doesn't my favorite
>> exchange that I configured show up in this list?!")
> 
> Interesting. Not sure this applies in any currency-domain in practice,
> but maybe a theory-issue to deal with. Still, then we could just list it
> as "incompatible", same as when an exchange is in our list but
> (eventually) is known to run an incompatible protocol version. So add
> 
> * incompatible (version or wire)

Yes, that's how we can deal with it in the UI.

In the wallet-core API, I still want to keep them separate, for example
to distinguish these things in our integration tests for error handling.

> I think that's overcomplicating it. I'd simply say that you cannot
> remove an exchange from the list unless either
> 
> 1) you have no coins from that exchange, or
> 2) you are in developer mode AND confirm (in warning dialog)
>    that you are happy to loose all the coins (value shown!)
>    you own from that exchange.

Okay, I'm fine with that.

> The exchange list for me _also_ implies that we are downloading /keys
> from those providers, and we must do so for any exchange for which we
> own coins. A user deleting an exchange from the list may have the
> expectation that we stop communicating with that exchange, which would
> be false if we still have coins.

Yeah.

> "Pending" or "Offline" as in: we don't have a current /keys (or /wire)
> reply from the exchange. This can be because we didn't yet get to
> download it ("pending"), or because our last attempt definitively failed
> (timeout/4xx/5xx-error on request, we have no network). Here we might
> want to show a detailed reason ("download pending", "500 internal server
> error", "bad request", "DNS lookup failed", "no Internet connectivity").

Makes sense, we already store this information in the wallet DB anyway.

> 
> Additionally, we may want something to show while we do our local
> signature checks. While we can determine that the exchange's
> denominations have a "reasonable fee structure" and are audited by a
> trusted auditor for that currency without much of a delay, checking ALL
> the signatures on the /keys may take some additional time. Here it may
> make sense to prioritize doing this for the exchange the user has
> currently selected (imagine a list of 100 exchanges and thus potentially
> 10k+ signature verifications...), and so then we need another state
> 
> * signatures (checked | invalid | pending)
> 
> which again would be shown as "signature check pending" (or
> theoretically "signatures invalid") while the "confirm" button is
> disabled (confirm would confirm/select the "default" exchange, so only
> the signature check on the prioritized/selected default matters).  Once
> the signatures have been checked, they of course _stay_ checked until
> the next /keys download, *and* we'd not show any message but simply
> enable the "confirm" button.

Agreed.


> Happy hacking!

I think we need some more spec-ing and designing before the real hacking
on the code can begin ;-)

We've discussed the full query on the exchange now (with some open
points).  However, there's still the "list" functionality.  The request
probably looks like this:

interface ExchangeListRequest {
  // Used to decide compatiblity
  talerWithdrawUri: string;
}

interface ExchangeListRespose {
  exchanges: {
    exchangeBaseUrl: string;
    // Incompatible exchanges are also returned,
    // as otherwise users might wonder why their expected
    // exchange is not there.
    compatibility: "compatible" |
      "incompatible-version" | "incompatible-wire";
    trustedDirectly: string;
    // Do we have electronic cash with this exchange?
    inUse: boolean;
    recommendedByBank: boolean;
    // This information is only returned if it's
    // already available to us, as the list query
    // must be fast!
    trustedViaAuditor: boolean | undefined;
    // The "reasonable-ness" of the exchange's fees, in context
    // of the withdrawn amount.
    // Only provided if available (if we've already queried
    // and checked this exchange before).
    feeStructureSummary: FeeStructureSummary | undefined;
  }[];
}

I do not expect the UI to use all of these, but the goal is to provide
something that we can use as the basis for iterating on the UI design
(and again, that has enough information to be useful for tests).

Also, the FeeStructureSummary is still totally unspecified.

- Florian