Re: [Taler] feedback requested: 002-wallet-exchange-management

[Florian Dold]

On 4/13/20 10:42 PM, Torsten Grote wrote:
> On 2020-04-09 06:16, Florian Dold wrote:
>> Please have a look: 
>> https://docs.taler.net/design-documents/002-wallet-exchange-management.html
> 
> Thanks a lot for writing this up. It helps a lot to have a document
> summarizing the outcome of a long email thread.
> 
>> An exchange might only be known the wallet temporarily.
> 
> typo?

Yep, thanks!

> 
> Aprt from that I am not sure we need temporary exchanges and thus
> garbage collection at all.
> 
>> During the withdrawal process, the bank can also suggest an exchange.
>> Unless the exchange is already known to the wallet, this exchange
>> will be added non-permanently to the wallet.
> 
> Could we only add the exchange if the user actually selects it? Or do we
> need to query it first to offer it as an option in exchange selection?

It could be that the exchange that the bank recommends is totally bogus,
e.g. for some different currency.  But I agree that we don't necessarily
need to query it to show it in the list, we can show an error later if
it turns out to be unsuitable.

> Or could we also list it, but display it differently and only query it
> once the user decided to try what the bank suggested?

Yes, that would be a good option.

>> If the user reviews a new exchange during withdrawal but then does
>> not decide to use it, will this exchange be permanent?
> 
> I'd say yes.

Hmm.  I really don't like the idea of being able to add some information
that's prominently visible in a way that can't be deleted.

>> interface QueryExchangeInfoRequest {
>>   ...
>>   // If true, the query already returns a result even if
>>   // /wire and denomination signatures weren't processed yet
>>   partial: boolean;
> 
> If think I had said this somewhere else before:
> Ideally, we can work without partial results.

I'm not sure, and that's why I included the option to do a partial query
;-)  Depending on the hardware of the user and the number of
denomination that the exchange offers, it might have noticeable latency
to verify all signatures we need to verify before the user can accept
the withdrawal operation.

If a client of wallet-core (e.g. the Android UI) doesn't need the faster
partial information, it can always pass this in the query.

> 
> What happens if a QueryExchangeInfoRequest fails, because the user made
> a typo in the baseUrl for example?
> 
>> interface QueryExchangeInfoResponse {
>>   ...
>>   currentTosVersion: string;
>>   acceptedTosVersion: string;
> 
> What's the reasoning here? Could we not simply have an tosAccepted
> boolean and if it is false, we need to accept?

Some exchange management view might want to allow the user to view the
accepted ToS and then the current ToS, and maybe even show a diff!  For
that we need to know these details.

But actually the acceptedTosVersion should be "string | undefined"!

> Depending on the outcome of the balance discussion, we might want to
> include the current balance hold by an exchange in the list response.
> 
> Are base URLs normalized somehow? I guess these would all be different
> exchanges?
> 
> * http://exchange.example.org
> * https://exchange.example.org
> * https://exchange.example.org/
> * https://exchange.example.org/#foo

We are doing normalization (and it *used* to be documented, but I have
to admit I can't currently find any documentation about it other than
the wallet's source code).

In the above list, the last three URLs would be considered the same
exchange.

I shall document this.

> Regarding manually setting trust, I think this is something to be
> avoided if at all possible as most users are not in a position to
> properly judge if an exchange can be trusted or not. It only opens a up
> a big attack vector.

Interesting point.  I guess by "trust", we mean "don't show a big
warning the next time this exchange is selected for withdrawal".  Maybe
trust is not the right word?  Any better suggestion?

If I "untrust" an exchange, it means I don't want it to be selected by
default (even if it's the last exchange I used), and that I have to
check some box again that I trust this exchange the next time I use it
for withdrawal.

I think directly trusted exchanges should be the exceptional case, but
if we don't allow it, other deployments (university, festival, ...) of
Taler would always be 2nd class citizens with this wallet.

> I saw public keys included in some responses. What's the use-case for
> the wallet here or could we leave those out? In general, there seems to
> be a tendency in the APIs I've seen to include as much information as
> possible. I'd do it the other way around and add as little as possible,
> only what is expected to be needed in the UI layer and add more later,
> if needed.

It definitely needs to be displayed as a way to verify that this is
really the exchange the user wants to use, in case they enter it
manually via an URL.

> 
>> SetExchangeTosAccepted
> 
> We should probably add a version field here to clarify which version of
> the ToS the user had accepted. Sure, wallet-core could just use the
> latest one, but what if that has changed since the user requested the ToS?

Ack!

> I hope the feedback is useful.

It was definitely useful!  We just need to agree on some details ;-)

- Florian