Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should the /pay API take a destination_amount? #256

Open
emschwartz opened this issue Aug 30, 2019 · 2 comments
Open

Should the /pay API take a destination_amount? #256

emschwartz opened this issue Aug 30, 2019 · 2 comments

Comments

@emschwartz
Copy link
Member

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 (#255) first.

@kincaidoneil
Copy link
Member

kincaidoneil commented Aug 30, 2019

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
Copy link
Member

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants