-
-
Notifications
You must be signed in to change notification settings - Fork 122
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
Implement rounding options for "Pay-By-Mail" payment type. #1059
base: master
Are you sure you want to change the base?
Conversation
Ready for review. I considered various use cases and went through multiple scenarios including:
|
Looks like the in-built tests need updated. You should observe the errors locally by running |
Unless you're in some weird state, you should be able to simply |
Thanks |
proto/src/main/proto/pb.proto
Outdated
NodeAddress arbitrator_signer = 36; | ||
bytes arbitrator_signature = 37; | ||
repeated string reserve_tx_key_images = 38; | ||
int32 round_to = 11; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't change the numbering of existing persistent types, so this should be appended to the end and the other fields unchanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved it to the end and resolved issues.
ea2fb5d
to
2a4d224
Compare
@woodser I addressed both of the issues you mentioned and I re-tested with normal and non-traditional (i.e. BCH) and everything seems to be working. If possible, please double-check thank you. |
@woodser Also, I think I want to try a ticket for the backend and was looking for a straight-forward, fairly easy one to start with. Do you think #1115 is a good candidate? If not, which one would you recommend to start with? No guarantees I will work on anything but just in case I decide to do it :) |
#1014
Adds a combo box with rounding options to the nearest integers 1, 5, 10 or 20.
Tested on 1.0.7 seems to be working fine.
All other relevant totals (i.e. Amount of XMR to sell, Min amount of XMR) are also updated as well.
There is one slight issue where the new combo box seems to re-render out-of-order so I will prob. submit another commit to resolve that but its not a functional issue.