search

dtr-org / unit-e

A digital currency for a new era of decentralized trust
https://unit-e.io
MIT License
45 stars 15 forks source link

Minor adjustments for the tests and codestyle #1066

Closed dsaveliev closed 5 years ago

dsaveliev commented 5 years ago

During the #1062 a couple of minor problems was found:

Signed-off-by: Dmitry Saveliev dima@thirdhash.com

cmihai commented 5 years ago

GetVirtualTransactionSize must have a sigOpCost as an argument. Right now this test passes just by coincidence.

I think it's not coincidental that the test passes, since the scripts used in the test have a sigop cost of 0 anyway. It doesn't hurt to add the parameter, but Bitcoin doesn't have it, so maybe we should keep everything as it is?

castarco commented 5 years ago

Although this PR is quite small, I think we should separate the fix to the p2p_fingerprint test from the style changes.

dsaveliev commented 5 years ago

I think it's not coincidental that the test passes, since the scripts used in the test have a sigop cost of 0 anyway. It doesn't hurt to add the parameter, but Bitcoin doesn't have it, so maybe we should keep everything as it is?

Not quite, TestMemPoolEntryHelper is initialized with sigOpCost = 4: https://github.com/dtr-org/unit-e/blob/c85d170adbe575b94000d0be3b5b9fb5f238740f/src/test/test_unite.h#L122 which goes further to CTxMemPoolEntry https://github.com/dtr-org/unit-e/blob/c85d170adbe575b94000d0be3b5b9fb5f238740f/src/test/test_unite.cpp#L169 which finally affects indexed_transaction_set https://github.com/dtr-org/unit-e/blob/c85d170adbe575b94000d0be3b5b9fb5f238740f/src/txmempool.cpp#L100 and finally affects the TX order. So, here we have a green test just because of fee based on nWeight, which match to the value, calculated inside of indexed_transaction_set: https://github.com/dtr-org/unit-e/blob/365cf0d4b5d580f5a202b519ab316ac1ea28cf0a/src/policy/policy.cpp#L265