aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2019-11-14test: add unit test for non-standard txs with too large scriptSigSebastian Falbesoner
The function IsStandardTx() returns rejection reason "scriptsig-size" if any one the inputs' scriptSig is larger than 1650 bytes.
2019-11-12fix Typo: "merkelRoot" -> "merkleRoot"ianliu
2019-11-08Merge #15934: Merge settings one place instead of five placesWladimir J. van der Laan
083c954b02a4e7d0708349eeaf3bac2b5947fb0e Add settings_tests (Russell Yanofsky) 7f40528cd50fc43ac0bd3e785de24d661adddb7a Deduplicate settings merge code (Russell Yanofsky) 9dcb952fe5f85529ab28e091af7534e72c21c90f Add util::Settings struct and helper functions. (Russell Yanofsky) e2e37cfe8af088bd8ea884be2f79f0f3cac555d5 Remove includeconf nested scope (Russell Yanofsky) 5a84aa880f6da0bac0e2144733fdef3b8558c761 Rename includeconf variables for clarity (Russell Yanofsky) dc8e1e75487461ec9bff433144f0db831b682403 Clarify emptyIncludeConf logic (Russell Yanofsky) Pull request description: This is a refactoring-only change that makes it easier to add a new settings source. This PR doesn't change behavior. The [`util_ArgsMerge`](https://github.com/bitcoin/bitcoin/blob/deb2327b435925c6a39ca654a79283b8eb6aeb86/src/test/util_tests.cpp#L626-L822) and [`util_ChainMerge`](https://github.com/bitcoin/bitcoin/blob/deb2327b435925c6a39ca654a79283b8eb6aeb86/src/test/util_tests.cpp#L843-L924) tests added in #15869 and #15988 were written specifically to confirm that ArgsManager settings are parsed, merged, and returned the same way before and after this change. This change: - Makes it easier to add new settings sources that can get merged with existing sources (see 70675c3e4975203ad6222ba2b00c83b4e4213793 from #15935). - Separates parsing of settings from merging of settings, and deduplicates merging code so it doesn't happen five different places ([GetArg](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L221-L244), [GetNetBoolArg](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L255-L261), [GetArgs](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L460-L467), [IsArgNegated](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L482-L491), [GetUnsuitableSectionOnlyArgs](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L343-L352)) in inconsistent ways. - Documents and tests current strange merging behaviors, so they be cleaned up in the future if resulting code simplifications and UX improvements warrant loss of backwards compatibility. The newly documented behaviors are: command line [ignored arguments](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/system.cpp#L323-L326) and [more ignored arguments](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/settings.cpp#L67-L72), and config file [reverse precedence](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/settings.cpp#L61-L65), [inconsistently applied top-level settings](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/settings.cpp#L55-L59), and [zombie values](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/settings.cpp#L101-L108). The original motivation for this change was to make it easy to add a new persistent setting source without introducing more bugs and inconsistencies. Two commits building on top of this to add a persistent `-wallet` setting are pretty straightforward and show how the new code can be extended: * 70675c3e4975203ad6222ba2b00c83b4e4213793 from #15935 – _Add \<datadir>/settings.json persistent settings storage_ * 04c80c40df9fc6f4734ba238ea7f65607cf88089 from #15937 – _Add loadwallet and createwallet RPC load_on_startup options_ ACKs for top commit: ariard: ACK 083c954 jnewbery: ACK 083c954b02a4e7d0708349eeaf3bac2b5947fb0e jamesob: ACK 083c954b02a4e7d0708349eeaf3bac2b5947fb0e Tree-SHA512: 5d106746a44d64d3963c4ef3f4a2fa668a4bedcc9018d3ea12c86beae2fda48a0b036241665837f68685712366f70f2e1faba84d193fa1f456013503097b7659
2019-11-08Merge #15931: Remove GetDepthInMainChain dependency on locked chain interfaceSamuel Dobson
36b68de5b2938722911db900ca299f7008780d01 Remove getBlockDepth method from Chain::interface (Antoine Riard) b66c429c56c85fa16c309be0b2bca9c25fdd3e1a Remove locked_chain from GetDepthInMainChain and its callers (Antoine Riard) 0ff03871add000f8b4d8f82aeb168eed2fc9dc5f Use CWallet::m_last_block_processed_height in GetDepthInMainChain (Antoine Riard) f77b1de16feee097a88e99d2ecdd4d84beb4f915 Only return early from BlockUntilSyncedToCurrentChain if current tip is exact match (Antoine Riard) 769ff05e48fb53d4b62c59060424a0fea71d0aab Refactor some importprunedfunds checks with guard clause (Antoine Riard) 5971d3848e09abf571e5308185275296127efca4 Add block_height field in struct Confirmation (Antoine Riard) 9700fcb47feca9d78e005b8d18b41148c8f6b25f Replace CWalletTx::SetConf by Confirmation initialization list (Antoine Riard) 5aacc3eff15b9b5bdc951f1e274f00d581f63bce Add m_last_block_processed_height field in CWallet (Antoine Riard) 10b4729e33f76092bd8cfa06d1a5e0a066436f76 Pass block height in Chain::BlockConnected/Chain::BlockDisconnected (Antoine Riard) Pull request description: Work starter to remove Chain::Lock interface by adding m_last_block_processed_height in CWallet and m_block_height in CMerkleTx to avoid GetDepthInMainChain having to keep a lock . Once this one done, it should ease work to wipe out more cs_main locks from wallet code. I think it's ready for a first round of review before to get further. - `BlockUntilSyncedToCurrent` : restrain isPotentialTip to isTip because we want to be sure that wallet see BlockDisconnected callbacks if its height differs from the Chain one. It means during a reorg, an RPC could return before the BlockDisconnected callback had been triggered. This could cause a tx that had been included in the disconnected block to be displayed as confirmed, for example. ~~- `AbandonTransaction` : in case of conflicted tx (nIndex = -1), we set its m_block_height to the one of conflicting blocks, but if this height is superior to CWallet::m_last_block_processed_height, that means tx isn't conflicted anymore so we return 0 as tx is again unconfirmed~~ After #16624, we instead rely on Confirmation. ~~- `AddToWalletIfInvolvingMe`: in case of block disconnected, transactions are added to mempool again, so we need to replace old txn in `mapWallet` with a height set to zero so we remove check on block_hash.IsNull~~ Already done in #16624 ACKs for top commit: jnewbery: @jkczyz you've ACKed an intermediate commit (github annoyingly orders commits in date order, not commit order). Did you mean to ACK the final commit in this branch (36b68de5b2938722911db900ca299f7008780d01). jkczyz: > @jkczyz you've ACKed an intermediate commit (github annoyingly orders commits in date order, not commit order). Did you mean to ACK the final commit in this branch ([36b68de](https://github.com/bitcoin/bitcoin/commit/36b68de5b2938722911db900ca299f7008780d01)). meshcollider: utACK 36b68de5b2938722911db900ca299f7008780d01 ryanofsky: Code review ACK 36b68de5b2938722911db900ca299f7008780d01. Changes since last review: new jkczyz refactor importprunedfunds commit, changed BlockUntilSyncedToCurrentChainChanges commit title and description, changed Confirmation struct field order and line-wrapped comment jnewbery: utACK 36b68de5b2938722911db900ca299f7008780d01 promag: Code review ACK 36b68de5b2938722911db900ca299f7008780d01. Tree-SHA512: 08b89a0bcc39f67c82a6cb6aee195e6a11697770c788ba737b90986b4893f44e90d1ab9ef87239ea3766508b7e24ea882b7199df41173ab27a3d000328c14644
2019-11-07Add settings_testsRussell Yanofsky
Co-authored-by: James O'Beirne <james.obeirne@gmail.com>
2019-11-07Deduplicate settings merge codeRussell Yanofsky
Get rid of settings merging code in util/system.cpp repeated 5 places, inconsistently: - ArgsManagerHelper::GetArg - ArgsManagerHelper::GetNetBoolArg - ArgsManager::GetArgs - ArgsManager::IsArgNegated - ArgsManager::GetUnsuitableSectionOnlyArgs Having settings merging code separated from parsing simplifies parsing somewhat (for example negated values can simply be represented as false values instead of partially cleared or emply placeholder lists). Having settings merge happen one place instead of 5 makes it easier to add new settings sources and harder to introduce new inconsistencies in the way settings are merged. This commit does not change behavior in any way.
2019-11-07Merge #16978: test: Seed test RNG context for each test case, print seedMarcoFalke
fae43a97ca947cd0802392e9bb86d9d0572c0fba test: Seed test RNG context for each test case, print seed (MarcoFalke) Pull request description: Debugging failing unit tests is hard if the failure is non-deterministic and the seed is not known. Fix that by printing the seed and making it possible to set the seed from outside. ACKs for top commit: davereikher: Tested ACK fae43a97ca947cd0802392e9bb86d9d0572c0fba Tree-SHA512: 33d848dd1f4180d3664ecf60e9810c2a93590c05276b2c46b1e4fe6e376b45916a46b90c803bb602750ab666da3a05ce499e550024685a90b8cc38fab6667cb8
2019-11-07Merge #17342: refactor: Clean up nScriptCheckThreadsMarcoFalke
5506ecfe7a65d5705616bc048f2f1735b89993fb [refactor] Replace global int nScriptCheckThreads with bool (John Newbery) d9957623b48a7c3eff0ac750d1245fabfb1843a2 [tests] Don't use TestingSetup in the checkqueue_tests (John Newbery) Pull request description: The meaning of this value is confusing. Refactor it and add comments. ACKs for top commit: sipa: ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb promag: ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb, only change was addressing my nits. laanwj: Code review ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb MarcoFalke: ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb 🥐 Tree-SHA512: 78536727c98d2c23f3c0f3f169131474fef9a4486ae65029011caf06eab30f6f70ff73a65b2fb04a5d969fc1150858d1c6ea4767f04d48c1eea6b829316d0e63
2019-11-07Merge #17384: test: Create new test libraryMarcoFalke
fa4c6fa9b1139791f45f1495d662c1c7cd2f7ed6 doc: Add documentation for new test/lib (MarcoFalke) faec28252cf4f8e754c689a7a44fd421f631db50 scripted-diff: test: Move setup_common to test library (MarcoFalke) Pull request description: Sorry for clickbait, this is only a move-only scripted-diff commit and one documentation commit. Longer term, someone who knows something about build systems can make this an actual library. Motivation for this is that each module gets compiled for each target that includes it. For example, setup_common is compiled 27 times (for the fuzz suite) and another 3 times for the other tests (bench, unit test, gui) ACKs for top commit: practicalswift: ACK fa4c6fa9b1139791f45f1495d662c1c7cd2f7ed6 -- diff looks correct and Travis is happy jonatack: ACK fa4c6fa9b1139791f45f1495d662c1c7cd2f7ed6 with the reserve that the commit messages (and PR description) contain the motivation for this change. Built, ran tests, light code review. ryanofsky: Code review ACK fa4c6fa9b1139791f45f1495d662c1c7cd2f7ed6. I didn't realize `lib` was actually name of existing directory, not a new name. But in any case this looks good and nice to have one scripted diff instead of two. Tree-SHA512: 2e176df90c60578276e4a6dc83ff57ff59d8e666ecf30c5ceacb8c326725da91baa4cac3dfa7a2e1605f58122a3e3e27e4938ff33e3a0ce7ea53afffebbf57a4
2019-11-06[refactor] Replace global int nScriptCheckThreads with boolJohn Newbery
The global nScriptCheckThreads int is confusing and is only needed for its int-ness in AppInitMain. Move all `-par` parsing logic there and replace the int nScriptCheckThreads with a bool g_parallel_script_checks. Also tidy up logic and improve comments.
2019-11-06[tests] Don't use TestingSetup in the checkqueue_testsJohn Newbery
It's only needed for a hardcoded int, which we can define locally.
2019-11-06doc: Add documentation for new test/libMarcoFalke
2019-11-06scripted-diff: test: Move setup_common to test libraryMarcoFalke
-BEGIN VERIFY SCRIPT- # Move files for f in $(git ls-files src/test/lib/); do git mv $f src/test/util/; done git mv src/test/setup_common.cpp src/test/util/ git mv src/test/setup_common.h src/test/util/ # Replace Windows paths sed -i -e 's|\\setup_common|\\util\\setup_common|g' $(git grep -l '\\setup_common') sed -i -e 's|src\\test\\lib\\|src\\test\\util\\|g' build_msvc/test_bitcoin/test_bitcoin.vcxproj # Everything else sed -i -e 's|/setup_common|/util/setup_common|g' $(git grep -l 'setup_common') sed -i -e 's|test/lib/|test/util/|g' $(git grep -l 'test/lib/') # Fix include guard sed -i -e 's|BITCOIN_TEST_SETUP_COMMON_H|BITCOIN_TEST_UTIL_SETUP_COMMON_H|g' ./src/test/util/setup_common.h sed -i -e 's|BITCOIN_TEST_LIB_|BITCOIN_TEST_UTIL_|g' $(git grep -l 'BITCOIN_TEST_LIB_') -END VERIFY SCRIPT-
2019-11-06Merge #17390: test: Add util_ArgParsing testWladimir J. van der Laan
286f197704e82045c762d332aba5d1ac52e0212d Add util_ArgParsing test (Russell Yanofsky) Pull request description: ArgsManager test coverage for parsing of integer and boolean values is currently very poor and doesn't give us a way of knowing whether changes to ArgsManager may unintentionally break backwards compatibility, so this adds a new test to catch regressions. ACKs for top commit: promag: ACK 286f197, more surprising results 😱 laanwj: ACK 286f197704e82045c762d332aba5d1ac52e0212d Tree-SHA512: 9e1db3ef87e55abbc280af60c088f35765a1f9e2ec20507ad0c1992027b875490016868dcb8cc287e6df279dd0e00f10550901af3de3d36287867249e0bd8207
2019-11-06Merge #17388: Add missing newline in util_ChainMerge testWladimir J. van der Laan
3645e4ca0033bb6365f41ef710111780c139370f Add missing newline in util_ChainMerge test (Russell Yanofsky) Pull request description: This was causing a lot of test cases not not be very meaningful because multiple configuration options were combined into one line. The changes in test output with this fix make sense and look like: ```diff - testnet=1 regtest=1 || test + testnet=1 regtest=1 || error: Invalid combination of -regtest, -testnet and -chain. Can use at most one. ``` Issue was reported and debugged by Wladimir J. van der Laan <laanwj@protonmail.com> in https://github.com/bitcoin/bitcoin/pull/17385#issuecomment-550033222 <!-- *** Please remove the following help text before submitting: *** Pull requests without a rationale and clear improvement may be closed immediately. --> <!-- Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly: * Any test improvements or new tests that improve coverage are always welcome. * All other changes should have accompanying unit tests (see `src/test/`) or functional tests (see `test/`). Contributors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change. * Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed. * Features are welcome, but might be rejected due to design or scope issues. If a feature is based on a lot of dependencies, contributors should first consider building the system outside of Bitcoin Core, if possible. * Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they *significantly* improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the [developer notes](/doc/developer-notes.md), stylistic code changes are usually rejected. --> <!-- Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time. --> ACKs for top commit: laanwj: ACK 3645e4ca0033bb6365f41ef710111780c139370f practicalswift: ACK 3645e4ca0033bb6365f41ef710111780c139370f -- diff looks correct Tree-SHA512: ca5bde9b9f553811d4827113f4880d15d7b8f4f1455b95bbf34c9a1512fdd53062f1a2133c50d9b54f94160a1ee77a54bc82681a5f3bf25d2b0d01f8a8e95165
2019-11-05Add util_ArgParsing testRussell Yanofsky
ArgsManager test coverage for parsing of integer and boolean values is currently very poor and doesn't give us a way of knowing whether changes to ArgsManager may unintentionally break backwards compatibility, so this adds a new test to catch regressions.
2019-11-05Merge #16805: logs: add timing information to FlushStateToDisk()Wladimir J. van der Laan
dcef9a2922317cb2849f397366b6c56d755db6c9 logs: add timing information to FlushStateToDisk() (James O'Beirne) 41edaf227a69bc4846d5996675e8763fdfe0f367 logs: add BCLog::Timer and related macros (James O'Beirne) Pull request description: It's currently annoying to detect FlushStateToDisk() calls when benchmarking since they have to be inferred from a drop in coins count from the `UpdateTip: ` log messages. This adds a new logging utility, `BCLog::Timer`, and some related macros that are generally useful for printing timing-related logging messages, and a message that is unconditionally written when the coins cache is flushed to disk. ``` 2019-09-04T20:17:51Z FlushStateToDisk: write block and undo data to disk completed (3ms) 2019-09-04T20:17:51Z FlushStateToDisk: write block index to disk completed (370ms) 2019-09-04T20:17:51Z FlushStateToDisk: write coins cache to disk (2068451 coins, 294967kB) completed (21481ms) ``` ACKs for top commit: laanwj: Thanks, ACK dcef9a2922317cb2849f397366b6c56d755db6c9 ryanofsky: Code review ACK dcef9a2922317cb2849f397366b6c56d755db6c9. No changes since last review other than moving code to new timer.h header Tree-SHA512: 6d61e48a062d3edb48d0e056a6f0b1f8031773cc99289ee4544f8349d24526b88519e1e304009d56e428f1eaf76c857bf8e7e1c0b6873a6f270306accb5edc3d
2019-11-05Add missing newline in util_ChainMerge testRussell Yanofsky
This was causing a lot of test cases not not be very meaningful because multiple configuration options were combined into one line. The changes in test output with this fix make sense and look like: ```diff - testnet=1 regtest=1 || test + testnet=1 regtest=1 || error: Invalid combination of -regtest, -testnet and -chain. Can use at most one. ``` Issue was reported and debugged by Wladimir J. van der Laan <laanwj@protonmail.com> in https://github.com/bitcoin/bitcoin/pull/17385#issuecomment-550033222
2019-11-05Merge #16540: test: Add ASSERT_DEBUG_LOG to unit test frameworkMarcoFalke
fa2c44c3ccc3e7a54e2afc862addd555d5a6e51b test: Add ASSERT_DEBUG_LOG to unit test framework (MarcoFalke) fa1936f57bbf5aebb1f8fc18701441d79219d443 logging: Add member for arbitrary print callbacks (MarcoFalke) Pull request description: Similar to `assert_debug_log` in the functional test framework Top commit has no ACKs. Tree-SHA512: aa9eaeca386b61d806867c04a33275f6eb4624fa5bf50f2928d16c83f5634bac96bcac46f9e8eda3b00b4251c5f12d7b01d6ffd84ba8e05c09eeec810cc31251
2019-11-05Merge #17363: test: add "diamond" unit test to MempoolAncestryTestsMarcoFalke
b2ff500fb3e4fa05de366ab1900825bea1f70377 test: add "diamond" unit test to MempoolAncestryTests (Sebastian Falbesoner) Pull request description: Approaches #17271 (_Missing Unit Test for Ancestors "diamond"_). If ancestors are represented more than once (in this case `ta` and `tb`), check that those are not overcounted. ACKs for top commit: laanwj: ACK b2ff500fb3e4fa05de366ab1900825bea1f70377 Tree-SHA512: 82a6573cc7f0e82bf6fcfe207d7ddecbf297d2a203d22e95b73d887e3cb280f45a3c5f649161561c1be1eb560ff81b9b385868f205d1c12284211c2377e5ad99
2019-11-05Pass block height in Chain::BlockConnected/Chain::BlockDisconnectedAntoine Riard
To do so we update CValidationInterface::BlockDisconnect to take a CBlockIndex pointing to the block being disconnected. This new parameter will be use in the following commit to establish wallet height.
2019-11-05Merge #17357: tests: Add fuzzing harness for Bech32 encoding/decodingMarcoFalke
b7541705d0abfbddf682a0134f3fa8a8e1d06cdf tests: Add fuzzing harness for Bech32 encoding/decoding (practicalswift) 85a34b1683233060c4500cfb8e3f5d84c8b8f0da tests: Move CaseInsensitiveEqual to test/util/str (practicalswift) Pull request description: Add fuzzing harness for Bech32 encoding/decoding. **Testing this PR** Run: ``` $ make distclean $ ./autogen.sh $ CC=clang CXX=clang++ ./configure --enable-fuzz \ --with-sanitizers=address,fuzzer,undefined $ make $ src/test/fuzz/bech32 -max_total_time=60 … ``` ACKs for top commit: jonatack: ACK b7541705d0abfbddf682a0134f3fa8a8e1d06cdf Tree-SHA512: ade01d30c6886a083b806dbfff08999cc0d08e687701c670c895e261ed242c789e8a0062d4ebbe8f82676b8f168dc37e83351a88822c9c0eab478572a9e1ec02
2019-11-05Merge #17243: p2p: add PoissonNextSend method that returns mockable timeMarcoFalke
1a8f0d5a74d5cc0000456932babf35301f5c1686 [tools] update nNextInvSend to use mockable time (Amiti Uttarwar) 4de630354fc6808b9b13b9e82da1a82f2f50f26a [tools] add PoissonNextSend method that returns mockable time (Amiti Uttarwar) Pull request description: Introduce a Poisson helper method that wraps the existing method to return `std::chrono::duration` type, which is mockable. Needed for https://github.com/bitcoin/bitcoin/pull/16698. ACKs for top commit: ajtowns: ACK 1a8f0d5a74d5cc0000456932babf35301f5c1686 MarcoFalke: re-ACK 1a8f0d5a74d5cc0000456932babf35301f5c1686 naumenkogs: ACK 1a8f0d5, and let's merge it and come back to it later. Tree-SHA512: 7e2325d7c55fc0b4357cb86b83e0c218ba269f678c1786342d8bc380bfd9696373bc24ff124b9ff17a6e761c62b2b44ff5247c3911e2afdc7cc5c20417e8290b
2019-11-05[tools] add PoissonNextSend method that returns mockable timeAmiti Uttarwar
2019-11-05tests: Add fuzzing harness for Bech32 encoding/decodingpracticalswift
2019-11-05tests: Move CaseInsensitiveEqual to test/util/strpracticalswift
2019-11-04logs: add BCLog::Timer and related macrosJames O'Beirne
Makes logging timing information about a block of code easier.
2019-11-04test: Reset global args between test suitesMarcoFalke
2019-11-04test: Add ASSERT_DEBUG_LOG to unit test frameworkMarcoFalke
2019-11-04test: add "diamond" unit test to MempoolAncestryTestsSebastian Falbesoner
Approaches #17271. If ancestors are represented more than once, check that those are not overcounted.
2019-11-04Merge #17228: test: Add RegTestingSetup to setup_commonMarcoFalke
fa0a731d007a8e76d2740a2e6ead2289de77e475 test: Add RegTestingSetup to setup_common (MarcoFalke) fa54b3e2485f701aa420c37f09a2859a5b805161 test: move-only ComputeFilter to src/test/lib/blockfilter (MarcoFalke) Pull request description: The default chain for `TestingSetup` is the main chain. However, any test that wants to mine blocks on demand needs to switch to regtest. This is done manually and in-line right now. Fix that by creating an explicit `RegTestingSetup` and use it where appropriate. Also, add a move-only commit to move `ComputeFilter` into the newly created unit test library. Both commits are part of #15845, but split up because they are useful on their own. ACKs for top commit: practicalswift: ACK fa0a731d007a8e76d2740a2e6ead2289de77e475 -- diff looks correct Tree-SHA512: 02b9765580b355ed8d1be555f8ae11fa6e3d575f5cb177bbdda0319378837e29de5555c126c477dc8a1e8a5be47335afdcff152cf2dea2fbdd1a988ddde3689b
2019-11-01Merge #17254: test: fix script_p2sh_tests OP_PUSHBACK2/4 missingMarcoFalke
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
2019-11-01test: Do not instantiate CAddrDB for static callHennadii Stepanov
2019-10-31Merge #17274: tests: Fix fuzzers eval_script and script_flags by re-adding ↵MarcoFalke
ECCVerifyHandle dependency 9cae3d5e94f4481e0d251c924314e57187a07a60 tests: Add fuzzer initialization (hold ECCVerifyHandle) (practicalswift) Pull request description: The fuzzers `eval_script` and `script_flags` require holding `ECCVerifyHandle`. This is a follow-up to #17235 which accidentally broke those two fuzzers. Sorry about the temporary breakage my fuzzing friends: it took a while to fuzz before reaching these code paths. That's why this wasn't immediately caught. Sorry. Top commit has no ACKs. Tree-SHA512: 67ebb155ba90894c07eac630e33f2f985c97bdf96dc751f312633414abeccdca20315d7d8f2ec4ee3ac810b666a1e44afb4ea8bc28165151cd51b623f816cac2
2019-10-30test: Add RegTestingSetup to setup_commonMarcoFalke
2019-10-30test: move-only ComputeFilter to src/test/lib/blockfilterMarcoFalke
2019-10-30Merge #15921: validation: Tidy up ValidationState interfaceWladimir J. van der Laan
3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf [validation] Remove fMissingInputs from AcceptToMemoryPool() (John Newbery) c428622a5bb1e37b2e6ab2c52791ac05d9271238 [validation] Remove unused first_invalid parameter from ProcessNewBlockHeaders() (John Newbery) 7204c6434b944f6ad51b3c895837729d3aa56eea [validation] Remove useless ret parameter from Invalid() (John Newbery) 1a37de4b3174d19a6d8691ae07e92b32fdfaef11 [validation] Remove error() calls from Invalid() calls (John Newbery) 067981e49246822421a7bcc720491427e1dba8a3 [validation] Tidy Up ValidationResult class (John Newbery) a27a2957ed9afbe5a96caa5f0f4cbec730d27460 [validation] Add CValidationState subclasses (John Newbery) Pull request description: Carries out some remaining tidy-ups remaining after PR 15141: - split ValidationState into TxValidationState and BlockValidationState (commit from ajtowns) - various minor code style tidy-ups to the ValidationState class - remove the useless `ret` parameter from `ValidationState::Invalid()` - remove the now unused `first_invalid` parameter from `ProcessNewBlockHeaders()` - remove the `fMissingInputs` parameter from `AcceptToMemoryPool()`, and deal with missing inputs the same way as other errors by using the `TxValidationState` object. Tip for reviewers (thanks ryanofsky!): The first commit ("[validation] Add CValidationState subclasses" ) is huge and can be easier to start reviewing if you revert the rote, mechanical changes: Substitute the commit hash of commit "[validation] Add CValidationState subclasses" for <CommitHash> in the commands below. ```sh git checkout <CommitHash> git grep -l ValidationState | xargs sed -i 's/BlockValidationState\|TxValidationState/CValidationState/g' git grep -l ValidationResult | xargs sed -i 's/BlockValidationResult\|TxValidationResult/ValidationInvalidReason/g' git grep -l MaybePunish | xargs sed -i 's/MaybePunishNode\(ForBlock\|ForTx\)/MaybePunishNode/g' git diff HEAD^ ``` After that it's possible to easily see the mechanical changes with: ```sh git log -p -n1 -U0 --word-diff-regex=. <CommitHash> ``` ACKs for top commit: laanwj: ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf amitiuttarwar: code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf. Also built & ran tests locally. fjahr: Code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf . Only nit style change and pure virtual destructor added since my last review. ryanofsky: Code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf. Just whitespace change and pure virtual destructor added since last review. Tree-SHA512: 511de1fb380a18bec1944ea82b513b6192df632ee08bb16344a2df3c40811a88f3872f04df24bc93a41643c96c48f376a04551840fd804a961490d6c702c3d36
2019-10-30Merge #17291: tests: Add fuzzing harness for ISO-8601 related functionsMarcoFalke
595cc9bcafb16709c0dd4a36d5921feb009ce430 docs: Add undefined to --with-sanitizers=fuzzer,address (practicalswift) d5dbb4898c437d3d2a5798d79c3986ba9e0b72f9 tests: Add fuzzing harness for ISO-8601 related functions (practicalswift) Pull request description: Add fuzzing harness for ISO-8601 related functions. **Testing this PR** Run: ``` $ CC=clang CXX=clang++ ./configure --enable-fuzz \ --with-sanitizers=address,fuzzer,undefined $ make $ src/test/fuzz/parse_iso8601 … ``` Top commit has no ACKs. Tree-SHA512: 8d4ad9e4eef546e97ea330cf518fdd7241c6f016d6c45c011369d5cdd832bbbc3564d1a990c953ffb33b0c05e58f5533e7b6fd77062f8484df36da1513567915
2019-10-30tests: Add fuzzing harness for ISO-8601 related functionspracticalswift
2019-10-30Merge #16839: Replace Connman and BanMan globals with NodeContext localWladimir J. van der Laan
362ded410b8cb1104b7ef31ff8488fec4824a7d5 Avoid using g_rpc_node global in wallet code (Russell Yanofsky) 8922d7f6b751a3e6b3b9f6fb7961c442877fb65a scripted-diff: Remove g_connman, g_banman globals (Russell Yanofsky) e6f4f895d5e42feaf7bfa5f41e80292aaa73cd7d Pass NodeContext, ConnMan, BanMan references more places (Russell Yanofsky) 4d5448c76b71c9d91399c31b043237091be2e5e7 MOVEONLY: Move NodeContext struct to node/context.h (Russell Yanofsky) 301bd41a2e6765b185bd55f4c541f9e27aeea29d scripted-diff: Rename InitInterfaces to NodeContext (Russell Yanofsky) Pull request description: This change is mainly a naming / organization change intended to simplify #10102. It: - Renames struct InitInterfaces to struct NodeContext and moves it from src/init.h to src/node/context.h. This is a cosmetic change intended to make the point of the struct more obvious. - Gets rid of BanMan and ConnMan globals making them NodeContext members instead. Getting rid of these globals has been talked about in past as a way to implement testing and simulations. Making them NodeContext members is a way of keeping them accessible without the globals. - Splits g_rpc_interfaces global into g_rpc_node and g_rpc_chain globals. This better separates node and wallet rpc methods. Node RPC methods should have access NodeContext, while wallet RPC methods should only have indirect access to node functionality via interfaces::Chain. - Adds NodeContext& references to interfaces::Chain class and the interfaces::MakeChain() function. This is needed to access ConnMan and BanMan instances without the globals. - Gets rid of redundant Node and Chain instances in Qt tests. This is needed due to the previous MakeChain change, and also makes test setup a little more straightforward. More cleanup could be done in the future, but it will require deduplication of bitcoind, bitcoin-qt, and TestingSetup init code. ACKs for top commit: laanwj: ACK 362ded410b8cb1104b7ef31ff8488fec4824a7d5 Tree-SHA512: 9ae6ff1e33423291d1e52056bac95e0874538390892a6e83c4c115b3c73155a8827c0191b46eb3d14e3b3f6c23ccb08095490880fbc3188026319c71739f7db2
2019-10-29Merge #17299: test: add reason checks for non-standard txs in test_IsStandardMarcoFalke
c1c6c410a66996b2d60d5172189b5a5ec8100842 test: add reason checks for non-standard txs in test_IsStandard (Sebastian Falbesoner) Pull request description: While taking a look at #17272 I noticed that for some reason the unit test `test_IsStandard` (which was not adapted to the policy change in the referenced PR commits) didn't fail as expected: https://github.com/bitcoin/bitcoin/blob/6a97e8a060f7632bbaee27d3de8035dc6ebe3895/src/test/transaction_tests.cpp#L758-L762 It turned out that `IsStandardTx()` returned `"dust"` as rejection reason (instead of the expected `"multi-op-return"`), leading to the conclusion that https://github.com/bitcoin/bitcoin/pull/17272/commits/5fe6f052bd37a16b2849e05f5cf18d7e194bc705 erroneously performs the `IsDust()` check also for TX_NULL_DATA transactions. To avoid cases like this in the future, this PR makes the unit test `test_IsStandard` more strict by also checking for the concrete reason after each occurence of `IsStandardTx()` returning false. ACKs for top commit: instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/17299/commits/c1c6c410a66996b2d60d5172189b5a5ec8100842 Tree-SHA512: c7419884cc52977c73f8f8c476eaebed80ba7bda4d03509d3f46dd977be911389f7b53daefa5ef31d2f7df9402243152e01e83f1b8a9fb300c19d1a0f69a89a9
2019-10-29[validation] Remove fMissingInputs from AcceptToMemoryPool()John Newbery
Handle this failure in the same way as all other failures: call Invalid() with the reasons for the failure.
2019-10-29[validation] Remove unused first_invalid parameter from ProcessNewBlockHeaders()John Newbery
No callers use the returned value in first_invalid. Remove it from the function signature and don't set it in the function.
2019-10-29[validation] Add CValidationState subclassesJohn Newbery
Split CValidationState into TxValidationState and BlockValidationState to store validation results for transactions and blocks respectively.
2019-10-29test: add reason checks for non-standard txs in test_IsStandardSebastian Falbesoner
2019-10-29Merge #17260: Split some CWallet functions into new LegacyScriptPubKeyManMarcoFalke
f201ba59ffd2e071a36a688b80d2cff9a9c44bb2 Refactor: Split up CWallet and LegacyScriptPubKeyMan and classes (Andrew Chow) 6702048f91089d7a565e5ca5f7c8dcd2ca405a85 MOVEONLY: Move key handling code out of wallet to keyman file (Andrew Chow) ab053ec6d1e766402f88947d29cd875a285e7280 Move wallet enums to walletutil.h (Andrew Chow) Pull request description: Moves key management functions into a new class LegacyScriptPubKeyMan. First two commits are move-only commits which move stuff out of wallet.{h/cpp} and into newly created scriptpubkeyman.{h/cpp}. Third commit changes several things in CWallet to use LegacyScriptPubKeyMan. First step in the wallet boxes refactor. Note that LegacyScriptPubKeyMan and ScriptPubKeyMan cannot be used standalone yet and are still very much tied into CWallet with both accessing functions within each other. This PR is to help reduce review burden. ACKs for top commit: Sjors: Code review ACK f201ba5. promag: Code review ACK f201ba59ffd2e071a36a688b80d2cff9a9c44bb2. ryanofsky: Code review ACK f201ba59ffd2e071a36a688b80d2cff9a9c44bb2 MarcoFalke: ACK f201ba59ffd2e071a36a688b80d2cff9a9c44bb2 Tree-SHA512: bdc0d8595a06233fe003afcf968a38e0e8cc584a6a89c5bcd05309ac29dca852391802d46763ef81a108d146d0f40c79ea5438e87234ed12b4b8360c9aec94c0
2019-10-28Merge #17266: util: Rename DecodeDumpTime to ParseISO8601DateTimeMarcoFalke
e7b02b54ccfb6b2e119a67799220f8d8d8b5cccd Add roundtrip and more tests to ParseISO8601DateTime and FormatISO8601DateTime (Elichai Turkel) 9e2c623be50ee7e586a411923b9ed136acfa2b3f Rename DecodeDumpTime to ParseISO8601DateTime and move to time.cpp (Elichai Turkel) Pull request description: As discussed in #17245. 1. Renamed the function. 2. Moved it from `rpcdump.cpp` to `time.cpp`. 3. Added a check if the time is less then epoch return 0 to prevent an overflow. 4. Added more edge cases tests and a roundtrip test. ACKs for top commit: laanwj: ACK e7b02b54ccfb6b2e119a67799220f8d8d8b5cccd MarcoFalke: ACK e7b02b54ccfb6b2e119a67799220f8d8d8b5cccd promag: Code review ACK e7b02b54ccfb6b2e119a67799220f8d8d8b5cccd. Moved code is correct, left a comment regarding the test change. Tree-SHA512: 703c21e09b2aabc992235149e67acba63d9d77a593ec8f6d2fec3eb63a7e5c406d56cbce6c6513ab32fba43367d073d2345f3b589843e3c5fe4f55ea3e00bf29
2019-10-28Avoid using g_rpc_node global in wallet codeRussell Yanofsky
Wallet code should use interfaces::Chain and not directly access to node state. Add a g_rpc_chain replacement global for wallet code to use, and move g_rpc_node definition to a libbitcoin_server source file so there are link errors if wallet code tries to access it.
2019-10-28scripted-diff: Remove g_connman, g_banman globalsRussell Yanofsky
-BEGIN VERIFY SCRIPT- sed -i 's:#include <interfaces/chain.h>:#include <banman.h>\n#include <interfaces/chain.h>\n#include <net.h>\n#include <net_processing.h>:' src/node/context.cpp sed -i 's/namespace interfaces {/class BanMan;\nclass CConnman;\nclass PeerLogicValidation;\n&/' src/node/context.h sed -i 's/std::unique_ptr<interfaces::Chain> chain/std::unique_ptr<CConnman> connman;\n std::unique_ptr<PeerLogicValidation> peer_logic;\n std::unique_ptr<BanMan> banman;\n &/' src/node/context.h sed -i '/std::unique_ptr<[^>]\+> \(g_connman\|g_banman\|peerLogic\);/d' src/banman.h src/net.h src/init.cpp sed -i 's/g_connman/m_context.connman/g' src/interfaces/node.cpp sed -i 's/g_banman/m_context.banman/g' src/interfaces/node.cpp sed -i 's/g_connman/m_node.connman/g' src/interfaces/chain.cpp src/test/setup_common.cpp sed -i 's/g_banman/m_node.banman/g' src/test/setup_common.cpp sed -i 's/g_connman/node.connman/g' src/init.cpp src/node/transaction.cpp sed -i 's/g_banman/node.banman/g' src/init.cpp sed -i 's/peerLogic/node.peer_logic/g' src/init.cpp sed -i 's/g_connman/g_rpc_node->connman/g' src/rpc/mining.cpp src/rpc/net.cpp src/rpc/rawtransaction.cpp sed -i 's/g_banman/g_rpc_node->banman/g' src/rpc/net.cpp sed -i 's/std::shared_ptr<CWallet> wallet =/node.context()->connman = std::move(test.m_node.connman);\n &/' src/qt/test/wallettests.cpp -END VERIFY SCRIPT-
2019-10-28Pass NodeContext, ConnMan, BanMan references more placesRussell Yanofsky
So g_connman and g_banman globals can be removed next commit.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-02 00:54:10 +0000