aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
2020-08-12test: add logging for p2p_feefilter.pySebastian Falbesoner
2020-08-11Merge #19674: refactor: test: use throwaway _ variable for unused loop countersfanquake
dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 refactor: test: use _ variable for unused loop counters (Sebastian Falbesoner) Pull request description: This tiny PR substitutes Python loops in the form of `for x in range(N): ...` by `for _ in range(N): ...` where applicable. The idea is indicating to the reader that a block (or statement, in list comprehensions) is just repeated N times, and that the loop counter is not used in the body, hence using the throwaway variable. This is already done quite often in the current tests (see e.g. `$ git grep "for _ in range("`). Another alternative would be using `itertools.repeat` (according to Python core developer Raymond Hettinger it's [even faster](https://twitter.com/raymondh/status/1144527183341375488)), but that doesn't seem to be widespread in use and I'm not sure about a readability increase. The only drawback I see is that whenever one wants to debug loop iterations, one would need to introduce a loop variable again. Reviewing this is basically a no-brainer, since tests would fail immediately if a a substitution has taken place on a loop where the variable is used. Instances to replace were found by `$ git grep "for.*in range("` and manually checked. ACKs for top commit: darosior: ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 instagibbs: manual inspection ACK https://github.com/bitcoin/bitcoin/pull/19674/commits/dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 practicalswift: ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 -- the updated code is easier to reason about since the throwaway nature of a variable is expressed explicitly (using the Pythonic `_` idiom) instead of implicitly. Explicit is better than implicit was we all know by now :) Tree-SHA512: 5f43ded9ce14e5e00b3876ec445b90acda1842f813149ae7bafa93f3ac3d510bb778e2c701187fd2c73585e6b87797bb2d2987139bd1a9ba7d58775a59392406
2020-08-09Merge #19631: test: Wait for 'cmpctblock' in p2p_compactblocks when it is ↵Wladimir J. van der Laan
expected 9e165d0de4c3cd168137fc85b8f31b371bd4e851 test: Wait for 'cmpctblock' in p2p_compactblocks when it is expected (Ben Woosley) Pull request description: This is a more narrowly-construed wait which eliminates the possibility of the wait being triggered by other messages. Note `received_block_announcement` reflect three possible messages: https://github.com/bitcoin/bitcoin/blob/edec7f7c254294cd5c46ae5cf304353d458bb852/test/functional/p2p_compactblocks.py#L34-L53 Prompted by looking into: #19449 ACKs for top commit: laanwj: Code review ACK 9e165d0de4c3cd168137fc85b8f31b371bd4e851 theStack: ACK https://github.com/bitcoin/bitcoin/pull/19631/commits/9e165d0de4c3cd168137fc85b8f31b371bd4e851 Tree-SHA512: bc4a9c8bf031c8a7efb40d9625feaa3fd1f56f3b75da7034944af71ccea44328a6c708ab0c13fea85fb7cf4fd9043fe90eb94a25e95b2d42be44c2962b4904ce
2020-08-09Merge #19657: test: Wait until is_connected in add_p2p_connectionWladimir J. van der Laan
fa4dfd215f62e88d668311701735c332c264fa2a test: Wait until is_connected in add_p2p_connection (MarcoFalke) Pull request description: Moving the wait_until from the individual test scripts to the test framework simplifies two tests ACKs for top commit: jnewbery: Code review ACK fa4dfd215f62e88d668311701735c332c264fa2a theStack: ACK https://github.com/bitcoin/bitcoin/pull/19657/commits/fa4dfd215f62e88d668311701735c332c264fa2a :coffee: Tree-SHA512: 36eda7eb323614a4c4f9215f1d7b40b9f9c4036d1c08eb701ea705f3e2986fdabd2fc558965a6aadabeed861034aeaeef3c00f968ca17ed7a27e42e506cda87d
2020-08-09Merge #19649: Restore test case for p2p transaction blindingWladimir J. van der Laan
566aada386e181c2ff40ef18ee660a819f485415 Test that wtxid relay peers add wtxid to reject filter (Gregory Sanders) 0fea6ede1b46f5137e8ea0fbacce169d97e4a5ee Restore test case for p2p transaction blinding (Gregory Sanders) Pull request description: Introduced in ca10a03addf70421893791c2c499e82fc494d60b then erroneously removed in 8d8099e97ab8af2126f6fbd223fbd82c52f2e85e. The restored line is how we are checking that the node will still re-request a specific txid given a witness-related failure. ACKs for top commit: fjahr: tACK 566aada386e181c2ff40ef18ee660a819f485415 Tree-SHA512: be2b75b5eddb88019b79cc798f9922ca7347ccbb2210b8d4eae93fdde62e2cbb614b5247cb2fbd7ee3577dbe053875a9b62c5747aace8617f12790b8fccdeab4
2020-08-07Merge #19620: Add txids with non-standard inputs to reject filterfanquake
9f88ded82b2898ca63d44c08072f1ba52f0e18d7 test addition of unknown segwit spends to txid reject filter (Gregory Sanders) 7989901c7eb62ca28b3d1e5d5831041a7267e495 Add txids with non-standard inputs to reject filter (Suhas Daftuar) Pull request description: Our policy checks for non-standard inputs depend only on the non-witness portion of a transaction: we look up the scriptPubKey of the input being spent from our UTXO set (which is covered by the input txid), and the p2sh checks only rely on the scriptSig portion of the input. Consequently it's safe to add txids of transactions that fail these checks to the reject filter, as the witness is irrelevant to the failure. This is helpful for any situation where we might request the transaction again via txid (either from txid-relay peers, or if we might fetch the transaction via txid due to parent-fetching of orphans). Further, in preparation for future witness versions being deployed on the network, ensure that WITNESS_UNKNOWN transactions are rejected in AreInputsStandard(), so that transactions spending v1 (or greater) witness outputs will fall into this category of having their txid added to the reject filter. ACKs for top commit: ajtowns: ACK 9f88ded82b2898ca63d44c08072f1ba52f0e18d7 - code review jnewbery: Code review ACK 9f88ded82b2898ca63d44c08072f1ba52f0e18d7 ariard: Code Review/Tested ACK 9f88ded naumenkogs: utACK 9f88ded82b2898ca63d44c08072f1ba52f0e18d7 jonatack: ACK 9f88ded82b2 Tree-SHA512: 1e93c0a5b68cb432524780ffc0093db893911fdfed9e2ed17f888e59114cc75d2a07062aefad4e5ce2e87c9270886117a8abb3c78fb889c9b9f31967f1777148
2020-08-06refactor: test: use _ variable for unused loop countersSebastian Falbesoner
substitutes "for x in range(N):" by "for _ in range(N):" indicates to the reader that a block is just repeated N times, and that the loop counter is not used in the body
2020-08-05Merge #19632: test: Catch decimal.InvalidOperation from TestNodeCLI#send_cliWladimir J. van der Laan
82fc4017b774aaff8799c2b6e8ba5370d94dbf4d test: Catch decimal.InvalidOperation from TestNodeCLI#send_cli (Ben Woosley) Pull request description: `decimal.InvalidOperation` is a special case of a float parsing error, which presumably should be handled in the same way as a general parsing error, rather than blow up. Alternatives include: logging the error, or re-raising with more information. Example log output: ``` File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 603, in sync_all self.sync_blocks(nodes) File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 568, in sync_blocks best_hash = [x.getbestblockhash() for x in rpc_connections] File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 568, in <listcomp> best_hash = [x.getbestblockhash() for x in rpc_connections] File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_node.py", line 571, in __call__ return self.cli.send_cli(self.command, *args, **kwargs) File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_node.py", line 639, in send_cli return json.loads(cli_stdout, parse_float=decimal.Decimal) File "/usr/lib64/python3.6/json/__init__.py", line 367, in loads return cls(**kw).decode(s) File "/usr/lib64/python3.6/json/decoder.py", line 339, in decode obj, end = self.raw_decode(s, idx=_w(s, 0).end()) File "/usr/lib64/python3.6/json/decoder.py", line 355, in raw_decode obj, end = self.scan_once(s, idx) decimal.InvalidOperation: [<class 'decimal.InvalidOperation'>] ``` See: https://travis-ci.org/github/bitcoin/bitcoin/jobs/713502326 ACKs for top commit: laanwj: ACK 82fc4017b774aaff8799c2b6e8ba5370d94dbf4d Tree-SHA512: 8c102b8bf831b05c5ca4b2e1feb5574dcbaed8cab0b2f22b013c5dfcb81788a38839a163dd1e2c6470ccbe5874214663b84485f45467738fd850ca38d539ae25
2020-08-05Merge #15382: util: add RunCommandParseJSONSamuel Dobson
31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0 [util] add RunCommandParseJSON (Sjors Provoost) c17f54ee535faaedf9033717403e1f775b5f1530 [ci] use boost::process (Sjors Provoost) 32128ba682033560d6eb2e4848a9f77a842016d2 [doc] include Doxygen comments for HAVE_BOOST_PROCESS (Sjors Provoost) 3c84d85f7d218fa27e9343c5cd1a55e519218980 [build] msvc: add boost::process (Sjors Provoost) c47e4bbf0b44f2de1278f9538124ec98ee0815bb [build] make boost-process opt-in (Sjors Provoost) 929cda5470f98d1ef85c05b1cad4e2fb9227e3b0 configure: add ax_boost_process (Sjors Provoost) 8314c23d7b39fc36dde8b40b03b6efbe96f85698 [depends] boost: patch unused variable in boost_process (Sjors Provoost) Pull request description: Prerequisite for external signer support in #16546. Big picture overview in [this gist](https://gist.github.com/Sjors/29d06728c685e6182828c1ce9b74483d). This adds a new dependency [boost process](https://github.com/boostorg/process/tree/boost-1.64.0). This is part of Boost since 1.64 which is part of `depends`. Because the minimum Boost version is 1.47, this functionality is skipped for older versions of Boost. Use `./configure --with-boost-process` to opt in, which checks for the presence of Boost::Process. We add `UniValue runCommandParseJSON(const std::string& strCommand)` to `system.{h,cpp}` which calls an arbitrary command and processes the JSON returned by it. This is currently only called by the test suite. ~For testing purposes this adds a new regtest-only RPC method `runcommand`, as well as `test/mocks/command.py` used by functional tests.~ (this is no longer the case) TODO: - [ ] review boost process in #15440 ACKs for top commit: achow101: ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0 hebasto: re-ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0, only rebased (verified with `git range-diff`) and removed an unintentional tab character since the [previous](https://github.com/bitcoin/bitcoin/pull/15382#pullrequestreview-458371035) review. meshcollider: Very light utACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0, although I am not very confident with build stuff. promag: Code review ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0, don't mind the nit. ryanofsky: Code review ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0. I left some comments below that could be ignored or followed up later. The current change is clean and comprehensive. Tree-SHA512: c506e747014b263606e1f538ed4624a8ad7bcf4e025cb700c12cc5739964e254dc04a2bbb848996b170e2ccec3fbfa4fe9e2b3976b191222cfb82fc3e6ab182d
2020-08-05Merge #19654: lint: Don't use TRAVIS_COMMIT_RANGE in commit message linterMarcoFalke
72351784b3df21a89f79076f4b814a6e700b6469 lint: Remove travis env var from commit linter (Fabian Jahr) Pull request description: #19439 was recently merged and seemed to work fine but I now noticed strange behavior when it was running in Travis, which I could not reproduce locally. It turns out `TRAVIS_COMMIT_RANGE` which is used in Travis to get the commits for the linter, uses all the commits that were in a push, which includes all rebase commits for example. This means that the linter can fail on a commit that the developer has never even seen before, which can be very confusing. See an example here which caused me to look into this: https://travis-ci.org/github/bitcoin/bitcoin/jobs/714296381 The commit that is reported as failing in my PR is not part of my PR. I think we rather want to use something like `git merge-base` to get the commit range by default and in Travis. I am leaving the env variable functionality in place with a different name but this is not a variable that can be expected to be present in the CI environments so the `merge-base` range should be used there by default. ACKs for top commit: hebasto: ACK 72351784b3df21a89f79076f4b814a6e700b6469, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: afb27bb386855cb8d5cf84fd3a6c11ef1160b25af6175ed0aa146bf04b9a26eb77298df70df0a855f8c46f19f08b3f62c49872c12974fcfa5526a15ee05b3c10
2020-08-04test addition of unknown segwit spends to txid reject filterGregory Sanders
2020-08-04test: Wait until is_connected in add_p2p_connectionMarcoFalke
2020-08-04Merge #19489: test: Fail wait_until early if connection is lostMarcoFalke
faa9a74c9e99eb43ba0d27fa906767ee88011aeb test: Fail wait_until early if connection is lost (MarcoFalke) Pull request description: Calling `minonode.wait_until` needs a connection to make progress (e.g. waiting for an inv), unless the mininode waits for the initial connection or for a disconnection. So for test development and failure debugging, fail early in all `wait_until`, unless opted out. ACKs for top commit: jnewbery: Code review ACK faa9a74c9e99eb43ba0d27fa906767ee88011aeb. Tree-SHA512: 4be850b96e23b87bc2ff42c028a5045d6f5cdbc9482ce6a6ba01cc5eb26710dab9e2ed547c363aac4bd5825151ee9996fb797261420b631bceeddbfa698d1dec
2020-08-04lint: Remove travis env var from commit linterFabian Jahr
2020-08-03Merge #18991: Cache responses to GETADDR to prevent topology leaksWladimir J. van der Laan
3bd67ba5a4ef3c20ef1f873b63c9f53a6c8608b6 Test addr response caching (Gleb Naumenko) cf1569e074505dbbb9d29422803dd31bb62072d4 Add addr permission flag enabling non-cached addr sharing (Gleb Naumenko) acd6135b43941fa51d52f5fcdb2ce944280ad01e Cache responses to addr requests (Gleb Naumenko) 7cc0e8101f01891aa8be093a00d993bb7579c385 Remove useless 2500 limit on AddrMan queries (Gleb Naumenko) ded742bc5b96e3215d69c11fb3628d224e7ae034 Move filtering banned addrs inside GetAddresses() (Gleb Naumenko) Pull request description: This is a very simple code change with a big p2p privacy benefit. It’s currently trivial to scrape any reachable node’s AddrMan (a database of all nodes known to them along with the timestamps). We do have a limit of one GETADDR per connection, but a spy can disconnect and reconnect even from the same IP, and send GETADDR again and again. Since we respond with 1,000 random records at most, depending on the AddrMan size it takes probably up to 100 requests for an spy to make sure they scraped (almost) everything. I even have a script for that. It is totally doable within couple minutes. Then, with some extra protocol knowledge a spy can infer the direct peers of the victim, and other topological stuff. I suggest to cache responses to GETADDR on a daily basis, so that an attacker gets at most 1,000 records per day, and can’t track the changes in real time. I will be following up with more improvements to addr relay privacy, but this one alone is a very effective. And simple! I doubt any of the real software does *reconnect to get new addrs from a given peer*, so we shouldn’t be cutting anyone. I also believe it doesn’t have any negative implications on the overall topology quality. And the records being “outdated” for at most a day doesn’t break any honest assumptions either. ACKs for top commit: jnewbery: reACK 3bd67ba5a4ef3c20ef1f873b63c9f53a6c8608b6 promag: Code review ACK 3bd67ba5a4ef3c20ef1f873b63c9f53a6c8608b6. ariard: Code Review ACK 3bd67ba Tree-SHA512: dfa5d03205c2424e40a3f8a41af9306227e1ca18beead3b3dda44aa2a082175bb1c6d929dbc7ea8e48e01aed0d50f0d54491caa1147471a2b72a46c3ca06b66f
2020-08-02Test that wtxid relay peers add wtxid to reject filterGregory Sanders
h/t Anthony Towns
2020-08-02Restore test case for p2p transaction blindingGregory Sanders
Introduced in ca10a03addf70421893791c2c499e82fc494d60b then erroneously removed in 8d8099e97ab8af2126f6fbd223fbd82c52f2e85e. The restored line is how we are checking that the node will still re-request a specific txid given a witness-related failure.
2020-07-31[build] msvc: add boost::processSjors Provoost
* AppVeyor boost-process vcpkg package. * Tell Boost linter to ignore it * Add HAVE_BOOST_PROCESS for MSVC build (bitcoin_config.h)
2020-07-30test: Catch decimal.InvalidOperation from TestNodeCLI#send_cliBen Woosley
decimal.InvalidOperation is a special case of a float parsing error, which presumably should be handled in the same way as a general parsing error, rather than blow up. Alternatives include: logging the error, or re-raising with more information. Example log output: File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 603, in sync_all self.sync_blocks(nodes) File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 568, in sync_blocks best_hash = [x.getbestblockhash() for x in rpc_connections] File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 568, in <listcomp> best_hash = [x.getbestblockhash() for x in rpc_connections] File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_node.py", line 571, in __call__ return self.cli.send_cli(self.command, *args, **kwargs) File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_node.py", line 639, in send_cli return json.loads(cli_stdout, parse_float=decimal.Decimal) File "/usr/lib64/python3.6/json/__init__.py", line 367, in loads return cls(**kw).decode(s) File "/usr/lib64/python3.6/json/decoder.py", line 339, in decode obj, end = self.raw_decode(s, idx=_w(s, 0).end()) File "/usr/lib64/python3.6/json/decoder.py", line 355, in raw_decode obj, end = self.scan_once(s, idx) decimal.InvalidOperation: [<class 'decimal.InvalidOperation'>]
2020-07-30test: Wait for 'cmpctblock' in p2p_compactblocks when it is expectedBen Woosley
This is a more narrowly-construed wait which eliminates the possibility of the wait being triggered by other messages. Co-authored-by: Billy Garrison <billygarrison.btc@gmail.com>
2020-07-30test: request parents of orphan from wtxid relay peerAnthony Towns
2020-07-30Merge #19439: script: Linter to check commit message formattingMarcoFalke
284a969cc082ae3c63ab523f22e71da86ad4ab20 Linter to check commit message formatting (Amir Ghorbanian) Pull request description: Write linter to check that commit messages have a new line before the body or no body at all. fixes issue #19091. ACKs for top commit: troygiorshev: ACK 284a969cc082ae3c63ab523f22e71da86ad4ab20 Reviewed, manually tested. Works great! fjahr: tested ACK 284a969cc082ae3c63ab523f22e71da86ad4ab20 adamjonas: utACK 284a969cc082ae3c63ab523f22e71da86ad4ab20 Tree-SHA512: fa278f090780b54e4fa6e2967a62b4c1a4da55d112ec1ad6dd7e1181ac490c5c1af0165524b5781b463fdd6d0f79fd3d95b5160184e6eca432ccff1189f77390
2020-07-30Merge #18011: Replace current benchmarking framework with nanobenchWladimir J. van der Laan
78c312c983255e15fc274de2368a2ec13ce81cbf Replace current benchmarking framework with nanobench (Martin Ankerl) Pull request description: Replace current benchmarking framework with nanobench This replaces the current benchmarking framework with nanobench [1], an MIT licensed single-header benchmarking library, of which I am the autor. This has in my opinion several advantages, especially on Linux: * fast: Running all benchmarks takes ~6 seconds instead of 4m13s on an Intel i7-8700 CPU @ 3.20GHz. * accurate: I ran e.g. the benchmark for SipHash_32b 10 times and calculate standard deviation / mean = coefficient of variation: * 0.57% CV for old benchmarking framework * 0.20% CV for nanobench So the benchmark results with nanobench seem to vary less than with the old framework. * It automatically determines runtime based on clock precision, no need to specify number of evaluations. * measure instructions, cycles, branches, instructions per cycle, branch misses (only Linux, when performance counters are available) * output in markdown table format. * Warn about unstable environment (frequency scaling, turbo, ...) * For better profiling, it is possible to set the environment variable NANOBENCH_ENDLESS to force endless running of a particular benchmark without the need to recompile. This makes it to e.g. run "perf top" and look at hotspots. Here is an example copy & pasted from the terminal output: | ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 2.52 | 396,529,415.94 | 0.6% | 25.42 | 8.02 | 3.169 | 0.06 | 0.0% | 0.03 | `bench/crypto_hash.cpp RIPEMD160` | 1.87 | 535,161,444.83 | 0.3% | 21.36 | 5.95 | 3.589 | 0.06 | 0.0% | 0.02 | `bench/crypto_hash.cpp SHA1` | 3.22 | 310,344,174.79 | 1.1% | 36.80 | 10.22 | 3.601 | 0.09 | 0.0% | 0.04 | `bench/crypto_hash.cpp SHA256` | 2.01 | 496,375,796.23 | 0.0% | 18.72 | 6.43 | 2.911 | 0.01 | 1.0% | 0.00 | `bench/crypto_hash.cpp SHA256D64_1024` | 7.23 | 138,263,519.35 | 0.1% | 82.66 | 23.11 | 3.577 | 1.63 | 0.1% | 0.00 | `bench/crypto_hash.cpp SHA256_32b` | 3.04 | 328,780,166.40 | 0.3% | 35.82 | 9.69 | 3.696 | 0.03 | 0.0% | 0.03 | `bench/crypto_hash.cpp SHA512` [1] https://github.com/martinus/nanobench ACKs for top commit: laanwj: ACK 78c312c983255e15fc274de2368a2ec13ce81cbf Tree-SHA512: 9e18770b18b6f95a7d0105a4a5497d31cf4eb5efe6574f4482f6f1b4c88d7e0946b9a4a1e9e8e6ecbf41a3f2d7571240677dcb45af29a6f0584e89b25f32e49e
2020-07-30Test addr response cachingGleb Naumenko
2020-07-30Add addr permission flag enabling non-cached addr sharingGleb Naumenko
2020-07-30Merge #19597: test: test decodepsbt fee calculation (count input value only ↵MarcoFalke
once per UTXO) 82dee87933ed0714976ff4eb9657acfc13c6de84 test: test decodepsbt fee calculation (count input value only once per UTXO) (Sebastian Falbesoner) Pull request description: Fixes #19523, adding a simple test to `rpc_psbt.py` that checks that the decodepsbt fee matches the one given by the wallet (`walletcreatefundedpsbt`). This is in particular important for PSBTs with segwit inputs that have both a witness- and a non-witness-UTXO type set. Example test run after reverting commit 75122780e2c46505d977e24c5612dfa9442ab754 ("Increment input value sum only once per UTXO in decodepsbt"): ``` $ test/functional/rpc_psbt.py 2020-07-26T11:31:44.862000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test__sutcd4y 20.00007580 2020-07-26T11:31:47.073000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/home/honeybadger/buidl/bitcoin_thestack/test/functional/test_framework/test_framework.py", line 118, in main self.run_test() File "test/functional/rpc_psbt.py", line 166, in run_test assert_equal(decoded['fee'], created_psbt['fee']) File "/home/honeybadger/buidl/bitcoin_thestack/test/functional/test_framework/util.py", line 49, in assert_equal raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args)) AssertionError: not(20.00007580 == 0.00007580) 2020-07-26T11:31:47.125000Z TestFramework (INFO): Stopping nodes ...... ``` ACKs for top commit: achow101: ACK 82dee87933ed0714976ff4eb9657acfc13c6de84 Tree-SHA512: 296b8a701f851d482ef6200c6cbf0cf0257a79a828ac6dbc39b05d8c2d839c6fdb9d3f5a084015295cfa3eac7c11faa2f2d52e619c11627b04c75150eead8330
2020-07-30Merge #19599: test: clean message_count and last_messageMarcoFalke
2c6a02e0248825e205e6deea4c38409044feb4ab Clean message_count and last_message (Troy Giorshev) Pull request description: From #19580 This PR changes comments to clarify the intended usage of `message_count` and `last_message`. Additionally it changes the only usage of `message_count` to use `last_message` instead, bringing the code into alignment with the intended usage. Note: Now `message_count` is completely unused. However, it is ready to be used (i.e. the supporting code works) and likely will be used in some test in the future. ACKs for top commit: jnewbery: utACK 2c6a02e0248825e205e6deea4c38409044feb4ab Tree-SHA512: 07c7684c9586de4f845e10d7aac36c1aab9fb56b409949c1c70d5ca705bc3971ca7d5943245a0472def4efd7b4e1c5dad2f713db5ead8fca08404daf4891e98b
2020-07-29Merge #19335: wallet: Cleanup and separate BerkeleyDatabase and BerkeleyBatchWladimir J. van der Laan
74507ce71eb61105fb3ae8460999099234ca7b8b walletdb: Remove BerkeleyBatch friend class from BerkeleyDatabase (Andrew Chow) 00f0041351bcd6ddbab110df1189f79ce011e192 No need to check for duplicate fileids in all dbenvs (Andrew Chow) d86efab37002841fd059251672e1ec1a977b743f walletdb: Move Db->open to BerkeleyDatabase::Open (Andrew Chow) 4fe4b3bf1b152877677a6115f82aefaf318dd514 walletdb: track database file use as m_refcount within BerkeleyDatabase (Andrew Chow) 65fb8807ac402d1e924fd85969b5837c192bf59f Combine BerkeleyEnvironment::Verify into BerkeleyDatabase::Verify (Andrew Chow) Pull request description: `BerkeleyBatch` and `BerkeleyDatabase` are kind of messy. The goal of this is to clean up them up so that they are logically separated. `BerkeleyBatch` currently handles the creation of the `BerkeleyDatabase`'s `Db` handle. This is instead moved into `BerkeleyDatabase` and is called by `BerkeleyBatch`. Instead of having `BerkeleyEnvironment` track each database's usage, have `BerkeleyDatabase` track this usage itself with the `m_refcount` variable that is present in `WalletDatabase`. Lastly, instead of having each `BerkeleyEnvironment` store the fileids of the databases open in it, have a global `g_fileids` to track those fileids. We were already checking fileid uniqueness globally (by checking the fileids in every environment when opening a database) so it's cleaner to do this with a global variable. All of these changes allow us to make `BerkeleyBatch` and `BerkeleyDatabase` no longer be friend classes. The diff of this PR is currently the same as in ##18971 Requires #19334 ACKs for top commit: laanwj: Code review ACK 74507ce71eb61105fb3ae8460999099234ca7b8b ryanofsky: Code review ACK 74507ce71eb61105fb3ae8460999099234ca7b8b. No changes since last review other than rebase Tree-SHA512: 845d84ee1a470e2bf5d2e2e3d7738183d8ce43ddd06a0bbd57edecf5779b2f55d70728b1b57f5daab0f078650a8d60c3e19dc30b75b36e7aa952ce268399d5f6
2020-07-27Clean message_count and last_messageTroy Giorshev
This commit clarifies the intended usage of message_count and last_message. Additionally it changes the only usage of message_count to using last_message instead, bringing the code further along the intended usage.
2020-07-26test: test decodepsbt fee calculation (count input value only once per UTXO)Sebastian Falbesoner
Checks that the RPC decodepsbt calculates the fee correctly, in particular for PSBTs with segwit inputs that have both a witness- and a non-witness-UTXO type set. Before commit 75122780e2c46505d977e24c5612dfa9442ab754 ("Increment input value sum only once per UTXO in decodepsbt") the values for those inputs were double counted.
2020-07-23Merge #15935: Add <datadir>/settings.json persistent settings storageMarcoFalke
9c69cfe4c54e38edd2f54303be2f8a53dcf5bad8 Add <datadir>/settings.json persistent settings storage. (Russell Yanofsky) eb682c5700e7a9176d0104d470b83ff9aa3589e8 util: Add ReadSettings and WriteSettings functions (Russell Yanofsky) Pull request description: Persistent settings are used in followup PRs #15936 to unify gui settings between bitcoin-qt and bitcoind, and #15937 to add a load_on_startup flag to the loadwallet RPC and maintain a dynamic list of wallets that should be loaded on startup that also can be shared between bitcoind and bitcoin-qt. ACKs for top commit: MarcoFalke: Approach re-ACK 9c69cfe4c54e38edd2f54303be2f8a53dcf5bad8 🌾 jnewbery: utACK 9c69cfe4c54e38edd2f54303be2f8a53dcf5bad8 Tree-SHA512: 39fcc6051717117c9141e934de1d0d3f739484be4685cdf97d54de967c8c816502b4fd0de12114433beaa5c5b7060c810fd8ae4e2b3ce7c371eb729ac01ba2e1
2020-07-23Merge #19473: net: Add -networkactive optionMarcoFalke
2aac093a3d60e446b85eebdf170ea6bed77bec92 test: Add test coverage for -networkactive option (Hennadii Stepanov) 3c58129b1293742a49aa196cb210ff345a7339e6 net: Log network activity status change unconditionally (Hennadii Stepanov) 62fe6aa87e4cdd8b06207abc1387c68d7bfc04c1 net: Add -networkactive option (Hennadii Stepanov) Pull request description: Some Bitcoin Core activity is completely local (offline), e.g., reindexing. The `setnetworkactive` RPC command is already present. This PR adds the corresponding command-line argument / config option, and allows to start the client with disabled p2p network by providing `-networkactive=0` or `-nonetworkactive`. This was done while reviewing #16981. ACKs for top commit: MarcoFalke: re-ACK 2aac093a3d60e446b85eebdf170ea6bed77bec92 🏠 LarryRuane: ACK 2aac093a3d60e446b85eebdf170ea6bed77bec92 Tree-SHA512: 446d791b46d7b556d7694df7b1f88cd4fbc09301fe4eaf036b45cb8166ed806156353cc03788a07b633d5887d5eee30a7c02a2d4307141c8ccc75e0a88145636
2020-07-22No need to check for duplicate fileids in all dbenvsAndrew Chow
Since we have .walletlock in each directory, we don't need the duplicate fileid checks across all dbenvs as it shouldn't be possible anyways.
2020-07-22test: Add test coverage for -networkactive optionHennadii Stepanov
2020-07-22Merge #18044: Use wtxid for transaction relayWladimir J. van der Laan
0a4f1422cd1c20e12a05d7ff1a2ef1d5e7c654bb Further improve comments around recentRejects (Suhas Daftuar) 0e20cfedb704c1f76bb727e2009867d3d503a03d Disconnect peers sending wtxidrelay message after VERACK (Suhas Daftuar) cacd85209e20fc0fd08f86eed23b6ef93484ffcf test: Use wtxid relay generally in functional tests (Fabian Jahr) 8d8099e97ab8af2126f6fbd223fbd82c52f2e85e test: Add tests for wtxid tx relay in segwit test (Fabian Jahr) 9a5392fdf67f1c5c90f52d3cdb3dea4f35d1609f test: Update test framework p2p protocol version to 70016 (Fabian Jahr) dd78d1d641178b473ab1156b71a837b9e686792b Rename AddInventoryKnown() to AddKnownTx() (Suhas Daftuar) 4eb515574e1012bc8ea5dafc3042dcdf4c766f26 Make TX_WITNESS_STRIPPED its own rejection reason (Suhas Daftuar) 97141ca442daea8fc9c307cf81a02b38dcc28fd8 Delay getdata requests from peers using txid-based relay (Suhas Daftuar) 46d78d47dea345329ba094310eec56ab00a02ddc Add p2p message "wtxidrelay" (Suhas Daftuar) 2d282e0cba9761574b6b43d134ca95f3052d7fd2 ignore non-wtxidrelay compliant invs (Anthony Towns) ac88e2eb619821ad7ae1d45d4b40be69051d3999 Add support for tx-relay via wtxid (Suhas Daftuar) 8e68fc246d09f1e6c6dfa8c676969d97c2eb4334 Add wtxids to recentRejects instead of txids (Suhas Daftuar) 144c38582042c3b9ec53bb97ba0644fc0114f8fb Add wtxids of confirmed transactions to bloom filter (Suhas Daftuar) 85c78d54af462996a0bca6cf97f91e1aa8778ae8 Add wtxid-index to orphan map (Suhas Daftuar) 08b39955ec7f84e835ab0b1366f0dd28dfd6ce03 Add a wtxid-index to mapRelay (Suhas Daftuar) 60f0acda713e7b9dc188aef54ef93981a93f4e44 Just pass a hash to AddInventoryKnown (Suhas Daftuar) c7eb6b4f1fe5bd76388a023529977674534334a7 Add wtxid to mempool unbroadcast tracking (Amiti Uttarwar) 2b4b90aa8f0440deacefb5997d7bd1f9f5c591b3 Add a wtxid-index to the mempool (Suhas Daftuar) Pull request description: Using txids (a transaction's hash, without witness) for transaction relay is problematic, post-segwit -- if a peer gives us a segwit transaction that fails policy checks, it could be because the txid associated with the transaction is definitely unacceptable to our node (regardless of the witness), or it could be that the transaction was malleated and with a different witness, the txid could be accepted to our mempool. We have a bloom filter of recently rejected transactions, whose purpose is to help us avoid redownloading and revalidating transactions that fail to be accepted, but because of this potential for witness malleability to interfere with relay of valid transactions, we do not use the filter for segwit transactions. This issue is discussed at some length in #8279. The effect of this is that whenever a segwit transaction that fails policy checks is relayed, a node would download that transaction from every peer announcing it, because it has no way presently to cache failure. Historically this hasn't been a big problem, but if/when policy for accepting segwit transactions were to change (eg taproot, or any other change), we could expect older nodes talking to newer nodes to be wasting bandwidth because of this. As discussed in that issue, switching to wtxid-based relay solves this problem -- by using an identifier for a transaction that commits to all the data in our relay protocol, we can be certain if a transaction that a peer is announcing is one that we've already tried to process, or if it's something new. This PR introduces support for wtxid-based relay with peers that support it (and remains backwards compatible with peers that use txids for relay, of course). Apart from code correctness, one issue to be aware of is that by downloading from old and new peers alike, we should expect there to be some bandwidth wasted, because sometimes we might download the same transaction via txid-relay as well as wtxid-relay. The last commit in this PR implements a heuristic I want to analyze, which is to just delay relay from txid-relay peers by 2 seconds, if we have at least 1 wtxid-based peer. I've just started running a couple nodes with this heuristic so I can measure how well it works, but I'm open to other ideas for minimizing that issue. In the long run, I think this will be essentially a non-issue, so I don't think it's too big a concern, we just need to bite the bullet and deal with it during upgrade. Finally, this proposal would need a simple BIP describing the changes, which I haven't yet drafted. However, review and testing of this code in the interim would be welcome. To do items: - [x] Write BIP explaining the spec here (1 new p2p message for negotiating wtxid-based relay, along with a new INV type) - [ ] Measure and evaluate a heuristic for minimizing how often a node downloads the same transaction twice, when connected to old and new nodes. ACKs for top commit: naumenkogs: utACK 0a4f1422cd1c20e12a05d7ff1a2ef1d5e7c654bb laanwj: utACK 0a4f1422cd1c20e12a05d7ff1a2ef1d5e7c654bb Tree-SHA512: d8eb8f0688cf0cbe9507bf738e143edab1f595551fdfeddc2b6734686ea26e7f156b6bfde38bad8bbbe8bec1857c7223e1687f8f018de7463dde8ecaa8f450df
2020-07-21Merge #19552: test: fix intermittent failure in p2p_ibd_txrelayMarcoFalke
12410b1feb80189061eb4a2b43421e53cbb758a8 test: fix intermittent p2p_ibd_txrelay race, add test_framework.py#wait_until (Jon Atack) Pull request description: To fix these intermittent failures in Travis CI. ``` 162/163 - p2p_ibd_txrelay.py failed, Duration: 2 s stdout: 2020-07-19T05:44:17.213000Z TestFramework (INFO): Check that nodes set minfilter to MAX_MONEY while still in IBD 2020-07-19T05:44:17.216000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/Users/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-x86_64-apple-darwin16/test/functional/test_framework/test_framework.py", line 117, in main self.run_test() File "/Users/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-x86_64-apple-darwin16/test/functional/p2p_ibd_txrelay.py", line 30, in run_test assert_equal(conn_info['minfeefilter'], MAX_FEE_FILTER) File "/Users/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-x86_64-apple-darwin16/test/functional/test_framework/util.py", line 49, in assert_equal raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args)) AssertionError: not(0E-8 == 0.09170997) 2020-07-19T05:44:17.293000Z TestFramework (INFO): Stopping nodes ``` At Marco's suggestion, cherry-picked part of #19134 to nicely simplify using `wait_until`. ACKs for top commit: vasild: ACK 12410b1fe Tree-SHA512: 615f509883682fd693e578b259cba35a9fa0bc519f1394e88c857e8b0650bfec5397bfa856cfa9e6d5ef81d0ee6ad02e4ad2b0eb0bd530b4c281cbe3e663790b
2020-07-21Merge #19205: script: previous_release.sh rewritten in pythonMarcoFalke
9c34aff39309b8adc99d347e07b6ddb5366498e9 Remove previous_release.sh (Brian Liotti) e1e5960e10a9329d9f55a3967d546ffbdd896030 script: Add previous_release.py (Brian Liotti) Pull request description: Closes #18132 Added functionality: 1) checks file hash before untarring when using the binary download option ACKs for top commit: fjahr: re-ACK 9c34aff39309b8adc99d347e07b6ddb5366498e9 Sjors: tACK 9c34aff39309b8adc99d347e07b6ddb5366498e9 Tree-SHA512: 323f11828736a372a47f048592de8b027ddcd75b38f312dfc73f7b495d1e078bfeb384d9cdf434b3e70f2c6c0ce2da2df48e9a6460ac0e1967c6829a411c52d5
2020-07-19test: fix intermittent p2p_ibd_txrelay race, add test_framework.py#wait_untilJon Atack
2020-07-19test: Use wtxid relay generally in functional testsFabian Jahr
2020-07-19test: Add tests for wtxid tx relay in segwit testFabian Jahr
Also cleans up some doublicate lines in the rest of the test. co-authored-by: Anthony Towns <aj@erisian.com.au>
2020-07-19test: Update test framework p2p protocol version to 70016Fabian Jahr
This new p2p protocol version allows to use WTXIDs for tx relay.
2020-07-19Delay getdata requests from peers using txid-based relaySuhas Daftuar
Using both txid and wtxid-based relay with peers means that we could sometimes download the same transaction twice, if announced via two different hashes from different peers. Use a heuristic of delaying txid-peer-getdata requests by 2 seconds, if we have at least one wtxid-based peer.
2020-07-19Add support for tx-relay via wtxidSuhas Daftuar
This adds a field to CNodeState that tracks whether to relay transactions with that peer via wtxid, instead of txid. As of this commit the field will always be false, but in a later commit we will add a way to negotiate turning this on via p2p messages exchanged with the peer.
2020-07-17Merge #19538: ci: Add tsan suppression for race in DatabaseBatchMarcoFalke
0cdf2a77ddfa1d53c6fbd830d557a3f20d7fc365 ci: add tsan debug symbols option (Russell Yanofsky) 9a2f12680b3f00a207f1cdd4e0c50a3c7613aefc ci: Add tsan suppression for race in DatabaseBatch (Hennadii Stepanov) Pull request description: Since #19325 was merged, the corresponding change in TSan suppression file gets required. This PR is: - an analogous to #19226 and #19450, and - a temporary workaround for CI fail like https://cirrus-ci.com/task/5741795508224000?command=ci#L4993 ACKs for top commit: MarcoFalke: ACK 0cdf2a77ddfa1d53c6fbd830d557a3f20d7fc365 Tree-SHA512: 7832f143887c8a0df99dea03e00694621710378fbe923e3592185fcd3658546a590693b513abffc5ab96e9ef76c9c4bff3330eeee69a0c5dbe7574f34c417220
2020-07-17Merge #19423: test: add functional test for txrelay during and after IBDMarcoFalke
cb31ee01b42af58c64b5d3c0eabae4bcd7fd67cf [test] feefilter during and after IBD (gzhao408) Pull request description: This is a followup to #19204 which uses `minfeefilter=MAX_MONEY` to effectively shut off txrelay, thereby reducing inv traffic, when nodes are in IBD. It was [missing](https://github.com/bitcoin/bitcoin/pull/19204#issuecomment-644040070) a functional test. ACKs for top commit: jnewbery: utACK cb31ee01b42af58c64b5d3c0eabae4bcd7fd67cf Tree-SHA512: a9effc8193fa95fb42a2f9c66b258cc7b0941fc04c1ce3a6092f4426c9bfc7e72f702aca559b3e30e90652497f411f22fae3cf5cdb6cfd6ef6d37fed712cda67
2020-07-16ci: Add tsan suppression for race in DatabaseBatchHennadii Stepanov
2020-07-16Merge #16525: Dump transaction version as an unsigned integer in RPC/TxToUnivWladimir J. van der Laan
e80259f1976545e4f1ab6a420644be0c32261773 Additionally treat Tx.nVersion as unsigned in joinpsbts (Matt Corallo) 970de70bdd3542e75b73c79b06f143168c361494 Dump transaction version as an unsigned integer in RPC/TxToUniv (Matt Corallo) Pull request description: Consensus-wise we already treat it as an unsigned integer (the only rules around it are in CSV/locktime handling), but changing the underlying data type means touching consensus code for a simple cleanup change, which isn't really worth it. See-also, https://github.com/rust-bitcoin/rust-bitcoin/pull/299 ACKs for top commit: sipa: ACK e80259f1976545e4f1ab6a420644be0c32261773 practicalswift: ACK e80259f1976545e4f1ab6a420644be0c32261773 ajtowns: ACK e80259f1976545e4f1ab6a420644be0c32261773 code review -- checked all other uses of tx.nVersion treat it as unsigned (except for policy.cpp:IsStandard anyway), so looks good. naumenkogs: ACK e80259f Tree-SHA512: 6760a2c77e24e9e1f79a336ca925f9bbca3a827ce02003c71d7f214b82ed3dea13fa7d9f87df9b9445cd58dff8b44a15571d821c876f22f8e5a372a014c9976b
2020-07-15[test] feefilter during and after IBDgzhao408
Co-authored-by: Jon Atack <jon@atack.com>
2020-07-15Merge #19512: p2p: banscore updates to gui, tests, release notesWladimir J. van der Laan
fa108d6a757838225179a8df942cfb6d99c98c90 test: update tests for peer discouragement (Jon Atack) 1a9f462caa63fa16d7b4415312d2032a42b3fe0b gui, doc: rm Ban Score in GUI Peers window/release notes updates (Jon Atack) Pull request description: This is the third `-banscore` PR in the mini-series described in #19464. See that PR for the intention and reasoning. - no longer display "Ban Score" in the GUI peers window and add a release note, plus release note fixups per https://github.com/bitcoin/bitcoin/pull/19464#pullrequestreview-447452052 - update tests (`src/test/denialofservice_tests.cpp` and `test/functional/p2p_leak.py`) from banning to discouragement and per https://github.com/bitcoin/bitcoin/pull/19464#issuecomment-658052518 ACKs for top commit: jnewbery: ACK fa108d6a757838225179a8df942cfb6d99c98c90 laanwj: ACK fa108d6a757838225179a8df942cfb6d99c98c90 Tree-SHA512: 58a449b3f47b8cb5490b34e4442ee8675bfad1ce48af4e4fd5c67715b0c1a596fb8e731d42e576b4c3b64627f76e0a68cbb1da9ea9f588a5932fe119baf40d50
2020-07-15Linter to check commit message formattingAmir Ghorbanian
Write linter to check that commit messages have a new line before the body or no body at all. reference: gist.github.com/agnivade/67b42d664ece2d4210c7 Fixes issue #19091.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-07 16:42:36 +0000