aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2019-07-30Merge #16433: txmempool: Remove unused default value ↵Jonas Schnelli
MemPoolRemovalReason::UNKNOWN 0000ff0aa763e3be524ac4537a41048a26529fb2 txmempool: Remove unused default value MemPoolRemovalReason::UNKNOWN (MarcoFalke) Pull request description: The `remove*` methods set the removal reason to `UNKNOWN` by default. This is nowhere used; Except in tests, where the value doesn't matter. Fix that by removing the confusing default. ACKs for top commit: practicalswift: utACK 0000ff0aa763e3be524ac4537a41048a26529fb2 promag: ACK 0000ff0aa763e3be524ac4537a41048a26529fb2. jonasschnelli: utACK 0000ff0aa763e3be524ac4537a41048a26529fb2 Tree-SHA512: ffc8b35dd3291a81225171577c743c8bb2645638cab02960b6361174cb68afd739aaab7ab8661d65de5750d37daf16bb7eee9338958d8609093a8d46c2ada1ab
2019-07-24Refactor out translation.hHennadii Stepanov
This is a prerequisite for introducing bilingual error messages. Note: #includes are arranged by clang-format-diff.py script.
2019-07-22txmempool: Remove unused default value MemPoolRemovalReason::UNKNOWNMarcoFalke
2019-07-18test: remove redundant setup in addrman_testszenosage
2019-07-11Merge #16227: Refactor CWallet's inheritance chainWladimir J. van der Laan
93ce4a0b6fb54efb1f424a71dfc09cc33307e5b9 Move WatchOnly stuff from SigningProvider to CWallet (Andrew Chow) 8f5b81e6edae9cb22559545de63f391d97c15701 Remove CCryptoKeyStore and move all of it's functionality into CWallet (Andrew Chow) 37a79a4fccbf6cd65a933594e24e59d36e674653 Move various SigningProviders to signingprovider.{cpp,h} (Andrew Chow) 16f8096e911e4d59292240a17e2d4004f0500b9e Move KeyOriginInfo to its own header file (Andrew Chow) d9becff4e13da8e182631baa79b9794c03d44434 scripted-diff: rename CBasicKeyStore to FillableSigningProvider (Andrew Chow) a913e3f2fbeb1352fc66f334d4f5f7332ea89ad7 Move HaveKey static function from keystore to rpcwallet where it is used (Andrew Chow) c7797ec65544bd23a2e571b2892e1bf512f2a485 Remove CKeyStore and squash into CBasicKeyStore (Andrew Chow) 1b699a5083b435c2b79f3951f94ac9f967d24f6c Add HaveKey and HaveCScript to SigningProvider (Andrew Chow) Pull request description: This PR compresses the `CWallet` chain of inheritance from 5 classes to 3 classes. `CBasicKeyStore` is renamed to `FillableSigningProvider` and some parts of it (the watchonly parts) are moved into `CWallet`. `CKeyStore` and `CCrypoKeyStore` are completely removed. `CKeyStore`'s `Have*` functions are moved into `SigningProvider` and the `Add*` moved into `FillableSigningProvider`, thus allowing it to go away entirely. `CCryptoKeyStore`'s functionality is moved into `CWallet`. The new inheritance chain is: ``` SigningProvider -> FillableSigningProvider -> CWallet ``` `SigningProvider` now is the class the provides keys and scripts and indicates whether keys and scripts are present. `FillableSigningProvider` allows keys and scripts to be added to the signing provider via `Add*` functions. `CWallet` handles all of the watchonly stuff (`AddWatchOnly`, `HaveWatchOnly`, `RemoveWatchOnly` which were previously in `CKeyStore`) and key encryption (previously in `CCryptoKeyStore`). Implements the 2nd [prerequisite](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#cwallet-subclass-stack) from the wallet restructure. ACKs for top commit: Sjors: re-ACK 93ce4a0; it keeps `EncryptSecret`, `DecryptSecret` and `DecryptKey` in `wallet/crypter.cpp`, but makes them not static. It improves alphabetical includes, reorders some function definitions, fixes commit message, brings back lost code comment. instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/16227/commits/93ce4a0b6fb54efb1f424a71dfc09cc33307e5b9 Tree-SHA512: 393dfd0623ad2dac38395eb89b862424318d6072f0b7083c92a0d207fd032c48b284f5f2cb13bc492f34557de350c5fee925da02e47daf011c5c6930a721b6d3
2019-07-11Merge #15649: Add ChaCha20Poly1305@Bitcoin AEADWladimir J. van der Laan
bb326add9f38f2a8e5ce5ee29d98ce08038200d8 Add ChaCha20Poly1305@Bitcoin AEAD benchmark (Jonas Schnelli) 99aea045d688059caf89c0e485fa427bd28eddd8 Add ChaCha20Poly1305@Bitcoin tests (Jonas Schnelli) af5d1b5f4a7b56628a76af21284c258d845894f0 Add ChaCha20Poly1305@Bitcoin AEAD implementation (Jonas Schnelli) Pull request description: This adds a new AEAD (authenticated encryption with additional data) construct optimised for small messages (like used in Bitcoins p2p network). Includes: #15519, #15512 (please review those first). The construct is specified here. https://gist.github.com/jonasschnelli/c530ea8421b8d0e80c51486325587c52#ChaCha20Poly1305Bitcoin_Cipher_Suite This aims for being used in v2 peer-to-peer messages. ACKs for top commit: laanwj: code review ACK bb326add9f38f2a8e5ce5ee29d98ce08038200d8 Tree-SHA512: 15bcb86c510fce7abb7a73536ff2ae89893b24646bf108c6cf18f064d672dbbbea8b1dd0868849fdac0c6854e498f1345d01dab56d1c92031afd728302234686
2019-07-10Merge #16237: Have the wallet give out destinations instead of keysWladimir J. van der Laan
8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf Add GetNewChangeDestination for getting new change Destinations (Andrew Chow) 33d13edd2bda0af90660e275ea4fa96ca9896f2a Replace CReserveKey with ReserveDestinatoin (Andrew Chow) 172213be5b174243dc501c1103ad5fe2fee67a16 Add GetNewDestination to CWallet to fetch new destinations (Andrew Chow) Pull request description: The wallet should give out destinations instead of keys. It should be the one that handles the conversion from key to destination and the setting of the label, not the caller. In order to do this, two new member functions are introduced `GetNewDestination()` and `GetNewChangeDestination()`. Additionally, `CReserveKey` is changed to be `ReserveDestination` and represents destinations whose keys can be returned to the keypool. ACKs for top commit: instagibbs: re-utACK https://github.com/bitcoin/bitcoin/pull/16237/commits/8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf sipa: ACK 8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf. Concept ACK as this gives a much cleaner abstraction to work with, and light code review ACK. laanwj: ACK 8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf Tree-SHA512: 5be7051409232b71e0ef2c1fd1a3e76964ed2f5b14d47d06edc2ad3b3687abd0be2803a1adc45c0433aa2c3bed172e14f8a7e9f4a23bff70f86260b5a0497500
2019-07-09Add GetNewDestination to CWallet to fetch new destinationsAndrew Chow
Instead of having the same multiple lines of code everywhere that new destinations are fetched, introduce GetNewDestination as a member function of CWallet which does the key fetching, label setting, script generation, and destination generation.
2019-07-09Move various SigningProviders to signingprovider.{cpp,h}Andrew Chow
Moves all of the various SigningProviders out of sign.{cpp,h} and keystore.{cpp,h}. As such, keystore.{cpp,h} is also removed. Includes and the Makefile are updated to reflect this. Includes were largely changed using: git grep -l "keystore.h" | xargs sed -i -e 's;keystore.h;script/signingprovider.h;g'
2019-07-09scripted-diff: rename CBasicKeyStore to FillableSigningProviderAndrew Chow
-BEGIN VERIFY SCRIPT- git grep -l "CBasicKeyStore" | xargs sed -i -e 's/CBasicKeyStore/FillableSigningProvider/g' -END VERIFY SCRIPT-
2019-07-09Remove CKeyStore and squash into CBasicKeyStoreAndrew Chow
2019-07-03Merge #16294: qt: test: Create at most one testing setupWladimir J. van der Laan
faa1e0fb1712b1f94334e42794163f79988270fd qt: test: Create at most one testing setup (MarcoFalke) Pull request description: It is assumed that ideally only one BasicTestingSetup exists at any point in time for each process (due to use of globals). This assumption is violated in the GUI tests, as a testing setup is created as the first step of the `main` function and then (sometimes) another one for the following test cases. So, the gui tests create two testing setups: * `BasicTestingSetup` in `main` (added in fa4a04a5a942d582c62773d815c7e1e9897975d0) * a testing setup for individual test cases Avoid that by destructing the testing setup in main after creation and then move the explicit `ECC_Stop` to the only places where it is needed (before and after `apptests`). ACKs for top commit: laanwj: code review ACK faa1e0fb1712b1f94334e42794163f79988270fd Tree-SHA512: b8edceb7e2a8749e1de3ea80bc20b6fb7d4390bf366bb9817206ada3dc8669a91416f4803c22a0e6c636c514e0c858dcfe04523221f8851b10deaf472f107d82
2019-07-03Add ChaCha20Poly1305@Bitcoin testsJonas Schnelli
2019-07-02Merge #14193: validation: Add missing mempool locksWladimir J. van der Laan
fa2b083c3feb0522baf652045efa6b73458761a3 [test] Add test to check mempool consistency in case of reorgs (MarcoFalke) fabeb1f613653a8c1560e4a093a9b6b7a069b60b validation: Add missing mempool locks (MarcoFalke) fa0c9dbf9156d64a4b9bff858da97825369a9134 txpool: Make nTransactionsUpdated atomic (MarcoFalke) Pull request description: Take the mempool read lock during reorgs, so that we don't accidentally read an inconsistent mempool. ACKs for top commit: laanwj: code review ACK fa2b083c3feb0522baf652045efa6b73458761a3 ryanofsky: utACK fa2b083c3feb0522baf652045efa6b73458761a3 [EDIT: was ~e284e422e75189794e24fe482819d8b1407857c3~, from bad copy and paste]. Changes since last review: rebase after #15976, adding vTxHashes lock annotation, adding new commit dropping mempool lock for nTransactionsUpdated and making it atomic to avoid deadlock between mempool lock and g_best_block_mutex Tree-SHA512: cfe7777993589087753e000e3736d79d320dca412383fb77b56bef8946a04049722bf888c11b6f722adf677165185c7e58b4a269f7c5fa25e84dda375f6c8a7d
2019-06-27qt: test: Create at most one testing setupMarcoFalke
2019-06-26tests: Reduce compilation time and unneccessary recompiles by removing ↵practicalswift
unused includes in tests
2019-06-25Merge #16252: test: Log to debug.log in all unit testsWladimir J. van der Laan
fabc57e07dc34c103552cb51c9277bb48ef97739 test: Log to debug.log in all tests (MarcoFalke) fa4a04a5a942d582c62773d815c7e1e9897975d0 test: use common setup in gui tests (MarcoFalke) fad3d2a624377de4b0311e6ddd446c36dafd1ddc test: Create data dir in BasicTestingSetup (MarcoFalke) Pull request description: This makes it easier to debug a frozen test or a test that failed. To debug a failed test, remove the line `fs::remove_all(m_path_root);`. The pull is done in three commits: * Create a datadir for every unit test once (and only once). This requires the `SetDataDir` function to go away. * Use the common setup in the gui unit tests. Some of those tests are testing the init sequence, so we'd have to undo some of what the testing setup did. * Log to the debug.log in all tests ACKs for top commit: laanwj: ACK fabc57e07dc34c103552cb51c9277bb48ef97739 Tree-SHA512: 73444210b88172669e2cd22c2703a1e30e105185d2d5f03decbdedcfd09c64ed208d3716c59c8bebb0e44214cee5c8095e3e995d049e1572ee98f1017e413665
2019-06-20test: Log to debug.log in all testsMarcoFalke
2019-06-20test: Create data dir in BasicTestingSetupMarcoFalke
2019-06-19Move ismine to wallet moduleAndrew Chow
2019-06-18Merge #12324: speed up Unserialize_impl for prevectorWladimir J. van der Laan
86b47fa741408b061ab0bda784b8678bfd7dfa88 speed up Unserialize_impl for prevector (Akio Nakamura) Pull request description: The unserializer for prevector uses `resize()` for reserve the area, but it's prefer to use `reserve()` because `resize()` have overhead to call its constructor many times. However, `reserve()` does not change the value of `_size` (a private member of prevector). This PR make the logic of read from stream to callback function, and prevector handles initilizing new values with that call-back and ajust the value of `_size`. The changes are as follows: 1. prevector.h Add a public member function named 'append'. This function has 2 params, number of elemenst to append and call-back function that initilizing new appended values. 2. serialize.h In the following two function: - `Unserialize_impl(Stream& is, prevector<N, T>& v, const unsigned char&)` - `Unserialize_impl(Stream& is, prevector<N, T>& v, const V&)` Make a callback function from each original logic of reading values from stream, and call prevector's `append()`. 3. test/prevector_tests.cpp Add a test for `append()`. ## A benchmark result is following: [Machine] MacBook Pro (macOS 10.13.3/i7 2.2GHz/mem 16GB/SSD) [result] DeserializeAndCheckBlockTest => 22% faster DeserializeBlockTest => 29% faster [before PR] # Benchmark, evals, iterations, total, min, max, median DeserializeAndCheckBlockTest, 60, 160, 94.4901, 0.0094644, 0.0104715, 0.0098339 DeserializeBlockTest, 60, 130, 65.0964, 0.00800362, 0.00895134, 0.00824187 [After PR] # Benchmark, evals, iterations, total, min, max, median DeserializeAndCheckBlockTest, 60, 160, 77.1597, 0.00767013, 0.00858959, 0.00805757 DeserializeBlockTest, 60, 130, 49.9443, 0.00613926, 0.00691187, 0.00635527 ACKs for top commit: laanwj: utACK 86b47fa741408b061ab0bda784b8678bfd7dfa88 Tree-SHA512: 62ea121ccd45a306fefc67485a1b03a853435af762607dae2426a87b15a3033d802c8556e1923727ddd1023a1837d0e5f6720c2c77b38196907e750e15fbb902
2019-06-17tests: Make coins_tests/updatecoins_simulation_test deterministicpracticalswift
2019-06-11Fix spelling errors identified by codespell 1.15.0Ben Woosley
After this commit, the only remaining output is: $ test/lint/lint-spelling.sh src/test/base32_tests.cpp:14: fo ==> of, for src/test/base64_tests.cpp:14: fo ==> of, for ^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/lint-spelling.ignore-words.txt Note: * I ignore several valid alternative spellings * homogenous is present in tinyformat, hence should be addressed upstream * process' is correct only if there are plural processes
2019-06-07[test] Add test to check mempool consistency in case of reorgsMarcoFalke
2019-06-06Merge #16129: refactor: Remove unused includesMarcoFalke
67f4e9c522 Include core_io.h from core_read.cpp (practicalswift) eca9767673 Make reasoning about dependencies easier by not including unused dependencies (practicalswift) Pull request description: Make reasoning about dependencies easier by not including unused dependencies. Please note that the removed headers are _not_ "transitively included" by other still included headers. Thus the removals are real. As an added bonus this change means less work for the preprocessor/compiler. At least 51 393 lines of code no longer needs to be processed: ``` $ git diff -u HEAD~1 | grep -E '^\-#include ' | cut -f2 -d"<" | cut -f1 -d">" | \ sed 's%^%src/%g' | xargs cat | wc -l 51393 ``` Note that 51 393 is the lower bound: the real number is likely much higher when taking into account transitively included headers :-) ACKs for commit 67f4e9: Tree-SHA512: 0c8868aac59813f099ce53d5307eed7962dd6f2ff3546768ef9e5c4508b87f8210f1a22c7e826c3c06bebbf28bdbfcf1628ed354c2d0fdb9a31a42cefb8fdf13
2019-06-06Merge #15689: netaddress: Update CNetAddr for ORCHIDv2Wladimir J. van der Laan
8be3f3063 netaddress: Update CNetAddr for ORCHIDv2 (Carl Dong) Pull request description: ``` The original ORCHID prefix was deprecated as of 2014-03, the new ORCHIDv2 prefix was allocated by RFC7343 as of 2014-07. We did not consider the original ORCHID prefix routable, and I don't see any reason to consider the new one to be either. ``` Would like to know if people think this kind of thing is even worth keeping the codebase updated for. Perhaps it'd be nice to write a devtool to pull the csv from [here](https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml) and generate the code. ACKs for commit 8be3f3: laanwj: utACK 8be3f3063 ryanofsky: utACK 8be3f306338e27b3fa3ad3bfb75f822d038909a5. Only change since last review is rebasing after #15718 merge. Tree-SHA512: 7c93317f597b1a6c1443e12dd690010392edb9d72a479a8201970db7d3444fbb99a80b98026caad6fbfbebb455ab4035d2dde79bc9263bfd1d0398cd218392e1
2019-06-05Merge #15985: Add test for GCC bug 90348Wladimir J. van der Laan
58e291cfa Add test for GCC bug 90348 (Pieter Wuille) Pull request description: This adds a test for GCC bug 90348 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90348), using a test case extracted from our own `sha256d64` test in crypto_tests.cpp, which was failing on some platforms. This is based on top of #15983 to make sure the bug doesn't trigger (it does in some Travis configurations without it). ACKs for commit 58e291: Tree-SHA512: 4dc9084e92dd143a53930e42bb68e33d922a2a2b891406b259d3a0bed4511dcc49e7447a7a8e4eb793a26e3eacb188ca293b71e0e061f9b3230f8e7fcfd29525
2019-06-02Make reasoning about dependencies easier by not including unused dependenciespracticalswift
2019-06-02Add test for GCC bug 90348Pieter Wuille
2019-05-29Merge #16046: util: Add type safe GetTimeWladimir J. van der Laan
fa013664ae23d0682a195b9bded85bc19c99536e util: Add type safe GetTime (MarcoFalke) Pull request description: There are basically two ways to get the time in Bitcoin Core: * get the system time (via `GetSystemTimeInSeconds` or `GetTime{Millis,Micros}`) * get the mockable time (via `GetTime`) Both return the same type (a plain int). This can lead to (test-only) bugs such as 99464bc38e9575ff47f8e33223b252dcea2055e3. Fix that by deprecating `GetTime` and adding a `GetTime<>` that returns the mockable time in a non-int type. The new util function is currently unused, but new code should it where possible. ACKs for commit fa0136: promag: utACK fa013664. Tree-SHA512: efab9c463f079fd8fd3030c479637c7b1e8be567a881234bd0f555c8f87e518e3b43ef2466128103db8fc40295aaf24e87ad76d91f338c631246fc703477e95c
2019-05-23util: Add type safe GetTimeMarcoFalke
2019-05-21Merge #13555: Tests: parameterize adjustment period in ↵MarcoFalke
versionbits_computeblockversion 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
2019-05-20parameterize hard coded numbers referring to miner conf windowJordan Baczuk
2019-05-16Merge #14047: Add HKDF_HMAC256_L32 and method to negate a private keyWladimir J. van der Laan
8794a4b3ae4d34a4cd21a7dee9f694eef7726a4f QA: add test for HKDF HMAC_SHA256 L32 (Jonas Schnelli) 551d489416339dae8f9d896013cd060a21406e2b Add HKDF HMAC_SHA256 L=32 implementations (Jonas Schnelli) 3b64f852e400c552f031697d6a86829dc6e74bd6 QA: add test for CKey::Negate() (Jonas Schnelli) 463921bb649d644f79f9d7f0f96f10aa0d165f76 CKey: add method to negate the key (Jonas Schnelli) Pull request description: This adds a limited implementation of `HKDF` (defined by rfc5869) that supports only HMAC-SHA256 and length output of 32 bytes (will be required for v2 transport protocol). This PR also includes a method to negate a private key which is useful to enforce public keys starting with 0x02 (or 0x03) (a requirement for the v2 transport protocol). The new `CKey::Negate()` method is pretty much a wrapper around `secp256k1_ec_privkey_negate()`. Including tests. This is a subset of #14032 and a pre-requirement for the v2 transport protocol. ACKs for commit 8794a4: Tree-SHA512: 5341929dfa29f5da766ec3612784baec6a3ad69972f08b5a985a8aafdae4dae36f104a2b888d1f5d1f33561456bd111f960d7e32c2cc4fd18e48358468f26c1a
2019-05-15netaddress: Update CNetAddr for ORCHIDv2Carl Dong
The original ORCHID prefix was deprecated as of 2014-03, the new ORCHIDv2 prefix was allocated by RFC7343 as of 2014-07. We did not consider the original ORCHID prefix routable, and I don't see any reason to consider the new one to be either.
2019-05-14Merge #15988: Add test for ArgsManager::GetChainNameMarcoFalke
f6bb11fd37 Add test for ArgsManager::GetChainName (Russell Yanofsky) 4b331159df Add unit test NextString, ForEachNoDup functions (Russell Yanofsky) 05bfee3451 util_SettingsMerge test cleanup (Russell Yanofsky) Pull request description: There was some test coverage previously, but it was limited and didn't test conflicting and negated arguments. ACKs for commit f6bb11: MarcoFalke: re-utACK f6bb11fd37f8a2c985786b688ea07699ba75780e Tree-SHA512: d03596614dc48584c7a9440117b107c6abb23fd4c7fa15fb4015351ec3de08b2656bc956ce05310663675672343d7a6aff35421657f29172080c7005045680b0
2019-05-13[refactor] interfaces: Add missing LockAnnotation for cs_mainMarcoFalke
2019-05-11QA: add test for HKDF HMAC_SHA256 L32Jonas Schnelli
2019-05-10Merge #15512: Add ChaCha20 encryption option (XOR)Jonas Schnelli
2dfe27517 Add ChaCha20 bench (Jonas Schnelli) 2bc2b8b49 Add ChaCha20 encryption option (XOR) (Jonas Schnelli) Pull request description: The current ChaCha20 implementation does not support message encryption (it can only output the keystream which is sufficient for the RNG). This PR adds the actual XORing of the `plaintext` with the `keystream` in order to return the desired `ciphertext`. Required for v2 message transport protocol. ACKs for commit 2dfe27: jnewbery: Looks good. utACK 2dfe2751713c814aea53b5a7563eb74ad1baea00. jnewbery: utACK 2dfe2751713c814aea53b5a7563eb74ad1baea00 sipa: utACK 2dfe2751713c814aea53b5a7563eb74ad1baea00 ryanofsky: utACK 2dfe2751713c814aea53b5a7563eb74ad1baea00. Changes since last review are just renaming the Crypt method, adding comments, and simplifying the benchmark. Tree-SHA512: 84bb234da2ca9fdc44bc29a786d9dd215520f81245270c1aef801ef66b6091b7793e2eb38ad6dbb084925245065c5dce9e5582f2d0fa220ab3e182d43412d5b5
2019-05-09Merge #15452: Replace CScriptID and CKeyID in CTxDestination with dedicated ↵Wladimir J. van der Laan
types 78e407ad0c26190a22de1bc8ed900164a44a36c3 GetKeyBirthTimes should return key ids, not destinations (Gregory Sanders) 70946e7fee54323ce6a5ea8aeb377e2c7c790bc6 Replace CScriptID and CKeyID in CTxDestination with dedicated types (Gregory Sanders) Pull request description: The current usage seems to be an overloading of meanings. `CScriptID` is used in the wallet as a lookup key, as well as a destination, and `CKeyID` likewise. Instead, have all destinations be dedicated types. New types: `CScriptID`->`ScriptHash` `CKeyID`->`PKHash` ACKs for commit 78e407: ryanofsky: utACK 78e407ad0c26190a22de1bc8ed900164a44a36c3. Only changes are removing extra CScriptID()s and fixing the test case. Sjors: utACK 78e407a meshcollider: utACK https://github.com/bitcoin/bitcoin/pull/15452/commits/78e407ad0c26190a22de1bc8ed900164a44a36c3 Tree-SHA512: 437f59fc3afb83a40540da3351507aef5aed44e3a7f15b01ddad6226854edeee762ff0b0ef336fe3654c4cd99a205cef175211de8b639abe1130c8a6313337b9
2019-05-07Add test for ArgsManager::GetChainNameRussell Yanofsky
There was some test coverage previously, but it was limited and didn't test conflicting and negated arguments.
2019-05-07Merge #15948: refactor: rename chainActiveMarcoFalke
486c1eea86 refactoring: remove unused chainActive (James O'Beirne) 631940aab2 scripted-diff: replace chainActive -> ::ChainActive() (James O'Beirne) a3a609079c refactoring: introduce unused ChainActive() (James O'Beirne) 1b6e6fcfd2 rename: CChainState.chainActive -> m_chain (James O'Beirne) Pull request description: This is part of the assumeutxo project: Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal --- This change refactors the `chainActive` reference into a `::ChainActive()` call. It also distinguishes `CChainState`'s `CChain` data member as `m_chain` instead of the current `chainActive`, which makes it easily confused with the global data. The active chain must be obtained via function because its reference will be swapped at some point during runtime after loading a UTXO snapshot. This change, though lengthy, should be pretty easy to review since most of it is contained within a scripted-diff. Once merged, the parent PR should be easier to review. ACKs for commit 486c1e: Sjors: utACK 486c1ee promag: utACK 486c1ee. practicalswift: utACK 486c1eea863a41e597ae4fddc392f446f2518b4b Tree-SHA512: 06ed8f9e77f2d25fc9bea0ba86436d80dbbce90a1e8be23e37ec4eeb26060483e60b4a5c4fba679cb1867f61e3921c24abeb9cabdfb4d0a9b1c4ddd77b17456a
2019-05-07Add unit test NextString, ForEachNoDup functionsRussell Yanofsky
Remove testcase generating code from util_SettingsMerge so it can be reused in new tests. The hash value expected in util_SettingsMerge changes as a result of this, but only because the testcases are generated in a different order, not because any cases are added or removed. It is possible to verify this with: SETTINGS_MERGE_TEST_OUT=new.txt test/test_bitcoin --run_test=util_tests/util_SettingsMerge git checkout HEAD~1 make test/test_bitcoin SETTINGS_MERGE_TEST_OUT=old.txt test/test_bitcoin --run_test=util_tests/util_SettingsMerge diff -u <(sort old.txt) <(sort new.txt) The new output is a little more readable, with simpler testcases sorted first.
2019-05-07util_SettingsMerge test cleanupRussell Yanofsky
Followup to #15869. Treat "-wallet" as the network-specific argument in test instead of "-server", to make test output clearer and be more consistent with bitcoind. Update embedded hash to match changed output from this.
2019-05-04Merge #15141: Rewrite DoS interface between validation and net_processingWladimir J. van der Laan
0ff1c2a838da9e8dc7f77609adc89124bbea3e2b Separate reason for premature spends (coinbase/locktime) (Suhas Daftuar) 54470e767bab37f9b7089782b1be73d5883bb244 Assert validation reasons are contextually correct (Suhas Daftuar) 2120c31521aa51aa1984ee33250b8320506d3a0f [refactor] Update some comments in validation.cpp as we arent doing DoS there (Matt Corallo) 12dbdd7a41bac73e51ed8f7b290b7671196bf9ea [refactor] Drop unused state.DoS(), state.GetDoS(), state.CorruptionPossible() (Matt Corallo) aa502b88d10c2c3ac56d9163555849b96dc4df1e scripted-diff: Remove DoS calls to CValidationState (Matt Corallo) 7721ad64f40a0c67edefaaf7353264d78df8803e [refactor] Prep for scripted-diff by removing some \ns which annoy sed. (Matt Corallo) 5e78c5734bb0c9aae7b0a7019a745b2d7059b3d9 Allow use of state.Invalid() for all reasons (Matt Corallo) 6b34bc6b6f54f85537494cbea3846d5d195a06d9 Fix handling of invalid headers (Suhas Daftuar) ef54b486d5333dfc85c56e6b933c81735196a25d [refactor] Use Reasons directly instead of DoS codes (Matt Corallo) 9ab2a0412e96e87956fe61257387683635213035 CorruptionPossible -> BLOCK_MUTATED (Matt Corallo) 6e55b292b0ea944897b6dc2f766446fd209af484 CorruptionPossible -> TX_WITNESS_MUTATED (Matt Corallo) 7df16e70e67c753c871797ce947ea09d7cb0e519 LookupBlockIndex -> CACHED_INVALID (Matt Corallo) c8b0d22698385f91215ce8145631e3d5826dc977 [refactor] Drop redundant nDoS, corruptionPossible, SetCorruptionPossible (Matt Corallo) 34477ccd39a8d4bfa8ad612f22d5a46291922185 [refactor] Add useful-for-dos "reason" field to CValidationState (Matt Corallo) 6a7f8777a0b193fae4f976196f3464ffac01bf1b Ban all peers for all block script failures (Suhas Daftuar) 7b999103e21509e1c2dec10f68e48744ffe90f55 Clean up banning levels (Matt Corallo) b8b4c80146780f9011abbd1be72343cc965c07b9 [refactor] drop IsInvalid(nDoSOut) (Matt Corallo) 8818729013e17c650a25f030b2b80e0997389155 [refactor] Refactor misbehavior ban decisions to MaybePunishNode() (Matt Corallo) 00e11e61c0211a62788611cd6a6714a393fdc26c [refactor] rename stateDummy -> orphan_state (Matt Corallo) f34fa719cf33a51d11f1d2219cbe73ccff6fd697 Drop obsolete sigops comment (Matt Corallo) Pull request description: This is a rebase of #11639 with some fixes for the last few comments which were not yet addressed. The original PR text, with some strikethroughs of text that is no longer correct: > This cleans up an old main-carryover - it made sense that main could decide what DoS scores to assign things because the DoS scores were handled in a different part of main, but now validation is telling net_processing what DoS scores to assign to different things, which is utter nonsense. Instead, we replace CValidationState's nDoS and CorruptionPossible with a general ValidationInvalidReason, which net_processing can handle as it sees fit. I keep the behavior changes here to a minimum, but in the future we can utilize these changes for other smarter behavior, such as disconnecting/preferring to rotate outbound peers based on them providing things which are invalid due to SOFT_FORK because we shouldn't ban for such cases. > > This is somewhat complementary with, though obviously conflicts heavily with #11523, which added enums in place of DoS scores, as well as a few other cleanups (which are still relevant). > > Compared with previous bans, the following changes are made: > > Txn with empty vin/vout or null prevouts move from 10 DoS > points to 100. > Loose transactions with a dependency loop now result in a ban > instead of 10 DoS points. > ~~BIP68-violation no longer results in a ban as it is SOFT_FORK.~~ > ~~Non-SegWit SigOp violation no longer results in a ban as it > considers P2SH sigops and is thus SOFT_FORK.~~ > ~~Any script violation in a block no longer results in a ban as > it may be the result of a SOFT_FORK. This should likely be > fixed in the future by differentiating between them.~~ > Proof of work failure moves from 50 DoS points to a ban. > Blocks with timestamps under MTP now result in a ban, blocks > too far in the future continue to not result in a ban. > Inclusion of non-final transactions in a block now results in a > ban instead of 10 DoS points. Note: The change to ban all peers for consensus violations is actually NOT the change I'd like to make -- I'd prefer to only ban outbound peers in those situations. The current behavior is a bit of a mess, however, and so in the interests of advancing this PR I tried to keep the changes to a minimum. I plan to revisit the behavior in a followup PR. EDIT: One reviewer suggested I add some additional context for this PR: > The goal of this work was to make net_processing aware of the actual reasons for validation failures, rather than just deal with opaque numbers instructing it to do something. > > In the future, I'd like to make it so that we use more context to decide how to punish a peer. One example is to differentiate inbound and outbound peer misbehaviors. Another potential example is if we'd treat RECENT_CONSENSUS_CHANGE failures differently (ie after the next consensus change is implemented), and perhaps again we'd want to treat some peers differently than others. ACKs for commit 0ff1c2: jnewbery: utACK 0ff1c2a838da9e8dc7f77609adc89124bbea3e2b ryanofsky: utACK 0ff1c2a838da9e8dc7f77609adc89124bbea3e2b. Only change is dropping the first commit (f3883a321bf4ab289edcd9754b12cae3a648b175), and dropping the temporary `assert(level == GetDoS())` that was in 35ee77f2832eaffce30042e00785c310c5540cdc (now c8b0d22698385f91215ce8145631e3d5826dc977) Tree-SHA512: e915a411100876398af5463d0a885920e44d473467bb6af991ef2e8f2681db6c1209bb60f848bd154be72d460f039b5653df20a6840352c5f7ea5486d9f777a3
2019-05-03scripted-diff: replace chainActive -> ::ChainActive()James O'Beirne
Though at the moment ChainActive() simply references `g_chainstate.m_chain`, doing this change now clears the way for multiple chainstate usage and allows us to script the diff. -BEGIN VERIFY SCRIPT- git grep -l "chainActive" | grep -E '(h|cpp)$' | xargs sed -i '/chainActive =/b; /extern CChain& chainActive/b; s/\(::\)\{0,1\}chainActive/::ChainActive()/g' -END VERIFY SCRIPT-
2019-05-03Add ChaCha20 encryption option (XOR)Jonas Schnelli
2019-05-02[refactor] Add useful-for-dos "reason" field to CValidationStateMatt Corallo
This is a first step towards cleaning up our DoS interface - make validation return *why* something is invalid, and let net_processing figure out what that implies in terms of banning/disconnection/etc. Behavior change: peers will now be banned for providing blocks with premature coinbase spends. Co-authored-by: Anthony Towns <aj@erisian.com.au> Suhas Daftuar <sdaftuar@gmail.com>
2019-05-02[refactor] drop IsInvalid(nDoSOut)Matt Corallo
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2019-04-30Merge #15849: Thread names in logs and deadlock debug toolsMarcoFalke
8722e54e56 threads: add thread names to deadlock debugging message (James O'Beirne) 383b186c28 threads: prefix log messages with thread names (James O'Beirne) ddd95ccb80 tests: add threadutil tests (James O'Beirne) ae5f2b6a6c threads: introduce util/threadnames, refactor thread naming (James O'Beirne) 188ca75e5f disable HAVE_THREAD_LOCAL on unreliable platforms (James O'Beirne) Pull request description: I'm resurrecting this one (from #13168) because I need it to make progress on #15735. It's now off by default and can be turned on with `-logthreadnames=1`. Ran some benchmarks (IBD from local peer from 500_000 -> 504_000) and it's within spitting distance either on or off: ### threadnames off (default) #### 2018-05-threadnames.3 vs. master (absolute) | name | iterations | 2018-05-threadnames.3 | master | |------------------------------------------------|-----------:|----------------------------|----------------------------| | ibd.local.500000.504000.dbcache=2048 | 3 | 376.1584 (± 9.2944) | 392.3414 (± 13.4238) | | ibd.local.500000.504000.dbcache=2048.mem-usage | 3 | 2236117.3333 (± 1845.9623) | 2238690.6667 (± 2669.3487) | #### 2018-05-threadnames.3 vs. master (relative) | name | iterations | 2018-05-threadnames.3 | master | |------------------------------------------------|-----------:|----------------------:|-------:| | ibd.local.500000.504000.dbcache=2048 | 3 | 1 | 1.043 | | ibd.local.500000.504000.dbcache=2048.mem-usage | 3 | 1 | 1.001 | ### threadnames on #### 2018-05-threadnames-take-2 vs. master (absolute) | name | iterations | 2018-05-threadnames-take-2 | master | |------------------------------------------------|-----------:|----------------------------|----------------------------| | ibd.local.500000.504000.dbcache=2048 | 3 | 367.6861 (± 0.3941) | 364.1667 (± 0.9776) | | ibd.local.500000.504000.dbcache=2048.mem-usage | 3 | 2238461.3333 (± 3697.8730) | 2237014.6667 (± 3307.6966) | #### 2018-05-threadnames-take-2 vs. master (relative) | name | iterations | 2018-05-threadnames-take-2 | master | |------------------------------------------------|-----------:|---------------------------:|-------:| | ibd.local.500000.504000.dbcache=2048 | 3 | 1.010 | 1.00 | | ibd.local.500000.504000.dbcache=2048.mem-usage | 3 | 1.001 | 1.00 | ``` ACKs for commit 8722e5: Empact: utACK https://github.com/bitcoin/bitcoin/pull/15849/commits/8722e54e56fd959fd4ff2321b36a7640dee440c5 jnewbery: utACK 8722e54e56fd959fd4ff2321b36a7640dee440c5 MarcoFalke: re-utACK 8722e54e56fd959fd4ff2321b36a7640dee440c5 (Only change since my previous review is DEFAULT_LOGTHREADNAMES=false and stylistic updates Tree-SHA512: 50af992708295b8d680cf10025262dd964e599a356bdfc1dfc84fb18c00afabcb34d3d12d551b0677ff81f8fccad0e17c1d5b24dfecb953a913bc77fdd1a4577