Forex #564

steveklabnik · 2014-04-10T20:29:58Z

This pull request will eventually fulfill the "Forex" milestone: https://github.com/balanced/balanced-api/issues?milestone=9&state=open

This is the very first step: debiting a card in €.

There are two things left to do before this is 'ready':

Have the charges broken down on the debits response
get together the full list of currencies to fix the schema

mjallday · 2014-04-10T20:32:42Z

features/forex.feature

+  This provides a better experience for customers who have accounts denominated
+  in other currencies, and reduces chargeback rates for those customers as well.
+
+  Scenario: Debit a card in â¬


â�¬ is either a new crypto currency or an encoding error...

Dammit. Yeah, should be €. fixing...

steveklabnik · 2014-04-10T21:18:18Z

Green! . ha!

... I guess this means we aren't actually validating the currency field server side. 😅

matthewfl · 2014-04-10T21:23:00Z

validate? fields? what? madness

did you assert that the field comes back with the same currency that you set?

steveklabnik · 2014-04-10T23:24:53Z

@mjallday point taken. we don't test the invoicing stuff here at all, right?

@mjallday

This reverts commit f8eb3b3. @mjallday schooled me about how we used to do this like this, but don't any more. Seems good. 👍

mahmoudimus · 2014-04-10T23:25:32Z

@steveklabnik @mjallday completely agree. Fees should in my opinion be a separate concern of the API.

steveklabnik · 2014-04-10T23:25:45Z

Reverted that commit.

steveklabnik · 2014-04-10T23:49:39Z

Bam. 157 currencies.

@matthewfl

Thanks @matthewfl

steveklabnik · 2014-04-11T00:40:07Z

There needs to be information on the response for the Debit that specifies by how much the escrow increased (in the escrow's or order escrow's currency). This is required for a marketplace to be able to audit the increase in their escrow balance.

steveklabnik · 2014-04-11T00:40:25Z

(above comment by @matin )

mjallday · 2014-04-11T00:44:54Z

Would a currency pair quote solve that? e.g. EUR/USD=1

If there was a forex quote resource you could query that ahead of time to get the exchange rate, submit to with the transaction (before the quote expires) to guarantee the transaction gets that rate (omitting the quote would give you a rate of whatever is current), and then link to the quote on the transaction to provide the auditing you are asking for.

mahmoudimus · 2014-04-11T00:54:00Z

@mjallday seeing what the converted number is and a convenient ratio on the debit's response is extremely useful.

@steveklabnik and @matin are correct, they'd like to audit their escrow balance, but I personally think no customer will run analytics by querying all the debits from our API. I think they'll request for a bulk export and or a way to view their invoices in a deep dive to see what their starting balance at the start of the day and all the invoice events generated for that day of business.

mjallday · 2014-04-11T00:56:37Z

no customer will run analytics by querying all the debits

they do build state machines that listen to webhooks and do their own reconciliation from that. (based on personal experience in irc helping implement such)

mahmoudimus · 2014-04-11T01:00:11Z

Let's get some customers to plz testify to this. I think they'd rather fetch a different resource (like invoices) 2 do their computations from that rather than querying all debits. That seems like a fools errand 2 me.

mjallday · 2014-04-11T01:19:14Z

TESTIFY BROTHER! CAN I GET AN AMEN?

https://botbot.me/freenode/balanced/2014-03-24/?tz=America/Los_Angeles

Read the conversation between jondkinney and myself.

Invoices are hard to use for reconciliation. The issue that I've heard with using invoices is that they have to trust us to get the math correct. If they want to verify this themselves then they need to build a state machine and adjust the escrow as txns go through various states.

This is digressing from the forex conversation, either way I believe we need to provide them with the exchange rate that the transaction was assigned.

steveklabnik · 2014-04-11T16:56:04Z

I'm going to have a discussion with Cath and Tim today about some of this, as well.

steveklabnik · 2014-04-11T21:24:38Z

Right, so there's two things here: first, that having the information also be in the debit rather than in an invoice would be useful, and that will probably be the same for forex.

So the real question is naming, I guess. Sometimes, a debit goes to an order, and sometimes, it doesn't, so escrow_amount seems bad. Thoughts?

matin · 2014-04-11T21:32:00Z

@steveklabnik can you explain what you mean by "the information"?

I assume you're trying to figure out how best to return the amount (in the currency of the escrow).

In the case where Balanced only supports a Order/escrow in USD, you can have the field be amount_in_usd.

What's stopping you from using that field name to keep it simple for now?

steveklabnik · 2014-04-11T21:54:43Z

Yes, your assumption was correct.

I guess I'm wary of creating yet another USD-specific name baked in. What happens when we have multiple currency escrows?

matin · 2014-04-11T21:59:11Z

What happens when we have multiple currency escrows?

We'll have learned a lot more than we know now about customer behavior and what people want. At which time, we can add a new revision and solve the problem based on everything that we've learned.

If you do accept that amount_in_usd works if we assume usd only, then the next step would presumably be amount_in_order_currency + order_currency. Not sure "order" is the right word but something following that format.

mahmoudimus · 2014-04-11T22:04:00Z

@matin why not just return the currency back w/ every amount?

matin · 2014-04-11T22:07:47Z

Assume the marketplace escrow or order is denominated in USD. A debit should return something like:

{
    "amount": 10000,
    "currency": "EUR",
    "amount_in_order_currency": 13187,
    "order_currency": "USD"
}

There's two currencies: the currency of the buyer and the currency of the marketplace/order

steveklabnik · 2014-04-11T22:31:57Z

Seems good.

matthewfl · 2014-04-11T22:41:58Z

fixtures/_models/debit.json

@@ -27,7 +27,164 @@
        "currency": {
            "type": "string",
            "enum": [
-                "USD"


can you make a specific currencies file for this similar to status

https://github.com/balanced/balanced-api/blob/master/fixtures/_models/status.json

I will eventually, this is changing.

ok, I just realized how long and unyielding that had become

All the currencies may come back at some point.

steveklabnik · 2014-04-15T20:45:42Z

I'm not happy with *_order_currency as a name, but generally, this is right. And once I get a final list of currencies, I will extract a specific file as @matthewfl suggests. But this should be good for actually implementing as of now.

mjallday · 2014-04-15T20:52:44Z

should _order_currency be _escrowed_currency? an order holds its funds in escrow. it also means that doing a GET on something that does not use an order can use the same fields so you do not need to parse something differently depending on the destination of funds.

that does not hold true for transfers however...

steveklabnik · 2014-04-15T20:53:33Z

Right, the problem with escrow is that with accounts & orders, it's not the right word...

matthewfl · 2014-04-15T21:08:59Z

how about having an object of the relevant currencies, something like

"amounts": {
   "usd": 12345,
   "btc": 9001
}

that seems somewhat awkward now that I have written it, but I think it might be nice to limit the number of top level fields on any resource

mjallday · 2014-04-15T21:39:17Z

i was going down the same path as you matthew, but agreed it feels odd.

@steveklabnik is this considered a backwards compatible change? i ask because tho the fields you're proposing are additional so far and there are no existing changes, you can no longer say that that escrow = sum(debits.amount) - sum(refunds.amount) - sum(credits.amount) + sum(reversals.amount).

matthewfl · 2014-04-15T21:46:31Z

my understanding is that this change is suppose to be compatible with rev1.1. That said, even if it was setup as not backwards compatible, I don't think people operating in a single currency should have to deal with the awkwardness of multi currency

steveklabnik · 2014-04-15T22:02:56Z

Orders already made that not true, right, @mjallday ?

steveklabnik · 2014-04-16T00:14:49Z

So, after discussing this with @mjallday , we should just make sure that this is clear to those who implement forex. It's still going to be backwards compatible, because until you do forex stuff, that's the same, and it's not a property we've ever explicitly guaranteed, but we should be careful about casually mentioning it.

steveklabnik · 2014-04-21T23:15:11Z

Updated this to use a currency reference so things don't get out of hand, as well as the full list of currencies, and renamed the two new fields to refer to 'captured' currency, since 'orders' was kinda confusing. Still not 100% happy with that name, but it's good enough.

😢

kyungmin · 2014-05-09T20:44:55Z

fixtures/_models/debit.json

+            "minimum": 1
+        },
+        "captured_currency": {
+	    "$ref": "_models/currency.json"
        },


How is captured_currency different from currency?

currency is your base currency (currently this will be USD for all accounts/escrows). captured_currency is the amount you captured in. e.g. you capture in 1EUR with an exchange rate of 1.2 you would see

currency: USD

amount: 120

captured_currency: EUR

amount_in_captured_currency: 100

Currently, currency is denominated in cents. Will captured_currency be also denominated the same way?

Also, if both currency and captured_currency are both USD, will amount_in_captured_currency be the same as amount or will it be null?

Yes, integers of the smallest value of the currency.

They'll be the same. null is evil!

@kyungmin and I have come to the conclusion that since this information basically doesn't ever change, it will go in the documentation, and we'll provide some sample JavaScript to make it even easier to format correctly. Adding that information to the response just leads to sending the same information over the wire over and over again for little benefit.

@matin noted. I struggled to come up with the right word here, and I guess I was wrong. Do you have any suggestions for what the right term is?

What about this?

{ "amount": 10000, "currency": "EUR", "amount_in": { "USD": 13755 } }

The response can include whatever currencies are relevant for that marketplace.

The other alternative I would suggest is amount_in_escrow_currency and escrow_currency. This is consistent with the language we use elsewhere.

Yes. Let's go with amount_in_escrow_currency and escrow_currency. I don't recall why I had an issue with that before, seems incredibly straightforward.

debit a card in a foreign currency

d94eb41

steveklabnik mentioned this pull request Apr 10, 2014

Forex support #100

Open

mjallday reviewed Apr 10, 2014
View reviewed changes

fix encoding error on € 😅

8b08372

Adding details for exposing the amounts

f8eb3b3

Revert "Adding details for exposing the amounts"

4307c8f

This reverts commit f8eb3b3. @mjallday schooled me about how we used to do this like this, but don't any more. Seems good. 👍

Add full currency list to the debit schema

f17f031

Ensure that we get the same currency back.

84ab490

Thanks @matthewfl

matthewfl reviewed Apr 11, 2014
View reviewed changes

Cut down on currencies, add order currency field.

a2f687e

All the currencies may come back at some point.

Adding currency object

3b64750

Refer to the correct model.

b5a3013

😢

kyungmin reviewed May 9, 2014
View reviewed changes

captured -> escrow in currency names

bfe1227

kyungmin mentioned this pull request May 13, 2014

Forex balanced/balanced-dashboard#1237

Open

8 tasks

steveklabnik closed this Dec 6, 2015

Forex #564

Forex #564

Conversation

steveklabnik commented Apr 10, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

steveklabnik commented Apr 10, 2014

matthewfl commented Apr 10, 2014

steveklabnik commented Apr 10, 2014

mahmoudimus commented Apr 10, 2014

steveklabnik commented Apr 10, 2014

steveklabnik commented Apr 10, 2014

steveklabnik commented Apr 11, 2014

steveklabnik commented Apr 11, 2014

mjallday commented Apr 11, 2014

mahmoudimus commented Apr 11, 2014

mjallday commented Apr 11, 2014

mahmoudimus commented Apr 11, 2014

mjallday commented Apr 11, 2014

steveklabnik commented Apr 11, 2014

steveklabnik commented Apr 11, 2014

matin commented Apr 11, 2014

steveklabnik commented Apr 11, 2014

matin commented Apr 11, 2014

mahmoudimus commented Apr 11, 2014

matin commented Apr 11, 2014

steveklabnik commented Apr 11, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

steveklabnik commented Apr 15, 2014

mjallday commented Apr 15, 2014

steveklabnik commented Apr 15, 2014

matthewfl commented Apr 15, 2014

mjallday commented Apr 15, 2014

matthewfl commented Apr 15, 2014

steveklabnik commented Apr 15, 2014

steveklabnik commented Apr 16, 2014

steveklabnik commented Apr 21, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment