Closed kincaidoneil closed 5 years ago
Regarding the asset scale, I do not see a clear benefit by making it configurable on a per request basis. IMO it should be a parameter on the settlement engine and the node's account.
Also, I'd expect that it's set to the amount of decimals of the currency, or some multiple of that such that you are able to make payments which are tinier than the minimum denomination (cf sub-sat payments in lightning)
e.g my account is configured to run with 9 decimals (1 satoshi), but the settlement engine is configured with 18. As such, I'd need to send 1e9 units (which would make 1 sat) before settlement happens
Regarding the asset scale, I do not see a clear benefit by making it configurable on a per request basis. IMO it should be a parameter on the settlement engine and the node's account.
If scale was configured you'd need a way for the SE to be made aware of the scale used by the connector and the connector to be made aware of the scale used by the SE.
This implies either:
IMO, it's simpler to just put the scale in the API calls
Added an initial set of edits putting it in the context of Interledger and explaining how the accounting operates.
All comments regarding content prior to the "Settlement Engine HTTP API" heading hopefully should be addressed; most things after that are yet-to-be addressed.
@emschwartz @adrianhopebailie Is there a preference on this?
I think we should just go with PUT
. Its really a nit picky issue that doesn't functionally change anything
Other than giving some example, we shouldn't repeat "asset or currency" all over. Instead, we should just define "asset" as being lots of things, and then use "asset" everywhere.
We shouldn't define anything in here in terms of "money" (assets can be any fungible thing) -- though I think it's good to actually use currency in our examples, which is what's there already.
Sounds good to me 👍
(I did like that "monetary unit" / "fractional monetary unit" are dictionary phrases, but agree they may not be general enough).
Alright! I think this is much improved and ready for a re-review. Changes from the original draft include:
Can we merge this now? This has been open for a really long time and we'd like to be able to refer to it. All remaining discussions can happen in follow-up PRs
This standardizes an agreed API between @emschwartz, @adrianhopebailie, @matdehaast, @sappenin and I, with some minor changes I am proposing from the existing Swagger definition.
Areas that may need improvement:
Thoughts? What's unclear or needs better explanation? What's still "unaccounted" for? (no pun intended...)