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

docs(fee): disambiguate fees #78

Merged
merged 4 commits into from Aug 30, 2022
Merged

docs(fee): disambiguate fees #78

merged 4 commits into from Aug 30, 2022

Conversation

cajubelt
Copy link
Contributor

@cajubelt cajubelt commented Aug 17, 2022

Previously, the meaning of fee was ambiguous because cryptoAmount and fiatAmount denoted an exchange rate that was singular for the quote, but fee could vary per account type if multiple account types are accepted for the same quote.

This PR disambiguates the meaning of fee by making it singular as well (one fee per quote).

As a side effect, providers will not be able to offer different fees per account type for the same quote. However, this was already the case because of how cryptoAmount and fiatAmount are defined, as the exact amount in/out for a transfer (fees and all!). This spec change is designed to avoid a breaking change on the CICO provider side by simply moving an optional field to a different place within the same quote response.

An alternative approach would be to move cryptoAmount and fiatAmount to where fee previously was, nested under fiatAccount in the quote response. This would allow for different fees per fiat account type, but would create a breaking change that is onerous for wallets and, likely, CICO providers. I'd prefer not to do this, since I'm not aware of any providers who rely on being able to levy different fees for different account types right now anyway. Of course, this is always an option for a v2 version of the spec.

moved feeFrequency and feeType to `quote`, for internal consistency. added clarity to what fee means.
@cajubelt cajubelt changed the title docs(fee): one fee per quote docs(fee): disambiguate fees Aug 17, 2022
@cajubelt cajubelt merged commit 6c778cd into main Aug 30, 2022
@cajubelt cajubelt deleted the cajubelt/quote-fees branch August 30, 2022 18:40
cajubelt added a commit to fiatconnect/fiatconnect-types that referenced this pull request Aug 30, 2022
cajubelt added a commit to fiatconnect/fiatconnect-sdk that referenced this pull request Sep 1, 2022
version bumped fiatconnect-types to keep in sync with [spec change](fiatconnect/specification#78)
cajubelt added a commit to fiatconnect/fiatconnect-sdk that referenced this pull request Sep 1, 2022
version bumped fiatconnect-types to keep in sync with [spec change](fiatconnect/specification#78)
cajubelt pushed a commit that referenced this pull request Sep 9, 2022
This addes to change in documentation in #78 by moving the
`QuoteResponse` fee entries to under `quote` from nested under
`fiatAccount`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants