Add per-account spreads

interledger / interledger-rs

An easy-to-use, high-performance Interledger implementation written in Rust

http://interledger.rs

Other

201 stars 70 forks source link

Add per-account spreads #301

Open kincaidoneil opened 5 years ago

kincaidoneil commented 5 years ago

To enable features such as:

Different currency pairs with different spreads
Different spreads based on how much trust/volume an account has, such as high spreads for low trust/volume accounts, or charging high trust/volume accounts a lower spread
Not charging any spread to node operator's account if they're receiving money (currently they are)

Originally posted by @kincaidoneil in https://github.com/interledger-rs/interledger-rs/pull/295

gakonst commented 4 years ago

This would be done as follows:

We keep the global spread set by the operator, and we add an optional per-account spread which will get used instead of the default if present.

IMO this should be implemented as a new function in the Account trait fn spread(&self) -> Option .

Then, in https://github.com/interledger-rs/interledger-rs/blob/master/crates/interledger-service-util/src/exchange_rates_service.rs#L124, it'd call to.spread().unwrap_or(self.spread) (or from.spread()? mhm)

@emschwartz @kincaidoneil

emschwartz commented 4 years ago

(or from.spread()? mhm)

Good question. Which account would you use, or would you use both?

I'm hesitant to add this now because I don't think it'll be used any time in the near future. It can be added later if there's a good use case for it.

kincaidoneil commented 4 years ago

We keep the global spread set by the operator, and we add an optional per-account spread which will get used instead of the default if present.

👍

I don't think it'll be used any time in the near future

Depending on how we support hosted/custodial connectors, this may prove necessary.

If hosted/child accounts fulfill packets to themselves to trigger a withdrawal, they shouldn't be double-penalized by the spread (i.e. they should have 0 spread). However, operators would set a spread for accounts connected to the broader network so it could be collected on all payments.

I think this is a useful feature on its own, but alternatively, is there a way to ensure the spread doesn't apply to "reflected" packets?

gakonst commented 4 years ago

As a v1 we could make it be a boolean instead of a specific amount. Operator would add hosted/child accounts as whitelisted and would not apply the spread, or only apply the global one.

Good question. Which account would you use, or would you use both?

As done in the PR, both equally contributing to the spread

gakonst commented 4 years ago

@kincaidoneil What do you think? cc @sappenin

If we don't think this is useful, think we might want to close the PR (and/or this issue)

kincaidoneil commented 4 years ago

I think more customization of spreads will eventually be necessary (use cases above), but we should wait for more feedback before implementing.

Averaging the spread of the two accounts seems hard to reason about (how do I set a minimum spread for a given set of accounts?), but as I was trying to think of an alternative, I realized there are lot of different permutations for how we could implement it:

Do spreads for an account only apply to outgoing packets, only incoming packets, both, or should there be separate per-account spreads for outgoing & incoming packets?
Should the spreads be averaged, added 1 - (a + b), or multiplied (1 - a) * (1 - b) between accounts?

Didn't seem like there's an obvious answer, so it's probably best to wait