Add unit tests for CPrivateSend::IsCollateralAmount #3310
Conversation
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
- The choice for "bad" values looks kind of arbitrary.. why not
0.00009999(1 duff too small) and0.00040001(1 duff too large)? Could also add0.00100000(10x min collateral) and0.00100001(the smallest denom), that would be ok-ish/kind of reasonable. - I don't think touching whitespaces in completely unrelated parts of code is a good idea just because btc (presumably) does it this way tbh. Would rather see this part removed.
Signed-off-by: Pasta <pasta@dashboost.org>
PastaPastaPasta
force-pushed
the
privatesend-unit-tests
branch
from
14173ee to
fce6483
Compare
|
split indentation into a separate commit adjusted the bad collateral values based on your feedback |
|
@PastaPastaPasta I think the point is that whitespace is not related to this change, not necessarily the commits. https://github.com/dashpay/dash/blob/master/CONTRIBUTING.md#pull-request-philosophy
|
|
Well, I figured I could either put it in a separate commit and leave it in this PR (and we merge without squashing), or I could put it in a separate branch and open a new PR 😂 I don't care which one I do, this route just seemed easier |
|
Well personally I prefer to review only related changesets so when I get to here it gets confusing and unrelated to the PR purpose: https://github.com/dashpay/dash/pull/3310/files#diff-9b2cfef005c1c5be84e5895e91be56dbL52 |
PastaPastaPasta
force-pushed
the
privatesend-unit-tests
branch
from
fce6483 to
6912c08
Compare
|
dropped that commit |
nmarley
left a comment
nmarley
left a comment
There was a problem hiding this comment.
dropped that commit
Thanks, looks much cleaner now. Just one more nit on order of filenames in the list.
Signed-off-by: Pasta <pasta@dashboost.org>
UdjinM6
changed the title
|
Can we add 16 milestone? |
* add privatesend collateral tests Signed-off-by: Pasta <pasta@dashboost.org> * move privatesend_tests.cpp in Makefile.test.include Signed-off-by: Pasta <pasta@dashboost.org>
4 participants
This adds a test for CPrivateSend::IsCollateralAmount
Most of the other PrivateSend features are difficult to do since they are wallet based, but I am looking into that.
Also, the indentation of test_dash.cpp has been adjusted since I had to adjust the file to get the test to work. (This is in line with how bitcoin does formatting fixes, but I guess I could revert that if need be)