rs-davila / currencyspot

Code and documents for building a decentralized FX market
MIT License
0 stars 0 forks source link

Misc feedback on first draft #1

Open lrettig opened 6 years ago

lrettig commented 6 years ago

https://github.com/rs-davila/currencyspot/blob/6e64289b749db48c0d1863713cc604d1bcd54279/contracts/CurrencyHedge.sol#L13

Why 5 decimal places? Seems kinda arbitrary

https://github.com/rs-davila/currencyspot/blob/6e64289b749db48c0d1863713cc604d1bcd54279/contracts/CurrencyHedge.sol#L17 https://github.com/rs-davila/currencyspot/blob/6e64289b749db48c0d1863713cc604d1bcd54279/contracts/CurrencyHedge.sol#L18

Strictly speaking, I think keeping track of institution and external account ID is unnecessary, and slightly antithetical to the pseudonymity common in the decentralized world (I have a gut feeling that, to the extent that it's necessary to associate external IDs with accounts or addresses in the blockchain, that should be done at a higher level by wallet software). Although I guess it could simplify accounting on our end somewhat. What do you think?

https://github.com/rs-davila/currencyspot/blob/6e64289b749db48c0d1863713cc604d1bcd54279/contracts/CurrencyHedge.sol#L24

How about more per-transaction metadata, e.g., transaction type, vendor type, a short description, etc.? Do we support all transactions or only a subset? (E.g. cash donations to charities and bill payments would usually be excluded.) How are refunds handled?

https://github.com/rs-davila/currencyspot/blob/6e64289b749db48c0d1863713cc604d1bcd54279/contracts/CurrencyHedge.sol#L46

Why restrict hedge creation to the owner? It seems more natural to me that the client would just submit payment in the form of ETH directly to the contract, creating the hedge. Otherwise you centralize the system unnecessarily.

What's the purpose of the "active" flag? If a user submits payment to the contract when she creates a hedge, shouldn't it automatically become active as soon as hedgeStart occurs?

https://github.com/rs-davila/currencyspot/blob/6e64289b749db48c0d1863713cc604d1bcd54279/contracts/CurrencyHedge.sol#L84

Pretty sure the beneficiary argument is unnecessary here, for this and several of the following functions. The data already lives inside the Hedge struct. If recordTransaction is called directly by the customer, you can just use msg.sender.

https://github.com/rs-davila/currencyspot/blob/6e64289b749db48c0d1863713cc604d1bcd54279/contracts/CurrencyHedge.sol#L130

I think you need to check absolute value here as it could be negative.

Misc:

rs-davila commented 6 years ago

Why 5 decimal places? Seems kinda arbitrary

If a contract today were set between USD and Vietnamese Dong, then the exchange rate is about 1 : 22733.50, or 0.00004399. Solidity can only store integers, so the first idea was to multiply by 10^5 and then truncate the number. Thus, this rate would actually be stored as '4'. However, that does mean that '4' will actually cover the range from 0.000040 to 0.000049, or 1 : 20,408 to 1 : 25,000. It would seem like you're losing significant data, but actually this spread is equivalent to US $0.000539. If the hedge were for, say, $10,000,000 (not expected for a 'retail' application, but not impossible), this would be $53.88 that could potentially be at risk.

There are a few different solutions: 1) Go to more decimal places and multiply by a larger order of 10. 2) Set a larger number for the home currency benchmark and store the actual integers in the contract. 3) Define the exchange rate such that it will be >= 1, multiply by 10^5 (or whatever other number) to preserve the decimals in the integer variable, and keep track of the ratio.

Example behavior of each suggestion above would look like: 1) Multiply the exchange rate by 10^12 instead of the currently-set 10^5. 2) Use $10,000 as the basis. Remove the 'refRate' variable, and instead store homeExch = 10000, hedgeExch = 2273350000. 3) Evaluate USD : VND outside the contract. If the result is < 1, then use the inverse of the ratio. Thus, refRate = 22733 and another variable refInverse = true.


Strictly speaking, I think keeping track of institution and external account ID is unnecessary, and slightly antithetical to the pseudonymity common in the decentralized world

I think you raise a good point, and I can't think of a strong argument to support keeping these two pieces of info on the blockchain. In "CurrencySpot Classic", Stripe was used to establish the link to a hedge buyer's credit or debit account, and to gain access to the account's transaction history. This functionality could stay in the wallet software, and doing so would also allow us to be more flexible and offer crypto-to-fiat and crypto-to-crypto hedges. We should, however, be mindful of KYC rules and how to accommodate them.


How about more per-transaction metadata, e.g., transaction type, vendor type, a short description, etc.? Do we support all transactions or only a subset? How are refunds handled?

So this gets to the heart of how much info should be maintained on the blockchain. As a coding exercise, we might want to be more maximalist and store lots of info, since it's more practice with contracts and blockchain. However, perhaps the blockchain should store only the bare minimum of transaction info, and the wallet software should be responsible for determining which transactions should and should not qualify. I think we should only allow transactions where a good or service is bought from a vendor or individual. So at this point we would not cover donations, bill payments, and general risk / derivatives trading. Here are some example cases that I think we would want to cover:

1) Alice travels to Thailand from the US, and while there buys a bunch of goods and services in baht. Before she travels, she has made a budget, and so buys a hedge so that exchange rates don't eat more out of her dollar.

2) Alice contracts Dopesh in Bangalore to do some web development work. At the time of their agreement, Alice and Dopesh agree on a price for the service, to be paid in installments. Alice and Dopesh thus buy a hedge together to limit exposure to rate changes (or perhaps use another, more direct mechanism?), or perhaps the platform they use buys a hedge for them.

So, perhaps the bare minimum of info required for each Transaction consists of: date, value, spot rate (or difference between spot and hedge rates), vendor name, short description / note.


Why restrict hedge creation to the owner? It seems more natural to me that the client would just submit payment in the form of ETH directly to the contract, creating the hedge.

Each hedge is between a client and the counterparty (CurrencySpot). When a client buys a hedge, CS would take out a corresponding counter-hedge, or have enough money on hand to cover some expected loss. I figured that CS or a similar counterparty would be the contract owner, and thus responsible for its creation, execution, and resolution. The counterparty can keep track of its hedge book by looking for transactions where it is the owner. I'm open to suggestions on how to disintermediate this process.


What's the purpose of the "active" flag? If a user submits payment to the contract when she creates a hedge, shouldn't it automatically become active as soon as hedgeStart occurs?

The purpose of the 'active' flag is primarily to govern the behavior of adding transactions. So, if the contract is active, then it's OK to add transactions to the hedge's record. If not, then an attempt to add transactions will result in an error. So, from that perspective, a hedge is not 'active' the moment it's created, since the hedge could be ordered at any point in time before the coverage period begins.


Pretty sure the beneficiary argument is unnecessary here, for this and several of the following functions. The data already lives inside the Hedge struct. If recordTransaction is called directly by the customer, you can just use msg.sender.

Assuming that the contract owner and msg.sender are the same, then yes, you're right. If, however, the counterparty is the contract owner, then the _beneficiary argument is required. This is tied to a larger question of how the system ultimately designed.


I think you need to check absolute value here as it could be negative.

I figured the wallet would calculate the difference between the hedge's reference rate and the current spot rate. If the hedge currency (e.g. VND) grows stronger, then the beneficiary would be reimbursed. Otherwise, nothing happens: if one USD can buy more VND than before, then there's nothing to reimburse.

There's a couple of consequences: 1) I think we want to record valid transaction details on the blockchain during the hedge period no matter what the rate difference is so that they can be verified by anyone. I think the 'by anyone' part is not yet coded into the contract. Need to sort that out.

2) The calculation of rateDiff should be structured such that a move against the beneficiary creates a positive number, and negative otherwise. I don't think absolute value would work, then.


It's pretty critical that we add unit tests.

Absolutely agreed. The JS frontend that I'm building is more for me to keep track of behavior and how things are working, as well as just practice more.


It probably makes sense to add some accounting-related functions for the owner to check e.g. the total active hedges, the total current payable amount, the total amount already paid out, etc.

Possibly. I wonder if the accounting functions should instead be built into the wallet application instead of on the blockchain, and instead limit contract functions in this area to getters.


"Ref rate" and "spot rate" terms are a little confusing. There are actually three currencies at play here: the home currency, the hedge currency, and ETH. Which pair does each represent?

I understand your logic, and you raise a key point. Your statement implies that a user pays for the hedge via ETH, which is paid when msg.sender creates the contract. The process I had in mind when coding this was as follows: 1) Beneficiary buys a hedge (basically a forward contract) from Service / Wallet. Payment could be made in ETH, BTC, or any currency that Beneficiary would like. Beneficiary specifies the desired terms of the hedge contract. 2) Service then takes the hedge (or bundles multiple hedges) to the FX forwards market and finds a Counterparty to buy (or sell; maybe the Counterparty needs to be paid) the other side of the contract. 3) At the end of the period, Beneficiary is reimbursed by Service / Wallet.

Fundamentally, you're right; this model is about being an intermediary between the big FX forwards market and a retail user. An alternative method would be to simply have individuals connect directly with each other. I think difference might be the key underlying theme for many of your comments. I haven't given a more fully decentralized version enough design thought, and perhaps we should sketch out these two paradigms and compare. What do you think?