Commit graph

1725 commits

Author SHA1 Message Date
Hennadii Stepanov
6fdfeebcc7
refactor: Replace RecursiveMutex with Mutex in rpc/server.cpp 2020-06-28 10:00:24 +03:00
MarcoFalke
fa8ec00061
rpc: Check that left section is not multiline 2020-06-26 13:46:29 -04:00
Andrew Chow
4638224f64 Add psbtbumpfee RPC 2020-06-25 15:32:11 -04:00
Andrew Chow
72f6bec1da rpc: show both UTXOs in decodepsbt 2020-06-24 16:31:42 -04:00
Wladimir J. van der Laan
bd93e32292 refactor: Replace HexStr(o.begin(), o.end()) with HexStr(o)
HexStr can be called with anything that bas `begin()` and `end()` functions,
so clean up the redundant calls.
2020-06-24 18:41:45 +02:00
Fabian Jahr
f17a4d1c4d
rpc: Add hash_type NONE to gettxoutsetinfo 2020-06-22 01:55:36 +02:00
Fabian Jahr
a712cf6f68
rpc: gettxoutsetinfo can specify hash_type (only legacy option for now) 2020-06-22 00:55:44 +02:00
MarcoFalke
fa32adf9dc
scripted-diff: TxoutType C++11 scoped enum class
-BEGIN VERIFY SCRIPT-
 # General rename helper: $1 -> $2
 rename_global() { sed -i "s/\<$1\>/$2/g" $(git grep -l "$1"); }

 # Helper to rename TxoutType $1
 rename_value() {
   sed -i "s/    TX_$1,/    $1,/g" src/script/standard.h;  # First strip the prefix in the definition (header)
   rename_global TX_$1 "TxoutType::$1";                    # Then replace globally
 }

 # Change the type globally to bring it in line with the style-guide
 # (clsses are UpperCamelCase)
 rename_global 'enum txnouttype' 'enum class TxoutType'
 rename_global      'txnouttype'            'TxoutType'

 # Now rename each enum value
 rename_value 'NONSTANDARD'
 rename_value 'PUBKEY'
 rename_value 'PUBKEYHASH'
 rename_value 'SCRIPTHASH'
 rename_value 'MULTISIG'
 rename_value 'NULL_DATA'
 rename_value 'WITNESS_V0_KEYHASH'
 rename_value 'WITNESS_V0_SCRIPTHASH'
 rename_value 'WITNESS_UNKNOWN'

-END VERIFY SCRIPT-
2020-06-21 06:41:55 -04:00
MarcoFalke
fa58469c77
rpc: Properly use underlying type in GetAllOutputTypes
Don't blindly assume it is int.

In practice this is usually `unsigned` or `int`, so this commit should
not change behavior.
2020-06-21 06:40:26 -04:00
MarcoFalke
fa41c65702
rpc: Simplify GetAllOutputTypes with the Join helper
This commit does not change behavior
2020-06-21 06:40:18 -04:00
Samuel Dobson
47a30ef0c6
Merge #19133: rpc, cli, test: add bitcoin-cli -generate command
22cb303cf0 rpc: add missing space in JSON parsing error message, update test (Jon Atack)
bf53ebef06 test: add multiwallet tests for bitcoin-cli -generate (Jon Atack)
4b859cfff9 cli: add multiwallet capability to GetNewAddress and -generate (Jon Atack)
18f93545a1 test: add tests for bitcoin-cli -generate (Jon Atack)
4818124137 cli: create bitcoin-cli -generate command (Jon Atack)
ff41a36900 cli: extract ParseResult() and ParseError() (Jon Atack)
f4185b26d9 cli: create GenerateToAddressRequestHandler class (Harris)
f7c65a3350 cli: create GetNewAddress() (Jon Atack)
9be7fd35c5 rpc: make generatetoaddress locals const (Jon Atack)
cb00510dba rpc: create rpc/mining.h, hoist default max tries values to constant (Jon Atack)

Pull request description:

  This PR continues and completes the work begun in #17700 working on issue #16000 to create a client-side version of RPC `generate`.

  Basically, `bitcoin-cli -generate` wraps calling `generatenewaddress` followed by `generatetoaddress [nblocks] [maxtries]` and prints the following:

  ```
  $ bitcoin-cli -generate
  {
    "address": "bcrt1qn4aszr2y2xvpa70y675a76wsu70wlkwvdyyln6"
    "blocks": [
      "01d2ebcddf663da90b28da7f6805115e2ba7818f16fe747258836646a43a0bb5",
    ]
  }

  $ bitcoin-cli -rpcwallet=wallet-name -generate 3 100
  {
    "address": "bcrt1q4cunfw0gnsj7g7e6mk0v0uuvvau9mwr09dj45l",
    "blocks": [
      "7a6650ca5e0c614992ee64fb148a7e5e022af842e4b6003f81abd8baf1e75136",
      "01d2ebcddf663da90b28da7f6805115e2ba7818f16fe747258836646a43a0bb5",
      "3f8795ec40b1ad812b818c177680841be319a3f6753d4e32dc7dfb5bafe5d00e"
    ]
  }
  ```

  Help doc:
  ```
  $ bitcoin-cli -h | grep -A5 "\-generate"
    -generate
         Generate blocks immediately, equivalent to RPC generatenewaddress
         followed by RPC generatetoaddress. Optional positional arguments
         are number of blocks to generate (default: 1) and maximum
         iterations to try (default: 1000000), equivalent to RPC
         generatetoaddress nblocks and maxtries arguments. Example:
         bitcoin-cli -generate 4 1000
  ```

  Quite a bit of test coverage turned out to be needed to cover the change and the different cases (arguments, multiwallet mode) and error-handling.

  This PR also improves some things that working on these changes brought to light.

  Credit to Harris Brakmić for the initial work in #17700.

ACKs for top commit:
  adamjonas:
    utACK 22cb303cf0
  meshcollider:
    utACK 22cb303cf0

Tree-SHA512: 94f67f632fe093d076f614e0ecff09ce7342ac6e424579200d5211a6615260e438d857861767fb788950ec6da0b26ef56dc8268c430012a3b3d4822b24ca6fbf
2020-06-21 22:24:07 +12:00
Ben Woosley
0a5ea32ce6
Prefer explicit uint160 conversion 2020-06-19 12:14:06 -07:00
MarcoFalke
fab80fef61
refactor: Remove unused EnsureChainman 2020-06-15 07:39:35 -04:00
Luke Dashjr
0f8f515445 RPC: Rephrase generatetoaddress help, and use PACKAGE_NAME 2020-06-15 08:13:49 +00:00
Hennadii Stepanov
d1ae7c0355
Make GetWarnings() return bilingual_str 2020-06-10 15:01:20 +03:00
Wladimir J. van der Laan
77b79fa6ef refactor: Error message bilingual_str consistency
- Move the decision whether to translate an error message to where it is
  defined. This simplifies call sites: no more `InitError(Untranslated(...))`.

- Make all functions in `util/error.h` consistently return a
  `bilingual_str`. We've decided to use this as error message type so
  let's roll with it.

This has no functional changes: no messages are changed, no new
translation messages are defined.
2020-06-09 15:39:44 +02:00
MarcoFalke
1b90a7b61a
Merge #19005: doc: Add documentation for 'checklevel' argument in 'verifychain' RPC…
501e6ab4e7 doc: Add documentation for 'checklevel' argument in 'verifychain' RPC call (Calvin Kim)

Pull request description:

  Rationale: When ```bitcoin-cli help verifychain``` is called, the user doesn't get any documentation about the ```checklevel``` argument, leading to issues like #18995.

  This PR addresses that issue and adds documentation for what each level does, and that each level includes the checks of the previous levels.

ACKs for top commit:
  jonatack:
    ACK 501e6ab4e7 `git diff 292ed3c 501e6ab` shows only change since last review is the verifychain RPCHelpMan edit; rebuild and retested manually anyway
  MarcoFalke:
    ACK 501e6ab4e7 🚝

Tree-SHA512: 09239f79c25b5c3022b8eb1f76198ba681305d7e8775038e46becffe5f6a14c572e0c5d06b0723fe9d4a015ec42c9f7ca7b80a2a93df0b1b66f5a84a80eeeeb1
2020-06-07 06:41:31 -04:00
Calvin Kim
501e6ab4e7 doc: Add documentation for 'checklevel' argument in 'verifychain' RPC call 2020-06-07 17:50:22 +09:00
MarcoFalke
0fc6ea216c
Merge #19096: Remove g_rpc_chain global
4a7253ab6c Remove g_rpc_chain global (Russell Yanofsky)
e783197bf0 refactor: replace RegisterWalletRPCCommands with GetWalletRPCCommands (Russell Yanofsky)

Pull request description:

  Replace with RPC request reference to new WalletContext struct similar to the existing NodeContext struct and reference.

  This PR is a followup to #18740 removing the g_rpc_node global.

  Some later PRs will follow this up and move more wallet globals to the WalletContext struct.

ACKs for top commit:
  MarcoFalke:
    ACK 4a7253ab6c 🎋
  ariard:
    Code Review ACK 4a7253a, feel free to ignore comment it's super nit.

Tree-SHA512: 5bb5561c89f81811ca5232a58bf450e230d4218e62471c03227d142395fd36131672e99cb88329b33b9680a235db01e8b9d1c1e2a18288349e57205528deabab
2020-06-05 08:29:18 -04:00
MarcoFalke
1f7fe59460
Merge #19111: Limit scope of all global std::once_flag
fa9c675591 Limit scope of all global std::once_flag (MarcoFalke)

Pull request description:

  `once_flag` is  a helper (as the name might suggest) to execute a callable only once. Thus, the scope of the flag does never need to extend beyond where the callable is called. Typically this is function scope.

  Move all the flags to function scope to
  * simplify code review
  * avoid mistakes where similarly named flags are accidentally exchanged
  * avoid polluting the global scope

ACKs for top commit:
  hebasto:
    ACK fa9c675591, tested on Linux Mint 19.3 (x86_64).
  promag:
    Code review ACK fa9c675591.

Tree-SHA512: 095a0c11d93d0ddcb82b3c71676090ecc7e3de3d5e7a2a63ab2583093be279242acac43523bbae2060b4dcfa8f92b54256a0e91fbbae78fa92d2d49e9db62e57
2020-06-02 07:04:19 -04:00
Jon Atack
22cb303cf0
rpc: add missing space in JSON parsing error message, update test 2020-06-02 08:50:57 +02:00
Jon Atack
9be7fd35c5
rpc: make generatetoaddress locals const
and named the same as in generatetodescriptor just above.
2020-06-02 08:23:34 +02:00
Jon Atack
cb00510dba
rpc: create rpc/mining.h, hoist default max tries values to constant 2020-06-01 15:08:36 +02:00
fanquake
a8327fd71f
Merge #19072: doc: Expand section on Getting Started
facef3d413 doc: Explain that anyone can work on good first issues, move text to CONTRIBUTING.md (MarcoFalke)
fae2fb2a19 doc: Expand section on Getting Started (MarcoFalke)
100000d1b2 doc: Add headings to CONTRIBUTING.md (MarcoFalke)
fab893e0ca doc: Fix unrelated typos reported by codespell (MarcoFalke)

Pull request description:

  Some random doc changes:

  * Add sections to docs, so that they can be linked to
  * Explain that anyone (even maintainers) are allowed to work on good first issues
  * Expand section on Getting Started slightly

ACKs for top commit:
  hebasto:
    ACK facef3d413
  fanquake:
    ACK facef3d413

Tree-SHA512: 8998e273a76dbf4ca77e79374c14efe4dfcc5c6df6b7d801e1e1e436711dbe6f76b436f9cbc6cacb45a56827babdd6396f3bd376a9426ee7be3bb9b8a3b8e383
2020-06-01 15:38:57 +08:00
MarcoFalke
826fe9c667
Merge #18807: [doc / test / mempool] unbroadcast follow-ups
9e1cb1adf1 [trivial/doc] Fix comment type (Amiti Uttarwar)
8f30260a67 [doc] Update unbroadcast description in RPC results (Amiti Uttarwar)
750456d6f2 [trivial] Remove misleading 'const' (Amiti Uttarwar)
fa32e676e5 [test] Manage node connections better in mempool persist test (Amiti Uttarwar)
1f94bb0c74 [doc] Provide rationale for randomization in scheduling. (Amiti Uttarwar)
9c8a55d9cb [mempool] Don't throw expected error message when upgrading (Amiti Uttarwar)
ba54983182 [test] Test that wallet transactions aren't rebroadcast before 12 hours (Amiti Uttarwar)
00d44a534b [test] P2P connection behavior should meet expectations (Amiti Uttarwar)
bd093ca15d [test] updates to unbroadcast test (Amiti Uttarwar)
dab298d9ab [docs] add release notes (Amiti Uttarwar)

Pull request description:

  This PR is a follow up to #18038 which introduced the idea of an unbroadcast set & focuses mostly on documentation updates and test fixes. One small functionality update to not throw an expected error in `LoadMempool` when you upgrade software versions.

  #18895 is another follow up to that addresses other functionality updates.

  Background context:
  The unbroadcast set is a mechanism for the mempool to track locally submitted transactions (via wallet or RPC). The node does a best-effort of delivering the transactions to the network via retries every 10-15 minutes until either a `GETDATA` is received or the transaction is removed from the mempool.

ACKs for top commit:
  MarcoFalke:
    ACK 9e1cb1adf1 👁
  gzhao408:
    ACK [`9e1cb1a`](9e1cb1adf1)

Tree-SHA512: 0cd51c4ca368b9dce92d50d73ec6e9df278a259e609eef2858f24cb8595ad07acc3db781d9eb0c351715f18fca5a2b4526838981fdb34a522427e9dc868bdaa6
2020-05-30 12:22:09 -04:00
MarcoFalke
fa9c675591
Limit scope of all global std::once_flag 2020-05-29 17:22:07 -04:00
Jonas Schnelli
8ad5f1c376
Merge #19106: util: simplify the interface of serviceFlagToStr()
189ae0c38b util: dedup code in callers of serviceFlagToStr() (Vasil Dimov)
fbacad1880 util: simplify the interface of serviceFlagToStr() (Vasil Dimov)

Pull request description:

  Don't take two redundant arguments in `serviceFlagToStr()`.

  Introduce `serviceFlagsToStr()` which takes a mask (with more than one
  bit set) and returns a vector of strings.

  As a side effect this fixes an issue introduced in
  https://github.com/bitcoin/bitcoin/pull/18165 due to which the GUI could
  print something like `UNKNOWN[1033] & UNKNOWN[1033] & UNKNOWN[2^10]`
  instead of `NETWORK & WITNESS`.

ACKs for top commit:
  MarcoFalke:
    ACK 189ae0c38b
  jonasschnelli:
    Tested ACK 189ae0c38b

Tree-SHA512: 000c490f16ebbba04458c62ca4ce743abffd344d375d95f5bbd5008742012032787655db2874b168df0270743266261dccf1693761906567502dcbac902bda50
2020-05-29 19:43:08 +02:00
Vasil Dimov
189ae0c38b
util: dedup code in callers of serviceFlagToStr()
Introduce `serviceFlagsToStr()` which hides the internals of the bitmask
and simplifies callers of `serviceFlagToStr()`.
2020-05-29 18:59:37 +02:00
Vasil Dimov
fbacad1880
util: simplify the interface of serviceFlagToStr()
Don't take two redundant arguments in `serviceFlagToStr()`.

As a side effect this fixes an issue introduced in
https://github.com/bitcoin/bitcoin/pull/18165 due to which the GUI could
print something like `UNKNOWN[1033] & UNKNOWN[1033] & UNKNOWN[2^10]`
instead of `NETWORK & WITNESS`.
2020-05-29 18:49:26 +02:00
Jonas Schnelli
e4bfd51acc
Merge #18452: qt: Fix shutdown when waitfor* cmds are called from RPC console
da73f1513a qt: Fix shutdown when waitfor* cmds are called from RPC console (Hennadii Stepanov)

Pull request description:

  On master (7eed413e72), if the GUI has been started with`-server=1`, `bitcoin-qt` hangs on shutdown during calling any of the `waitfor*` commands in the GUI RPC console.

  This PR suggests minimal changes to fix this bug.

  Fix #17495

ACKs for top commit:
  jonasschnelli:
    utACK da73f1513a

Tree-SHA512: 469f5332945a5f2c57d19336cda5df79b123ccc494aea6d58a85eb1293be52708b2b9c5bb6bc2c402a90b7b4e9e8d7ab8fe84cf201cf7ce612c9290c57e43681
2020-05-29 15:49:46 +02:00
Jonas Schnelli
de369c7ea5
Merge #18165: Consolidate service flag bit-to-name conversion to a shared serviceFlagToStr function
c31bc5bcfd Consolidate service flag bit-to-name conversion to a shared serviceFlagToStr function (Luke Dashjr)
cea91a1e40 Bugfix: GUI: Use unsigned long long type to avoid implicit conversion of MSB check (Luke Dashjr)

Pull request description:

  Side effect: this results in the RPC showing unknown service bits as "UNKNOWN[n]" like the GUI.

  Note that there is no common mask-to-`vector<string>` function because both GUI and RPC would need to iterate through it to convert to their desired target formats.

ACKs for top commit:
  jonasschnelli:
    utACK ~~cea91a1e40e12029140ebfba969ce3ef2965029c~~ c31bc5bcfd

Tree-SHA512: 32c7ba8ac7ef2d4087f4f317447ae93a328ec9fb9ad81301df2fbaeeb21a3db7a503187a369552b05a9414251b7cf8e15bcde74c1ea2ef36591ea7ffb6721f60
2020-05-29 11:06:08 +02:00
Russell Yanofsky
4a7253ab6c Remove g_rpc_chain global
Replace with RPC request reference to new WalletContext struct similar to the
existing NodeContext struct and reference.

This PR is a followup to 25ad2c623a
https://github.com/bitcoin/bitcoin/pull/18740 removing the g_rpc_node global.

Some later PRs will follow this up and move more wallet globals to the
WalletContext struct.

Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
2020-05-28 02:13:19 -04:00
MarcoFalke
55b4c65bd1
Merge #16127: More thread safety annotation coverage
5478d6c099 logging: thread safety annotations (Anthony Towns)
e685ca1992 util/system.cpp: add thread safety annotations for dir_locks (Anthony Towns)
a788789948 test/checkqueue_tests: thread safety annotations (Anthony Towns)
479c5846f7 rpc/blockchain.cpp: thread safety annotations for latestblock (Anthony Towns)
8b5af3d4c1 net: fMsgProcWake use LOCK instead of lock_guard (Anthony Towns)
de7c5f41ab wallet/wallet.h: Remove mutexScanning which was only protecting a single atomic bool (Anthony Towns)
c3cf2f5501 rpc/blockchain.cpp: Remove g_utxosetscan mutex that is only protecting a single atomic variable (Anthony Towns)

Pull request description:

  In a few cases we need to use `std::mutex` rather than the sync.h primitives. But `std::lock_guard<std::mutex>` doesn't include the clang thread safety annotations unless you also use clang's C library, which means you can't indicate when variables should be guarded by `std::mutex` mutexes.

  This adds an annotated version of `std::lock_guard<std::mutex>` to threadsafety.h to fix that, and modifies places where `std::mutex` is used to take advantage of the annotations.

  It's based on top of #16112, and turns the thread safety comments included there into annotations.

  It also changes the RAII classes in wallet/wallet.h and rpc/blockchain.cpp to just use the atomic<bool> flag for synchronisation rather than having a mutex that doesn't actually guard anything as well.

ACKs for top commit:
  MarcoFalke:
    ACK 5478d6c099 🗾
  hebasto:
    re-ACK 5478d6c099, only renamed s/`MutexGuard`/`LockGuard`/, and dropped the commit "test/util_threadnames_tests: add thread safety annotations" since the [previous](https://github.com/bitcoin/bitcoin/pull/16127#pullrequestreview-414184113) review.
  ryanofsky:
    Code review ACK 5478d6c099. Thanks for taking suggestions! Only changes since last review are dropping thread rename test commit d53072ec730d8eec5a5b72f7e65a54b141e62b19 and renaming mutex guard to lock guard

Tree-SHA512: 7b00d31f6f2b5a222ec69431eb810a74abf0542db3a65d1bbad54e354c40df2857ec89c00b4a5e466c81ba223267ca95f3f98d5fbc1a1d052a2c3a7d2209790a
2020-05-27 19:31:33 -04:00
MarcoFalke
fab893e0ca
doc: Fix unrelated typos reported by codespell 2020-05-27 12:37:08 -04:00
MarcoFalke
13397dc78f
Merge #19056: rpc: Make gettxoutsetinfo/GetUTXOStats interruptible
fa756928c3 rpc: Make gettxoutsetinfo/GetUTXOStats interruptible (MarcoFalke)
fa7fc5a8e0 rpc: factor out RpcInterruptionPoint from dumptxoutset (MarcoFalke)

Pull request description:

  Make it interruptible, so that shutdown doesn't block for up to one hour.

  Fixes (partially) #13217

ACKs for top commit:
  Empact:
    Code Review ACK fa756928c3
  laanwj:
    Code review ACK fa756928c3

Tree-SHA512: 298261e0ff7d79fab542b8f6828cc0ac451cbafe396d5f0816c9d36437faba1330f5c4cb2a25c5540e202bfb9783da6ec858bd453056ce488d21e36335d3d42c
2020-05-26 07:33:43 -04:00
Amiti Uttarwar
8f30260a67 [doc] Update unbroadcast description in RPC results 2020-05-25 11:27:07 -07:00
Samuel Dobson
24f7029064
Merge #18594: cli: display multiwallet balances in -getinfo
5edad5ce5d test: add -getinfo multiwallet functional tests (Jon Atack)
903b6c117f rpc: drop unused JSONRPCProcessBatchReply size arg, refactor (Jon Atack)
afce85eb99 cli: use GetWalletBalances() functionality for -getinfo (Jon Atack)
9f01849a49 cli: create GetWalletBalances() to fetch multiwallet balances (Jon Atack)
743077544b cli: lift -rpcwallet logic up to CommandLineRPC() (Jon Atack)
29f2cbdeb7 cli: extract connection exception handler, -rpcwait logic (Jon Atack)

Pull request description:

  This PR is a client-side version of #18453, per review feedback there and [review club discussions](https://bitcoincore.reviews/18453#meeting-log). It updates `bitcoin-cli -getinfo` on the client side to display wallet name and balance for the loaded wallets when more than one is loaded (e.g. you are in "multiwallet mode") and `-rpcwallet=` is not passed; otherwise, behavior is unchanged.

  before
  ```json
  $ bitcoin-cli -getinfo -regtest
  {
    "version": 199900,
    "blocks": 15599,
    "headers": 15599,
    "verificationprogress": 1,
    "timeoffset": 0,
    "connections": 0,
    "proxy": "",
    "difficulty": 4.656542373906925e-10,
    "chain": "regtest",
    "balance": 0.00001000,
    "relayfee": 0.00001000
  }

  ```
  after
  ```json
  $ bitcoin-cli -getinfo -regtest
  {
    "version": 199900,
    "blocks": 15599,
    "headers": 15599,
    "verificationprogress": 1,
    "timeoffset": 0,
    "connections": 0,
    "proxy": "",
    "difficulty": 4.656542373906925e-10,
    "chain": "regtest",
    "balances": {
      "": 0.00001000,
      "Encrypted": 0.00003500,
      "day-to-day": 0.00000120,
      "side project": 0.00000094
    }
  }
  ```
  -----

  `Review club` discussion about this PR is here: https://bitcoincore.reviews/18453

  This PR can be manually tested by building, creating/loading/unloading several wallets with `bitcoin-cli createwallet/loadwallet/unloadwallet` and running `bitcoin-cli -getinfo` and `bitcoin-cli -rpcwallet=<wallet-name> -getinfo`.

  `wallet_multiwallet.py --usecli` provides regression test coverage on this change, along with `interface_bitcoin_cli.py` where this PR adds test coverage.

  Credit to Wladimir J. van der Laan for the idea in https://github.com/bitcoin/bitcoin/issues/17314 and https://github.com/bitcoin/bitcoin/pull/18453#issuecomment-605431806.

ACKs for top commit:
  promag:
    Tested ACK 5edad5ce5d.
  jnewbery:
    utACK 5edad5ce5d
  meshcollider:
    Code review ACK 5edad5ce5d

Tree-SHA512: 4ca36c5f6c49936b40afb605c44459c1d5b80b5bd84df634007ca276b3f6c102a0cb382f9d528370363ee32c94b0d7ffa15184578eaf8de74179e566c5c5cee5
2020-05-24 00:17:38 +12:00
MarcoFalke
793e0ff22c
Merge #18698: Make g_chainman internal to validation
fab6b9d18f validation: Mark g_chainman DEPRECATED (MarcoFalke)
fa1d97b256 validation: Make ProcessNewBlock*() members of ChainstateManager (MarcoFalke)
fa24d49098 validation: Make PruneOneBlockFile() a member of ChainstateManager (MarcoFalke)
fa84b1cd84 validation: Make LoadBlockIndex() a member of ChainstateManager (MarcoFalke)
fa05fdf0f1 net: Pass chainman into PeerLogicValidation (MarcoFalke)
fa7b626d7a node: Add chainman alias for g_chainman (MarcoFalke)

Pull request description:

  The global `g_chainman` has recently been introduced in #17737. The chainstate manager is primarily needed for the assumeutxo feature, but it can also simplify testing in the future.

  The goal of this pull is to make the global chainstate manager internal to validation, so that all external code does not depend on globals and that unit or fuzz tests can pass in their (potentially mocked) chainstate manager.

  I suggest reviewing the pull request commit-by-commit. It should be relatively straightforward refactoring that does not change behavior at all.

ACKs for top commit:
  ryanofsky:
    Code review ACK fab6b9d18f. Had to be rebased but still looks good

Tree-SHA512: dcbf114aeef4f8320d466369769f22ce4dd8f46a846870354df176c3de9ff17c64630fbd777e7121d7470d7a8564ed8d37b77168746e8df7489c6877e55d7b4f
2020-05-23 07:58:13 -04:00
MarcoFalke
fa756928c3
rpc: Make gettxoutsetinfo/GetUTXOStats interruptible
Also, add interruption points to scantxoutset
2020-05-22 15:53:50 -04:00
MarcoFalke
fa7fc5a8e0
rpc: factor out RpcInterruptionPoint from dumptxoutset 2020-05-22 15:52:37 -04:00
fanquake
ad3a61c5f5
Merge #18895: p2p: unbroadcast followups: rpcs, nLastResend, mempool sanity check
651f1d816f [test] wait for inital broadcast before comparing mempool entries (gzhao408)
9d3f7eb986 [mempool] sanity check that all unbroadcast txns are in mempool (gzhao408)
a7ebe48b94 [rpc] add unbroadcast info to mempool entries and getmempoolinfo (gzhao408)
d160069604 [wallet] remove nLastResend logic (gzhao408)

Pull request description:

  Followup to #18038 by amitiuttarwar which introduces the unbroadcast set: "a mechanism for the mempool to track locally submitted transactions" and decreases the frequency of rebroadcast from 10-15 minutes to 12-36 hours.

  This PR addresses some of the outstanding TODOs building on top of it:
  - remove `nLastResend` logic, which is used to ensure rebroadcast doesn't happen again if no new block has been mined (makes sense in 10-15 min period, but not necessary for 12-36 hour period). (#18038 [comment](https://github.com/bitcoin/bitcoin/pull/18038#discussion_r416826914))
  - expose unbroadcast info via RPCs, for more informative queries and testing (#18038 [comment](https://github.com/bitcoin/bitcoin/pull/18038#discussion_r416837980))
  - add sanity check to verify unbroadcast transactions still exist in mempool before rebroadcasting (#18038 [comment](https://github.com/bitcoin/bitcoin/pull/18038#discussion_r416861609))

ACKs for top commit:
  naumenkogs:
    Code review ACK 651f1d816f
  amitiuttarwar:
    ACK 651f1d816f 🎉
  MarcoFalke:
    Review ACK 651f1d816f

Tree-SHA512: d5327e95ef39d44152b48df5c610502ae11c168f43dbbfb2885340c93d1ba9426eb3a5794573f5fc843502109cb3ffb63efa3f2db4f8f112efcde8f76d9a8845
2020-05-22 07:51:51 +08:00
MarcoFalke
fa1d97b256
validation: Make ProcessNewBlock*() members of ChainstateManager 2020-05-21 09:56:16 -04:00
MarcoFalke
25ad2c623a
Merge #18740: Remove g_rpc_node global
b3f7f375ef refactor: Remove g_rpc_node global (Russell Yanofsky)
ccb5059ee8 scripted-diff: Remove g_rpc_node references (Russell Yanofsky)
6fca33b2ed refactor: Pass NodeContext to RPC and REST methods through util::Ref (Russell Yanofsky)
691c817b34 Add util::Ref class as temporary alternative for c++17 std::any (Russell Yanofsky)

Pull request description:

  This PR removes the `g_rpc_node` global, to get same benefits we see removing other globals and make RPC code more testable, modular, and reusable.

  This uses a hybrid of the approaches suggested in #17548. Instead of using `std::any`, which isn't available in c++11, or `void*`, which isn't type safe, it uses a small new `util::Ref` helper class, which acts like a simplified `std::any` that only holds references, not values.

  Motivation for writing this was to provide an simpler alternative to #18647 by Harris Brakmić (brakmic) which avoids some shortcomings of that PR (https://github.com/bitcoin/bitcoin/pull/18647#issuecomment-617878826)

ACKs for top commit:
  MarcoFalke:
    re-ACK b3f7f375ef, only change is adding back const and more tests 🚾
  ajtowns:
    ACK b3f7f375ef

Tree-SHA512: 56292268a001bdbe34d641db1180c215351503966ff451e55cc96c9137f1d262225d7d7733de9c9da7ce7d7a4b34213a98c2476266b58c89dbbb0f3cb5aa5d70
2020-05-21 06:53:39 -04:00
Jon Atack
903b6c117f
rpc: drop unused JSONRPCProcessBatchReply size arg, refactor 2020-05-21 10:24:26 +02:00
gzhao408
a7ebe48b94 [rpc] add unbroadcast info to mempool entries and getmempoolinfo
- expose info about number of txns in unbroadcast set and whether a mempool entry's tx has passed initial broadcast
- makes rpcs more informative and allows for more explicit testing, eg tracking if tx is in unbroadcast set
before and after originating node connects to peers (adds this in mempool_unbroadcast.py)
- adds mempool method IsUnbroadcastTx to query for tx inclusion in  mempool's unbroadcast set
2020-05-19 14:23:13 -07:00
MarcoFalke
aa8d76806c
Merge #17946: Fix GBT: Restore "!segwit" and "csv" to "rules" key
412d5fe879 QA: feature_segwit: Check that template "rules" includes "!segwit" as appropriate (Luke Dashjr)
2abe8cc3b7 Bugfix: Include "csv","!segwit" in "rules" (Luke Dashjr)

Pull request description:

  #16060 removed CSV & segwit from versionbits, breaking the "rules" key returned by GBT.

  Without this, miners don't know they're mining segwit blocks, and should fall back to pre-segwit block creation.

ACKs for top commit:
  sipa:
    ACK 412d5fe879
  jnewbery:
    Tested ACK 412d5fe879.

Tree-SHA512: 825d72e257dc0dd4941f2fe498d8d4f4f2a21b9505cd21a8f9eb7fb5d6d7dd9219347928cf90bb57a777920ce24295859763e64fa8a22ebb58fc2380f80f5615
2020-05-19 08:54:23 -04:00
Anthony Towns
479c5846f7 rpc/blockchain.cpp: thread safety annotations for latestblock 2020-05-19 16:33:10 +10:00
Hennadii Stepanov
2bec309ad6
refactor: Remove unused bool parameter in RPCNotifyBlockChange() 2020-05-19 02:39:45 +03:00
fanquake
b9c504cbc4
Merge #18742: miner: Avoid stack-use-after-return in validationinterface
7777f2a4bb miner: Avoid stack-use-after-return in validationinterface (MarcoFalke)
fa5ceb25fc test: Remove UninterruptibleSleep from test and replace it by SyncWithValidationInterfaceQueue (MarcoFalke)
fa770ce7fe validationinterface: Rework documentation, Rename pwalletIn to callbacks (MarcoFalke)
fab6d060ce test: Add unregister_validation_interface_race test (MarcoFalke)

Pull request description:

  When a validationinterface has itself unregistered in one thread, but is about to get executed in another thread [1], there is a race:

  * The validationinterface destructing itself
  * The validationinterface getting dereferenced for execution

  [1] 64139803f1/src/validationinterface.cpp (L82-L83)

  This happens in the miner. More generally it happens everywhere where at least one thread is generating notifications and another one is unregistering a validationinterface.

  This issue has been fixed in commit ab31b9d6fe, but the fix has not been applied to the miner.

  Example where this happened in practice: https://travis-ci.org/github/bitcoin/bitcoin/jobs/675322230#L4414

ACKs for top commit:
  promag:
    Code review ACK 7777f2a4bb.
  laanwj:
    Code review ACK 7777f2a4bb

Tree-SHA512: 8087119243c71ba18a823a63515f3730d127162625d8729024278b447af29e2ff206f4840ee3d90bf84f93a2c5ab73b76c7e7044c83aa93b5b51047a166ec3d3
2020-05-14 20:40:55 +08:00
MarcoFalke
7777f2a4bb
miner: Avoid stack-use-after-return in validationinterface
This is achieved by switching to a shared_ptr.

Also, switch the validationinterfaces in the tests to use shared_ptrs
for the same reason.
2020-05-13 19:58:20 -04:00