Commit graph

2083 commits

Author SHA1 Message Date
Wladimir J. van der Laan cb11324a63
Merge #17051: tests: Add deserialization fuzzing harnesses
897849d8c2 tests: Add deserialization fuzzing harnesses (practicalswift)
16f0a186dc tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift)

Pull request description:

  Add deserialization fuzzing harnesses.

  **Testing this PR**

  Run:

  ```
  $ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
  $ make
  $ contrib/devtools/test_fuzzing_harnesses.sh 'addr_info|block_file_info|block_filter|block_header|ext_key|ext_pub_key|fee_rate|flat_file|key_origin|merkle_block|mutable_transaction|out_point|partial_merkle_tree|partially_signed_transaction|prefilled_transaction|psbt_input|psbt_output|pub_key|script_deserialize|sub_net|tx_in' 10
  ```

  `test_fuzzing_harnesses.sh` can be found in PR #17000.

ACKs for top commit:
  laanwj:
    thanks, ACK 897849d8c2

Tree-SHA512: 5a270a3002cc23b725f7b35476a43777b2b00b4d089cc006372e2fcc7afa430afaa3c1430f778ae08fc53dd85a13e7bd2fab0449c319f676423226e189a417f6
2019-12-06 09:45:26 +01:00
MarcoFalke 910a4301b1
Merge #17522: test: Wait until mempool is loaded in wallet_abandonconflict
dddd09eb33 test: Wait until mempool is loaded in wallet_abandonconflict (MarcoFalke)

Pull request description:

  This might or might not fix intermittent issues such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/28724018#L4091

  I believe the mempool was not loaded fully after the restart, in which case it was not dumped either on the next restart. Thus, the previous mempool was attempted to be loaded a second time, which succeeded and contained the txs.

ACKs for top commit:
  laanwj:
    ACK dddd09eb33

Tree-SHA512: ab7061f946b5e5388f825dddceadb125f5197b24af3a7fcf1e700235d106a323419a56bfb4d84a2e27442e0de63e540c623b704343d83a98deaab3c02fcbdcbe
2019-12-05 15:52:17 -05:00
fanquake 5aee0e2163
Merge #17650: util: remove unwanted fields from bitcoin-cli -getinfo
01c8701559 util: remove unwanted fields from bitcoin-cli -getinfo (malevolent)

Pull request description:

  Removed the following fields from -getinfo: protocolversion, walletversion and keypoololdest. This change closes #17314 .

ACKs for top commit:
  laanwj:
    ACK 01c8701559
  achow101:
    ACK 01c8701559
  practicalswift:
    ACK 01c8701559 -- diff looks correct

Tree-SHA512: c98f2e8a3fee04d46766f70cb88f4e49e892a4424eff8940a7d48e9e808597b702427225788f984f5c3641591fd8d86acee56774afde1d57a4259c31d971ea08
2019-12-04 14:59:12 -05:00
MarcoFalke fa1bfc476c
ci: ubsan report_error_type=1 and add suppressions 2019-12-04 12:16:38 -05:00
MarcoFalke fa69cef13e
test: Print stderr when subprocess fails 2019-12-04 11:25:32 -05:00
malevolent 01c8701559 util: remove unwanted fields from bitcoin-cli -getinfo
In accordance with #17314, Removing noisy fields from -getinfo. Fields removed: protocolversion, walletversion and keypoololdest. In addition to changing bitcoin-cli -getinfo, there is another change to test/functional/interface_bitcoin_cli.py. This change deletes tests that utilize removed -getinfo calls.
2019-12-04 06:48:53 +00:00
MarcoFalke 69ca6cdd66
Merge #17599: ci: Run functional tests on s390x
fabd71076c ci: Print free disk space (MarcoFalke)
fad9fdbea5 test: Properly deserialize integers in little-endian (MarcoFalke)
fa94fc10c8 ci: Run functional tests on s390x (MarcoFalke)

Pull request description:

Top commit has no ACKs.

Tree-SHA512: 98ba77eb56f283131fdaeb393fda86cc308f1bf9781e1e0e5736b8d616528dc8ff2e494d55ba107c138083025c66a59e382fcfa9962d4349a5fd6cbbc52484c3
2019-12-03 10:34:11 -05:00
MarcoFalke 2b6575d989
Merge #17643: wallet: Fix origfee return for bumpfee with feerate arg
02afb0c550 Fix origfee return for bumpfee with feerate arg (Gregory Sanders)

Pull request description:

  fixes https://github.com/bitcoin/bitcoin/issues/17642 and adds a simple test that would have caught it

ACKs for top commit:
  achow101:
    ACK 02afb0c550

Tree-SHA512: 303e392e05407f204dffe360689b5bb5dc77fd462dd0e489bc0b6c8f94f89ab7fe2bd8cb47e4dc6dc5c23a619826d15f3bf6b02b2c8e96402fbb51953c462e2d
2019-12-03 10:25:34 -05:00
Gregory Sanders 02afb0c550 Fix origfee return for bumpfee with feerate arg 2019-12-01 20:54:16 -05:00
fanquake 19698ac6bc
Merge #17568: wallet: fix when sufficient preset inputs and subtractFeeFromOutputs
eadd1304c8 tests: Add a test for funding with sufficient preset inputs and subtractFeeFromOutputs (Andrew Chow)
ff330badd4 Default to bnb_used = false as there are many cases where BnB is not used (Andrew Chow)

Pull request description:

  #17290 introduced a bug where, when we had preset inputs that covered the amount being sent and subtractFeeFrromOutputs was being used, transaction funding would result in a `Fee exceeds maximum configured by -maxtxfee` error. This was happening because we weren't setting `bnb_used = false` when the preset inputs were used as it should have been. This resulted in a too high fee because the change would go to fees accidentally.

  Apparently this particular case doesn't have a test, so I've added one as well.

ACKs for top commit:
  Sjors:
    ACK eadd130. I can't get this new test to fail on macOS (without this PR). It passes whether or not I compile with `--enable-debug`. It does fail on Ubuntu. Yay undefined behavior... Anyway, it's a useful test.
  fanquake:
    ACK eadd1304c8
  instagibbs:
    utACK eadd1304c8

Tree-SHA512: 7286c321f78666eea558cc591174630d210263594df41cab1065417510591ee514ade0e1d0cec8af09a785757da68de82592b013e8fe8d4966cec3254368706e
2019-12-01 12:23:44 -05:00
Wladimir J. van der Laan 1f59885d27
Merge #17361: script: Lint Gitian descriptors with ShellCheck
17f81e9648 script: Enable SC2001 rule for Gitian scripts (Hennadii Stepanov)
61bb21b418 script: Enable SC2155 rule for Gitian scripts (Hennadii Stepanov)
577682d9e8 script: Enable SC2006 rule for Gitian scripts (Hennadii Stepanov)
14aded46df script: Lint Gitian descriptors with ShellCheck (Hennadii Stepanov)

Pull request description:

  This PR extracts shell scripts from Gitian descriptors (`contrib/gitian-descriptors/`) and checks for ShellCheck warnings as any other one.

  Some non-controversial warnings are fixed.

ACKs for top commit:
  practicalswift:
    ACK 17f81e9648 -- diff looks correct

Tree-SHA512: bdfa3d35bbb65ff634c90835d75c3df63e958b558599771d21366724f5cf64da83a68957d926e926a99c3704b9529e96a17697dc8d9ff3adf7154d9cb1999a8d
2019-11-28 10:18:22 +01:00
Hennadii Stepanov 17f81e9648
script: Enable SC2001 rule for Gitian scripts 2019-11-27 19:27:56 +02:00
Hennadii Stepanov 61bb21b418
script: Enable SC2155 rule for Gitian scripts
Also pwd command is replaced with $PWD variable everywhere for
consistency.
2019-11-27 19:25:29 +02:00
Andrew Chow eadd1304c8 tests: Add a test for funding with sufficient preset inputs and subtractFeeFromOutputs 2019-11-26 13:02:46 -05:00
MarcoFalke fad9fdbea5
test: Properly deserialize integers in little-endian 2019-11-25 15:27:50 -05:00
Jon Atack 33f5fc32e5
test: add rpc getaddressinfo labels test coverage 2019-11-24 23:08:38 +01:00
Jon Atack 0f3539ac6d
test: add listlabels test in wallet_labels.py 2019-11-24 23:07:10 +01:00
Hennadii Stepanov 3ed5e6819a
refactor: Nuke coincontrol circular dependency 2019-11-23 08:30:03 +02:00
Samuel Dobson 4effd67bf4
Merge #17387: wallet_importmulti: use addresses of the same type as being imported
b84e776fd1 wallet_importmulti: use addresses of the same type as being imported (Andrew Chow)

Pull request description:

  When constructing an import from the solving data of an address, make sure that the original address is the same type as the one that will be imported.

  See also: https://github.com/bitcoin/bitcoin/pull/17374#issuecomment-550036931

  Part of #17261

ACKs for top commit:
  Sjors:
    Code review ACK b84e776
  meshcollider:
    Tested re-ACK b84e776fd1

Tree-SHA512: 53c49c63af8cbade0116a62beddc77df1a411d8ed76571c3053f6aff096f41a5325421a188bab3dcacfda69bb28fdff6ba921ddd80f29c4abbadb3b58fda884c
2019-11-23 10:20:04 +13:00
251 e20c72f9f0 Fire TransactionRemovedFromMempool from mempool
This commit fires TransactionRemovedFromMempool callbacks from the
mempool and cleans up a bunch of code.
2019-11-21 21:05:38 +01:00
Wladimir J. van der Laan 69a6f1ad1f
Merge #17513: refactor, qt: Nuke some circular dependencies
5f50599ae7 refactor: Cleanup headers from walletmodel.h (Hennadii Stepanov)
a53e9895db refactor: Nuke walletmodel circular dependency (Hennadii Stepanov)
49c4211c04 refactor: Nuke walletmodeltransaction circular dep (Hennadii Stepanov)
567cb44eb9 refactor: Nuke guiutil circular dependency (Hennadii Stepanov)
73b5505cfe refactor: Move SendCoinsRecipient in own header (Hennadii Stepanov)

Pull request description:

  This PR gets rid of the following circular dependencies:
  - `qt/guiutil` -> `qt/walletmodel` -> `qt/optionsmodel` -> `qt/guiutil`
  - `qt/walletmodel` -> `qt/walletmodeltransaction` -> `qt/walletmodel`
  - `qt/paymentserver` -> `qt/walletmodel` -> `qt/paymentserver`

ACKs for top commit:
  Sjors:
    ACK 5f50599ae7
  instagibbs:
    code review ACK 5f50599ae7
  practicalswift:
    ACK 5f50599ae7 -- diff looks correct
  promag:
    ACK 5f50599ae7.

Tree-SHA512: 070686ac82b5c68c3ef1b8b4c16b4b916b84d80d1e92e42287fdd9454671bea54779c0d2db4db623750aaaf180beaba212137190d6a427113905e2c4be5c60c5
2019-11-21 19:38:39 +01:00
practicalswift 897849d8c2 tests: Add deserialization fuzzing harnesses 2019-11-21 17:53:06 +00:00
practicalswift 16f0a186dc tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus 2019-11-21 17:52:35 +00:00
MarcoFalke 5ff798c39b
Merge #17439: refactor: Use proper MAX_SCRIPT_ELEMENT_SIZE constants consistently
cb9d830a00 test: Use proper MAX_SCRIPT_ELEMENT_SIZE (Hennadii Stepanov)
402ee706d8 refactor: Use proper MAX_SCRIPT_ELEMENT_SIZE const (Hennadii Stepanov)

Pull request description:

  This PR replaces well-known "magic" numbers with proper `MAX_SCRIPT_ELEMENT_SIZE` constants.

ACKs for top commit:
  practicalswift:
    ACK cb9d830a00 -- diff looks correct and change appears to be complete
  instagibbs:
    utACK cb9d830a00

Tree-SHA512: 5fa033275d6df7e35962c38bfdf09a7b5cd7ef2ccdd5e30a39ba47d0c21ac779a5559c23f5ef5bfd4293be0fc639e836a308bbedf0e34717e1eead983b389bbd
2019-11-21 10:11:06 -05:00
Sebastian Falbesoner 8f2d7737cc test: add functional test for non-standard txs with too large scriptSig
A transaction is rejected by the mempool with reason "scriptsig-size" if any of
the inputs' scriptSig is larger than 1650 bytes.
2019-11-20 11:01:27 +01:00
MarcoFalke dddd09eb33
test: Wait until mempool is loaded in wallet_abandonconflict 2019-11-19 14:29:43 -05:00
fanquake b4a1da9ef8
Merge #17515: Remove straggling OpenSSL references from doc and build
ea3c7e585c test: Remove libssl-dev packages from CI scripts (Wladimir J. van der Laan)
7ea55264b9 test: remove lsan suppression for libcrypto (Wladimir J. van der Laan)
2d7066527a build: remove libcrypto as internal dependency in libbitcoinconsensus.pc (Wladimir J. van der Laan)
278751ea11 doc: Remove ssl as a required dependency from build-unix (Wladimir J. van der Laan)

Pull request description:

  Some doc and build cleanups following #17265.

  I intentionally left the libssl-dev install in `gitian-win-signer.yml`, as it's necessary for the ossl signer.

ACKs for top commit:
  MarcoFalke:
    ACK ea3c7e585c 🗯
  jamesob:
    ACK ea3c7e585c
  practicalswift:
    ACK ea3c7e585c - nice!
  fanquake:
    ACK ea3c7e585c - thanks.

Tree-SHA512: 67ea35bdd6d6e512d69e6734713534c88cae033a2ed695677ea15c3e3d5ff570374e342775c88e60877fa43a19047853e7b2a433e2c9a4349a5c423726a7457e
2019-11-19 11:38:06 -05:00
Hennadii Stepanov a53e9895db
refactor: Nuke walletmodel circular dependency 2019-11-19 17:05:35 +02:00
Hennadii Stepanov 49c4211c04
refactor: Nuke walletmodeltransaction circular dep 2019-11-19 17:05:35 +02:00
Hennadii Stepanov 567cb44eb9
refactor: Nuke guiutil circular dependency
Co-authored-by: Gregory Sanders <gsanders87@gmail.com>
2019-11-19 17:07:18 +02:00
Wladimir J. van der Laan 7ea55264b9 test: remove lsan suppression for libcrypto 2019-11-19 15:03:44 +01:00
fanquake a67352161c
test: skip tool_wallet test when bitcoin-wallet isn't compiled 2019-11-18 16:48:52 -05:00
fanquake e9277baed6
test: skip wallet_listreceivedby test when the cli isn't compiled 2019-11-18 16:47:41 -05:00
fanquake 621d398750
test: skip bitcoin_cli test when the cli isn't compiled 2019-11-18 16:19:15 -05:00
Wladimir J. van der Laan ca1eeba0b0
Merge #17488: test: fix "bitcoind already running" warnings on macOS
1c23ea5fe6 test: fix bitcoind already running warnings on macOS (fanquake)

Pull request description:

  On macOS, `pidof` installed via brew returns b'' rather than None.
  Account for this, to remove spurious warnings from the test_runner.

ACKs for top commit:
  laanwj:
    ACK 1c23ea5fe6

Tree-SHA512: 640f4323d4105eac5c7abb52daf80486d5d3b4a074720490ceeb97c3dd8d73a3de9a988d2550f1e2076c620bb10d452b2959d8b723d2ee64f499878909824e31
2019-11-18 14:23:41 +01:00
Wladimir J. van der Laan 24647a09e7
Merge #17470: ci: Use clang-8 for fuzzing to run on aarch64 ci systems
fa2ec9f451 fuzz: Bump timeout in test_runner to accomodate for slow arm64 CPUs (MarcoFalke)
fa6e01b2f3 ci: Use clang-8 for fuzzing to run on aarch64 ci systems (MarcoFalke)

Pull request description:

  Ubuntu bionic clang is clang version 6, which does not come with libfuzzer. So the ci system breaks down when run on aarch64.

  Fix that by using clang-8

  For reference, the previous error on my ci system was:

  ```
  /usr/bin/ld: cannot find /usr/lib/llvm-6.0/lib/clang/6.0.0/lib/linux/libclang_rt.fuzzer-aarch64.a: No such file or directory

ACKs for top commit:
  laanwj:
    ACK fa2ec9f451

Tree-SHA512: 4954dbc36c444d1ae145290115eea6291753c9810c92003ab8d75433c3fe3bfee439d3a99dc394418275527157a8b89f04038c8b16e08c69ec9ded50fb869e70
2019-11-18 14:02:04 +01:00
fanquake 1c23ea5fe6
test: fix bitcoind already running warnings on macOS
On macOS, pidof installed via brew returns b'' rather than None.
Account for this, to remove spurious warnings from the test_runner.
2019-11-15 16:03:47 -05:00
MarcoFalke 422ec33d45
Merge #17322: Fix input size assertion in wallet_bumpfee.py
38516f9078 Fix input size assertion in wallet_bumpfee.py (Gregory Sanders)

Pull request description:

  I was investigating a curious error for https://github.com/bitcoin/bitcoin/pull/17290 and realized that this check should have caught that error earlier in the test.

  The loop is intended to ensure that only a single input exists the entire time until the change output disappears, a single additional bump occurs, then it leaves the loop.

Top commit has no ACKs.

Tree-SHA512: 1d2d6ef535ec2c55f516ee5de11352386ceac6bedaabc6842229a486d9f28d35310ad5f57bfcc1f1e654fc397ecff29ec33256f9b3da897500b7e1635004b63a
2019-11-15 14:02:01 -05:00
Gregory Sanders 38516f9078 Fix input size assertion in wallet_bumpfee.py 2019-11-15 13:58:51 -05:00
MarcoFalke fa2ec9f451
fuzz: Bump timeout in test_runner to accomodate for slow arm64 CPUs 2019-11-14 13:50:20 -05:00
MarcoFalke fac942ca57
test: Remove fragile assert_memory_usage_stable 2019-11-14 10:56:57 -05:00
MarcoFalke 8237889e8d
Merge #17435: test: check custom ancestor limit in mempool_packages.py
49997813a4 test: check custom ancestor limit in mempool_packages.py (Sebastian Falbesoner)

Pull request description:

  The functional test `mempool_packages.py` starts one node with default ancestor/descendant limit settings and one with a custom, reduced ancestor limit (currently `-limitancestorcount=5`). The effect of the latter had not been tested yet though. This is approached in this PR by checking on the expected mempool contents of node1 after the node0 ancestor tests are done, via the following three conditions:
  - the # of txs in the node1 mempool is equal to the the limit
  - all txs in node1 mempool are a subset of txs in node0 mempool
  - the node1 mempool txs match the start of the constructed tx-chain

  Note that this still doesn't *fully* check the expected mempool of node1 (e.g. that it isn't influenced by `prioritisetransaction` RPC on node0), hence I add another TODO. In the future it would make sense to also set a custom descendant limit when the second TODO about checking node1's mempool is approached: 89e93135ae/test/functional/mempool_packages.py (L228)

ACKs for top commit:
  MarcoFalke:
    ACK 49997813a4 👲

Tree-SHA512: d3a1d19fb49731238ad08ee7c02e2fa81a227e3b4ef3340d68598de42ddb62be9161134f6b8e08fa76b8c9faa02fecfa01111159642e20e9f358292a757b7608
2019-11-12 14:53:34 -05:00
João Barbosa a5e77959c8 rpc: Expose block height of wallet transactions 2019-11-11 22:32:44 +00:00
Sebastian Falbesoner 49997813a4 test: check custom ancestor limit in mempool_packages.py
To test the custom ancestor limit on node1 (passed by the argument
-limitancestorcount), we check for three conditions:
    -> the # of txs in the node1 mempool is equal to the the limit
    -> all txs in node1 mempool are a subset of txs in node0 mempool
    -> the node1 mempool txs match the start of the constructed tx-chain
2019-11-11 22:37:00 +01:00
Hennadii Stepanov cb9d830a00
test: Use proper MAX_SCRIPT_ELEMENT_SIZE 2019-11-11 12:04:48 +02:00
fanquake 270616228b
Merge #17362: test: speed up wallet_avoidreuse, add logging
0e7c90eb37 test: speed up wallet_avoidreuse.py (Jon Atack)
6d50b2606e test: add logging to wallet_avoidreuse.py (Jon Atack)

Pull request description:

  Inspired by PRs #17340 and #15881.

  - add logging
  - pass -whitelist in `set_test_params` to speed up transaction relay

  `wallet_avoidreuse.py` is not intended to test P2P transaction relay/timing, so it should be fine to do this here. This reduces test run time variability and speeds up the test by 2-3 times on average.

  Test run times in seconds:

  - before: 20, 24, 22, 17, 27, 40, 30

  - after: 10, 10, 8, 9, 10, 7, 8

ACKs for top commit:
  MarcoFalke:
    ACK 0e7c90eb37 🐊
  fanquake:
    ACK 0e7c90eb37

Tree-SHA512: 6d954a0aaf402c9594201626b59d29263479059e68fa5155bb44ed973cd0c3347729dd78b78b4d5a2275e45da365dc1afb4cc7e3293dea33fcc2e3e83a39faf5
2019-11-07 11:59:51 -05:00
Jon Atack 0e7c90eb37
test: speed up wallet_avoidreuse.py
Use -whitelist to speed up transaction relay.

The wallet_avoidreuse.py test is not intended to test transaction relay/timing,
so it should be fine to do this here.

This greatly reduces test run time variability and speeds up the test by 2-3
times on average, e.g. on my system from 20-30 seconds down to 8-10 seconds.
2019-11-07 10:03:28 +01:00
Jon Atack 6d50b2606e
test: add logging to wallet_avoidreuse.py 2019-11-07 10:03:26 +01:00
MarcoFalke e65b4160e9
Merge #17340: Tests: speed up fundrawtransaction test
af7bae7340 [tests] Don't stop-start unnecessarily in rpc_fundrawtransaction.py (John Newbery)
9a8505299b [tests] Use -whitelist in rpc_fundrawtransaction.py (John Newbery)
646b593bbd [tests] Speed up rpc_fundrawtransaction.py (John Newbery)

Pull request description:

  Speed up rpc_fundrawtransaction.py

  Most of the time in rpc_fundrawtransaction.py is spent waiting for
  unconfirmed transactions to propagate. Net processing adds a poisson
  random delay to the time it will INV transactions with a mean interval
  of 5 seconds. Calls like the following:

  ```
  self.nodes[2].sendrawtransaction(signedTx['hex'])
  self.sync_all()
  self.nodes[1].generate(1)
  ````

  will therefore introduce a delay waiting for the mempools to sync.
  Instead just generate the block on the node that sent the transaction:

  ```
  self.nodes[2].sendrawtransaction(signedTx['hex'])
  self.nodes[2].generate(1)
  ```

  rpc_fundrawtransaction.py is not intended to be a test for transaction
  relay, so it's ok to do this.

ACKs for top commit:
  MarcoFalke:
    ACK af7bae7340 🛴

Tree-SHA512: db3407d871bfdc99a02e7304b07239dd3585ac47f27f020f1a70608b7f6386b134343c01f3e4d1c246ce734676755897671999695068d6388602fb042d178780
2019-11-06 15:18:41 -05:00
John Newbery af7bae7340 [tests] Don't stop-start unnecessarily in rpc_fundrawtransaction.py
This was only added in c1dde3a949 to match
behaviour when `encryptwallet` would restart the node. It's not required
for the test (and slows things down).
2019-11-06 14:56:35 -05:00