=
I have the following comments:

There's no need to serialize RecurringPaymentDetails as = bytes here. It's done that way outside of PaymentDetails in order to = support digital signatures over protobufs that may have extensions the = wallet app isn't aware of, but it's a pain and inside PaymentDetails = (and therefore for most extensions) it shouldn't be necessary. So you = can just use "optional RecurringPamentDetails recurring_payments =3D = 8;"

OK, we'll fix = it.

There's only 4 possibilities here = for recurrences. That seems rather restrictive. Is the cost of being = more expressive really so high? Why not allow more flexible = specification of periods?

If there's no payment_frequency_type field then what happens? A quirk of = protobufs to be aware of is that making an enum field "required" can = hurt backwards compatibility. Because it will be expressed using a = languages underlying enum type, if there's a new enum member added later = old software that attempts to deserialize this will throw exceptions = because the new "unknown" member would be unrepresentable in the old = model. Making the field optional avoids this problem (it will be treated = as missing instead) but means software needs to be written to know what = to do when it can't read the enum value / sees enum values from the = future.

I hope the = explanation above answers the questions.

I assume the = amounts are specified in terms of satoshi, and timestamps are UNIX time, = but better to make that = explicit.

Yes.

Seems there's an implicit value constraint that = max_payment_amount <=3D max_payment_per_period. What happens if that = constraint is violated? Best to document that.

As explained = above, contract would define none, 1 or both conditions. First the = merchant should not return such 'conditions' but if it does the client = should not accept the contract. If the client decides to accept it = anyway, then the wallet just verifies both conditions are met separately = regardless of whether there is such violation and if so, makes the = payment.

What's the "merchant ID" namespace = thing about? What's it for? What happens if I set my competitors = merchant ID there?

I = agree, we can easily get rid of it.

What's the = "subscription ID"? Is this stuff not duplicative/redundant with the = existing merchant_data = field?

In an = ideal world the merchant should return unique subscriptionId (UUID for = instance). That subscriptionId is used in the code to identify the = contracts associated with the subscription. The merchant_data if i = understand correctly the payment protocol is opaque from the client = point of view, so it cannot be used by the client for that = purpose.

In what situations would you have >1 contract per = payment request? I'm not sure I understand why it's repeated. Presumably = if there are zero contracts included the data should be ignored, or an = error thrown and the entire payment request rejected? Which should it = be?
=

= There are many example where that could happen; for instance if = you subscribe to a service, then later decide to downgrade to a = lower product. The merchant may decide to only let you downgrade at the = end of your paid period-- to avoid generating extra credit-- and in that = situation you end up with two contracts: One for the current product you = are in and one for the future product you will end up on when the = downgrade becomes effective.

It's unclear to = me given such a contract when the payment should actually occur. For = instance if it's "monthly" then what day in the month would the payment = occur?

As = outlined above in the introduction, the protocol is designed in such a = way that the wallet does not have to know what is the exact date when = payment should occur, but instead polls the merchant for pending = payments. There are many situations when specifying an exact payment = date is not an option so that flexibility is essential. A simple example = would be for a customer who started subscribing on the 31th of a month. = Since there will be months with 28/29/30 days, the payment date would = change depending on the = month.

You'll notice I = moved the comments to be above the field definitions. I know the current = proto isn't done that way, but let's change it - long comments are good = and putting them above the field definitions encourages people to write = enough detail without being put off by line length constraints
=

Fine.

I = think the next step would be to talk to BitPay/get Jeff+Stephen involved = because I know they have customers that really want recurring payments, = and those guys will have a clearer idea of customer requirements than we = do. I feel uncomfortable with designing or reviewing in a vacuum without = some actual people who would use it chiming in, as I don't really know = much about the underlying business = processes.

We = are totally open to receive feedbacks from them.. How do we bring them = in the discussion?

I have some other comments about the bitcoinj = implementation specifically - for instance, we don't have a "wallet = directory" concept: everything goes into the wallet file. So we'll need = to think about how to structure the code to allow that. Also, just using = a background polling thread is likely not flexible enough, as on some = platforms you can't stay running all the time (e.g. Android) without = upsetting people, but the underlying OS can wake you up at the right = times, so wallet apps should have an ability to control wakeup tasks. = But we can discuss that over on the bitcoinj list specifically. Let's = keep this thread for the general protocol = design.

Ok that makes = sense.

BIP 70 is indeed implemented = in Bitcoin Core on the C++ side, so that isn't a concern. It could be = done there too.

Great to = know.

= --Apple-Mail=_95094412-BF43-4F67-807D-5F2DC2CB942C--