search

interledger / rfcs

Specifications for Interledger and related protocols
https://interledger.org
Other
454 stars 111 forks source link

docs(0028): update spec with monetization tag observation changes #527

Closed sublimator closed 5 years ago

sublimator commented 5 years ago

Overview

sublimator commented 5 years ago

@sharafian

sharafian commented 5 years ago

We should also have some tips for implementors about CORS and the Web-Monetization-Id header. This is something I didn't even think about at first because extensions ignore CORS but in some cases you need to serve an OPTIONS endpoint that has Access-Control-Allow-Headers set to allow Web-Monetization-Id

sublimator commented 5 years ago

Putting aside the fact it is a breaking change: I think "id" and "Monetization ID" are more appropriate. The former is nice and short (leaving wiggle room?? Actually, I could be easily moved to s/requestId/monetizationId) and the latter matches the existing Web-Monetization-ID header.

The meta tags and are no longer bound to a single page "request" and streams can stop and start.

Back to "breaking": Could we live with the pain of the change? Is it worth it for conceptual clarity? How many people would it effect? When would be the time to make changes like this?

Does "request" still make sense to you? Would it make sgnse to the uninitiated?

I guess one could define non enumerable properties on the events with deprecation warnings. Shrug.

Your call

sharafian commented 5 years ago

The meta tags and are no longer bound to a single page "request" and streams can stop and start.

The main reason to say requestId was to make it clear that the ID was ephemeral (i.e. unique to each SPSP request). A monetizationId might seem like it refers to the user rather than to the SPSP request.

I could maybe get webMonetizationId (because it matches the header) or just id because it's pretty general. But I still feel like requestId isn't innapropriate.

  • monetization stop event ?

:+1: I think this makes sense. We might also want to add a new monetization state: stopped. So you can tell the difference between initializing ("pending"), stopped ("stopped"), and not supported (undefined). This also makes it less confusing when you check if web monetization is supported on a page that doesn't use it. "pending" is a confusing state if there's no meta tag present.

  • specify both sent and received currency code/scale explicitly

:+1: STREAM exposes both of these so it makes sense to expose them to the page. You'd want to use the source currency preferences for display (like counters) and you'd want to use the destination currency preferences to check how much the user has paid (e.g. to adjust paid video quality or to trigger loading of a piece of exclusive content)

traviscrist commented 5 years ago

The meta tags and are no longer bound to a single page "request" and streams can stop and start.

The main reason to say requestId was to make it clear that the ID was ephemeral (i.e. unique to each SPSP request). A monetizationId might seem like it refers to the user rather than to the SPSP request.

I could maybe get webMonetizationId (because it matches the header) or just id because it's pretty general. But I still feel like requestId isn't innapropriate.

If we make it id or webMonetizationId it no longer feels short lived, the update above which removes that its only on page loads also sorta implies this id is a long lived id. I feel like what we put here should not just be id since its a short lived id, its not long lived. webMonetizationSessionId could work but that's getting really long.

sublimator commented 5 years ago

@sharafian

just id because it's pretty general.

Clarity through ambiguity TM :)

@traviscrist

it no longer feels short lived,

I think I understand.

We moved to allow dynamic <meta> tags as a way of supporting SPA, such as coil.com (which I suppose will end up using server side rendering for SEO, but regardless, I'm sure you can imagine the use cases, such as a long page of videos that stop/start as you scroll ).

For better or worse, I suppose I've come to think of a monetization as a concept/thing.

I'll try and illustrate why monetizationId made sense to me:

The event names: monetizationstart monetizationstop

The meta tag: <meta name='monetization' />

The (content request) header: Web-Monetization-Id

Look at the relationships between the various entities:

Entities Relationship
<meta name="monetization">:requestId 1:1
SPSP resolve request: requestId M:1 (see: pause/resume)
Content request: requestId M:1 (see: reconnects while streaming videos)

When do we generate a new requestId now that we have moved to supporting dynamic meta tags? Whenever we see a new monetization meta tag.

Note, that in the table above I'm using request in the sense of a http request, rather than in a more abstract sense. Kind of cheating, but there is a strict mapping of meta tag to an identifier. To me the current request in requestId maps more closely to the request for content than the SPSP request.

I recall that requestId was at first dubbed correlationId. Used for correlating a monetization stream with later requests for content/services/etc.

If you guys aren't really board, given it's a break change, should probably just leave it as requestId

sublimator commented 5 years ago

@sharafian @traviscrist Thoughts re: monetizationpause/ monetizationresume events? Would be interesting to get input from cinnamon guys.

Note that often people open a youtube tab to put some music on, change tabs while still listening and then do something else.

sublimator commented 5 years ago

@sharafian

We should also have some tips for implementors about CORS "pending" is a confusing state if there's no meta tag present.

Noted

sublimator commented 5 years ago

@traviscrist Do you have any specific concerns with easing the requirement of only one meta per "page load" (which I assume means a single http request)? In particular, any security concerns? That was one aspect that nags at me.

We were discussing an https requirement at one point, but then you'd really have to be the browser to be able to get to the raw request bytes to ensure it wasn't inserted dynamically. Failing that, you'd have to request the page again, which I doubt would be workable in all cases.

traviscrist commented 5 years ago

When do we generate a new requestId now that we have moved to supporting dynamic meta tags?

Maybe it should have been called pageLoadId the whole idea behind the id changing on page load is to prevent tracking a user with this id by forcing it the be fresh on each page load. This is my main concern in relaxing the requirement that this be unique per page load. The name of it should just go with that requirement, I'm not really set on any name just that this id not be another vector for tracking user behavior since that could slow down the adoption of the web monetization spec.

Note that often people open a youtube tab to put some music on, change tabs while still listening and then do something else.

If the page does not reload then the id would not change, if it does then it would need to be correlated again with that user.

@traviscrist Do you have any specific concerns with easing the requirement of only one meta per "page load" (which I assume means a single http request)? In particular, any security concerns? That was one aspect that nags at me.

Where is this change in the proposed changes here? I don't see that requirement being eased any where in the proposed changes. Maybe I missed it though.

sublimator commented 5 years ago

YouTube question wasn't really concerning the correlation id. Just something that occurred to me. The extension currently pauses monetization when a tab is not active.

We moved the random id generation from per page load to per meta content change.

Late here, will answer tomorrow in more detail

traviscrist commented 5 years ago

We moved the random id generation from per page load to per meta content change.

So this means that if the page is reloaded then the id stays the same?

I understand changing the id on the meta content change but I think we should also keep the requirement that it change/(be unique) on a page load as well. From what I can tell these changes relax that requirement which makes it so the id no longer needs to be unique per page reload.

sublimator commented 5 years ago

So this means that if the page is reloaded then the id stays the same? No. The requirements were not intended to be relaxed. It's just that the language needs tightening up.

The idea (and latest impl in the extension) is to generate a new uuid.v4 every time it sees a new meta tag, and it's not persisted between page loads. Reload the page and you'll get a new id.

traviscrist commented 5 years ago

@sublimator Chatted with ben on this, I now see the idea that when the page is reloaded the meta tag would get picked up and then a new uuid would be generated each time.

Right now I don't think that's very clear in the updated documentation. Its also not clear how this would work for a static site since they meta tag would be part of the page. If we can add some clarity around when the uuid is generated and the overall flow of the meta tag being picked up to the uuid being created that would resolve my concerns about it.

I'm also good with calling it monetizationId, I think it makes sense, its a breaking change but if we are going to change it we should change it now.

sublimator commented 5 years ago

I don't think that's very clear in the updated documentation

Agreed. Still very rough. Thanks for highlighting it

I'm also good with calling it monetizationId

@sharafian What about you?

sublimator commented 5 years ago

@sharafian

Payment Pointer or SPSP url

Do we have a single term that covers both ? Note that the emitted monetization events currently use paymentPointer If not, do we need to constantly put Payment Pointer or SPSP url or just a single well placed mention that it's possible to also use an SPSP url.

Does the fact that you might need to use an SPSP url mean the design of payment pointers is lacking somehow?

sublimator commented 5 years ago

@sharafian I believe that was already fixed in a subsequent fix

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is important, please feel free to bring it up on the next Interledger Community Group Call or in the Gitter chat.

sharafian commented 5 years ago

Is this PR blocked on anything? @sublimator @traviscrist

sublimator commented 5 years ago

I think the plan was to get experience with things working in the extension and then make this PR match that.

traviscrist commented 5 years ago

I think the plan was to get experience with things working in the extension and then make this PR match that.

I thought this was looking good, I left one comment about the request id, @sharafian if you and I review this one more time then I think it could be merged. I'd rather make this change from request id to monetization id sooner than wait. Its kinda a big change so the sooner we make it the better.

sublimator commented 5 years ago

This is still very much a draft, hence the TODOs. I guess we could just merge this in now after a bit of cleanup though as it more closely matches reality and it is taking time. I didn’t understand Ben’s question about “what’s the hold up?” properly before.

As for a follow up: I think there are ways we could change the names, while keeping the old fields (non enumerable) which console.warn deprecation messages upon access if in our heart of hearts we want to change some names.I think the amount/asset etc, could do with clarifying specification of src/dest. I’d do it now/soon before much adoption.

sublimator commented 5 years ago

I watched the Sublime Text 2 to Sublime Text 3 transition, which went from Python 2 to Python 3 and a naming convention change for the plugin api. There was a transition period with compatibility layers but it was possible to make a clean break.

Changes from v1 to v2 included using json for conf over xml. There were far less users then.

In the end it was worth it.

I don’t think we should be afraid to make changes in the early days, especially if it can be done in a non breaking way.

As for ‘requestId’, it’s kind of grown on me/could live with it, but that’s typical once you no longer have fresh eyes.

adrianhopebailie commented 5 years ago

@sublimator @sharafian @traviscrist

This spec is likely to move from here to something in the W3C format as soon as @brucelawson and I have submitted this to the Web Platform Incubator CG (WICG) later this week.

I'd recommend we look at already migrating this over to that doc (incorporating these changes), linking to it from here, deprecating this and making further changes there.

@sublimator - do you want to have a stab at that or would you prefer I do it?

The WICG has a template for new proposals which I've already used to put the following together: https://github.com/adrianhopebailie/web-monetization

The main entry point for the community is the explainer which is expected to be updated regularly based on feedback and then the spec (index.html) should be updated whenever there is consensus on changes.

The spec will need to be a balance of "what is implemented already" and "what the community says should be in the spec". i.e. Initially the implementation drives the spec but as we get more implementations the spec will become the reference and implementations will need to follow.

Keep in mind that the spec is targeted at browser vendors (they are the implementors) not web developers. BUT, the API design should make sense to developers. So, for example, the choice of property names should consider only what the developer cares about (i.e. not SPSP, STREAM or ILP).

sublimator commented 5 years ago

@adrianhopebailie

I defer to your greater expertise here. I'm sure Ben and Travis will want to have some input before it's 'thrown to the wolves' though

sharafian commented 5 years ago

@adrianhopebailie Right now there's no accurate spec of WM other than this PR so I think it's important to get this PR merged. Your spec that you linked looks good, I'm wondering where I can leave feedback on it

adrianhopebailie commented 5 years ago

@sharafian

Right now there's no accurate spec of WM other than this PR so I think it's important to get this PR merged.

Agree, let's get it merged and then we can migrate it to the W3C template. Can we aim to have that done this week?

Your spec that you linked looks good, I'm wondering where I can leave feedback on it

The WICG process is to have a dedicated repo for proposals so use the issue list on that repo to log any comments about the spec or the explainer.

sharafian commented 5 years ago

Merging on behalf of Niq