WICG / webmonetization

Proposed Web Monetization standard
https://webmonetization.org
Other
457 stars 153 forks source link

fix: various #285

Closed adrianhopebailie closed 1 year ago

adrianhopebailie commented 1 year ago

I have tried to address some of the discussion in the main link_rel PR. I think that this helps to make the whole spec a little cleaner.

The goal is that the WM spec should not have too much detail about ILP or Open Payments as these are evolving and leaking details into this spec makes changes hard.

fix: math in examples As pointed out by @sublimator

fix: payment pointer (not endpoint) Replace "payment end-point" with payment pointer

fix: send amount in event Fix ambiguity about send vs receive amount and add warning about changes

fix: removed some leaking external protocol refs Adjust some wording to remove ambiguity about Open Payments concepts.

fix: former editors, editor details Move former editors into former editors block

adrianhopebailie commented 1 year ago

@sublimator - I think that this part is key to the whole separation of concerns:

Otherwise, establish a new payment session.

This is where the browser (having successfully fetched the payment pointer) does processing and interfaces with the WM Provider to create a payment session.

The specifics of this are opaque to the website. If this session goes stale and is re-established later then there is no need for the website to know that.

sublimator commented 1 year ago

We should probably make a struct for amounts, probably matching OP (value, not amount). I was liking PaymentAmountCurrency but if we are using OP ...

sublimator commented 1 year ago

@adrianhopebailie

This is where the browser (having successfully fetched the payment pointer) does processing and interfaces with the WM Provider to create a payment session. The specifics of this are opaque to the website. If this session goes stale and is re-established later then there is no need for the website to know that.

Thanks for the clarification. By that you mean there is no need to emit another "load" event. My objection is with mandating that only the browser processes the payment pointer and the tradeoffs involved, but we can have that discussion elsewhere.

adrianhopebailie commented 1 year ago

I was liking PaymentAmountCurrency but if we are using OP ...

Me too. I don't think the protocol used on the backend should be relevant we should use WebIDL types that already exist where we can.

adrianhopebailie commented 1 year ago

@AlexLakatos - this is ready to merge into the new spec. WDYT?

adrianhopebailie commented 1 year ago

@AlexLakatos @sabineschaller @uchibeke I'd like to merge this in so we can wrap up review of #193 ASAP

I've responded to/resolved all the outstanding questions as far as I can tell.

AlexLakatos commented 1 year ago
2. Introducing `MonetizationCurrencyAmount` or `PaymentCurrencyAmount` and replacing `assetCode` and `assetScale`

The Google team reviewed this, it's good to go. Here's what they said:

Your plan of introducing a MonetizationCurrencyAmount interface with the same members as PaymentCurrencyAmount and linking to the validation steps in the PaymentRequest API SGTM! 👍 From implementation perspective, you would be able to reuse PaymentRequests' existing validation code, which is great for easier code maintenance.

The way PaymentRequest API has "solved" the same issue with the total amount on PaymentRequestEvent is by using an object data-type instead. Your solution seems somewhat better. 

P.S.: Perhaps the PaymentRequest API can consider converting PaymentCurrencyAmount into an interface in the future as well, so that our PaymentRequestEvent is better defined and the PaymentCurrencyAmount would be more easily shared among multiple APIs. No concrete plans for this now, though.

sublimator commented 1 year ago

For the record, I'm not totally convinced about sourceAmount only, but basically lgtm

Emitting an event with a source amount in a different asset than the page is set up to receive can cause problems because it would require the page to keep a list of exchange rates to do anything meaningful with the amount. For example, if a page is set up to receive payments in USD, but an event is emitted with a source amount in EUR, the page would need to know the exchange rate between USD and EUR to determine the amount of USD received. This can be cumbersome and potentially unreliable.

Even if the source amount can be converted to the desired asset, it cannot be fully verified by the recipient account. The recipient can only really trust and verify what they actually received. Thus they would need to consult the incomingPayment.

It is possible for a user to send more than one asset as a source amount, which would further complicate the page's ability to use this information. The page would need to know the exchange rate for each asset and then convert them all to the desired asset. It could not simply assume that all amounts will be in a single asset.

If the method of comparing source amounts requires only the three-letter currency codes, and not the issuer, it can be susceptible to issues. In practice, a USD issued by one exchange may be worth relatively more or less than one issued on another due to various factors such as withdrawal times or limits at the exchange. So, relying solely on the currency code may not be a reliable indicator of the actual value of the asset.

It may be difficult to determine from the monetization event whether an amount is user-initiated or not. One possible way to do this is to set a threshold for the source amount in a particular asset. For example, if the source amount in USD crosses a certain threshold, the page could interpret it as a user-initiated payment and display a "thank you" message. However, this approach may not always be reliable, as there could be other reasons why the source amount crosses the threshold, such as a system error or a test payment.