003929c0d55532038d5bf6fc0ff4a20628710fae refactor: add [[noreturn]] attribute where applicable (fanquake)
Pull request description:
Similar to #10843. We could build with `-Wmissing-noreturn`, however that would also mean modifying something like `--suppress-external-warnings` to suppress warnings for leveldb, which I don't think we want to do. In any case, the functions where this is applicable are only added/removed very rarely.
ACKs for top commit:
vasild:
ACK 003929c0d55532038d5bf6fc0ff4a20628710fae
Tree-SHA512: 33dfa6547d6b84f38a941f24d4c2effe8fde7b93dbc0b27a9309716420e4a879fdbe689d789fa5439d65f5f78292f89fd9dc1b61c97acf69316dfed954086705
f7264fff0a098f8b6354c7373b8790791c25dd07 Check if Cjdns address is valid (Lucas Ontivero)
Pull request description:
CJDNS addresses start with 0xFC and for that reason if a netaddr was unserialized with network type cjdns but its address prefix is not 0xFC then that netaddr should be considered invalid.
ACKs for top commit:
jonatack:
ACK f7264fff0a098f8b6354c7373b8790791c25dd07
practicalswift:
cr ACK f7264fff0a098f8b6354c7373b8790791c25dd07: patch looks correct
theStack:
ACK f7264fff0a098f8b6354c7373b8790791c25dd07 ✔️
Tree-SHA512: 5300df2ffbbd69c40271b6d8df96cca98eb3e1ee76aba62c9c76025d083788ab1f1332775890c63b06e02ca593863a867cd53956bce5962383e8450487898669
fa2204f6adef493079d1ca5148b0fdc2b55816e6 streams: Accept URef obj for VectorReader unserialize (MarcoFalke)
Pull request description:
Missed in commit 172f5fa738. An URef may collapse into an LRef or RRef depending on context. There is no reason to forbid RRef in `VectorReader::operator>>`, so add it for consistency.
ACKs for top commit:
ryanofsky:
Code review ACK fa2204f6adef493079d1ca5148b0fdc2b55816e6, just expanded test since last review
Tree-SHA512: 09ff4e8a918e15b08cebd8c125d37e78bfb3a635c38546fc8454a97a882b2c81c55ef552243617e78744799d31127e6fbf78c4e319c030480b370aab6f38b645
9a19c9ada53e84d1ee8521b48f656faad48b737a Always define the raii_event_tests test suite (Craig Andrews)
Pull request description:
The test suite must always be defined (even when EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is not defined) so that the test harness doesn't fail due to not being able to find the raii_event_tests test.
This improves upon 95f97f4 actually fixing https://github.com/bitcoin/bitcoin/issues/9493
ACKs for top commit:
MarcoFalke:
ACK 9a19c9ada53e84d1ee8521b48f656faad48b737a 🎹
Tree-SHA512: 3c42f17a9b5d56c8841f3aa9ac19da91c10aff210026266f31f7eb98a62528740d7c518c121452b68e8f801d6c80ecfb627d137ec6ed533289fa3beb08b4f176
b3c4d9bac6910f6c28f6008c5ca7064a315fd2a5 test: rename test suite name "tx_validationcache_tests" to match filename (Sebastian Falbesoner)
Pull request description:
Quoting `src/test/README.md`, '`Adding test cases`':
> "The file naming convention is `<source_filename>_tests.cpp`
> and such files should wrap their tests in a test suite
> called `<source_filename>_tests`."
Currently the unit test source file `txvalidationcache_tests.cpp` contains a unit test suite with the name `tx_validationcache_tests`, which is fixed by this PR. The following shell script shows that this is the only mismatch and for all other unit test source files the test suite names are correct:
```
#!/bin/bash
shopt -s globstar
for test_full_filename in **/*_tests.cpp; do
test_name_file=`basename $test_full_filename .cpp`
test_name_suite=`sed -n "s/^.*TEST_SUITE(\(.*_tests\).*$/\1/p" $test_full_filename`
if [ $test_name_file != $test_name_suite ]; then
echo "TestFilename: $test_name_file != TestSuitname: $test_name_suite"
fi
done
```
ACKs for top commit:
practicalswift:
ACK b3c4d9bac6910f6c28f6008c5ca7064a315fd2a5 -- expected naming is better than unexpected naming :)
kristapsk:
ACK b3c4d9bac6910f6c28f6008c5ca7064a315fd2a5
Tree-SHA512: 29d409b1eb22057ee2cc407508e2580d2bc03f412401df11b8ecf77be5ada6bda8f7d2cb5338c5e079490fa12242c1fd6230a09e47252c1b0d9fe535a828ca4c
a5a2654bbc43b5c208418872e5d4c0acbadda5de test: add missing #include to fix compiler errors (Karl-Johan Alm)
Pull request description:
I believe this fixes AppVeyor errors in master. Will close if that is not the case.
Closes#17976
ACKs for top commit:
fanquake:
ACK a5a2654bbc43b5c208418872e5d4c0acbadda5de - glad the fix turned out to be this simple.
Tree-SHA512: 8fed8c2050d0f435e7ed6db1c2927d5daccc3540c6cf9e57e644d0931a740359550a5270201c893f40200960101f11cd039d807d4ed0190f1e0c674f86fd7290
529d332fbfe633d60845a97e1a06f552bd63d0d4 test: add IsRFC2544 tests (Mark Tyneway)
419ef3b7cc04e3ab26252d7024da847dfd5ab1a3 CNetAddr: fix IsRFC2544 comment (Mark Tyneway)
Pull request description:
The comment describing the functionality of `CNetAddr::IsRFC2544` is incorrect.
46d6930f8c/src/netaddress.h (L57)
It should actually read `198.18.0.0/15` based on [RFC 3330](https://tools.ietf.org/html/rfc3330):
```
198.18.0.0/15 - This block has been allocated for use in benchmark
tests of network interconnect devices. Its use is documented in
[RFC2544].
```
See [RFC 2544](https://tools.ietf.org/html/rfc2544) here.
See the implementation here:
47d981e827/src/netaddress.cpp (L142-L145)
This PR also adds tests for the minimum and maximum values that are valid RFC 2544 addresses.
ACKs for top commit:
practicalswift:
ACK 529d332fbfe633d60845a97e1a06f552bd63d0d4
laanwj:
ACK 529d332fbfe633d60845a97e1a06f552bd63d0d4
promag:
ACK 529d332fbfe633d60845a97e1a06f552bd63d0d4, nit could squash.
jonatack:
ACK 529d332fbfe633d60845a97e1a06f552bd63d0d4
Tree-SHA512: 954a9582856d77564e0ea5fd2e3d287d0cfc4ecfe0588115692d01005e8ca7ad8ab20ff390ded867dc91af2bfb758d4e73a336e6c0b7798846c30a6d69b8ae3d
76303f65f92a0fbe9a90c0e807554a6daa860636 test: add unit test for non-standard txs with wrong nVersion (Dominik Spicher)
Pull request description:
Takes care of one of the missing cases of #17394: nVersion must be within the allowed range.
ACKs for top commit:
instagibbs:
ACK 76303f65f9
Tree-SHA512: 94464f781cf70a5616f7cea2014ae0a97a338c34411cc989c60389de2ce00368374811db78c919bda30e0ebf341fb830998a5e97c124dd8afc8feb726cedfd3a
70ed2ab7ef9e7ebf56f77b7c410a345ff455938f Add unit test for DB creation with unicode path (Aaron Clauson)
Pull request description:
An issue arose when attempting to switch back to the main repo version of leveldb when the bitcoin data directory uses a unicode path. The leveldb windows file IO wrapper was using the *A ANSI win32 calls instead of the Unicode *W ones. This unit test will catch if the path created by leveldb doesn't match what we're expecting. For more info see https://github.com/google/leveldb/issues/755.
ACKs for top commit:
laanwj:
ACK 70ed2ab7ef9e7ebf56f77b7c410a345ff455938f
Tree-SHA512: fc6dbd3aa26a439016e63e8d4d931f218ce99094fc7887a13b54562ad4133047020288ecbcd622a8309f422ee1eda5df50bcb8c8e44442af36ed57b22c069004
fab7d14ea5a4305317d66f35beb3225a07823d42 test: Check that wait_until returns if time point is in the past (MarcoFalke)
Pull request description:
Add an explicit regression test for the condvar bug (#18227), so that this doesn't happen again
ACKs for top commit:
laanwj:
ACK fab7d14ea5a4305317d66f35beb3225a07823d42
Tree-SHA512: 6ec0d0b3945cae87a001e367af34cca1953a8082b4a0d9f8a20d30acd1f36363e98035d4eb173ff786cf6692d352d41f960633415c46394af042eb44e3b5ad71
5710dadf9b282524fddf42c682351cd8022ed7bf test: fix script_p2sh_tests OP_PUSHBACK2/4 missing (kodslav)
Pull request description:
Cleans up #15140 which fixes commit 6b25f29a91 where opcodes were lost in translation.
ACKs for top commit:
laanwj:
code review ACK 5710dadf9b282524fddf42c682351cd8022ed7bf
Tree-SHA512: 3f7fbcaf0dd199626d9ec9fdf3c5b5c5c2a91c4cfe81fae5b1d5662a48e52cf4bd27c94f8f42ebdfe7a076c5d600ada5661a6902b03eb5dc3dc953f4524345ac
4896bacc00549c14f3284f5a2b61fb848ac31be0 Add testcase to simulate bitcoin schema in leveldb (MapleLaker)
Pull request description:
Resurrecting #14125 with updates based on comments of closed PR
ACKs for top commit:
laanwj:
ACK 4896bacc00549c14f3284f5a2b61fb848ac31be0
dongcarl:
ACK 4896bacc00549c14f3284f5a2b61fb848ac31be0
Tree-SHA512: 3290ea7e1e998901d5ee8921d1d76cec399cae30ac1911a45b86826afed47cee1acf92bd6438f1fa11ed785a3b17abdcb1c169bc0419945eda9fe4c089d0b6eb
efd2474d17098c754367b844ec646ebececc7c74 util: CBufferedFile fixes (Larry Ruane)
Pull request description:
The `CBufferedFile` object guarantees its user is able to "rewind" the data stream (that's being read from a file) up to a certain number of bytes, as specified by the user in the constructor. This guarantee is not honored due to a bug in the `SetPos` method.
Such rewinding is done in `LoadExternalBlockFile()` (currently the only user of this object), which deserializes a series of `CBlock` objects. If that function encounters something unexpected in the data stream, which is coming from a `blocks/blk00???.dat` file, it "rewinds" to an earlier position in the stream to try to get in sync again. The `CBufferedFile` object does not actually rewind its file offset; it simply repositions its internal offset, `nReadPos`, to an earlier position within the object's private buffer; this is why there's a limit to how far the user may rewind.
If `LoadExternalBlockFile()` needs to rewind (call `blkdat.SetPos()`), the stream may not be positioned as it should be, causing errors in deserialization. This need to rewind is probably rare, which is likely why this bug hasn't been noticed already. But if this object is used elsewhere in the future, this could be a serious problem, especially as, due to the nature of the bug, the `SetPos()` _sometimes_ works.
This PR adds a unit test for `CBufferedFile` that fails due to this bug. (Until now it has had no unit tests.) The unit test provides good documentation and examples for developers trying to understand `LoadExternalBlockFile()` and for future users of this object.
This PR also adds code to throw an exception from the constructor if the rewind argument is not less than the buffer size (since that doesn't make any sense).
Finally, I discovered that the object is too restrictive in one respect: When the deserialization methods call this object's `read` method, a check ensures that the number of bytes being requested is less than the size of the buffer (adjusting for the rewind size), else it throws an exception. This restriction is unnecessary; the object being deserialized can be larger than the buffer because multiple reads from disk can satisfy the request.
ACKs for top commit:
laanwj:
ACK ~after squash.~ efd2474d17098c754367b844ec646ebececc7c74
mzumsande:
I had intended to follow up earlier on my last comment, ACK efd2474d17098c754367b844ec646ebececc7c74. I reviewed the code, ran tests and did a successful reindex on testnet with this branch.
Tree-SHA512: 695529e0af38bae2af4e0cc2895dda56a71b9059c3de04d32e09c0165a50f6aacee499f2042156ab5eaa6f0349bab6bcca4ef9f6f9ded4e60d4483beab7e4554
41d484d5c88f057e75cfdd8b88587b9a12a61744 doc: Delete stale URL in test README (Michael Folkson)
Pull request description:
The resource on the Boost unit test framework previously linked to in src/test/README.md was a stale URL.
Instead of deleting it, I've replaced it with an alternative resource on the framework on [boost.org](https://www.boost.org/doc/libs/1_45_0/libs/test/doc/html/utf/tutorials.html).
ACKs for top commit:
promag:
ACK 41d484d5c88f057e75cfdd8b88587b9a12a61744.
hebasto:
ACK 41d484d5c88f057e75cfdd8b88587b9a12a61744, the removed link is really obsolete.
fanquake:
ACK 41d484d5c88f057e75cfdd8b88587b9a12a61744 - Thanks.
Tree-SHA512: 764f12548441bde615f77b7a2ca7c5188b4ab936972d16b84960fbd8604d4cbd224415bc59ce839e7e63293aa84fd97f31a69e38734e531231cdb0e148d2e1bd
5c3c24cf9eab7bf19a9201599e93955cace5c154 test: remove redundant setup in addrman_tests (zenosage)
Pull request description:
#10765 make this default behavior. No reason to keep these line.
Top commit has no ACKs.
Tree-SHA512: 545eea9c2d0741a75708f288f2c8752534ecaa6d54a9d014ef9afa295b0d075007704b64809eec090023703f47753e8ec755d22c9ccecf57b75f6898f6b708dd
f8995807e4 tests: Make coins_tests/updatecoins_simulation_test deterministic (practicalswift)
Pull request description:
Make `coins_tests/updatecoins_simulation_test` deterministic.
Before:
```
$ contrib/devtools/test_deterministic_coverage.sh 1000
[2019-06-15 05:36:20] Measuring coverage, run #1 of 1000
[2019-06-15 05:38:05] Measuring coverage, run #2 of 1000
[2019-06-15 05:39:49] Measuring coverage, run #3 of 1000
[2019-06-15 05:41:38] Measuring coverage, run #4 of 1000
[2019-06-15 05:43:16] Measuring coverage, run #5 of 1000
...
[2019-06-16 18:25:23] Measuring coverage, run #880 of 1000
[2019-06-16 18:27:12] Measuring coverage, run #881 of 1000
[2019-06-16 18:29:33] Measuring coverage, run #882 of 1000
[2019-06-16 18:33:00] Measuring coverage, run #883 of 1000
[2019-06-16 18:35:32] Measuring coverage, run #884 of 1000
The line coverage is non-deterministic between runs. Exiting.
The test suite must be deterministic in the sense that the set of lines executed at least
once must be identical between runs. This is a necessary condition for meaningful
coverage measuring.
--- gcovr.run-1.txt 2019-06-15 05:38:05.282359029 +0200
+++ gcovr.run-884.txt 2019-06-16 18:37:23.518298374 +0200
@@ -269,7 +269,7 @@
test/bloom_tests.cpp 320 320 100%
test/bswap_tests.cpp 13 13 100%
test/checkqueue_tests.cpp 223 222 99% 169
-test/coins_tests.cpp 478 472 98% 52,68,344-345,511,524
+test/coins_tests.cpp 478 474 99% 52,68,511,524
test/compilerbug_tests.cpp 18 18 100%
test/compress_tests.cpp 27 27 100%
test/crypto_tests.cpp 268 268 100%
@@ -401,5 +401,5 @@
zmq/zmqpublishnotifier.h 5 0 0% 12,31,37,43,49
zmq/zmqrpc.cpp 23 3 13% 16,18,20,23,33-35,37,40-47,51,62,64-65
------------------------------------------------------------------------------
-TOTAL 53323 28305 53%
+TOTAL 53323 28307 53%
------------------------------------------------------------------------------
```
After:
```
$ contrib/devtools/test_deterministic_coverage.sh 1000
[2019-06-15 05:36:20] Measuring coverage, run #1 of 1000
[2019-06-15 05:38:05] Measuring coverage, run #2 of 1000
[2019-06-15 05:39:49] Measuring coverage, run #3 of 1000
[2019-06-15 05:41:38] Measuring coverage, run #4 of 1000
[2019-06-15 05:43:16] Measuring coverage, run #5 of 1000
...
$
```
ACKs for commit f89958:
MarcoFalke:
ACK f8995807e4f08fd0266899e3e227903f06da6ab1 (checked that the randomness state of g_insecure_rand_ctx is the same after three test runs)
Tree-SHA512: 796d362b050c5750e351de1126b62f0f2c8e2d712cf01b6e1a3e2cc6ef92fa68439a32fc24c76d34bce4d553aee4ae4ea88a036c56eb9e25979649a19c59c3e5
2c448d6bc7 parameterize hard coded numbers referring to miner conf window (Jordan Baczuk)
Pull request description:
Replace hard coded values (eg. 2016) with `mainnetParams.nMinerConfirmationWindow` where appropriate. This parameterizes hard coded values in the unit test that refer to the `Miner Confirmation Window`, which currently is `2016`. This includes values not exactly 2016 but which were derived from it. Also changed `int` to `uint32_t` where appropriate to avoid compiler warnings. This makes one source of truth, and also helps people who might be adjusting this value in testing so the unit tests don't break.
ACKs for commit 2c448d:
Tree-SHA512: 9262e0b89c1baf7857b49fe2221b2b00f948f61317b321c4871a9182a86d6f8aadeb59d6b133e8a213cc9b31b4a417888fb1ad31caef16ccbbab1de33c4b8459
3ccfa34b32 convert C-style (void) parameter lists to C++ style () (Arvid Norberg)
Pull request description:
In C, an empty parameter list, `()`, means the function takes any arguments, and `(void)` means the function does not take any parameters.
In C++, an empty parameter list means the function does not take any parameters.
So, C++ still supports `(void)` parameter lists with the same semantics, why change to `()`?
1. removing the redundant `void` improves signal-to-noise ratio of the code
2. using `(void)` exposes a rare inconsistency in that a template taking a template `(T)` parameter list, cannot be instantiated with `T=void`
Tree-SHA512: be2897b6c5e474873aa878ed6bac098382cd21866aec33752fe40b089a6331aa6263cae749aba1b4a41e8467f1a47086d32eb74abaf09927fd5a2f44a4b2109a
# Conflicts:
# src/qt/rpcconsole.cpp
f6ee177f7 Remove unused AES-128 code (practicalswift)
Pull request description:
Remove unused AES-128 code.
As far as I can tell this AES-128 code has never been in use in the project (outside of testing/benchmarking).
The AES-256 code is used in `CCrypter::Encrypt`/`CCrypter::Decrypt` (`src/wallet/crypter.cpp`).
Trivia: 0.15% of the project's C++ LOC count (excluding dependencies) is trimmed off:
```
$ LOC_BEFORE=$(git grep -I "" HEAD~1 -- "*.cpp" "*.h" ":(exclude)src/leveldb/" ":(exclude)src/secp256k1/" ":(exclude)src/univalue/" | wc -l)
$ LOC_AFTER=$(git grep -I "" -- "*.cpp" "*.h" ":(exclude)src/leveldb/" ":(exclude)src/secp256k1/" ":(exclude)src/univalue/" | wc -l)
$ bc <<< "scale=4; ${LOC_AFTER}/${LOC_BEFORE}"
.9985
```
:-)
Tree-SHA512: 9588a3cd795a89ef658b8ee7323865f57723cb4ed9560c21de793f82d35e2835059e7d6d0705e99e3d16bf6b2a444b4bf19568d50174ff3776caf8a3168f5c85
fa694f706c37a1c512a7c346591a63c5e09ee239 test: Add tests for truncated scripts (MarcoFalke)
Pull request description:
Previously not covered by any test
Tree-SHA512: 9f99659bdf3947271074938456a2fe64f5b39fc868e9aa474cec199a536ae5d7428f1cfa7f361936b71b09ee4c426261e6b25668fa77b8416b30dbe4ddb357f0
cf4b0327ed92ca8a1533cdf6c2b0015fd9b56397 Use std::numeric_limits<UNSIGNED>::max()) instead of (UNSIGNED)-1 (practicalswift)
6b82fc59eb19004e54f910261a40d5e1b9e44b42 Use const in COutPoint class (Hennadii Stepanov)
Pull request description:
Refactoring:
- all cases of using `(uint32_t) -1` in `COutPoint` class are replaced with const;
- also all remaining instances of `(UNSIGNED)-1` transformed to `std::numeric_limits<UNSIGNED>::max()` (by @practicalswift).
Tree-SHA512: fc7fe9838b6e5136d8b97ea3d6f64c4aaa1215f4369832df432cab017396620bb6e30520a64180ceab6de222562ac11eab243a78dfa5a658ba018835a34caa19
9e0a514112 Add compile time checking for all cs_main runtime locking assertions (practicalswift)
Pull request description:
Add compile time checking for `cs_main` runtime locking assertions.
This PR is a subset of #12665. The PR was broken up to make reviewing easier.
The intention is that literally all `EXCLUSIVE_LOCKS_REQUIRED`/`LOCKS_EXCLUDED`:s added in this PR should follow either directly or indirectly from `AssertLockHeld(…)`/`AssertLockNotHeld(…)`:s already existing in the repo.
Consider the case where function `A(…)` contains `AssertLockHeld(cs_foo)` (without
first locking `cs_foo` in `A`), and that `B(…)` calls `A(…)` (without first locking `cs_main`):
* It _directly_ follows that: `A(…)` should have an `EXCLUSIVE_LOCKS_REQUIRED(cs_foo)` annotation.
* It _indirectly_ follows that: `B(…)` should have an `EXCLUSIVE_LOCKS_REQUIRED(cs_foo)` annotation.
Tree-SHA512: 120e7410c4c223dbc7d42030b1a19e328d01a55f041bb6fb5eaac10ac35cb0c5d469b9b3bda6444731164c73b88ac6495a00890672b107d9305e891571f64dd6
# Conflicts:
# src/validation.cpp
# src/validation.h
# src/wallet/feebumper.cpp
# src/wallet/rpcwallet.cpp
# src/wallet/wallet.h
# Conflicts:
# src/wallet/wallet.h
4b7091a842 Replace median fee rate with feerate percentiles (Marcin Jachymiak)
Pull request description:
Currently, the `medianfeerate` statistic is calculated from the feerate of the middle transaction of a list of transactions sorted by feerate.
This PR instead uses the value of the 50th percentile weight unit in the block, and also calculates the feerate at the 10th, 25th, 75th, and 90th percentiles. This more accurately corresponds with what is generally meant by median feerate.
Tree-SHA512: 59255e243df90d7afbe69839408c58c9723884b8ab82c66dc24a769e89c6d539db1905374a3f025ff28272fb25a0b90e92d8101103e39a6d9c0d60423a596714
e306be742932d4ea5aca0ea4768e54b2fc3dc6a0 Use 72 byte dummy signatures when watching only inputs may be used (Andrew Chow)
48b1473c898129a99212e2db36c61cf93625ea17 Use 71 byte signature for DUMMY_SIGNATURE_CREATOR (Andrew Chow)
18dfea0dd082af18dfb02981b7ee1cd44d514388 Always create 70 byte signatures with low R values (Andrew Chow)
Pull request description:
When creating signatures for transactions, always make one which has a 32 byte or smaller R and 32 byte or smaller S value. This results in signatures that are always less than 71 bytes (32 byte R + 32 byte S + 6 bytes DER + 1 byte sighash) with low R values. In most cases, the signature will be 71 bytes.
Because R is not mutable in the same way that S is, a low R value can only be found by trying different nonces. RFC 6979 for deterministic nonce generation has the option to specify additional entropy, so we simply use that and add a uin32_t counter which we increment in order to try different nonces. Nonces are sill deterministically generated as the nonce used will the be the first one where the counter results in a nonce that results in a low R value. Because different nonces need to be tried, time to produce a signature does increase. On average, it takes twice as long to make a signature as two signatures need to be created, on average, to find one with a low R.
Having a fixed size signature makes size calculations easier and also saves half a byte of transaction size, on average.
DUMMY_SIGNATURE_CREATOR has been modified to produce 71 byte dummy signatures instead of 72 byte signatures.
Tree-SHA512: 3cd791505126ce92da7c631856a97ba0b59e87d9c132feff6e0eef1dc47768e81fbb38bfbe970371bedf9714b7f61a13a5fe9f30f962c81734092a4d19a4ef33
d5f745a5c7 trivial: correct typos (Varunram)
3be70ba400 trivial: Fixed typos and cleaned up language (William Robinson)
Pull request description:
This rebases and fixes some of the outstanding nits in #13010. Let either merge quickly or close for now.
Tree-SHA512: 4cc1a5f854f2d6a19332334e2608a19e2be6b97dc09114c8186237ea77ee4b62372ebf6841a61cca548cedb47f0e6f11d4c0aba51a71949cd5aff8cef88204d6
417b6c1d2990ffc78c029442e027797d724a101f bitcoinconsensus: invalid flags should be set to bitcoinconsensus_error type, add test cases covering bitcoinconsensus error codes (Thomas Kerin)
Pull request description:
A check was added to the bitcoinconsensus verify_script codepath to ensure that callers only used _exposed_ interpreter flags. I think this error should be written to `bitcoinconsensus_err* err` and not returned by verify_script?
I modified the check so it indicates the error using *err like the others, and added tests covering the error codes.
Tree-SHA512: 8ab370e56956a7d4740f83475e6078774affd663ac92383a02b85295da550f1b4f7a7a68f32ed5c5bcb39d98e2f15ec0b76de8399887e7763eb7c1e21d131093
fa84723e73 amount: Move CAmount CENT to unit test header (MarcoFalke)
Pull request description:
`CAmount` is currently not type-safe. Exporting a constant (`CENT`) that is commonly not referred to by that name might be confusing. `CENT` is only used in two places prior to this commit (`ParseMoney` and `MIN_CHANGE`). So replace these with constants relative to `COIN` and move `CENT` to the unit test header.
Tree-SHA512: 5273e96d8664ced6ae211abde2e20bc763e6e99f89404eec02c621f29e1d235e5f9b1ade933743843fae16fc24b643f883deda9221e3d9fd31229d2ab63a914f
ed12d5df1ba52b5ef3dd3799de26bb5e1d3fc654 index: Fix for indexers skipping genesis block. (Jim Posen)
Pull request description:
This fixes a bug where indexers would skip processing of the genesis block. Preserves the current behavior of omitting genesis block transaction from the index.
Tree-SHA512: 092fd3d629bf1ef279566217c668cc913a8b8e012d811d0e544231894c49a0c0c179537ac4727c39b9bf407479541745d79c4e118db6f0795a2b848d0fe62cbf
ec3073a274bf7affe1b8c87a10f75d126f5ac027 index: Move index DBs into index/ directory. (Jim Posen)
89eddcd365e9a2218648f5cc5b9f22b28023f50a index: Remove TxIndexDB from public interface of TxIndex. (Jim Posen)
2318affd27de436ddf9d866a4b82eed8ea2e738b MOVEONLY: Move BaseIndex to its own file. (Jim Posen)
f376a4924109af2496b5fd16a787299eb039f1c8 index: Generalize logged statements in BaseIndex. (Jim Posen)
61a1226d87d80234b2be123c5cad07534c318cfb index: Extract logic from TxIndex into reusable base class. (Jim Posen)
e5af5fc6fb4658599b940d1d50853129b31b8766 db: Make reusable base class for index databases. (Jim Posen)
9b0ec1a7f9ffae816fd5ca32ff7e7559640b6f6d db: Remove obsolete methods from CBlockTreeDB. (Jim Posen)
Pull request description:
This refactors most of the logic in TxIndex into a reusable base class for other indices. There are two commits moving code between files, which may be be more easily reviewed using `git diff --color-moved` (https://blog.github.com/2018-04-05-git-217-released/).
The motivation for this is to support BIP 157 by indexing block filters.
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/bitcoin/bitcoin/13243)
<!-- Reviewable:end -->
Tree-SHA512: 0857f04df2aa920178dab2eb8e57984d8eb4d5010deca9971190358479e05b6672ccca2a08af0a7ac9fe02afb947be84cf35a3693204d0667263c6add2959cbf
