balanced / balanced-api

Balanced API specification.
220 stars 72 forks source link

Expand transaction_id keyspace #633

Closed msherry closed 9 years ago

msherry commented 10 years ago

transaction_id resembles a US phone number -- NNN-NNN-NNNN. There are only 9,999,999,999 possible values for this field.

According to the first birthday problem calculator I found online (http://lazycackle.com/Probability_of_repeated_event_online_calculator__birthday_problem_.html), it only takes ~120,000 transactions before there is a 50% chance of finding a duplicate, and we have marketplaces that perform that many debits in a matter of months. It would be good to expand this keyspace in order to provide less chance of collision -- otherwise, why provide this field at all?

mjallday commented 10 years ago

are you thinking of expanding number of digits or allowing [0-9A-Z]?

msherry commented 10 years ago

Allowing letters would probably be the least disruptive -- if users of the api have fixed-length database fields holding these values, they should continue to work with no modifications. If we expanded the length, there's a chance that could cause issues with very strict schemas. transactions_id values are already mixed letters/numbers, so existing types should still be ok.

msherry commented 10 years ago

If there are no comments on this, I'm just going to do it. This is becoming annoying.

cieplak commented 10 years ago

+1 add some unicode characters

mjallday commented 9 years ago

@matin do you see any issues with changing the transaction_id spec from [A-Z][0-9]{3}-[0-9]{3}-[0-9]{4}" to [A-Z][A-Z0-9]{3}-[A-Z0-9]{3}-[A-Z0-9]{4}?

matin commented 9 years ago

@msherry @mjallday What if the transaction_id wasn't unique? The purpose is to make search easier. If you put in a transaction_id in the search and only get 3 results, that seems to still accomplish the goal.

mjallday commented 9 years ago

the transaction_id is submitted to the underlying banks and used as a trace number that's common to balanced and the bank. i believe that's where the issue lies since we can have collisions when we try to reconcile. amirite @msherry?

mahmoudimus commented 9 years ago

Two things here:

mjallday commented 9 years ago

isn't the transaction_id used as a trace id if the customer needs to contact the bank? if so then i do not think this is leaky.

mahmoudimus commented 9 years ago

@mjallday that's a good point - fwiu, it only appears on AMEX txns anyway and only some issuing banks. Wouldn't appears_on_statement_as be more appropriate?

Here's what I get on my cc statement:

image

msherry commented 9 years ago

It's what we use to identify/look up transactions in remote systems, so no, this is not leaky.

Per the email I sent internally earlier, I'm implementing this tomorrow unless specific objections and alternatives are brought up.

msherry commented 9 years ago

This has been implemented.