Commit graph

14097 commits

Author SHA1 Message Date
MarcoFalke
6b2210f101
Merge #16713: Ignore old versionbit activations to avoid 'unknown softforks' warning
fdb3e8f8b2 Ignore old versionbit activations (Anthony Towns)

Pull request description:

  PR 16060 removed the CSV and Segwit BIP9 softfork definitions and hard-coded ('buried') the activation heights. The versionbits code will warn users if an undefined softfork has been signalled in block header versions, and removing the CSV/Segwit definitions caused those warnings to be triggered.

  Change the BIP 9 warning code to only check for unknown softforks after the segwit activation height.

ACKs for top commit:
  MarcoFalke:
    ACK fdb3e8f8b2
  ajtowns:
    ACK fdb3e8f8b2 for what it's worth
  achow101:
    ACK fdb3e8f8b2
  Sjors:
    ACK fdb3e8f8b2. It makes the bit 0 warning go away in mainnet and testnet QT when a new block arrives. I think the code is clear enough.
  jonatack:
    ACK fdb3e8f8b2

Tree-SHA512: e6fd34e8902f8c7affb28e8951803e47d542710d5f1229000746656a37ee59d754439fc33e36b7eef87544262e5aac374645db91b74cb507e73514003ca7a67f
2019-09-27 15:25:53 -04:00
Wladimir J. van der Laan
a6c8aed1f1
Merge #16817: rpc: Fix casing in getblockchaininfo to be inline with other fields
1a02edb3f2 [RPC] Fix casing in getblockchaininfo to be inline with the rest of the response (Dan Gershony)

Pull request description:

  The response in the RPC result `startTime` is camel cased while the rest of the response seems to be lower cased.

  If this was intentional please ignore and close this PR.

  Note: RPC field case changes might break existing callers

ACKs for top commit:
  laanwj:
    ACK 1a02edb3f2

Tree-SHA512: 6f0eaf2b4aaf73c9a9bf1fbd4af59af5f95fc012fa88f94e050e6ae273b3ad647f5729df53bfce91e1a925fe4fd7b14818908bb6131a81413a555137d1007d7c
2019-09-27 15:11:00 +02:00
Dan Gershony
1a02edb3f2 [RPC] Fix casing in getblockchaininfo to be inline with the rest of the response
The response in the RPC result `starttime` is camel cased while the rest of the response seems to be lower cased.

If this was intentional please ignore this PR.

Note: case might break existing callers

Reflect the change in the test data

Change to snake case
2019-09-26 15:20:55 +01:00
Wladimir J. van der Laan
ab765c2ec7
Merge #16577: util: CBufferedFile fixes and unit test
efd2474d17 util: CBufferedFile fixes (Larry Ruane)

Pull request description:

  The `CBufferedFile` object guarantees its user is able to "rewind" the data stream (that's being read from a file) up to a certain number of bytes, as specified by the user in the constructor. This guarantee is not honored due to a bug in the `SetPos` method.

  Such rewinding is done in `LoadExternalBlockFile()` (currently the only user of this object), which deserializes a series of `CBlock` objects. If that function encounters something unexpected in the data stream, which is coming from a `blocks/blk00???.dat` file, it "rewinds" to an earlier position in the stream to try to get in sync again. The `CBufferedFile` object does not actually rewind its file offset; it simply repositions its internal offset, `nReadPos`, to an earlier position within the object's private buffer; this is why there's a limit to how far the user may rewind.

  If `LoadExternalBlockFile()` needs to rewind (call `blkdat.SetPos()`), the stream may not be positioned as it should be, causing errors in deserialization. This need to rewind is probably rare, which is likely why this bug hasn't been noticed already. But if this object is used elsewhere in the future, this could be a serious problem, especially as, due to the nature of the bug, the `SetPos()` _sometimes_ works.

  This PR adds a unit test for `CBufferedFile` that fails due to this bug. (Until now it has had no unit tests.) The unit test provides good documentation and examples for developers trying to understand `LoadExternalBlockFile()` and for future users of this object.

  This PR also adds code to throw an exception from the constructor if the rewind argument is not less than the buffer size (since that doesn't make any sense).

  Finally, I discovered that the object is too restrictive in one respect: When the deserialization methods call this object's `read` method, a check ensures that the number of bytes being requested is less than the size of the buffer (adjusting for the rewind size), else it throws an exception. This restriction is unnecessary; the object being deserialized can be larger than the buffer because multiple reads from disk can satisfy the request.

ACKs for top commit:
  laanwj:
    ACK ~after squash.~ efd2474d17
  mzumsande:
    I had intended to follow up earlier on my last comment, ACK efd2474d17. I reviewed the code, ran tests and did a successful reindex on testnet with this branch.

Tree-SHA512: 695529e0af38bae2af4e0cc2895dda56a71b9059c3de04d32e09c0165a50f6aacee499f2042156ab5eaa6f0349bab6bcca4ef9f6f9ded4e60d4483beab7e4554
2019-09-26 13:38:39 +02:00
fanquake
fdfaeb67de
Merge #16956: validation: Make GetWitnessCommitmentIndex public
fa607c2292 validation: Make GetWitnessCommitmentIndex public (MarcoFalke)

Pull request description:

  `GenerateCoinbaseCommitment` is public and can be used in unit tests to update the witness commitment after the list of txs in a block has been changed. However, for it to work, the existing commitment (added by default in `CreateNewBlock`) must be removed (and thus its index must be known).

  Make that possible by exposing the `GetWitnessCommitmentIndex` helper function in the header.

ACKs for top commit:
  jb55:
    ACK fa607c2292
  jamesob:
    ACK fa607c2292
  promag:
    ACK fa607c2292.
  fanquake:
    ACK fa607c2292 - This unblocks work in #15845.

Tree-SHA512: d563aa2c201d5fb4874e506a28f468c37e457cc8a20229c377178af08c22d3be44e19ee6e8e524b6de99236cd5f2c9e39b8009d88c26854aa774737912bd5889
2019-09-26 17:07:20 +08:00
Wladimir J. van der Laan
ae3902ee3f
Merge #16928: gui: Rename address checkbox back to bech32
fa7847d99b gui: Rename address checkbox back to bech32 (MarcoFalke)

Pull request description:

  This is the wording that has been used in the previous release, so translations should still exist for it.

  Fixes: #16924

ACKs for top commit:
  promag:
    ACK fa7847d99b.
  laanwj:
    ACK fa7847d99b

Tree-SHA512: 0ac6c47fe5eb2145b609a30fd3f56052d3e08abe6c67fc74b6d209a55a4df509c52f13eb1c759520a4fa43916ece0e6d4cefef87e061b51114a6582db911944a
2019-09-25 16:21:39 +02:00
Wladimir J. van der Laan
742cd77f6f
Merge #16929: test: follow-up to rpc: default maxfeerate value as BTC/kB
6659810e2f test: use named args for sendrawtransaction calls (Jon Atack)
5c1cd78b7e doc: improve rawtransaction code/test docs (Jon Atack)
acc14c5093 test: fix incorrect value in rpc_rawtransaction.py (Jon Atack)

Pull request description:

  Follow-up to PR #16521.

  - Fix incorrect value in rpc_rawtransaction test as per https://github.com/bitcoin/bitcoin/pull/16521/files#r325842308
  - Improve the code docs
  - Use named arguments as per https://github.com/bitcoin/bitcoin/pull/16521/files#r310715127

  Happy to squash or keep only the first commit if the others are too fixup-y.

ACKs for top commit:
  laanwj:
    ACK 6659810e2f

Tree-SHA512: bf5258f23802ab3ba3defb8791097e08e63f3e2af21023f832cd270dc88d1fa04349e921d69f9f5fedac5dce5cd3c1cc46b48febbede4bc18dccb8be994565b2
2019-09-25 11:51:31 +02:00
MarcoFalke
fa607c2292
validation: Make GetWitnessCommitmentIndex public 2019-09-24 11:16:05 -04:00
Jon Layton
c15ac2c0aa doc: Move doxygen intro to file for USE_MDFILE_AS_MANPAGE
doc: Change header to notitle
2019-09-23 19:22:06 -04:00
fanquake
3ce8298888
Merge #15558: Don't query all DNS seeds at once
6170ec5d3a Do not query all DNS seed at once (Pieter Wuille)

Pull request description:

  Before this PR, when we don't have enough connections after 11 seconds, we proceed to query all DNS seeds in a fixed order, loading responses from all of them.

  Change this to to only query three randomly-selected DNS seed. If 11 seconds later we still don't have enough connections, try again with another one, and so on.

  This reduces the amount of information DNS seeds can observe about the requesters by spreading the load over all of them.

ACKs for top commit:
  Sjors:
    ACK 6170ec5d3
  sdaftuar:
    ACK 6170ec5d3a
  jonasschnelli:
    utACK 6170ec5d3a - I think the risk of a single seeder codebase is orthogonal to this PR. Such risks could also be interpreted differently (diversity could also increase the risk based on the threat model).
  fanquake:
    ACK 6170ec5d3a - Agree with the reasoning behind the change. Did some testing with and without `-forcednsseed` and/or a `peers.dat` and monitored the DNS activity.

Tree-SHA512: 33f6be5f924a85d312303ce272aa8f8d5e04cb616b4b492be98832e3ff37558d13d2b16ede68644ad399aff2bf5ff0ad33844e55eb40b7f8e3fddf9ae43add57
2019-09-23 12:53:50 +08:00
Jon Atack
5c1cd78b7e
doc: improve rawtransaction code/test docs 2019-09-21 16:01:20 +02:00
MarcoFalke
fa7847d99b
gui: Rename address checkbox back to bech32
This is the wording that has been used in the previous release
2019-09-21 08:25:58 -04:00
fanquake
630ec7bf41
Merge #16900: doc: Fix doxygen comment for SignTransaction in rpc/rawtransaction_util
fa8d65f071 doc: Fix doxygen comment for SignTransaction in rpc/rawtransaction_util (MarcoFalke)

Pull request description:

  The param `coins` to `SignTransaction` is final and can thus not be extended (as suggested by the doc).

ACKs for top commit:
  practicalswift:
    ACK fa8d65f071 -- const correctness is good and diff looks correct
  fanquake:
    ACK fa8d65f071

Tree-SHA512: 041e159f2c3cf96e296173c31f3e5f35bbc7711cc888aa4bf08aaa8c65c95ee7f7672f65396690a9af45795a618eea0fadde7fb02d29ec85f1b4df5e6d9e0c7a
2019-09-20 16:25:59 +08:00
MarcoFalke
fa8d65f071
doc: Fix doxygen comment for SignTransaction in rpc/rawtransaction_util 2019-09-19 13:34:50 -04:00
MarcoFalke
7d4bc60f1f
Merge #16743: refactor: move LoadChainTip/RelayBlocks under CChainState
3cf36736e5 refactoring: move ReplayBlocks under CChainState (James O'Beirne)
bcf73d3b84 refactoring: move LoadChainTip to CChainState method (James O'Beirne)
f5809d5b13 doc: fix CChainState::ActivateBestChain doc (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

  ---

  Move more chainstate-related functionality to methods on CChainState. Nothing too interesting here, but needed to work with multiple chainstates. And brief to review. :)

  Also fixes doc on ActivateBestChain.

ACKs for top commit:
  MarcoFalke:
    ACK 3cf36736e5
  ryanofsky:
    Can confirm. utACK 3cf36736e5. Removes wrapper functions and removes more  ::ChainActive() and ::ChainstateActive() calls than it adds, so seems good.

Tree-SHA512: 4bf8a1dd454ca9d61c85f6736910fa7354c57acc0002e3a8e5ce494035d8280e4c20e066f03478eeff7d44195e7912c282a486526da9be53854b478b961affaa
2019-09-19 10:45:10 -04:00
fanquake
9bf5768dd6
Merge #16885: doc: Update tx-size-small comment with relevant CVE disclosure
c4b0c08f7c Update tx-size-small comment with relevant CVE disclosure (Gregory Sanders)

Pull request description:

  Code first introduced under https://github.com/bitcoin/bitcoin/pull/11423 with essentially no description and no discussion.

ACKs for top commit:
  MarcoFalke:
    ACK c4b0c08f7c
  fanquake:
    ACK c4b0c08f7c

Tree-SHA512: 95d5c92998b8b1e944c477dbaee265b62612b6e815099ab31d9ff580b4dff777abaf7f326a284644709f918aa1510412d62310689b1250ef6e64de7b19ca9f71
2019-09-19 08:51:30 +08:00
Gregory Sanders
c4b0c08f7c Update tx-size-small comment with relevant CVE disclosure 2019-09-18 16:21:44 -04:00
Wladimir J. van der Laan
0ee0474234
Merge #16521: rpc: Use the default maxfeerate value as BTC/kB
2dfd6834ef test: Add test for default maxfeerate in sendrawtransaction (Joonmo Yang)
261843e4be wallet/rpc: Use the default maxfeerate value as BTC/kB (Joonmo Yang)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/issues/16382

  This patch tries to treat `maxfeerate` in sendrawtransaction/testmempoolaccept RPC as a rate(BTC/kB) instead of an absolute value(BTC).
  The included test case checks if the new behavior works correctly, by using the transaction with an absolute fee of ~0.02BTC, where the fee rate is ~0.2BTC/kB.
  This test should be failing if the default `maxfeerate` is 0.1BTC, but pass if the default value is 0.1BTC/kB

ACKs for top commit:
  laanwj:
    ACK 2dfd6834ef (ACKs by Sjors and MarcoFalke above for trivially different code)

Tree-SHA512: a1795bffe8a182acef8844797955db1f60bb0c0ded97148f3572dc265234d5219271a3a7aa0b6418a43f73b2b2720ef7412ba169c99bb1cdcac52051f537d6af
2019-09-18 16:49:18 +02:00
Wladimir J. van der Laan
dd8cf82e96
Merge #15146: Solve SmartOS FD_ZERO build issue
b4fd0ca9be Include cstring for sanity_test_fdelt if required (Ben Woosley)
7fb886b1b1 [moveonly] Split glibc sanity_test_fdelt out (Ben Woosley)

Pull request description:

  SmartOS FD_ZERO is implemented in a way that requires
  an external declaration of memcpy. We can not simply
  include cstring in the existing file because
  sanity_test_memcpy is attempting to replace memcpy.

  Instead split glibc_sanity into fdelt and memcpy files,
  and include <cstring> in glibc_sanity/fdelt.cpp.

  Fixes #13581, see also #13619

ACKs for top commit:
  laanwj:
    Code review an lightly tested (but not on SmartOS) ACK b4fd0ca9be

Tree-SHA512: 231306da291ad9eca8ba91bea1e9c27b6c2e96e484d1602e1c2cf27761202f9287ce0bc19fefd000943d2b449d0e5929cd39e2f7e09cf930d89fa520228ccbec
2019-09-18 16:33:23 +02:00
Wladimir J. van der Laan
72d30d668a
Merge #16512: rpc: Shuffle inputs and outputs after joining psbts
c0b5d97103 Test that joinpsbts randomly shuffles the inputs (Andrew Chow)
6f405a1d3b Shuffle inputs and outputs after joining psbts (Andrew Chow)

Pull request description:

  `joinpsbts` currently just adds the inputs and outputs in the order of that the PSBTs were provided. This makes it extremely easy to identify which outputs belong to which inputs. This PR changes that so that all of the inputs and outputs are shuffled in the joined transaction.

ACKs for top commit:
  instagibbs:
    utACK c0b5d97103
  jonatack:
    ACK c0b5d97103 modulo suggestions for later.

Tree-SHA512: 14a0b7aae07d92e6d2c76a3a3b228b481e1964cb7d34f97515bdda18e2ea05a9f97c5a22affc143b86ae8b95c3cb239849fb54219d65512bc2112264dca915c8
2019-09-18 16:19:47 +02:00
Wladimir J. van der Laan
408c920381
Merge #16400: refactor: Rewrite AcceptToMemoryPoolWorker() using smaller parts
4a87c5cfdf [refactor] Rewrite AcceptToMemoryPoolWorker() using smaller parts (Suhas Daftuar)

Pull request description:

  This is in preparation for re-using these validation components for a new version of AcceptToMemoryPool() that can operate on multiple transactions ("package relay").

ACKs for top commit:
  MarcoFalke:
    re-ACK 4a87c5cfdf (did the rebase myself and arrived at the same result, mod whitespace)
  laanwj:
    ACK 4a87c5cfdf

Tree-SHA512: b0495c026ffe06146258bace3d5e0c9aaf23fa65f89f258abc4af5980812e68e63a799f1d923e78ac1ee6bcafaf1222b2c2690a527df9b65dff7b48a013f154e
2019-09-18 16:15:47 +02:00
Wladimir J. van der Laan
feb162d500
Merge #14696: qa: Add explicit references to related CVE's in p2p_invalid_block test.
0c62e3aa73 New regression testing for CVE-2018-17144, CVE-2012-2459, and CVE-2010-5137. (lucash-dev)
38bfca6bb2 Added comments referencing multiple CVEs in tests and production code. (lucash-dev)

Pull request description:

  This functional test includes two scenarios that test for regressions of vulnerabilities, but they are only briefly described. There are freely available documents explaining in detail the issues, but without explicit mentions, the developer trying to maintain the code needs an additional step of digging in commit history and PR conversations to figure it out.
  Added comments to explicitly mention  CVE-2018-17144 and CVE-2012-2459, for more complete documentation.
  This improves developer experience by making understanding the tests easier.

ACKs for top commit:
  laanwj:
    ACK 0c62e3aa73, checked the CVE numbers, thanks for adding documentation

Tree-SHA512: 3ee05351745193b8b959e4a25d50f25a693b2d24b0732ed53cf7d5882df40b5dd0f1877bd5c69cffb921d4a7acf9deb3cc1160b96dc730d9b5984151ad06b7c9
2019-09-18 16:00:54 +02:00
MarcoFalke
b3067f4338
Merge #16865: test: add some unit tests for merkle.cpp
8573429d46 test: add some unit tests for merkle.cpp (soroosh-sdi)

Pull request description:

  following situations are covered:
  - empty block
  - one Tx
  - Merkle root of a block with odd Txs should not change with repeating
  last one
  - Merkle root is computed with combining Merkle root of left subtree and right subtree
  - block witness is Merkle root of a block when setting first Tx
  to zero.

  Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>

ACKs for top commit:
  laanwj:
    ACK 8573429d46

Tree-SHA512: e12228171de8f8480f173c9d9d0359f00f46bf09075e0767f5f1a367478a1b7b6d177d230f7e930914915cd2c6b66b18d24b1682f1233c38e97954ba331e5773
2019-09-17 14:28:10 -04:00
James O'Beirne
3cf36736e5 refactoring: move ReplayBlocks under CChainState 2019-09-17 09:45:38 -04:00
James O'Beirne
bcf73d3b84 refactoring: move LoadChainTip to CChainState method 2019-09-17 09:45:38 -04:00
James O'Beirne
f5809d5b13 doc: fix CChainState::ActivateBestChain doc 2019-09-17 09:45:38 -04:00
soroosh-sdi
8573429d46 test: add some unit tests for merkle.cpp
following situations are covered:
- empty block
- one Tx
- Merkle root of a block with odd Txs should not change with repeating
last one
- Merkle root is computed with combining Merkle root of left subtree and right subtree
- block witness is Merkle root of a block when setting first Tx
to zero.

Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>
2019-09-16 23:36:43 +04:30
Suhas Daftuar
4a87c5cfdf [refactor] Rewrite AcceptToMemoryPoolWorker() using smaller parts
This is in preparation for re-using these validation components for a new
version of AcceptToMemoryPool() that can operate on multiple transactions
("package relay").
2019-09-16 11:07:40 -04:00
MarcoFalke
fa27eded3a
qt: Run "make translate" in ./src 2019-09-16 09:45:11 -04:00
Wladimir J. van der Laan
eb812257a3
Merge #16847: doc: add comments clarifying how local services are advertised
82e53f37e1 doc: add comments clarifying how local services are advertised (James O'Beirne)

Pull request description:

  Recent questions have come up regarding dynamic service registration
  (see https://github.com/bitcoin/bitcoin/pull/16442#discussion_r308702676
  and the assumeutxo project ~~which needs to dynamically flip NODE_NETWORK~~).

  While investigating how dynamic service registration might work, I was
  confused about how we convey local services to peers. This adds some
  documentation that hopefully clarifies this process.

ACKs for top commit:
  laanwj:
    ACK 82e53f37e1
  darosior:
    ACK 82e53f37e1

Tree-SHA512: a30c1020387d7a75b5b3cdde45f8b7f2ae46293da97e6227b2ee17e290b93deb5b16c0bbc2b1676972300e5c3c2ad74eb8b3910d6b93e028dac1ae2700468ef9
2019-09-16 13:15:05 +02:00
Wladimir J. van der Laan
871d3ae45b
Merge #16757: doc: CChainState return values
fa912a8ad5 doc: move-only ActivateBestChain doxygen comment to header (MarcoFalke)
fa99efd054 doc: ActivateBestChainStep return value (MarcoFalke)

Pull request description:

  It will always return true, unless a system error such as #15305 occurred

ACKs for top commit:
  laanwj:
    ACK fa912a8ad5

Tree-SHA512: d439da844a467f9705014b946d7d987fb62cb63fe6a325b2fdbbb73a6578fc0ade3f60892044f02face43948204fc4e3c9fa70d108233d4ca8eef27984059689
2019-09-16 12:58:22 +02:00
fanquake
a953429a0e
Merge #16822: gui: Create wallet menu option follow-ups
cad3ab5db8 gui: fix autofocus in CreateWalletActivity::askPassphrase() (Jon Atack)
539d9403af gui: fix passphrase labels/tooltip in createwalletdialog/askpassphrasedialog (Jon Atack)
43aa9b0d79 gui: rename encrypt(), blank(), and askPasshprase() (Jon Atack)

Pull request description:

  Closes #16820. The wallet [name escaping issue](https://github.com/bitcoin/bitcoin/pull/15450#pullrequestreview-282297760) in that issue predates #15450 and is fixed by #16826.

  - [x]  rename encrypt() to encryptWallet(), and blank() to makeBlankWallet() // EDIT: updated to
          isEncryptWalletChecked()
          isDisablePrivateKeysChecked()
          isMakeBlankWalletChecked()
  - [x]  fix naming of askPasshprase() to askPassphrase()
  - [x]  fix passphrase labels and tooltip in createwalletdialog.ui and askpassphrasedialog.ui
  - [x]  fix grammar of labels in askpassphrase dialog and WalletController::closeWallet
  - [x]  fix autofocus in CreateWalletActivity::askPassphrase()

  Squashed down to three commits.

  Reviewers, to test manually: build, launch the gui wallet, and look at labels/tooltips/focus with the create wallet, encrypt wallet, change password, and close wallet commands.

ACKs for top commit:
  jb55:
    Approach ACK cad3ab5db8
  instagibbs:
    code review and tACK cad3ab5db8
  fanquake:
    ACK cad3ab5db8

Tree-SHA512: b441fbf8f8cd370dd692bac24f0d3c1b32fc7d947b6c3a2c9ba7cf0bc175a72b3460440f2f10f7632c0e8e0f8e65fe15615a30c46e2c7763bf258c504b457dd6
2019-09-16 16:52:00 +08:00
fanquake
9debfd0e28
Merge #16863: doc: Add a missing closing parenthesis in the bitcoin-wallet's help
b6233a4985 bitcoin-wallet: Add a missing closing parenthesis in the help (darosior)

Pull request description:

ACKs for top commit:
  kristapsk:
    utACK b6233a4985
  fanquake:
    ACK b6233a4985

Tree-SHA512: acf18633fdca4bd73838fcaa0ebe4121dd0b5308daa77c4458ec4c98a9e8aa6d9d6580a48c884147438af14e670b0606c1e76f72d1d7efd221c4da419061beed
2019-09-16 08:21:46 +08:00
Samuel Dobson
b0a7a76c9d
Merge #16873: rpc: fix regression in gettransaction
1b41c2c8a1 test: improve gettransaction test coverage (Jon Atack)
0f34f54888 rpc: fix regression in gettransaction (Jon Atack)

Pull request description:

  Closes #16872.

  PR #16866 renamed the `decode` argument in gettransaction to `verbose` to make it more consistent with other RPC calls like getrawtransaction. However, it inadvertently overloaded the "details" field when `verbose` is passed. The result is that the original "details" field is no longer returned correctly, which seems to be a breaking API change.

  This PR:

  - takes the simplest path to restoring the "details" field by renaming the decoded one back to "decoded" while leaving the `verbose` argument for API consistency, which was the main intent of #16866,

  - addresses [this comment](https://github.com/bitcoin/bitcoin/pull/16185#discussion_r320740413) by mentioning in the RPC help that the new decoded field is equivalent to decoderawtransaction, and

  - updates the help, functional test, and release note.

  Reviewers, to test this manually, build and run `bitcoin-cli help gettransaction` and `bitcoin-cli gettransaction <wallet txid> false true`, and verify that the command returns both `details` and `decoded` fields.

ACKs for top commit:
  jnewbery:
    tACK 1b41c2c8a1

Tree-SHA512: 287edd5db7ed58fe8b548975aba58628bd45ed708b28f40174f10a35a455d89f796fbf27430aa881fc376f47aabda8803f74d4d100683bd86577a02279091cf3
2019-09-16 09:44:34 +12:00
Jonas Schnelli
a40ccbb195
Merge #16858: Qt: advise users not to switch wallets when opening a BIP70 URI.
1153caf78e Qt: advise users not to switch wallets when opening a BIP70 URI. (James Hilliard)

Pull request description:

  It would probably be a good idea to have something like this before #15584 is merged.

ACKs for top commit:
  jonasschnelli:
    utACK 1153caf78e
  fanquake:
    ACK 1153caf78e

Tree-SHA512: 6e682dd280c44eaafb1206c32439df42a20173c33297bf93dd607f0a7a2faec8e2d17fff83c85027083ebd11a71795b443e707992251574370dd1d46b7bff060
2019-09-15 20:08:03 +02:00
Jon Atack
0f34f54888
rpc: fix regression in gettransaction
PR 16866 renamed the 'decode' argument in gettransaction to 'verbose' to make it more consistent with other RPC calls like getrawtransaction.

However, it seems it inadvertently overloaded the 'details' fields when 'verbose' is passed. The result is that the original 'details' fields are no longer returned, which seems to be a breaking API change.

This PR takes the simplest path to restoring the 'details' fields by renaming them from 'details' back to 'decoded', while leaving the 'verbose' argument for API consistency.

It also addresses [this comment](https://github.com/bitcoin/bitcoin/pull/16185#discussion_r320740413) to mention that the 'decoded' field is identical to decoderawtransaction.

Update the RPC help, functional test, and release note.
2019-09-14 20:17:19 +02:00
John Newbery
7dee8f4808 [wallet] Rename 'decode' argument in gettransaction method to 'verbose'
This makes the RPC method consistent with other RPC methods that have a
'verbose' option.

Change the name of the return object from 'decoded' to details.

Update help text.
2019-09-13 22:33:46 +03:00
darosior
b6233a4985
bitcoin-wallet: Add a missing closing parenthesis in the help 2019-09-13 16:59:06 +02:00
Wladimir J. van der Laan
fb4f5beb6e
Merge #16854: Prevent UpdateTip log message from being broken up
f390d8556b Prevent UpdateTip log message from being broken up (Steven Roose)

Pull request description:

  We had some occurrences of log messages colliding:
  ```
  $ grep ThreadRPC $(find . -type f -name debug.log -print | grep bitcoind.datadir ) | awk '{$1=""; print $0}' | sort -u
   ThreadRPCServer method=sendrawtransaction user=bitcoinrpc
   ThreadRPCServer method=sendtoaddress user=bitcoinrpc
   ThreadRPCServer method=signrawtransactionwithkey user=bitcoinrpc
   UpdateTip: new best=0026711b58616e8d4db7d9bc9f46b746ec535521feb3e0f077e83925e45daa8a height=630 version=0x20000000 log2_work=10.301496 tx=633 date='2019-09-10T00:43:41Z' progress=1.000000 cache=0.1MiB(811txo)ThreadRPCServer method=getblockheader user=bitcoinrpc
   UpdateTip: new best=466304b9c3ed652373d30207f0d99551c568c2dbc648518c077cb40826039e5f height=643 version=0x20000000 log2_work=10.330917 tx=647 date='2019-09-10T00:43:43Z' progress=1.000000 cache=0.1MiB(915txo)ThreadRPCServer method=getblockheader user=bitcoinrpc
  ```

ACKs for top commit:
  jonasschnelli:
    utACK f390d8556b
  laanwj:
    ACK f390d8556b

Tree-SHA512: 15f9836d6cac171fc2b7a36a65ea8db77727559e5a558114e8c767bf059b20067afa7d6a88cfc365e9f5fa21ab5bc462d92fd01997a299fa8ed1f5c1046d1ba6
2019-09-13 15:32:20 +02:00
Jonas Schnelli
884f7cc81b
Merge #16714: gui: add prune to intro screen with smart default
9924bce317 [gui] intro: enable pruning by default unless disk is big (Sjors Provoost)
c8de347a9d [gui] intro: add prune preference (Sjors Provoost)
1bbc49d207 [gui] intro: inform caller if intro was shown (Sjors Provoost)
1957103786 [gui] add explicit prune setter (Sjors Provoost)
1bccf6a52d [node] add forceSetArg to interface (Sjors Provoost)

Pull request description:

  This adds a checkbox to the intro screen to enable pruning from the get go.

  If the user has plenty of space, it's unchecked by default:

  <img width="671" alt="big" src="https://user-images.githubusercontent.com/10217/63641289-10339000-c6ac-11e9-98d7-caf64dff0da6.png">

  If the user has insufficient space it's checked by default:
  <img width="897" alt="low" src="https://user-images.githubusercontent.com/10217/63641276-d4002f80-c6ab-11e9-9f5b-a53472f814ff.png">

  When the user has barely enough space and is likely to need pruning in the near future, this is shown in yellow and we also check the prune box:

  <img width="662" alt="medium" src="https://user-images.githubusercontent.com/10217/63641294-1c1f5200-c6ac-11e9-8ecb-6b69e42b1ece.png">

  The cut-off for this 10 GB above `m_assumed_blockchain_size` (`=240` in `chainparams.cpp`).

  If the user launches the first time with `-prune=...` then we disable the check box and display the correct size (rounded to GB):
  <img width="658" alt="Schermafbeelding 2019-08-24 om 20 23 14" src="https://user-images.githubusercontent.com/10217/63641351-09594d00-c6ad-11e9-94fe-fe5ed562e109.png">

  The 2 GB default matches the settings default. The user can't change it in the intro screen, but can change it later. I'm tempted to increase that default to 10 GB, and then have the intro screen reduce it if space is really tight.

  Tips for testing:
  * move your existing data dir elsewhere
  * wipe data dir at every restart (behavior is different if it exists)
  * launch with `bitcoin-qt -resetguisettings -lang=en` (there's some space issues in different languages)
  * fake your free space by changing `intro.cpp` line 90: `freeBytesAvailable = 5000000000; // 5 GB`
  * try both testnet and mainnet, because settings are seperate. In particular note how step 7 in `GuiMain` switches where `QTSettings settings` points to; this had me thoroughly confused on testnet, because I was setting them too early.

ACKs for top commit:
  jonasschnelli:
    Tested ACK 9924bce317
  ryanofsky:
    utACK 9924bce317. The changes are very logical, and implement the feature in a clean that way that doesn't add a lot of complication and shouldn't interfere with future improvements. I looked at Luke's branch too, and I think there's also a lot of great stuff there that seems fully compatible with this change.

Tree-SHA512: 9523961451c53aebd347716976bc3a4a398f989dc21e9bbbd357060bd11a8f46c435f068bd421bb31ccb08e55445ef67bc347d8d19a4fb8fde9d6d3f9a3bcbb0
2019-09-12 15:00:51 +02:00
Steven Roose
f390d8556b
Prevent UpdateTip log message from being broken up 2019-09-12 12:33:09 +01:00
James Hilliard
1153caf78e Qt: advise users not to switch wallets when opening a BIP70 URI. 2019-09-12 14:11:34 +03:00
James O'Beirne
82e53f37e1 doc: add comments clarifying how local services are advertised
Recent questions have come up regarding dynamic service registration
(see https://github.com/bitcoin/bitcoin/pull/16442#discussion_r308702676
and the assumeutxo project, which needs to dynamically flip NODE_NETWORK).

While investigating how dynamic service registration might work, I was
confused about how we convey local services to peers. This adds some
documentation that hopefully clarifies this process.
2019-09-11 10:24:44 -04:00
fanquake
2296fe65f5
Merge #16251: Improve signrawtransaction error reporting
ec4c79326b signrawtransaction*: improve error for partial signing (Anthony Towns)
3c481f8921 signrawtransactionwithkey: better error messages for bad redeemScript/witnessScript (Anthony Towns)

Pull request description:

  Two fixes for `signrawtransactionwith{key,wallet}` (in addition to #16250): one that checks redeemScript/witnessScript matches scriptPubKey (and if both are provided that they match each other sanely), and the other changes the warning when some-but-not-all the signatures for a CHECKMULTISIG are provided to something that suggests more signatures may be all that's required.

  Fixes: #13218
  Fixes: #14823

ACKs for top commit:
  instagibbs:
    utACK ec4c79326b
  achow101:
    Code Review ACK ec4c79326b
  meshcollider:
    utACK ec4c79326b

Tree-SHA512: 0c95c91d498e85b834662b9e5c83f336ed5fd306be7701ce1dbfa0836fbeb448a267a796585512f7496e820be668b07c2a0a2f45e52dc23f09ee7d9c87e42b35
2019-09-11 15:37:13 +10:00
Larry Ruane
efd2474d17 util: CBufferedFile fixes 2019-09-10 07:53:09 -06:00
MarcoFalke
fa912a8ad5
doc: move-only ActivateBestChain doxygen comment to header 2019-09-10 15:48:23 +03:00
MarcoFalke
750c2fbf26
Merge #16680: Preparations for more testchains
3bf9d8cac0 Testchains: Qt: Simplify network/chain styles (Jorge Timón)
052c54ecb0 Testchains: Generic selection with -chain=<str> in addition of -testnet and -regtest (Jorge Timón)

Pull request description:

  Separated from #8994 as suggested by MarcoFalke and Sjors in https://github.com/bitcoin/bitcoin/pull/8994#issuecomment-522555390

  You can't really test the qt changes on their own, so to test them, use #8994 .

ACKs for top commit:
  MarcoFalke:
    ACK 3bf9d8cac0

Tree-SHA512: 5b5e6083ebc0a44505a507fac633e7af18037c85e5e73f5d1e6f7e730575d3297ba8a31d1c2441df623b273f061c32d8fa324f4aa6bead01d23e88582029b568
2019-09-10 12:44:01 +03:00
Wladimir J. van der Laan
33c466a642
Merge #16787: rpc: Human readable network services
66740f460a doc: add a release note for the new field in 'getpeerinfo' and 'getnetworkinfo' (darosior)
6564f58c87 rpc/net: decode the services flags in a new entry (darosior)

Pull request description:

  This is a reopen of https://github.com/bitcoin/bitcoin/pull/15511#issuecomment-527087370 since there have been concept ACKs from sdaftuar and Sjors.

  This adds a new entry to `getpeerinfo` and `getnetworkinfo` which decodes the network services flags.

  Here is a truncated output of `getpeerinfo`:
  ```
  "services": "000000000000040d",
  "servicesnames": "NODE_NETWORK | NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED",
  "relaytxes": true,
  ```
  And one of `getnetworkinfo`:
  ```
  "localservices": "0000000000000409",
  "localservicesnames": "NODE_NETWORK | NODE_WITNESS | NODE_NETWORK_LIMITED",
  "localrelay": true,
  ```

  Fixes #16780.

ACKs for top commit:
  MarcoFalke:
    unsigned ACK 66740f460a
  laanwj:
    ACK 66740f460a

Tree-SHA512: 0acc37134b283f56004a41243903d7790cb01591ddf0342489bd05f3a2c780563075373ba5fd55180fa15632e8968ffa11a979b8afece75a6a2e891342601440
2019-09-10 09:03:14 +02:00
Anthony Towns
ec4c79326b signrawtransaction*: improve error for partial signing
Thanks to Danial Jaffy (tipu) for reporting this issue.
2019-09-10 15:41:50 +10:00
Anthony Towns
3c481f8921 signrawtransactionwithkey: better error messages for bad redeemScript/witnessScript
This adds checks to ensure the redeemScript/witnessScript actually
correspond to the provided scriptPubKey, and, if both are provided,
that they are sensibly related to each other.

Thanks to github user passionofvc for raising this issue.
2019-09-10 15:41:50 +10:00