-
Notifications
You must be signed in to change notification settings - Fork 47
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
Few minor suggestions #75
Comments
Thanks for the feedback and I'm really glad RP2 is useful to you (please consider leaving a star on RP2 and DaLI's pages, if you'd like to support these projects)! Here are my thoughts on the issues you raised:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Thank you for this library, I've found it extremely useful. I just have few minor suggestions
Fee only transactions are introduced to address issues #16 and #4. I was wondering whether something like
fee
,fee_currency
,fee_fiat_value
,fee_fiat_value_currency
could be used instead. With this, there is no need to create two transactions for a single transaction. Also, I found current fee structure design slightly confusing as if I am correct, one can pass either onlycrypto_fee
orfiat_fee
, but not both, which is a deviation from the current library standard. Throughout the library, the keyword fiat is used to address fiat value and both crypto and fiat related params get populated. E.gcrypto_in
andfiat_in
related params all get populated instead of either onlycrypto_in
orfiat_in
getting populated. I think using something likefee
,fee_currency
,fee_fiat_value
,fee_fiat_value_currency
, will help the fee structure also adhere to the current practice and make the code easier to follow.asset
andfiat
instead ofasset
,crypt
, andfiat
Currently there are three main keywords,
asset
,crypto
, andfiat
. However, I think using onlyasset
andfiat
seems quite fine. So I was wondering whether we could just useasset
,asset_in
,asset_sent
, etc instead of having bothasset
andcrypto_in
,crypto_sent
, etc. This will also make code easier to follow for a few corner cases wherecrypto_in
orcrypto_out
gets populated even whenasset
is a fiat currencyThe text was updated successfully, but these errors were encountered: