Forgive me if I missed it, but the spec doesnt look like it can handle only handle periods of per week, per month, per quarter rather than 'n period'. I take Paypal as a reference example for subscription payments where you can set recurring to every: n days, n weeks, n months, n years. That way a quarterly payment is every 3 months. This fine granularity is necessary because sometime a payment scheme can be per 4 weekly rather than per monthly. So in summary the spec needs daily as an option, and to specify the recurring cycle as every n*period (one of daily, weekly, monthly, yearly): and you can drop quarterly since it's just expressed as per 3*monthly. Drak On 25 February 2014 16:29, Mike Hearn wrote: > Hey there, > > So the essence of this protocol is as follows: > > enum PaymentFrequencyType { > > WEEKLY = 1; > > MONTHLY = 2; > > QUARTERLY = 3; > > ANNUAL = 4; > } > message RecurringPaymentDetails { > // Namespace for the merchant such as org.foo.bar > > required string merchant_id = 1; > > // Id for the recurring subscription > required bytes subscription_id = 2; > > // Contracts associated with a given subscription > > repeated RecurringPaymentContract contracts = 3; > > } > message RecurringPaymentContract { > > // Unique id for a given contract > > required bytes contract_id = 1; > > // URL to poll to get the next PaymentRequest > > required string polling_url = 2; > > // Timestamp; when this contract starts > required uint64 starts = 3; > > // Timestamp; when this contract should be considered invalid > > optional uint64 ends = 4; > > // Expected payment frequency > optional PaymentFrequencyType payment_frequency_type = 5; > > // Max payment amount within that frequency (e.g. no more than 5 BTC per month) > > optional uint64 max_payment_per_period = 6; > > // Max payment amount (e.g. no more than 3 BTC per payment) > > optional uint64 max_payment_amount = 7; > > } > > I have the following comments: > > 1. 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 = 8;" > > 2. 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? > > 3. 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. > > 4. I assume the amounts are specified in terms of satoshi, and > timestamps are UNIX time, but better to make that explicit. > > 5. Seems there's an implicit value constraint that max_payment_amount > <= max_payment_per_period. What happens if that constraint is violated? > Best to document that. > > 6. What's the "merchant ID" namespace thing about? What's it for? What > happens if I set my competitors merchant ID there? > > 7. What's the "subscription ID"? Is this stuff not > duplicative/redundant with the existing merchant_data field? > > 8. 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? > > 9. 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? > > 10. 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 > > > 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. > > 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. > > BIP 70 is indeed implemented in Bitcoin Core on the C++ side, so that > isn't a concern. It could be done there too. > > > > ------------------------------------------------------------------------------ > Flow-based real-time traffic analytics software. Cisco certified tool. > Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer > Customize your own dashboards, set traffic alerts and generate reports. > Network behavioral analysis & security monitoring. All-in-one tool. > > http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk > _______________________________________________ > Bitcoin-development mailing list > Bitcoin-development@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/bitcoin-development > >