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

Consider renaming settlement related variables #120

Open
gakonst opened this issue Jul 1, 2019 · 5 comments
Open

Consider renaming settlement related variables #120

gakonst opened this issue Jul 1, 2019 · 5 comments
Labels

Comments

@gakonst
Copy link
Member

gakonst commented Jul 1, 2019

I think we can rename the min_balance variable to something like: credit_limit. It's slightly clearer I think.

settle_to could also be renamed to something more descriptive, not sure yet what: I'm thinking along the lines of a "safe floor" such that settlements aren't triggered too often.

Sidenote: Does it ever make sense to set settle_to above threshold? Think not?

https://github.com/emschwartz/interledger-rs/blob/master/crates/interledger-api/src/lib.rs#L61
https://github.com/emschwartz/interledger-rs/blob/master/crates/interledger-store-redis/src/account.rs#L35

@emschwartz
Copy link
Member

Credit limit seems fine for negative minimum balances but strange if you ever had a positive minimum balance (it would be a negative credit limit). Having a positive minimum balance would be one way to implement a kind of signup fee, though there might be more straightforward ways of implementing it.

"Safe floor" seems less descriptive to me. Happy to consider alternatives but settle_to seems fine to me because it is the amount you will settle the balance down to.

Sidenote: Does it ever make sense to set settle_to above threshold? Think not?

Agreed that that doesn't make sense

@emschwartz
Copy link
Member

@gakonst Do you still think we should change the names or can we close this issue?

@gakonst
Copy link
Member Author

gakonst commented Aug 15, 2019

I've got used to them at this point so let's close it.

@gakonst gakonst closed this as completed Aug 15, 2019
@kincaidoneil
Copy link
Member

Reopening this since it's confusing that the maximum balance in the settlement RFC and ilp-connector is the minimum balance in the Rust and Java connectors, since they each define positive/negative balances differently.

Evan agrees that renaming minimum to "credit limit" is clearer, but some open questions are:

  • Can we implement this in a backwards compatible way? (e.g. "minimum" may still be used as an alias for "credit limit")
  • Should credit_limit be limited to non-negative values? (I think this is the most intuitive interpretation, since other things like sign-up fees can be implemented within settlement engines)

@kincaidoneil kincaidoneil reopened this Oct 28, 2019
@emschwartz
Copy link
Member

We should make a decision on this issue before finalizing #557

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

No branches or pull requests

3 participants