Mike Hearn [ARCHIVE] on Nostr: 📅 Original date posted:2014-02-25 📝 Original message:Hey there, So the essence ...

📅 Original date posted:2014-02-25
📝 Original message: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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxfoundation.org/pipermail/bitcoin-dev/attachments/20140225/bb748c61/attachment.html>;