search

interledger / interledger-rs

An easy-to-use, high-performance Interledger implementation written in Rust
http://interledger.rs
Other
201 stars 70 forks source link

Should the /pay API take a destination_amount? #256

Open emschwartz opened 5 years ago

emschwartz commented 5 years ago

Currently, the /pay HTTP API endpoint accepts a source_amount and will send STREAM packets until that amount has been sent. We may want to add the option to specify a destination_amount instead (setting either amount would be supported).

The tricky part about the destination_amount-based sending is that the exchange rate could fluctuate -- for good or malicious reasons -- while the payment is being sent, potentially causing the sender to send a lot of money.

We could add a max_source_amount field along with it, but it's not totally clear how the user or the node would set that. The best option might be to use STREAM to get a quote first and then base the max_source_amount on the first observed exchange rate + some slippage. Alternatively, if we want to make sure the user is aware of the current exchange rate, we could make the max_source_amount a required field if you set the destination_amount so that the user has to explicitly call the quote endpoint (https://github.com/interledger-rs/interledger-rs/issues/255) first.

kincaidoneil commented 5 years ago

To protect against malicious receivers, I think the STREAM client needs to always check the rate against an external provider. This would make specifying the destination amount much more secure, since single path probing is very easy for the recipient to manipulate.

(One simple idea is to just use the same rate provider the router uses).

One important note: with this exchange rate check, if I'm specifying a destination_amount, the endpoint must also require the destination asset (presumably I already knew it). One attack vector or footgun is I think Dave accepts ETH, so I say "send destination amount of 1." It resolves the payment pointer, and it turns out the destination asset is... BTC! So the STREAM client calculates my source rate to BTC and sends many multiples what I originally intended to send.

The best option might be to use STREAM to get a quote first and then base the max_source_amount on the first observed exchange rate + some slippage.

I agree we should also support a max_source_amount field, but unless the user manually checks some external rate provider, that's still insecure, since it would be trivial to identify and manipulate the probing packets

kincaidoneil commented 5 years ago

Also... when/if we add that exchange rate logic, we definitely need to fix this: https://github.com/interledger-rs/interledger-rs/blob/master/crates/interledger-stream/src/client.rs#L121

Right now intermediary connectors can steal ALL the money!