flash-oss / medici

Double-entry accounting system for nodejs + mongoose
MIT License
307 stars 84 forks source link

multi currency account #18

Closed nicolasburtey closed 4 years ago

nicolasburtey commented 4 years ago

If you were to implement a multi-currency version of Medici, how would you do it?

I guess

.debit('Assets:Cash', 1000, {meta}) would need another parameters, ie: .debit('Assets:Cash', 1000, {meta}, "USD")

A transaction would still equal out with multiple currencies. ie Alice sending 10 USD for 12 EUR to Bob:

.credit('Alice', 10,  {}, 'USD')
.debit('Bob", 10, {}, USD')
.debit('Alice', 12,  {}, 'EUR')
.credit('Alice, 12, {}, 'EUR')

Now the issue would be that, if we try to fetch a balance like this, we will have currency co-mingled.

ie:

const balance = await myBook.balance({
  account: "Assets:Accounts Receivable",
  client: "Joe Blow"
});

So we would need to have a mandatory currency field here. And filter that out.

I'm wondering what the most efficient way to do it would be?

Simply adding a currency field in transaction, and modifying the balance function? Any other idea?

koresar commented 4 years ago

Hm... Having multiple currencies in a same book? That doesn't sound like a double-entry accounting system. I'd recommend creating a separate book for a new currency. That the only safe way to go. Otherwise you can get yourself into a lot of troubles.

Medici checks every commit is zero balanced. See source code: https://github.com/koresar/medici/blob/a1d43c3cee03f0d2f84badad29324bef678c67de/src/entry.js#L130 Every commit must balance out to zero.

I would discourage the two following solutions because adding dollars to euros is plain wrong.

The Medici module have a feature of "sub accounts".

You can have a "USD" account and then sub accounts for Alice and Bob.

.credit('USD:Alice', 10)
.debit('USD:Bob", 10)
.debit('EUR:Alice', 12)
.credit('EUR:Alice', 12)

Or Alice and Bob can have a USD wallet sub account.

.credit('Alice:USD', 10)
.debit('Bob:USD", 10)
.debit('Alice:EUR', 12)
.credit('Alice:EUR', 12)
nicolasburtey commented 4 years ago

Thanks for the quick feedback!

Hm... Having multiple currencies in the same book? That doesn't sound like a double-entry accounting system. I'd recommend creating a separate book for a new currency. That the only safe way to go.

The issue is if you do cross-currency transactions. I don't think you could do such transaction atomically if you have them in 2 books?

Ledger has a useful wiki on it.

Would there be some constraint to have more than 3 sub accounts (performance?). If accounts need to finish with currency, there 3 may not be enough.

But as shown on the ledger page and their -X option, there could be a need to convert between currency as well.

koresar commented 4 years ago

Can be done in one book.

Alice converting AUD->EUR within her multicurrency wallet:

books
  .credit("AUD:Expenses:Marketing", 2000.00, { client: 'Alice' })
  .debit("AUD:Equity:Trading:Currency:AUD:EUR", 2000.00, { client: 'Alice' })
  .credit("EUR:Equity:Trading:Currency:AUD:EUR", 1000.00, { client: 'Alice' })
  .debit("EUR:Liabilities:Accounts Payable:EUR", 1000.00, { client: 'Alice' })

Just make sure that the sum for AUD is always 0. And for EUR.

To get total AUD balance:

const balance = await myBook.balance({
  account: "AUD",
  client: "Alice"
});

Not sure if that's helpful.

nicolasburtey commented 4 years ago

Let me play with it and see. Just a question to understand the behavior or Medici.

The documentation says:

Accounts are divided into up to three levels, separated by a colon

But after skimming through the code I have not seen this limitation. Ie: AUD:Equity:Trading:Currency:AUD:EUR will have 6 sub-accounts (and not be limited by 3) in total.

Is that correct?

koresar commented 4 years ago

Documentation sadly is outdated. I also do not remember any sub accounts limitations. Sleep about that. Would you be able to submit a PR with the README changes for that sentence?

On Sat., 18 Apr. 2020, 05:17 nicolasburtey, notifications@github.com wrote:

Let me play with it and see. Just a question to understand the behavior or Medici.

The documentation says:

Accounts are divided into up to three levels, separated by a colon

But after skimming through the code I have not seen this limitation. Ie: AUD:Equity:Trading:Currency:AUD:EUR will have 6 sub-accounts (and not be limited by 3) in total.

Is that correct?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/koresar/medici/issues/18#issuecomment-615420079, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMMEL2YQ3RWYGV34FL7S3LRNCTMPANCNFSM4MJ4E75A .

nicolasburtey commented 4 years ago

Just proposed a PR to remove what I believe is not correct in the README

nicolasburtey commented 4 years ago

actually I might have talked too quickly: https://github.com/koresar/medici/blob/e6a2c27fa28fd748bb7c349b16a9e4ccc0f943d5/src/entry.js#L38

nicolasburtey commented 4 years ago

will continue seeing how to integrate multi-currency and will try to pull a PR if I find a good way forward

koresar commented 4 years ago

Sounds awesome!

On Tue., 21 Apr. 2020, 07:02 nicolasburtey, notifications@github.com wrote:

will continue seeing how to integrate multi-currency and will try to pull a PR if I find a good way forward

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/koresar/medici/issues/18#issuecomment-616806964, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMMELYB2CTIR4V7EQ2VEFLRNSZ45ANCNFSM4MJ4E75A .

koresar commented 4 years ago

Thinking out loud, maybe we should just remove this limitation?

nicolasburtey commented 4 years ago

probably. do you know why it was here in the first place? to limit index size?

koresar commented 4 years ago

I don't know exactly. But I suspect that the first version of the medici API allowed to pass account, subaccount, subsubaccount.

The newer (current) API let you pass these as a concatenated string "Assets:Something:Somethingelse"

nicolasburtey commented 4 years ago

I have currently being using

.credit('Alice', 10,  {currency: 'USD'})
.debit('Alice', 9, {currency: 'EUR'})
.credit('Bob', 9,  {currency: 'EUR'})
.debit('Bob', 10, {currency: 'USD'})

with no issue so far. as long as every balance() call includes currency the balance seems correctly fetch with the currency flag.

closing this for now.

koresar commented 4 years ago

thank you for the thread!