Commit graph

15494 commits

Author SHA1 Message Date
Pieter Wuille
f63dec189c [REFACTOR] Initialize PrecomputedTransactionData in CheckInputScripts
Add a default constructor to `PrecomputedTransactionData`, which doesn't
initialize the struct's members. Instead they're initialized inside the
`CheckInputScripts()` function. This allows a later commit to add the
spent UTXOs to that structure.
2020-04-11 21:32:45 -04:00
MarcoFalke
a5623ba89f
Merge #18588: Revert "Merge #16367: Multiprocess build support"
f29bd546ec Revert "Merge #16367: Multiprocess build support" (MarcoFalke)

Pull request description:

  Reverting the changes temporarily is going to help with the following:

  * Discussion about the next steps for the multiprocess concept and the experimental libmultiprocess library without having code already commited in the master branch, potentially influencing the discussion
  * Allowing for more conceptual as well as code review ACKs to accumulate, since the pull only had one ACK (two if I count mine, which didn't make it to GitHub)

  Can be reviewed with `git diff HEAD HEAD~2 | wc` or `git diff 1b307613604883daea4913a65da30ae073c9dc4d~ | wc` (should be all zeros)

  Context here: https://github.com/bitcoin/bitcoin/pull/16367#issuecomment-612260496

ACKs for top commit:
  ryanofsky:
    Code review ACK f29bd546ec. Confirmed revert with
  fanquake:
    ACK f29bd546ec

Tree-SHA512: 3ce06c30de23c81c2d69cfb3ada20b3458c48efda1a5ba96aee678e946c499f701bc83e9eae91580f0156c0f30a90e5d015ef8b1806ad611d433c482fa55723e
2020-04-11 10:05:35 -04:00
MarcoFalke
f29bd546ec Revert "Merge #16367: Multiprocess build support"
This reverts the changes made in merge commit
1b30761360:

This reverts commit b919efadff.
This reverts commit d54f64c6c7.
This reverts commit 787f40668d.
This reverts commit d630646662.
This reverts commit e6e44eedd5.
2020-04-10 19:38:21 -04:00
MarcoFalke
3eb8b1c392
Merge #17905: gui: Avoid redundant tx status updates
96cb597325 gui: Avoid redundant tx status updates (Russell Yanofsky)

Pull request description:

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).

  In `TransactionTablePriv::index`, avoid calling `interfaces::Wallet::tryGetTxStatus` if the status is up to date as of the most recent `NotifyBlockTip` notification.  Store height from the most recent notification in a new `ClientModel::cachedNumBlocks` variable in order to check this.

  This avoids floods of IPC traffic from `tryGetTxStatus` with #10102 when there are a lot of transactions. It might also make the GUI a little more efficient even when there is no IPC.

ACKs for top commit:
  promag:
    Code review ACK 96cb597325.
  hebasto:
    ACK 96cb597325

Tree-SHA512: fce597bf52a813ad4923110d0a39229ea09e1631e0d580ea18cffb09e58cdbb4b111a40a9a9270ff16d8163cd47b0bd9f1fe7e3a6c7ebb19198f049f8dd1aa46
2020-04-10 16:34:30 -04:00
Vasil Dimov
2599d13c94
rpc: Remove deprecated migration code
Don't accept a second argument to `sendrawtransaction` and
`testmempoolaccept` of type `bool`. Actually even the code before this
change would not accept `bool`, but it would print a long explanatory
message when rejecting it: "Second argument must be numeric (maxfeerate)
and no longer supports a boolean. To allow a transaction with high fees,
set maxfeerate to 0."

This was scheduled for removal in 6c0a6f73e.
2020-04-10 21:27:01 +02:00
MarcoFalke
51e2ce45d6
Merge #17693: rpc: Add generateblock to mine a custom set of transactions
7524b6479c Add tests for generateblock (Andrew Toth)
dcc8332543 Add generateblock rpc (Andrew Toth)

Pull request description:

  The existing block generation rpcs for regtest, `generatetoaddress` and `generatetodescriptor`, mine everything in the mempool up to the block weight limit. This makes it difficult to test a system for several scenarios where a different set of transactions are mined. For example:
  - Testing the common scenario where a transaction is replaced in the mempool but the replaced transaction is mined instead.
  - Testing for a double-spent transaction where a transaction that conflicts with the mempool is mined.
  - Testing for non-standard transactions that are mined.
  - Testing the scenario where several blocks are mined without a specific transaction in the mempool being included in a block.

  This PR introduces a new rpc, `generateblock`, that takes an array of raw transactions and txids and mines only those and the coinbase. Any txids must be in the mempool, but the raw txs can be anything conforming to consensus rules. The coinbase can be specified as either an address or descriptor.

  This reopens #17653 since it was closed by mistake.

  Thanks to instagibbs for code suggestions that I used here.

ACKs for top commit:
  MarcoFalke:
    re-ACK 7524b6479c 📁

Tree-SHA512: 857106007465b5b9b8a84b6d07c17cbf8378a33a72d32ff79abea1d5ab4babb4d53a11ddbb14595aa1fac9dfa1391e3a11403d742f69951beea2f683e8a01cd4
2020-04-10 13:12:30 -04:00
MarcoFalke
10358a381a
Merge #17737: Add ChainstateManager, remove BlockManager global
c9017ce3bc protect g_chainman with cs_main (James O'Beirne)
2b081c4568 test: add basic tests for ChainstateManager (James O'Beirne)
4ae29f5f0c use ChainstateManager to initialize chainstate (James O'Beirne)
5b690f0aae refactor: move RewindBlockIndex to CChainState (James O'Beirne)
89cdf4d569 validation: introduce unused ChainstateManager (James O'Beirne)
8e2ecfe249 validation: add CChainState.m_from_snapshot_blockhash (James O'Beirne)

Pull request description:

  This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):

  Parent PR: #15606
  Issue: #15605
  Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal

  ---

  This changeset introduces `ChainstateManager`, which is responsible for creating and managing access to multiple chainstates. Until we allow chainstate creation from UTXO snapshots (next assumeutxo PR?) it's basically unnecessary, but it is a prerequisite for background IBD support.

  Changes are also made to the initialization process to make use of `g_chainman` and thus clear the way for multiple chainstates being loaded on startup.

  One immediate benefit of this change is that we no longer have the `g_blockman` global, but instead have the ChainstateManager inject a reference of its shared BlockManager into any chainstate it creates.

  Another immediate benefit is that uses of `ChainActive()` and `ChainstateActive()` are now covered by lock annotations. Because use of `g_chainman` is annotated to require cs_main, these two functions subsequently follow.

  Because of whitespace changes, this diff looks bigger than it is. E.g., 4813167d98 is most easily reviewed with
  ```sh
  git show --color-moved=dimmed_zebra -w 4813167d98
  ```

ACKs for top commit:
  MarcoFalke:
    re-ACK c9017ce3bc 📙
  fjahr:
    Code Review Re-ACK c9017ce3bc
  ariard:
    Code Review ACK c9017ce
  ryanofsky:
    Code review ACK c9017ce3bc. No changes since last review other than a straight rebase

Tree-SHA512: 3f250d0dc95d4bfd70852ef1e39e081a4a9b71a4453f276e6d474c2ae06ad6ae6a32b4173084fe499e1e9af72dd9007f4a8a375c63ce9ac472ffeaada41ab508
2020-04-10 13:02:01 -04:00
MarcoFalke
a9213bbe75
Merge #18422: [consensus] MOVEONLY: Move single-sig checking EvalScript code to EvalChecksig
14e8cf974a [consensus] MOVEONLY: Move single-sig checking EvalScript code to EvalChecksig (Pieter Wuille)

Pull request description:

  This is another small refactor pulled out of the Schnorr/Taproot PR #17977.

  This is in preparation for adding different signature verification rules,
  specifically tapscript (BIP 342), which interprets opcode 0xac and 0xad
  as Schnorr signature verifications.

ACKs for top commit:
  sipa:
    ACK 14e8cf974a, verified move-only.
  MarcoFalke:
    ACK 14e8cf974a, reviewed with "git show 14e8cf974a --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space -W" 👆
  fjahr:
    Code-review ACK 14e8cf974a, verified that it's move-only.
  instagibbs:
    code review ACK 14e8cf974a, verified move-only
  theStack:
    Code-Review ACK 14e8cf974a
  jonatack:
    ACK 14e8cf974a

Tree-SHA512: af2efce9ae39d5ec01db5b9ef0ff383fe252ef5f33b3483927308ae17d91a619266cb45951f32ea1ce54807a4c0f052bcdefb47e244465d3a726393221c227b1
2020-04-10 12:59:29 -04:00
MarcoFalke
99d6a5be8b
Merge #17999: refactor: Add ChainClient setMockTime, getWallets methods
3ce16ad2f9 refactor: Use psbt forward declaration (Russell Yanofsky)
1dde238f2c Add ChainClient setMockTime, getWallets methods (Russell Yanofsky)

Pull request description:

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).

  These changes are needed to set mock times, and get wallet interface pointers correctly when
  wallet code is running in a different process from node code in #10102

ACKs for top commit:
  MarcoFalke:
    re-ACK 3ce16ad2f9 🔙
  promag:
    Code review ACK 3ce16ad2f9.

Tree-SHA512: 6c093bfcd68adf5858a1aade4361cdb7fb015496673504ac7a93d0bd2595215047184551d6fd526baa27782331cd2819ce45c4cf923b205ce93ac29e485b5dd8
2020-04-10 12:57:35 -04:00
MarcoFalke
1b30761360
Merge #16367: Multiprocess build support
b919efadff depends: Use default macos clang compiler (Russell Yanofsky)
d54f64c6c7 Add multiprocess travis configuration (Russell Yanofsky)
787f40668d Set LD_LIBRARY_PATH consistently in travis tests (Russell Yanofsky)
d630646662 libmultiprocess depends build (Russell Yanofsky)
e6e44eedd5 Multiprocess build changes (Russell Yanofsky)

Pull request description:

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).

  This splits autotools, depends build, and travis changes out of #10102, so code changes and build system changes can be reviewed separately.

ACKs for top commit:
  hebasto:
    re-ACK b919efadff, rebased only since my [previous](https://github.com/bitcoin/bitcoin/pull/16367#issuecomment-605514556) review.

Tree-SHA512: ebc5e403cc99a0d9629ed7fe1595e01d57e6d1255cbf03968a3196ff6f528f734c78060fdc065724ee1f923bcc5aa2b29470fcb36a7f15957eb57c76d58178a4
2020-04-10 12:55:12 -04:00
MarcoFalke
4eb1eeb02c
Merge #18504: build: Drop bitcoin-tx and bitcoin-wallet dependencies on libevent
01a3392b1b Drop bitcoin-wallet dependency on libevent (Russell Yanofsky)
0660119ac3 Drop unintended bitcoin-tx dependency on libevent (Russell Yanofsky)

Pull request description:

  This fixes compile errors trying to build bitcoin-tx and bitcoin-wallet without libevent, which were reported by Luke Dashjr in https://github.com/bitcoin/bitcoin/issues/18465

  The fix avoiding `bitcoin-tx` dependency on libevent just adds a conditional build rule. This is implemented in the first commit (more details in commit description).

  The fix avoiding `bitcoin-wallet` dependency on libevent requires minor code changes, because `bitcoin-wallet` (unlike `bitcoin-tx`) links against code that calls `urlDecode` / `evhttp_uridecode`. This fix is implemented in the second commit (again details in the commit description).

ACKs for top commit:
  jonasschnelli:
    utACK 01a3392b1b.

Tree-SHA512: d2245e912ab494cccceeb427a1eca8e55b01a0006ff93eebcfb5461ae7cecd1083ac2de443d9db036b18bdc6f0fb615546caaa20c585046f66d234937f74870a
2020-04-10 12:52:37 -04:00
MarcoFalke
6ab96ec546
Merge #18574: cli: call getbalances.ismine.trusted instead of getwalletinfo.balance
5df0877f91 test: update and harden interface_bitcoin_cli tests (Jon Atack)
75019774c9 cli -getinfo: use getbalances instead of deprecated getwalletinfo balance (Jon Atack)

Pull request description:

  Extracted from #18453 to preserve that PR as a discussion on multiwallet RPC/CLI.

  This PR updates `bitcoin-cli -getinfo` to fetch the wallet balance from `getbalances` in order to no longer depend on `getwalletinfo.balance` which was deprecated a year ago in facfb41.

  I found this when removing the getwalletinfo() `balance`, `unconfirmed_balance`, and `immature_balance` fields to see what broke from depending on them.

  I didn't see any perceivable change in `-getinfo` run time from the change.

  Test coverage for this change is provided by `test/functional/interface_bitcoin_cli.py`, which the second commit updates to (a) no longer depend on getwalletinfo.balances and (b) test the -getinfo blockcount and balance fields against non-default, non-zero values.

ACKs for top commit:
  robot-visions:
    ACK 5df0877
  MarcoFalke:
    ACK 5df0877
  vasild:
    re-ACK 5df0877f9
  promag:
    Code review ACK 5df0877f91.
  theStack:
    ACK 5df0877f91

Tree-SHA512: 0dd8c62f915b1c0112e42b132dcf74a141bdd1f51e7c17d4a698b374ec296f4f9836f7058dbe237cf24f9bfb32ea5000e14f7089e2e86472d9c6a175be26e910
2020-04-10 12:50:51 -04:00
MarcoFalke
a840dab2a5
Merge #18541: rpc: Make verifychain default values static, not depend on global args
fad691cafe rpc: Make verifychain default values static, not depend on global args (MarcoFalke)

Pull request description:

  This fixes several issues:

  * The documentation is not compile-time static and depends on run-time arguments, making it impossible to host it on a static resource like a website or pdf. See also a similar change in the wallet rpc code: #18499
  * The same call (relying on default values) will run different code on different machines, depending on the command line args that were used to start the server. This might lead to hard-to-debug-remote issues.

  This is a small behaviour change, and I will add release notes.

ACKs for top commit:
  theStack:
    ACK fad691cafe
  promag:
    Code review ACK fad691cafe.

Tree-SHA512: 1c7a253ff0ec13a973b10d3777b71c70954ded5805b65a3ab06317327014de4cd0601d71d30c6ce89a581722c150cb5567acc1bd3e0c789cb51bab6ef0dcfc4a
2020-04-10 11:21:57 -04:00
MarcoFalke
3347ca4881
Merge #18526: Remove PID file at the very end
7fcdec0f32 Remove PID file at the very end (Hennadii Stepanov)

Pull request description:

  While reproducing the bug from #18517, I've noticed that the `bitcoind.pid` file has already been removed when the `bitcoind` hangs.

  This PR makes `Shutdown()` keep the `bitcoind.pid` file available until the end.

ACKs for top commit:
  MarcoFalke:
    ACK 7fcdec0f32
  emilengler:
    utACK 7fcdec0f32
  promag:
    Code review ACK 7fcdec0f32.
  theStack:
    Code review ACK 7fcdec0f32

Tree-SHA512: 9732ef34e137dbee70a06d922b316b8ea7b9a1c959cf8861b6940cd789336dc19ee468a4c3a28d95d1458076a48270c676b0ff27fec30cf57eced6ddab0a2a9b
2020-04-10 11:06:27 -04:00
MarcoFalke
29893ec875
Merge #18454: net: Make addr relay mockable, add test
fa1da3d4bf test: Add basic addr relay test (MarcoFalke)
fa1793c1c4 net: Pass connman const when relaying address (MarcoFalke)
fa47a0b003 net: Make addr relay mockable (MarcoFalke)

Pull request description:

  As usual:

  * Switch to std::chrono time to be type-safe and mockable
  * Add basic test that relies on mocktime to add code coverage

ACKs for top commit:
  naumenkogs:
    utACK  fa1da3d
  promag:
    ACK fa1da3d4bf (fabe56e44b6f683e24e37246a7a8851190947cb3 before https://github.com/bitcoin/bitcoin/pull/18454#issuecomment-607866453), fa5bf23d527a450e72c2bf13d013e5393b664ca3 was dropped since last review.

Tree-SHA512: 0552bf8fcbe375baa3cab62acd8c23b2994efa47daff818ad1116d0ffaa0b9e520dc1bca2bbc68369b25584e85e54861fe6fd0968de4f503b95439c099df9bd7
2020-04-10 10:12:46 -04:00
MarcoFalke
405713d00f
Merge #18529: Add fuzzer version of randomized prevector test
b1d24d1d03 Reorder the test instructions by number (Pieter Wuille)
c2ccadc26a Merge and generalize case 3 and case 6 (Pieter Wuille)
402ad5aaca Only run sanity check once at the end (Pieter Wuille)
eda8309bfc Assert immediately rather than caching failure (Pieter Wuille)
55608455cb Make a fuzzer-based copy of the prevector randomized test (Pieter Wuille)

Pull request description:

  The current prevector test effectively randomly generates a number of operations to perform on a prevector and a normal vector, and checks consistency between the two.

  By converting this into a fuzzer the operations can be targetted rather than random.

ACKs for top commit:
  MarcoFalke:
    ACK b1d24d1d03 🍬

Tree-SHA512: 2b5c62abcd5fee94f42db03400531484d98c59e7f4308e0e683c61aabcd9ce42f85c5d058d2d5e7f8221124f71d2112b6a5f3c80e5d0fdae265a70647747e92f
2020-04-09 15:00:57 -04:00
Jon Atack
75019774c9
cli -getinfo: use getbalances instead of deprecated getwalletinfo balance 2020-04-09 15:42:43 +02:00
Sebastian Falbesoner
5eae034996 net: limit BIP37 filter lifespan (active between 'filterload' and 'filterclear')
Previously, a default match-everything bloom filter was set for every peer,
i.e. even before receiving a 'filterload' message and after receiving a
'filterclear' message code branches checking for the existence of the filter
by testing the pointer "pfilter" were _always_ executed.
2020-04-09 11:26:24 +02:00
MarcoFalke
661bd5dea3
Merge #18363: tests: Add fuzzing harness for HTTPRequest, libevent's evhttp and related functions
cdfb8e7afa tests: Add fuzzing harness for HTTPRequest, libevent's evhttp and related functions (practicalswift)

Pull request description:

  Add fuzzing harness for `HTTPRequest`, `libevent`'s `evhttp` and related functions.

ACKs for top commit:
  laanwj:
    ACK cdfb8e7afa

Tree-SHA512: da481afed5eb3232d3f3d0583094e56050e6234223dfcb356d8567fe0616336eb1b78c5e6821325fc9767e385e5dfaf3c96f0d35ffdb67f18d74f9a9a9464e24
2020-04-09 02:45:37 +08:00
MarcoFalke
3410fe6887
Merge #18521: fuzz: Add process_messages harness
fa6a008434 fuzz: Add process_messages harness (MarcoFalke)

Pull request description:

ACKs for top commit:
  practicalswift:
    Tested ACK fa6a008434

Tree-SHA512: 2d8788308c7f45c97ca003378f58a9d51f51265958557a65e5e505b1666b4cb928f0d010622870175090a0ad25e2d10b41f26f4eef14b6ff334a024baa250f8c
2020-04-09 00:14:31 +08:00
MarcoFalke
4c59236376
Merge #18533: scripted-diff: Replace strCommand with msg_type
7777e3624f scripted-diff: Replace strCommand with msg_type (MarcoFalke)

Pull request description:

  Receiving a message is not a command, but simply a message of some type

ACKs for top commit:
  promag:
    ACK 7777e3624f.
  naumenkogs:
    ACK 7777e36
  practicalswift:
    ACK 7777e3624f -- I've always thought the `strCommand` name is confusing :)
  theStack:
    ACK 7777e36

Tree-SHA512: 662bac579064c621191916274314b85111cfb4df488f00893ceb16def1c47af4b2a0f34cd7349722099b5a9d23160edb8eb999841f1d64af3e0da02e4870b4bf
2020-04-09 00:12:39 +08:00
MarcoFalke
bfef72d0fb
Merge #18565: tests: Add fuzzing harnesses for classes/functions in policy/fees.h, checkqueue.h and cuckoocache.h. Add fuzzing coverage.
283bd72156 tests: Add coverage of {,Incremental}DynamicUsage(const std::set<X, Y>& s) to existing fuzzer (practicalswift)
bf76000493 tests: Add fuzzing harness for classes/functions in cuckoocache.h (practicalswift)
57890b2555 tests: Add fuzzing harness for classes/functions in checkqueue.h (practicalswift)
2df5701e90 tests: Add coverage of GetVirtualTransactionSize(...) to existing fuzzer (practicalswift)
7b9a2dc864 tests: Add fuzzing harness for AdditionOverflow(...) (practicalswift)
44fb2a596b tests: Add fuzzing harness for FeeFilterRounder (practicalswift)

Pull request description:

  Includes:

  ```
  tests: Add fuzzing harness for FeeFilterRounder
  tests: Add fuzzing harness for classes/functions in checkqueue.h
  tests: Add fuzzing harness for classes/functions in cuckoocache.h
  tests: Add coverage of {,Incremental}DynamicUsage(const std::set<X, Y>& s) to existing fuzzer
  tests: Add coverage of GetVirtualTransactionSize(...) to existing fuzzer
  tests: Add fuzzing harness for AdditionOverflow(...)
  ```

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core.

ACKs for top commit:
  MarcoFalke:
    ACK 283bd72156

Tree-SHA512: 2361edfb5c47741b22d9fb996836c5250c5a26bc5e956039ea6a0c55ba2d36c78f241d66f85bc02f5b85b9b83d5fde56a5c4702b9d1b7ac4a9a3ae391ca79eaa
2020-04-08 23:34:19 +08:00
practicalswift
283bd72156 tests: Add coverage of {,Incremental}DynamicUsage(const std::set<X, Y>& s) to existing fuzzer 2020-04-08 14:45:27 +00:00
practicalswift
bf76000493 tests: Add fuzzing harness for classes/functions in cuckoocache.h 2020-04-08 14:45:27 +00:00
practicalswift
57890b2555 tests: Add fuzzing harness for classes/functions in checkqueue.h 2020-04-08 14:45:27 +00:00
practicalswift
2df5701e90 tests: Add coverage of GetVirtualTransactionSize(...) to existing fuzzer 2020-04-08 14:45:27 +00:00
practicalswift
7b9a2dc864 tests: Add fuzzing harness for AdditionOverflow(...) 2020-04-08 14:45:27 +00:00
practicalswift
44fb2a596b tests: Add fuzzing harness for FeeFilterRounder 2020-04-08 14:45:27 +00:00
Russell Yanofsky
13d2a33537 Fix unregister_all_during_call cleanup
Use TestingSetup fixture to fix unregister_all_during_call test not calling
UnregisterBackgroundSignalScheduler, which could trigger an assert in
RegisterBackgroundSignalScheduler when called in later tests

Failure reported by fanquake <fanquake@gmail.com>
https://github.com/bitcoin/bitcoin/pull/18551#issuecomment-610974251
2020-04-08 10:11:46 -04:00
Pieter Wuille
8508473094 Avoid non-trivial global constants in SHA-NI code 2020-04-07 13:57:34 -07:00
Russell Yanofsky
2276339a17 Add test for UnregisterAllValidationInterfaces bug
Bug in MainSignalsInstance::Clear could cause validation interface callbacks to
be deleted during execution if UnregisterAllValidationInterfaces was called
more than once.

Bug was introduced in https://github.com/bitcoin/bitcoin/pull/18524 and is
fixed by https://github.com/bitcoin/bitcoin/pull/18551
2020-04-07 12:54:41 -07:00
Pieter Wuille
3c61abbbc8 Do not clear validationinterface entries being executed
The previous code for MainSignalsInstance::Clear would decrement the reference
count of every interface, including ones that were already Unregister()ed but
still being executed.
2020-04-07 12:53:40 -07:00
MarcoFalke
1b151e3ffc
Merge #18532: rpc: Avoid initialization-order-fiasco on static CRPCCommand tables
fa1a92224d rpc: Avoid initialization-order-fiasco on static CRPCCommand tables (MarcoFalke)

Pull request description:

  Currently the fiasco is only theoretical because all content of the table are compile-time constants. However, the fiasco materializes should they ever become run-time constants (e.g. #18531).

ACKs for top commit:
  promag:
    ACK fa1a92224d.
  practicalswift:
    ACK fa1a92224d -- fiasco bad :)

Tree-SHA512: cccadb0ad56194599b74f04264d74c34fa865958580a850efc6474bbdc56f30cadce6b2e9a6ad5472ff46c3f4c793366acd8090fad409a45b25d961f2d89da19
2020-04-07 23:46:17 +08:00
MarcoFalke
63dad67348
Merge #18546: Bugfix: Wallet: Safely deal with change in the address book [part 2]
7a2ecf16df Wallet: Change IsMine check in CWallet::DelAddressBook from assert to failure (Luke Dashjr)
2952c46b92 Wallet: Replace CAddressBookData.name with GetLabel() method (Luke Dashjr)
d7092c392e QA: Test that change doesn't turn into non-change when spent in an avoid-reuse wallet (Luke Dashjr)

Pull request description:

  Follow-up to #18192, not strictly necessary for 0.20

ACKs for top commit:
  MarcoFalke:
    re-ACK 7a2ecf16df, only change is adding an assert_equal in the test 🔰
  jnewbery:
    utACK 7a2ecf16df

Tree-SHA512: e0933ee40f705b751697dc27249e1868ed4874254b174ebdd0a7150125d8c818402e66df2371718c7eeb90e67ee2317215fb260aa9b9d7b9b45ee436de2988ff
2020-04-07 18:47:38 +08:00
Pieter Wuille
b1d24d1d03 Reorder the test instructions by number 2020-04-06 14:51:38 -07:00
Pieter Wuille
c2ccadc26a Merge and generalize case 3 and case 6 2020-04-06 14:39:42 -07:00
Pieter Wuille
402ad5aaca Only run sanity check once at the end 2020-04-06 14:39:42 -07:00
Pieter Wuille
eda8309bfc Assert immediately rather than caching failure 2020-04-06 14:39:38 -07:00
Pieter Wuille
55608455cb Make a fuzzer-based copy of the prevector randomized test 2020-04-06 14:25:25 -07:00
Hennadii Stepanov
56fe839e4e
qt: Fix Window -> Minimize menu item 2020-04-07 00:19:19 +03:00
Luke Dashjr
7a2ecf16df Wallet: Change IsMine check in CWallet::DelAddressBook from assert to failure 2020-04-06 20:52:04 +00:00
Luke Dashjr
2952c46b92 Wallet: Replace CAddressBookData.name with GetLabel() method 2020-04-06 20:52:04 +00:00
MarcoFalke
c5966a87d1
Merge #18192: Bugfix: Wallet: Safely deal with change in the address book
b5795a7886 Wallet: Add warning comments and assert to CWallet::DelAddressBook (Luke Dashjr)
6d2905f57a Wallet: Avoid unnecessary/redundant m_address_book lookups (Luke Dashjr)
c751d886f4 Wallet: Avoid treating change-in-the-addressbook as non-change everywhere (Luke Dashjr)
8e64b8c84b Wallet: New FindAddressBookEntry method to filter out change entries (and skip ->second everywhere) (Luke Dashjr)
65b6bdc2b1 Wallet: Add CAddressBookData::IsChange which returns true iff label has never been set (Luke Dashjr)
144b2f85da Wallet: Require usage of new CAddressBookData::setLabel to change label (Luke Dashjr)
b86cd155f6 scripted-diff: Wallet: Rename mapAddressBook to m_address_book (Luke Dashjr)

Pull request description:

  In many places, our code assumes that presence in the address book indicates a non-change key, and absence of an entry in mapAddressBook indicates change.

  This no longer holds true after #13756 (first released in 0.19) since it added a "used" DestData populated even for change addresses. Only avoid-reuse wallets should be affected by this issue.

  Thankfully, populating DestData does not write a label to the database, so we can retroactively fix this (so long as the user didn't see the change address and manually assign it a real label).

  Fixing it is accomplished by:

  * Adding a new bool to CAddressBookData to track if the label has ever been assigned, either by loading one from the database, or by assigning one at runtime.
  * `CAddressBookData::IsChange` and `CWallet::FindAddressBookEntry` are new methods to assist in excluding change from code that doesn't expect to see them.
  * For safety in merging, `CAddressBookData::name` has been made read-only (the actual data is stored in `m_label`, a new private member, and can be changed only with `setLabel` which updates the `m_change` flag), and `mapAddressBook` has been renamed to `m_address_book` (to force old code to be rebased to compile).

  A final commit also does some minor optimisation, avoiding redundant lookups in `m_address_book` when we already have a pointer to the `CAddressBookData`.

ACKs for top commit:
  ryanofsky:
    Code review ACK b5795a7886. Pretty clever and nicely implemented fix!
  jonatack:
    ACK b5795a7886 nice improvements -- code review, built/ran tests rebased on current master ff53433fe4 and tested manually with rpc/cli
  jnewbery:
    Good fix. utACK b5795a788.

Tree-SHA512: 40525185a0bcc1723f602243c269499ec86ecb298fecb5ef24d626bbdd5e3efece86cdb1084ad7eebf7eeaf251db4a6e056bcd25bc8457b417fcbb53d032ebf0
2020-04-07 03:51:18 +08:00
Wladimir J. van der Laan
c31bcaf203
Merge #18458: net: Add missing cs_vNodes lock
fa369651c5 net: Add missing cs_vNodes lock (MarcoFalke)

Pull request description:

  Fixes #18457

ACKs for top commit:
  promag:
    Code review ACK fa369651c5.
  laanwj:
    ACK fa369651c5

Tree-SHA512: 60d7000f2f3d480bb0953ce27a0020763e7102da16a0006b619e0a236cfc33cbd4f83d870e9f0546639711cd877c1f9808d419184bbc153bb328885417e0066c
2020-04-06 21:06:09 +02:00
Wladimir J. van der Laan
75021e80ee
Merge #18487: rpc: Fix rpcRunLater race in walletpassphrase
7b8e15728d rpc: Fix rpcRunLater race in walletpassphrase (João Barbosa)

Pull request description:

  Release locks before calling `rpcRunLater`.

  Quick explanation: `rpcRunLater` leads to `event_free` which calls `event_del` which can wait for the event callback to finish if it's already running and that callback will try to lock wallet mutex - which is already locked in http thread.

  Fixes #14995 , fixes #18482. Best reviewed with whitespace changes hidden.

ACKs for top commit:
  MarcoFalke:
    ACK 7b8e15728d, only tested that this avoids the node freezing. Did not look at how libevent works or how the deadlock happens or if this breaks other stuff. 📞
  ryanofsky:
    Code review ACK 7b8e15728d. Just updated comment since last review

Tree-SHA512: 17874a2fa7b0e164fb0d7ee4cb7d59650275b8c03476fb291d60af8b758495457660d3912623fb26259fefe84aeba21c0a9e0c6467982ba511f19344ed5413ab
2020-04-06 20:29:35 +02:00
MarcoFalke
c0b389b335
Merge #18484: rpc: Correctly compute redeemScript from witnessScript for signrawtransaction
cd3b1569d9 Correctly compute redeemScript from witnessScript for signrawtransaction (Andrew Chow)

Pull request description:

  `ParsePrevouts` uses `GetScriptForWitness` on the given witnessScript to find the corresponding redeemScript. This is incorrect when the witnessScript is either a P2PK or P2PKH script as it returns the corresponding P2WPK script instead of turning the witnessScript into a P2WSH script. Instead this should make the script a `WitnessV0ScriptHash` destination and get the script for that.

  Test cases are also added. These will fail on master with a `redeemScript does not correspond to witnessScript`

  Reported on [Bitcointalk](https://bitcointalk.org/index.php?topic=5236818.0)

ACKs for top commit:
  MarcoFalke:
    weak ACK cd3b1569d9, only checked that the test fails without the code change 🚰
  instagibbs:
    utACK cd3b1569d9

Tree-SHA512: afac671dbb52ce88bfb4a9ca3dd6065427ad52c9778d0549ad40e9286778f308adad24fb3b3c3089545d7f88c57c53d41224fd7a4bb207550eff2fe06600118f
2020-04-07 00:59:48 +08:00
MarcoFalke
fad691cafe
rpc: Make verifychain default values static, not depend on global args 2020-04-07 00:53:49 +08:00
Wladimir J. van der Laan
fdeb445a34
Merge #18524: refactor: drop boost::signals2 in validationinterface
d6815a2313 refactor: drop boost::signals2 in validationinterface (Russell Yanofsky)

Pull request description:

  Stop using boost::signals2 internally in validationinterface. Replace with std::list and Add/Remove/Clear/Iterate helper functions.

  Motivation for change is to reduce dependencies and avoid issues happening with boost versions before 1.59: https://github.com/bitcoin/bitcoin/issues/18517, https://github.com/bitcoin/bitcoin/pull/18471

ACKs for top commit:
  MarcoFalke:
    ACK d6815a2313
  laanwj:
    ACK d6815a2313
  hebasto:
    re-ACK d6815a2313
  promag:
    ACK d6815a2313.

Tree-SHA512: 4fc0f14a8446e8616cc142af6c3d36815f3254525d30348ba8e4d4bc74c249a5a8c9bc119bdd1be7ebd7abe0b784bc0c5551a3e156a766890cb2fdd891a95919
2020-04-06 16:46:07 +02:00
Hennadii Stepanov
7fcdec0f32
Remove PID file at the very end 2020-04-06 17:12:32 +03:00
practicalswift
cdfb8e7afa tests: Add fuzzing harness for HTTPRequest, libevent's evhttp and related functions 2020-04-06 13:58:51 +00:00
Wladimir J. van der Laan
adac12ae73
Merge #18506: net: Hardcoded seeds update for 0.20
0eeb0468e7 net: Hardcoded seeds update for 0.20 (Wladimir J. van der Laan)

Pull request description:

  Update hardcoded seeds from http://bitcoin.sipa.be/seeds.txt.gz,
  according to release process.

  Output from makeseeds.py:
  ```
    IPv4   IPv6  Onion Pass
  1364173 244127   2454 Initial
  1364173 244127   2454 Skip entries with invalid address
  1129552 213117   2345 After removing duplicates
  1129548 213117   2345 Skip entries from suspicious hosts
  338216 191944   2249 Enforce minimal number of blocks
  336851 188993   2189 Require service bit 1
    6998   1520    150 Require minimum uptime
    5682   1290     89 Require a known and recent user agent
    5622   1279     89 Filter out hosts with multiple bitcoin ports
     512    146     89 Look up ASNs and limit results per ASN and per net
  ```

Top commit has no ACKs.

Tree-SHA512: ce1c2cda18dd5bd22586a5283a0877f3bd890437cc29dc1d85452ba4a4d28032f591c8b37f3329e8e649556cf83750b6949a068fad76d1773853d93014609da0
2020-04-06 13:38:32 +02:00
fanquake
516ebe8a62
Merge #18514: test: remove rapidcheck integration and tests
9e071b0089 test: remove rapidcheck integration and tests (fanquake)

Pull request description:

  Whilst the property tests are interesting, ultimately [rapidcheck](https://github.com/emil-e/rapidcheck) integration in this repository has not gained much traction. We have a limited number of tests, and they are rarely (if ever) run. Have discussed this with Chris Stewart.

ACKs for top commit:
  practicalswift:
    ACK 9e071b0089

Tree-SHA512: d0c12af3163382eee8413da420c63e39265a7b700709a05d518445832d45e049aed9508e32524db5228fe3ac114609a00b7bb890be047c07032e44a5ef4611e9
2020-04-06 09:48:21 +08:00
Russell Yanofsky
e6e44eedd5 Multiprocess build changes
autotools and automake changes to support multiprocess execution.

This adds a new --enable-multiprocess flag, and build configuration code to
detect libraries needed for multiprocess support. The --enable-multiprocess
flag builds new bitcoin-node and bitcoin-gui executables, which are updated in
https://github.com/bitcoin/bitcoin/pull/10102 to communicate across processes.
But for now they are functionally equivalent to existing bitcoind and
bitcoin-qt executables.
2020-04-05 21:48:21 -04:00
MarcoFalke
7777e3624f
scripted-diff: Replace strCommand with msg_type
-BEGIN VERIFY SCRIPT-
sed -i 's/\<strCommand\>/msg_type/g' ./src/net_processing.cpp ./src/test/fuzz/process_message.cpp
-END VERIFY SCRIPT-
2020-04-06 08:00:34 +08:00
MarcoFalke
fa1a92224d
rpc: Avoid initialization-order-fiasco on static CRPCCommand tables 2020-04-06 00:20:00 +08:00
MarcoFalke
fa6a008434
fuzz: Add process_messages harness 2020-04-05 10:46:24 +08:00
Harris
691e2a7af7
build: create test_fuzz library from src/test/fuzz/fuzz.cpp 2020-04-05 01:01:13 +02:00
MarcoFalke
4830077494
Merge #18510: fuzz: Add CScriptNum::getint coverage
faa64af960 fuzz: Add CScriptNum::getint coverage (MarcoFalke)

Pull request description:

  Add coverage for

  * https://marcofalke.github.io/btc_cov/fuzz.coverage/src/script/script.h.gcov.html#311
  * https://marcofalke.github.io/btc_cov/fuzz.coverage/src/script/script.h.gcov.html#511

ACKs for top commit:
  practicalswift:
    ACK faa64af960 -- more fuzzing coverage is better than less fuzzing coverage :)

Tree-SHA512: 1a66a2edc3740e8c286049f6c27458c59c45b01052e51684eec0e1be63ffcee94b4ba3d41d88ad715ceb3e4754fd997cf03899085982454905e86d0553d58199
2020-04-05 04:53:19 +08:00
MarcoFalke
e16da90d95
Merge #18518: fuzz: Extend descriptor fuzz test
fa0189955a fuzz: Extend descriptor fuzz test (MarcoFalke)

Pull request description:

ACKs for top commit:
  practicalswift:
    ACK fa0189955a

Tree-SHA512: 6d6a6417f06d90732bbf055ff54102530d6956f3082f1ff65598f790d588170768aee98e4835996876d28bca2a9c62f22fe122c3fc7eafd4b7660696f72f9835
2020-04-05 04:49:55 +08:00
MarcoFalke
16b6d3422b
Merge #18519: fuzz: Extend script fuzz test
fa86edf66d fuzz: Extend script fuzz test (MarcoFalke)

Pull request description:

ACKs for top commit:
  practicalswift:
    ACK fa86edf66d

Tree-SHA512: 611adee9e673183e67f9711e49289fa59e410bb3ac1bb3fcbb7f1ed331bf0d288c7065e256a82eb41a30a4afe53544c836463cf58865d6e40b18795c8716e57c
2020-04-05 04:48:47 +08:00
MarcoFalke
4839560ee1
Merge #18407: tests: Add proof-of-work fuzzing harness
acf269e146 tests: Add proof-of-work fuzzing harness (practicalswift)

Pull request description:

  Add proof-of-work fuzzing harness.

Top commit has no ACKs.

Tree-SHA512: dcdfa211cf1ec3018b61f378bb0f95793bbbe5d00e2f4d17f9db2c7263fe8ce919760c56cae7122c62c82e05c90e7056eb1778871674bdb3c42869e5fe4c2b60
2020-04-05 04:41:07 +08:00
practicalswift
acf269e146 tests: Add proof-of-work fuzzing harness 2020-04-04 17:23:50 +00:00
Russell Yanofsky
d6815a2313 refactor: drop boost::signals2 in validationinterface
Stop using boost::signals2 internally in validationinterface. Replace with
std::list and Add/Remove/Clear/Iterate helper functions.

Motivation for change is to reduce dependencies and avoid issues happening with
boost versions before 1.59:

https://github.com/bitcoin/bitcoin/issues/18517
https://github.com/bitcoin/bitcoin/pull/18471
2020-04-04 11:44:39 -04:00
MarcoFalke
c8971547d9
Merge #18499: rpc: Make rpc documentation not depend on call-time rpc args
fab32557f2 rpc: Make rpc documentation not depend on rpc args (MarcoFalke)

Pull request description:

  This is required to host the documentation on a static resource (like a website or pdf)

ACKs for top commit:
  emilengler:
    utACK fab32557f2
  promag:
    ACK fab32557f2.

Tree-SHA512: 3ca2691c7fbd5f17c75df2887753da152f66521dcb7dee4c29af6339fdea011cecdd51f825b96bde9c6aaf82f4d915cbd5aacb52e4eae3898d9dbc216f627171
2020-04-04 03:15:00 +08:00
MarcoFalke
e35e118656
Merge #18508: RPC: Fix more formatting nits
f32ab443a9 Bugfix: RPC: JSON null is not "None" (Luke Dashjr)
26dcf39581 Bugfix: RPC: Don't use a continuation elipsis after an elision elipsis (Luke Dashjr)
eca65caadc Bugfix: RPC: Add missing commas and correct indentation of explicit ELISION (Luke Dashjr)

Pull request description:

  1. listsinceblock had a double ellipsis (elision + continuation); this looks ugly, just one is needed.
  2. Elision ellipsis wasn't getting a comma, so was truncated to `".."` by comma-removal code.
  3. Elision ellipsis was indented incorrectly (as if it was a subitem).
  4. Similarly, type "none" would get truncated to `"Non"`, when it should really be `"null"` anyway.

ACKs for top commit:
  MarcoFalke:
    ACK f32ab443a9 🐰

Tree-SHA512: 34e1c72673790ed11cdee838d64ea5e0ac498de19258df99d54b5322e003060123c65ad27ac2fd4729a1dfe52066a0629602a132b1ef85d4154affd99a065a3f
2020-04-04 03:08:05 +08:00
MarcoFalke
fa86edf66d
fuzz: Extend script fuzz test 2020-04-04 01:32:17 +08:00
MarcoFalke
fa0189955a
fuzz: Extend descriptor fuzz test 2020-04-04 01:16:19 +08:00
fanquake
9e071b0089
test: remove rapidcheck integration and tests 2020-04-03 22:47:59 +08:00
Wladimir J. van der Laan
0eeb0468e7 net: Hardcoded seeds update for 0.20
Update hardcoded seeds from seeds_emzy.txt seeds_lukejr.txt
seeds_sipa.txt seeds_sjors.txt, according to release process.

Output from makeseeds.py:
```
  IPv4   IPv6  Onion Pass
1364173 244127   2454 Initial
1364173 244127   2454 Skip entries with invalid address
1129552 213117   2345 After removing duplicates
1129548 213117   2345 Skip entries from suspicious hosts
338216 191944   2249 Enforce minimal number of blocks
336851 188993   2189 Require service bit 1
  6998   1520    150 Require minimum uptime
  5682   1290     89 Require a known and recent user agent
  5622   1279     89 Filter out hosts with multiple bitcoin ports
   512    146     89 Look up ASNs and limit results per ASN and per net
```
2020-04-03 16:29:26 +02:00
fanquake
08c4994969
Merge #18503: init: Replace URL_WEBSITE with PACKAGE_URL
fad2f68353 init: Replace URL_WEBSITE with PACKAGE_URL (MarcoFalke)

Pull request description:

  This is needed for rebranding efforts such as #18489

ACKs for top commit:
  hebasto:
    ACK fad2f68353, tested on Linux Mint 19.3:
  fanquake:
    ACK fad2f68353 - clicked a link.

Tree-SHA512: c26e18cd328d3dd3fd7e25413e1bab780026687a148f126b8673e5f6cc13249f6c16689e45eba9da1545915c6001f96cd33f4e656c08cda3eae1c3fd88da23ea
2020-04-03 19:11:07 +08:00
MarcoFalke
faa64af960
fuzz: Add CScriptNum::getint coverage 2020-04-03 09:02:34 +08:00
MarcoFalke
dce6f3b29b
Merge #18383: refactor: Check for overflow when calculating sum of tx outputs
f65c9ad40f Check for overflow when calculating sum of outputs (Elichai Turkel)

Pull request description:

  This was reported by practicalswift here #18046
  The exact order of the if, is important, we first do `!MoneyRange(tx_out.nValue)` to make sure the amount is non-negative. and then `std::numeric_limits<CAmount>::max() - tx_out.nValue < nValueOut` checks that the addition cannot overflow (if we won't check that the amount is positive this check can also overflow! (by doing something like `max - -max`))
  and only then we make sure that the some is also valid `!MoneyRange(nValueOut + tx_out.nValue)`
  if any of these conditions fail we throw.

  the overflowing logic:
  ```
  a + b > max // we want to fail if a+b is more than the maximum -> will overflow
  b > max - a
  max - a < b
  ```

  Closes: #18046

ACKs for top commit:
  MarcoFalke:
    ACK f65c9ad40f, checked that clang with O2 produces identical binaries 💕
  practicalswift:
    ACK f65c9ad40f
  instagibbs:
    utACK f65c9ad40f
  vasild:
    ACK f65c9ad40f modulo `s/assert.h/cassert/`

Tree-SHA512: 512d6cf4762f24c41cf9a38da486b17b19c634fa3f4efbdebfe6608779e96fc3014d5d2d29adb8001e113152c0217bbd5b3900ac4edc7b8abe77f82f36209e33
2020-04-03 06:50:29 +08:00
Luke Dashjr
b5795a7886 Wallet: Add warning comments and assert to CWallet::DelAddressBook 2020-04-02 16:37:42 +00:00
Luke Dashjr
6d2905f57a Wallet: Avoid unnecessary/redundant m_address_book lookups 2020-04-02 16:37:41 +00:00
João Barbosa
7b8e15728d rpc: Fix rpcRunLater race in walletpassphrase 2020-04-02 17:25:27 +01:00
Luke Dashjr
c751d886f4 Wallet: Avoid treating change-in-the-addressbook as non-change everywhere 2020-04-02 16:25:17 +00:00
Luke Dashjr
8e64b8c84b Wallet: New FindAddressBookEntry method to filter out change entries (and skip ->second everywhere) 2020-04-02 16:02:56 +00:00
Luke Dashjr
65b6bdc2b1 Wallet: Add CAddressBookData::IsChange which returns true iff label has never been set 2020-04-02 16:02:07 +00:00
Luke Dashjr
144b2f85da Wallet: Require usage of new CAddressBookData::setLabel to change label 2020-04-02 16:01:36 +00:00
Luke Dashjr
b86cd155f6 scripted-diff: Wallet: Rename mapAddressBook to m_address_book
Previous versions assumed absence of an entry in mapAddressBook indicated change.
This no longer holds true (due to bugs) and will shortly be made intentional.
Renaming the field helps ensure that old code using mapAddressBook directly gets checked for necessary rebasing.

-BEGIN VERIFY SCRIPT-
sed -i -e 's/mapAddressBook/m_address_book/g' $(git grep -l 'mapAddressBook' ./src)
-END VERIFY SCRIPT-
2020-04-02 16:00:28 +00:00
Luke Dashjr
f32ab443a9 Bugfix: RPC: JSON null is not "None" 2020-04-02 15:28:05 +00:00
Luke Dashjr
26dcf39581 Bugfix: RPC: Don't use a continuation elipsis after an elision elipsis 2020-04-02 15:24:01 +00:00
Luke Dashjr
eca65caadc Bugfix: RPC: Add missing commas and correct indentation of explicit ELISION 2020-04-02 15:24:00 +00:00
MarcoFalke
fa1793c1c4
net: Pass connman const when relaying address 2020-04-02 21:56:20 +08:00
Wladimir J. van der Laan
b83565625e
Merge #18500: chainparams: Bump assumed valid hash
222253e37d chainparams: Bump assumed valid hash (MarcoFalke)

Pull request description:

  See https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-branch-off

ACKs for top commit:
  fanquake:
    ACK 222253e37d - matches output on my node. Also have 3.8GB for the chainstate dir. If anyone wants to know what commands they should run / what to check; I have some docs on [assumevalid updates](https://github.com/fanquake/core-review/blob/master/update-assumevalid.md).
  elichai:
    ACK 222253e37d
  theStack:
    ACK 222253e37d
  instagibbs:
    ACK 222253e37d all mainnet params, did not check testnet

Tree-SHA512: 856e75f0c6663ab478768d80618f11ac109ef30058661f58ed7f2a3a615dd5fb8f8d648c996eff77949036c1633119085b25cd4c25250a0850c943c02cddf3d7
2020-04-02 15:43:49 +02:00
Wladimir J. van der Laan
6bdd515ccf
Merge #16923: wallet: Handle duplicate fileid exception
9eefc6e92f gui: Delete progress dialog instead of hidding it (João Barbosa)
ee9e88ba27 wallet: Handle duplicate fileid exception (João Barbosa)

Pull request description:

  Handle the duplicate fileid exception thrown at `CheckUniqueFileid` in tow cases:
   - when duplicate wallets are set on the command line - catch in `LoadWallets`;
   - when a duplicate wallet is loaded dynamically - catch in `LoadWallet`.

  Fixes #16776.

ACKs for top commit:
  jonatack:
    Re-ACK 9eefc6e92f no change since last review 68e0ff0e1f530c942721aab49cf67ffc07104628
  hebasto:
    re-ACK 9eefc6e92f

Tree-SHA512: 46e3c1cd6708b54e2d1c4973a74c8d5428822e04cecbc147cf200eb034efa385e867bd749c7c639020e83c9813fae8fed64a851bdd99abf60c33b07e0363f5d5
2020-04-02 15:07:55 +02:00
MarcoFalke
fad2f68353
init: Replace URL_WEBSITE with PACKAGE_URL 2020-04-02 20:52:47 +08:00
Russell Yanofsky
01a3392b1b Drop bitcoin-wallet dependency on libevent
Don't require urlDecode function in wallet code since urlDecode implementation
currently uses libevent. Just call urlDecode indirectly though URL_DECODE
function pointer constant if available.

In bitcoind and bitcoin-qt, URL_DECODE is implemented and used to interpret RPC
wallet requests. In bitcoin-wallet, URL_DECODE is null to avoid depending on
libevent.
2020-04-02 08:35:10 -04:00
Russell Yanofsky
0660119ac3 Drop unintended bitcoin-tx dependency on libevent
Don't include util/url.cpp to libbitcoin_util.a when libevent isn't available.
This fixes a compile error trying to build bitcoin-tx without libevent reported
by Luke Dashjr in https://github.com/bitcoin/bitcoin/issues/18465

Fixes #18465
2020-04-02 08:28:11 -04:00
Ben Woosley
a46484c8b3
build: Detect gmtime_* definitions via configure
This improves the portability of the codebase and fixes compilation
with mingw-w64 7.0+.

Co-authored-by: fanquake <fanquake@gmail.com>
2020-04-02 12:31:54 +08:00
MarcoFalke
222253e37d
chainparams: Bump assumed valid hash 2020-04-02 05:34:47 +08:00
MarcoFalke
fab32557f2
rpc: Make rpc documentation not depend on rpc args 2020-04-02 00:25:13 +08:00
Vasil Dimov
0753efd9dc
rpc: Remove deprecated "size" from mempool txs
Remove the "size" property of a mempool transaction from RPC replies.

Deprecated in e16b6a718 in 0.19, about 1 year ago.
2020-04-01 14:19:34 +02:00
Wladimir J. van der Laan
740614e3c3 qt: Translations update pre-branch 2020-04-01 12:49:15 +02:00
Andrew Chow
cd3b1569d9 Correctly compute redeemScript from witnessScript for signrawtransaction
ParsePrevouts uses GetScriptForWitness on the given witnessScript
to find the corresponding redeemScript. This is incorrect when the
witnessScript is either a P2PK or P2PKH script as it returns the
corresponding P2WPK script instead of turning the witnessScript
into a P2WSH script. Instead this should make the script a
WitnessV0ScriptHash destination and get the script for that.

Test cases are also added.
2020-03-31 18:41:52 -04:00
MarcoFalke
7777703958
doc: Explain new test logging 2020-03-31 17:11:47 -04:00
MarcoFalke
72ef366564
Merge #18472: test: Remove unsafe BOOST_TEST_MESSAGE
fa3cc0bfc4 test: Remove unsafe BOOST_TEST_MESSAGE (MarcoFalke)

Pull request description:

  Fixes #17987

  Can be tested with

  ```
  ./src/test/test_bitcoin -l test_suite  -- DEBUG_LOG_OUT

ACKs for top commit:
  fjahr:
    tested ACK fa3cc0bfc4
  mzumsande:
    Tested ACK fa3cc0bfc4

Tree-SHA512: f63b110d77882cd7c0d7574ff6c9c948db8febb3400ecdac45164746b587b0fa223463041801271b3959267ddc1d9a4a67ba76939e242e7dd2f92a2834a400a0
2020-03-31 15:39:15 -04:00
Fabian Jahr
bda84a08a0
rpc: Add documentation for deactivating settxfee 2020-03-31 17:07:33 +02:00
Russell Yanofsky
48973402d8 wallet: Avoid use of Chain::Lock in CWallet::GetKeyBirthTimes
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change only affects behavior in the case where wallet last block processed
falls behind the chain tip, where it will treat the last block processed as the
current tip.
2020-03-31 08:36:02 -05:00
Russell Yanofsky
e958ff9ab5 wallet: Avoid use of Chain::Lock in CWallet::CreateTransaction
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change only affects behavior in the case where wallet last block processed
falls behind the chain tip, where it may set a different lock time.
2020-03-31 08:36:02 -05:00
Russell Yanofsky
c0d07dc4cb wallet: Avoid use of Chain::Lock in CWallet::ScanForWalletTransactions
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change affects behavior in a few small ways.

- If there's no max_height specified, percentage progress is measured ending at
  wallet last processed block instead of node tip

- More consistent error reporting: Early check to see if start_block is on the
  active chain is removed, so start_block is always read and the triggers an
  error if it's unavailable
2020-03-31 08:36:02 -05:00
Russell Yanofsky
1be8ff280c wallet: Avoid use of Chain::Lock in rescanblockchain
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change only affects behavior in the case where wallet last block processed
falls behind the chain tip. The rescanblockchain error height error checking
will just be stricter in this case and only accept values up to the last
processed height
2020-03-31 08:36:02 -05:00
Russell Yanofsky
3cb85ac594 wallet refactor: Avoid use of Chain::Lock in CWallet::RescanFromTime
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change has no effect on behavior.
2020-03-31 08:36:02 -05:00
Russell Yanofsky
f7ba881bc6 wallet: Avoid use of Chain::Lock in listsinceblock
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change only affects behavior in the case where wallet last block processed
falls behind the chain tip. Previously listsinceblock might not have returned
all transactions up to the claimed "lastblock" value in this case, resulting in
race conditions and potentially missing transactions in cases where
listsinceblock was called in a loop like
https://github.com/bitcoin/bitcoin/issues/14338#issuecomment-426706574
2020-03-31 08:36:02 -05:00
Russell Yanofsky
bc96a9bfc6 wallet: Avoid use of Chain::Lock in importmulti
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change only affects behavior in the case where wallet last block processed
falls behind the chain tip, in which case it may use a more accurate rescan
time.
2020-03-31 08:36:02 -05:00
Russell Yanofsky
25a9fcf9e5 wallet: Avoid use of Chain::Lock in importwallet and dumpwallet
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change only affects behavior in the case where wallet last block processed
falls behind the chain tip, in which case it will use more accurate backup and
rescan timestamps.
2020-03-31 08:36:02 -05:00
Russell Yanofsky
c1694ce6bb wallet: Avoid use of Chain::Lock in importprunedfunds
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change only affects behavior in the case where wallet last block processed
falls behind the chain tip, in which case the "Block not found in chain" error
will be stricter and not allow importing data from a blocks between the wallet
last processed tip and the current node tip.
2020-03-31 08:36:02 -05:00
Russell Yanofsky
ade5f87971 wallet refactor: Avoid use of Chain::Lock in qt wallettests
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change doesn't affect behavior.
2020-03-31 08:36:02 -05:00
Russell Yanofsky
f6da44ccce wallet: Avoid use of Chain::Lock in tryGetTxStatus and tryGetBalances
This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

It also helps ensure the GUI display stays up to date in the case where the
node chain height runs ahead of wallet last block processed height.
2020-03-31 08:36:02 -05:00
Russell Yanofsky
bf30cd4922 refactor: Add interfaces::FoundBlock class to selectively return block data
FoundBlock class allows interfaces::Chain::findBlock to return more block
information without having lots of optional output parameters. FoundBlock class
is also used by other chain methods in upcoming commits.

There is mostly no change in behavior. Only exception is
CWallet::RescanFromTime now throwing NonFatalCheckError instead of
std::logic_error.
2020-03-31 08:36:02 -05:00
João Barbosa
9eefc6e92f gui: Delete progress dialog instead of hidding it 2020-03-31 14:23:43 +01:00
João Barbosa
ee9e88ba27 wallet: Handle duplicate fileid exception 2020-03-31 14:23:43 +01:00
Wladimir J. van der Laan
9a2b5f22c1
Merge #18338: Fix wallet unload race condition
41b0baf43c gui: Handle WalletModel::unload asynchronous (João Barbosa)
ab31b9d6fe Fix wallet unload race condition (Russell Yanofsky)

Pull request description:

  This PR consists in two fixes. The first fixes a concurrency issues with `boost::signals2`. The second fixes a wallet model destruction while it's being used.

  From boost signal documentation at https://www.boost.org/doc/libs/1_72_0/doc/html/signals2/thread-safety.html:

  > When a signal is invoked by calling signal::operator(), the invocation first acquires a lock on the signal's mutex. Then it obtains a handle to the signal's slot list and combiner. Next it releases the signal's mutex, before invoking the combiner to iterate through the slot list.

  This means that `UnregisterValidationInterface` doesn't prevent more calls to that interface. The fix consists in capturing the `shared_ptr<CValidationInterface>` in each internal slot.

  The GUI bug is fixed by using a `Qt::QueuedConnection` in the `WalletModel::unload` connection.

ACKs for top commit:
  ryanofsky:
    Code review ACK 41b0baf43c. Only change is moving assert as suggested
  hebasto:
    ACK 41b0baf43c, tested on Linux Mint 19.3.

Tree-SHA512: 4f712d8de65bc1214411831250de5dc0a9fd505fb84da5baf9f2cc4d551bc3abffc061616f00afe43dba7525af2cd96c9b54aeead9383145e3b8801f25d85f50
2020-03-31 15:07:06 +02:00
Wladimir J. van der Laan
f2880e21ef
Merge #18160: gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged
0933a37078 gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged (João Barbosa)

Pull request description:

  Each 250ms the slot `WalletModel::pollBalanceChanged` is called which, at worst case, calls `Wallet::GetBalance`. This is a waste of resources since most of the time there aren't new transactions or new blocks. Fix this by early checking if cache is dirty or not.

  The actual balance computation can still hang the GUI thread but that is tracked in #16874 and should be fixed with a solution similar to #17135.

ACKs for top commit:
  hebasto:
    ACK 0933a37078, I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged.
  jonasschnelli:
    utACK 0933a37078
  instagibbs:
    ACK 0933a37078
  ryanofsky:
    Code review ACK 0933a37078, but I would prefer (not strongly) for #17905 to be merged first. This PR can be simpler if it is based on #17905, so tryGetBalances can just be left alone instead of changing into to a more complicated tryGetBalancesIfNeeded function, and then getting changed back later when we want to optimize it out.
  jonatack:
    ACK 0933a37078 based primarily on code review, despite a lot of manual testing with a large 177MB wallet.

Tree-SHA512: 18db35bf33a7577666658c8cb0b57308c8474baa5ea95bf1468cd8531a69857d8915584f6ac505874717aa6aabeb1b506ac77630f8acdb6651afab89275e38a1
2020-03-31 14:23:30 +02:00
MarcoFalke
fa3cc0bfc4
test: Remove unsafe BOOST_TEST_MESSAGE 2020-03-30 15:18:42 -04:00
fanquake
6a11d9e330
Merge #18433: serialization: prevent int overflow for big Coin::nHeight
e980214bc4 serialization: prevent int overflow for big Coin::nHeight (pierrenn)

Pull request description:

  This is an attempt to fix fuzzer issues 1,2,8 reported by practicalswift here : https://github.com/bitcoin/bitcoin/issues/18046

  The fuzzer harness doesn't prevent deserialization of unrealistic high values for `Coin::nHeight`.  In the [provided examples](https://github.com/bitcoin/bitcoin/issues/18046), we have :
  - `blockundo_deserialize` : the varint `0x8DD88DD700` is deserialized as `3944983552` in `Coin::nHeight` (`TxInOutFormatter::Unser`)
  - `coins_deserialize` : the varint `0x8DD5D5EC40` is deserialized as `3939874496` similarly
  - `txundo_deserialize`: the varint `0x8DCD828F01` is deserialized as `3921725441` in `Coin::nHeight` (`Coin::Unserialize`)

  Since `Coin::nHeight` is 31 bit long, multiplying a large value by 2 triggers the fuzzer.

  AFAIK those values are unrealistic (~70k years for the smallest..). I've looked a bit a reducing the range of values the fuzzer can deserialize, but this seems to be too much code change for not much.

  Hence this PR chooses to static cast `nHeight` when re-serializing; it seems to be the less intrusive/safest way to prevent the fuzzer output.

  Another more "upstream" approach would be to limit `Coin::nHeight` values to something more realistic, e.g. `0xFFFFFFF` (~5k years) :

  de3a30bab2/src/undo.h (L39) and de3a30bab2/src/coins.h (L71)

  Thanks !

  NB: i was also not sure about the component/area to prefix the PR/commit with.. ?

ACKs for top commit:
  practicalswift:
    ACK e980214bc4 -- patch looks correct
  promag:
    ACK e980214bc4.
  sipa:
    utACK e980214bc4
  MarcoFalke:
    re-ACK e980214bc4 🎑
  ryanofsky:
    Code review ACK e980214bc4. Just removed ternary ? 1 : 0 and replaced / 2 with >> 1 since last review

Tree-SHA512: 905fc9e5e52a6857abee4a1c863751767835965804bb8c39474f27a120f65399ff4ba7a49ef1da0ba565379f8c12095bd384b6c492cf06776f01b2db68d522b8
2020-03-30 22:16:12 +08:00
MarcoFalke
fa369651c5
net: Add missing cs_vNodes lock 2020-03-29 11:45:46 -04:00
MarcoFalke
5f9cd62f33
Merge #18455: tests: Add fuzzing harness for functions/classes in flatfile.h, merkleblock.h, random.h, serialize.h and span.h
11a520f679 tests: Add fuzzing harness for functions/classes in random.h (practicalswift)
64d277bbbc tests: Add fuzzing harness for LimitedString (serialize.h) (practicalswift)
f205cf7fef tests: Add fuzzing harness for functions/classes in span.h (practicalswift)
9718f38f54 tests: Add fuzzing harness for functions/classes in merkleblock.h (practicalswift)
a16ea051f9 tests: Add fuzzing harness for functions/classes in flatfile.h (practicalswift)

Pull request description:

  * Add fuzzing harness for functions/classes in `flatfile.h`
  * Add fuzzing harness for functions/classes in `merkleblock.h`
  * Add fuzzing harness for functions/classes in `span.h`
  * Add fuzzing harness for `LimitedString` (`serialize.h`)
  * Add fuzzing harness for functions/classes in `random.h`

Top commit has no ACKs.

Tree-SHA512: 6f7e0f946f1062d51216990cde9672b4e896335152548ace3d8711e4969c3e3c8566d01d915b72adcda5c1caa9c2e34da6b7473b55a229f5b77239d3b0ba4b67
2020-03-29 10:32:05 -04:00
practicalswift
11a520f679 tests: Add fuzzing harness for functions/classes in random.h 2020-03-29 13:17:04 +00:00
practicalswift
64d277bbbc tests: Add fuzzing harness for LimitedString (serialize.h) 2020-03-29 13:17:04 +00:00
practicalswift
f205cf7fef tests: Add fuzzing harness for functions/classes in span.h 2020-03-29 13:17:04 +00:00
practicalswift
9718f38f54 tests: Add fuzzing harness for functions/classes in merkleblock.h 2020-03-29 13:17:04 +00:00
practicalswift
a16ea051f9 tests: Add fuzzing harness for functions/classes in flatfile.h 2020-03-29 13:17:04 +00:00
MarcoFalke
6cfb3dbbdb
Merge #18391: doc: Update init and reduce-traffic docs for -blocksonly
621e86ee8d Update -blocksonly documentation (glowang)

Pull request description:

  When -blocksonly is set to 1, it interacts with the -walletbroadcast
  parameter and sets it to 0.

  This behavior is not captured by the current documentation, which
  claims that -blocksonly does not impact any wallet transactions at
  all.

  Fixes #17294

ACKs for top commit:
  MarcoFalke:
    ACK 621e86ee8d

Tree-SHA512: f47bfb40a196c23e62505e1d4f79094011ac7c21fc9b920fad60cdadb5c4f48e993be1f015e26e568ce329967c24848fd7b665a6cffd3881f4cfcd2fd0081ed8
2020-03-29 08:15:55 -04:00
glowang
621e86ee8d Update -blocksonly documentation
When -blocksonly is set to 1, it interacts with the -walletbroadcast
parameter and sets it to 0 if it has not been set already.This behavior
is not captured by the current documentation, which claims that -blocksonly
does not impact any wallet transactions.

Update the max number of outgoing peers from 8 to 10, due to the
addition of two -blocksonly peers.
2020-03-29 05:12:30 -07:00
MarcoFalke
27a82d347e
Merge #18459: rpc: remove unused getbalances() code
6e0d82c55b rpc: remove unused getbalances() code (Jon Atack)

Pull request description:

  This line from 999931cf8f appears to be extraneous and replaced 2 lines after by `UniValue balances{UniValue::VOBJ};`.

ACKs for top commit:
  Empact:
    ACK 6e0d82c55b
  hebasto:
    ACK 6e0d82c55b, the `obj` local variable is not used until the end of the scope.

Tree-SHA512: a220ca9cda091e78144d9b7fbe4bf90e8338d6e8c8dc7bea27a8e62f3a8ac1d983ad12a48a0a3366b2d8b9586878dfc69c1ec34bf846b34c91e42cda48a59850
2020-03-28 17:09:52 -04:00
Jon Atack
6e0d82c55b
rpc: remove unused getbalances() code 2020-03-28 20:25:53 +01:00
MarcoFalke
6b4f182806
Merge #18444: RPC: Remove final comma for last entry of fixed-size arrays/objects in RPCResult
c34164896c Bugfix: RPC: Remove final comma for last entry of fixed-size Arrays and Objects in RPCResult (Luke Dashjr)

Pull request description:

  JSON doesn't allow a trailing comma in arrays

Top commit has no ACKs.

Tree-SHA512: 761502a05f447afc09c120f13bf23abd2aee83a7f5e5dadaf54c7e1c0c1280d83ee041ca6ca45998fb561e41b32d01067ec52a187c3bcc9d53303ea813bc212c
2020-03-28 15:13:13 -04:00
Wladimir J. van der Laan
1668c80bdc
Merge #18449: util: Remove unused itostr
faaf1cb5b9 util: Replace i64tostr with ToString (MarcoFalke)
fac96fff62 util: Remove unused itostr (MarcoFalke)

Pull request description:

  Currently unused, but if someone really needed to use a helper with this functionality in the future, they could use `ToString`.

ACKs for top commit:
  laanwj:
    ACK faaf1cb5b9
  promag:
    Code review ACK faaf1cb5b9.

Tree-SHA512: 42180c03f51d677f7b69da23c7868bdd88944335fad0752fcc307f2c3e3c69f1cc1b316ac0875bcefb9a69c5d55200d7cf66843ea4c0f0f26baf7a054b96c1bb
2020-03-28 19:26:45 +01:00
Luke Dashjr
c34164896c Bugfix: RPC: Remove final comma for last entry of fixed-size Arrays and Objects in RPCResult
JSON doesn't allow a trailing comma in Arrays/Objects
2020-03-28 17:32:28 +00:00
pierrenn
e980214bc4 serialization: prevent int overflow for big Coin::nHeight 2020-03-28 08:38:07 +09:00
MarcoFalke
fa47a0b003
net: Make addr relay mockable 2020-03-27 17:58:51 -04:00
Wladimir J. van der Laan
f2c416bcf5
Merge #16995: Fix gcc 9 warnings
ff9c671b11 refactor: Work around GCC 9 `-Wredundant-move` warning (Russell Yanofsky)
b837b334db net: Fail instead of truncate command name in CMessageHeader (Wladimir J. van der Laan)

Pull request description:

  Fixes all 3 from #16992 (see commits)

  - net: Fail instead of truncate command name in CMessageHeader
  - refactor: Use std::move workaround for unique_ptr upcast only when necessary

ACKs for top commit:
  practicalswift:
    ACK ff9c671b11 -- patch looks correct
  sipa:
    utACK ff9c671b11
  ryanofsky:
    Code review ACK ff9c671b11. Looks good and seems to pass travis, modulo a timeout on one build
  hebasto:
    ACK ff9c671b11, tested on Fedora 31:

Tree-SHA512: 52d8c13aaf0d56f9bc546a98d7f853eae21f7e325b202fdeb2286b19a9a0ee308634c644b039f60ad8043421e382381cbf1bce58d9f807547f928621c7d245d0
2020-03-27 20:04:46 +01:00
Andrew Toth
a1d5b12ec0 Merge getreceivedby tally into GetReceived function 2020-03-27 14:26:00 -04:00
João Barbosa
41b0baf43c gui: Handle WalletModel::unload asynchronous
This change prevents deleting a WalletModel instance while it's
being used.
2020-03-27 15:17:35 +00:00
Russell Yanofsky
ab31b9d6fe Fix wallet unload race condition
Currently it's possible for ReleaseWallet to delete the CWallet pointer while
it is processing BlockConnected, etc chain notifications.

To fix this, unregister from notifications earlier in UnloadWallet instead of
ReleaseWallet, and use a new RegisterSharedValidationInterface function to
prevent the CValidationInterface shared_ptr from being deleted until the last
notification is actually finished.
2020-03-27 15:17:35 +00:00
Andrew Toth
dcc8332543 Add generateblock rpc 2020-03-27 11:00:10 -04:00
MarcoFalke
faaf1cb5b9
util: Replace i64tostr with ToString 2020-03-27 10:14:08 -04:00
Sjors Provoost
764bfe4cba
[psbt] add file size limit 2020-03-27 14:59:19 +01:00
Sjors Provoost
1cd8dc2556
[gui] load PSBT
co-authored-by: Glenn Willen <gwillen@nerdnet.org>
2020-03-27 14:59:19 +01:00
Sjors Provoost
f6895301f7
[gui] save PSBT to file
co-authored-by: Glenn Willen <gwillen@nerdnet.org>
2020-03-27 14:59:19 +01:00
Sjors Provoost
1d05a9d80b
Move DEFAULT_MAX_RAW_TX_FEE_RATE to node/transaction.h
So it can be used in the GUI.
2020-03-27 14:58:39 +01:00
MarcoFalke
fac96fff62
util: Remove unused itostr 2020-03-27 08:59:06 -04:00
fanquake
7eed413e72
Merge #18398: rpc: fix broken RPCExamples for waitforblock(height)
ef35604c9c rpc: fix broken RPCExamples for waitforblock(height) (Sebastian Falbesoner)

Pull request description:

  This PR fixes several broken RPCExamples from the "blockchain" category:
  - `HelpExampleCli` for `waitforblock` (disturbing comma between arguments)
  - `HelpExampleCli` for `waitforblockheight` (disturbing comma between arguments)
  - `HelpExampleRpc` for `waitforblockheight` (disturbing quotation marks around integer argument)

  Note that the CLI example for `waitforblockheight` would also work with the first argument in quotation marks (in contrast to the RPC example), but I removed them as well as they are not needed.

  Outputs for the non-working examples in the master branch:
  ```
  $ ./bitcoin-cli waitforblock "0000000000079f8ef3d2c688c244eb7a4570b24c9ed7b4a8c619eb02596f8862", 1000
  error code: -8
  error message:
  blockhash must be of length 64 (not 65, for '0000000000079f8ef3d2c688c244eb7a4570b24c9ed7b4a8c619eb02596f8862,')
  ```

  ```
  $ ./bitcoin-cli waitforblockheight "100", 1000
  error: Error parsing JSON:100,
  ```

  ```
  $ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "waitforblockheight", "params": ["100", 1000]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
  Enter host password for user '__cookie__':
  {"result":null,"error":{"code":-1,"message":"JSON value is not an integer as expected"},"id":"curltest"}
  ```

  Outputs for the fixed examples in the PR branch:
  ```
  $ ./bitcoin-cli waitforblock "0000000000079f8ef3d2c688c244eb7a4570b24c9ed7b4a8c619eb02596f8862" 1000
  {
    "hash": "0000000000000000000910ae4d56120e0ddd55c0552e80ed12dba147abc68080",
    "height": 622416
  }
  ```

  ```
  $ ./bitcoin-cli waitforblockheight 100 1000
  {
    "hash": "0000000000000000000910ae4d56120e0ddd55c0552e80ed12dba147abc68080",
    "height": 622416
  }
  ```

  ```
  $ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "waitforblockheight", "params": [100, 1000]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
  Enter host password for user '__cookie__':
  {"result":{"hash":"0000000000000000000910ae4d56120e0ddd55c0552e80ed12dba147abc68080","height":622416},"error":null,"id":"curltest"}
  ```

ACKs for top commit:
  fanquake:
    ACK ef35604c9c

Tree-SHA512: b98c6681d1aa24b3ee3ef4ef450cb630082a9f8695af18f3b6d418e5b0b1e472b787ccf6397cd719b4d5fe0082ea5f1d0ca553c1cc56066ee2d288be34c601e3
2020-03-27 15:24:11 +08:00
Wladimir J. van der Laan
b53af72b82
Merge #18416: util: Limit decimal range of numbers ParseScript accepts
9ab14e4d21 Limit decimal range of numbers ParseScript accepts (pierrenn)

Pull request description:

  Following up on this suggestion : https://github.com/bitcoin/bitcoin/pull/18413#issuecomment-602966490, prevent the output of `atoi64` in the `core_read.cpp:ParseScript` helper to send to `CScriptNum::serialize` values wider than 32-bit.

  Since the `ParseScript` helper is only used by the tool defined in `bitcoin-tx.cpp`, this only prevents users to provide too much unrealistic values.

ACKs for top commit:
  laanwj:
    ACK 9ab14e4d21

Tree-SHA512: ee228269d19d04e8fee0aa7c0ae2bb0a2b437b8e574356e8d9b2279318242057d51fcf39a842aa3afe27408d0f2d5276df245d07a3f4828644a366f80587b666
2020-03-27 08:02:51 +01:00
pierrenn
9ab14e4d21 Limit decimal range of numbers ParseScript accepts 2020-03-27 15:51:05 +09:00
fanquake
54646167db
Merge #18388: Make VerifyWitnessProgram use a Span stack
2b0fcff7f2 Make VerifyWitnessProgram use a Span stack (Pieter Wuille)

Pull request description:

  Here is a follow-up to #18002, again with the goal of simplifying (potential) BIP341 code.

  Instead of passing a begin and end iterator of the initial stack to `ExecuteWitnessScript`, they are turned into a `Span<const valtype>`, representing a span of `valtype`s in memory. This allows `VerifyWitnessProgram` to operate on that span directly, instead of juggling iterators around (which would be exacerbated by #17977 if trying to avoid copying the stack).

ACKs for top commit:
  ajtowns:
    ACK 2b0fcff7f2
  elichai:
    ReACK on the diff 2b0fcff7f2
  instagibbs:
    re-ACK 2b0fcff7f2
  theStack:
    re-ACK 2b0fcff7f2
  Empact:
    ACK 2b0fcff7f2
  jnewbery:
    utACK 2b0fcff7f2

Tree-SHA512: 38eb4ce17f1947674c1c274caa40feb6ea8266bd96134d9cf1bc41e6fbf1114d4dde6c7a9e26e1ca8f3d0155429ef0911cc8ec0c1037d8fe7d6ec7f9e7184e93
2020-03-27 14:49:50 +08:00
MarcoFalke
e3154aacf4
Merge #18445: tests: Add fuzzing harnesses for functions/classes in chain.h and protocol.h
7834c3b9ec tests: Add fuzzing harness for functions/classes in chain.h (practicalswift)
d7930c4326 tests: Add fuzzing harness for functions/classes in protocol.h (practicalswift)

Pull request description:

  Add fuzzing harnesses for functions/classes in `chain.h` and `protocol.h`.

Top commit has no ACKs.

Tree-SHA512: ac2d66bc678ebba0ffbbc42e77806eaf3bb07413ff19219c7a83b171ccd4601e0aa8546ee7ffe8018ca4de12d080f79f693d184cc337c234cde641803279f00c
2020-03-26 20:37:48 -04:00
MarcoFalke
0dc6218c79
Merge #18270: util: Fail to parse whitespace-only strings in ParseMoney(...) (instead of parsing as zero)
100213c5c2 util: Fail to parse space-only strings in ParseMoney(...) (instead of parsing as zero) (practicalswift)

Pull request description:

  Fail to parse whitespace-only strings in `ParseMoney(...)` (instead of parsing as `0`).

  This is a follow-up to #18225 ("util: Fail to parse empty string in `ParseMoney`") which made `ParseMoney("")` fail instead of parsing as `0`.

  Context: https://github.com/bitcoin/bitcoin/pull/18225#issuecomment-592994765

  Current non-test call sites:

  ```
  $ git grep ParseMoney ":(exclude)src/test/"
  src/bitcoin-tx.cpp:    if (!ParseMoney(strValue, value))
  src/init.cpp:        if (!ParseMoney(gArgs.GetArg("-incrementalrelayfee", ""), n))
  src/init.cpp:        if (!ParseMoney(gArgs.GetArg("-minrelaytxfee", ""), n)) {
  src/init.cpp:        if (!ParseMoney(gArgs.GetArg("-blockmintxfee", ""), n))
  src/init.cpp:        if (!ParseMoney(gArgs.GetArg("-dustrelayfee", ""), n))
  src/miner.cpp:    if (gArgs.IsArgSet("-blockmintxfee") && ParseMoney(gArgs.GetArg("-blockmintxfee", ""), n)) {
  src/util/moneystr.cpp:bool ParseMoney(const std::string& str, CAmount& nRet)
  src/util/moneystr.h:NODISCARD bool ParseMoney(const std::string& str, CAmount& nRet);
  src/wallet/wallet.cpp:        if (!ParseMoney(gArgs.GetArg("-mintxfee", ""), n) || 0 == n) {
  src/wallet/wallet.cpp:        if (!ParseMoney(gArgs.GetArg("-fallbackfee", ""), nFeePerK)) {
  src/wallet/wallet.cpp:        if (!ParseMoney(gArgs.GetArg("-discardfee", ""), nFeePerK)) {
  src/wallet/wallet.cpp:        if (!ParseMoney(gArgs.GetArg("-paytxfee", ""), nFeePerK)) {
  src/wallet/wallet.cpp:        if (!ParseMoney(gArgs.GetArg("-maxtxfee", ""), nMaxFee)) {
  ```

ACKs for top commit:
  Empact:
    ACK 100213c5c2
  sipa:
    ACK 100213c5c2
  theStack:
    ACK 100213c5c2

Tree-SHA512: cadfb1ac8276cf54736c3444705f2650e7a08023673aedc729fabe751ae80f6c490fc0945ee38dbfd02c95e4d9853d1e4c84f5d3c310f44eaf3585afec8a4c22
2020-03-26 20:25:55 -04:00
practicalswift
7834c3b9ec tests: Add fuzzing harness for functions/classes in chain.h 2020-03-26 21:21:34 +00:00