Commit graph

1813 commits

Author SHA1 Message Date
MarcoFalke
a063647413
Merge bitcoin/bitcoin#23280: init: Coalesce Chainstate loading sequence between {,non-}unittest codepaths
7f15eff2dd style-only: Remove redundant scope in *Chainstate (Carl Dong)
89bec827fd Collapse the 2 cs_main locks in LoadChainstate (Carl Dong)
3b1584b794 Remove all #include // for * comments (Carl Dong)
9a5a5a3d08 test/setup: Use LoadChainstate (Carl Dong)
c541da0d62 node/chainstate: Add options for in-memory DBs (Carl Dong)
ceb9790341 node/caches: Remove intermediate variables (Carl Dong)
ac4bf138b8 node/caches: Extract cache calculation logic (Carl Dong)
15f2e33bb3 validation: VerifyDB only needs Consensus::Params (Carl Dong)
4da9c076d1 node/chainstate: Decouple from ShutdownRequested (Carl Dong)
05441c2dc5 node/chainstate: Decouple from GetTime (Carl Dong)
2414ebc18b init: Delay RPC block notif until warmup finished (Carl Dong)
8d466a8504 Move -checkblocks LogPrintf to AppInitMain (Carl Dong)
aad8d59789 node/chainstate: Reduce coupling of LogPrintf (Carl Dong)
b345979a2b node/chainstate: Decouple from concept of uiInterface (Carl Dong)
ca7c0b934d Split off VerifyLoadedChainstate (Carl Dong)
adf4912d77 node/chainstate: Remove do/while loop (Carl Dong)
975235ca0a Move init logistics message for BAD_GENESIS_BLOCK to init.cpp (Carl Dong)
8715658983 Move mempool nullptr Assert out of LoadChainstate (Carl Dong)
9162a4f93e node/chainstate: Decouple from concept of NodeContext (Carl Dong)
c7a5c46e6f node/chainstate: Decouple from ArgsManager (Carl Dong)
ae9121f958 node/chainstate: Decouple from stringy errors (Carl Dong)
cbac28b72f node/chainstate: Decouple from GetTimeMillis (Carl Dong)
cb64af9635 node: Extract chainstate loading sequence (Carl Dong)

Pull request description:

  This PR:
  1. Coalesce the Chainstate loading sequence between `AppInitMain` and `*TestingSetup` (which makes it more tested)
  2. Makes the Chainstate loading sequence reusable in preparation for future work extracting out our consensus engine.

  Code-wise, this PR:
  1. Extracts `AppInitMain`'s Chainstate loading sequence into a `::LoadChainstateSequence` function
  2. Makes this `::LoadChainstateSequence` function reusable by
      1. Decoupling it from various concepts (`ArgsManager`, `uiInterface`, etc)
      2. Making it report errors using an `enum` rather than by setting a `bilingual_str`
  3. Makes `*TestingSetup` use this new `::LoadChainstateSequence`

  Reviewers: Aside from commentary, I've also included `git diff` flags of interest in the commit messages which I hope will aid review!

ACKs for top commit:
  ryanofsky:
    Code review ACK 7f15eff2dd. Thanks for updates!
  MarcoFalke:
    review ACK 7f15eff2dd 💳

Tree-SHA512: fb9a6cbd1c511a52b477c62a5e68e53a8be5dec2fff0e44a279966afb91efbab44bf1fe7c6b1519f8464ecc25f42dd4bae8e1efbf55ee91fc90fa0b92e3a83e2
2021-12-10 17:17:43 +01:00
Andrew Chow
81521173ba Merge global xpubs in joinpsbts and combinepsbts 2021-12-10 08:29:47 -05:00
Andrew Chow
35670df866 Add global_xpubs to decodepsbt 2021-12-10 08:29:47 -05:00
Andrew Chow
a4cf810174 Output proprietary type info in decodepsbt 2021-12-10 08:29:47 -05:00
Andrew Chow
10ba0b593d Output psbt version in decodepsbt 2021-12-10 08:29:26 -05:00
MarcoFalke
011d6e429b
Merge bitcoin/bitcoin#22514: psbt: Actually use SIGHASH_DEFAULT for PSBT signing
c0405ee27f rpc: Document that DEFAULT is for Taproot, ALL for everything else (Andrew Chow)
d3992669df psbt: Actually use SIGHASH_DEFAULT (Andrew Chow)
eb9a1a2c59 psbt: Make sighash_type std::optional<int> (Andrew Chow)

Pull request description:

  Make the behavior align with the help text by actually using SIGHASH_DEFAULT as the default sighash for signing PSBTs.

ACKs for top commit:
  Sjors:
    re-utACK c0405ee27f

Tree-SHA512: 5199fb41de416b2f10ac451f824e7c94b428ba11fdb9e50f0027c692e959ce5813a340c34a4e52d7aa128e12008303d80939a693eff36a869720e45442119828
2021-12-10 10:17:36 +01:00
MarcoFalke
9f7661c0c4
Merge bitcoin/bitcoin#19499: p2p: Make timeout mockable and type safe, speed up test
fadc0c80ae p2p: Make timeout mockable and type safe, speed up test (MarcoFalke)
fa6d5a238d scripted-diff: Rename m_last_send and m_last_recv (MarcoFalke)

Pull request description:

  Use type-safe time for better code readability/maintainability and mockable time for better testability. This speeds up the p2p_timeout test.

  This is also a bugfix for intermittent test issues like: https://cirrus-ci.com/task/4769904156999680?command=ci#L2836

  Fixes #20654

ACKs for top commit:
  laanwj:
    Code review ACK fadc0c80ae
  naumenkogs:
    ACK fadc0c80ae

Tree-SHA512: 28c6544c97f188c8a0fbc80411c74ab74ffd055885322c325aa3d1c404b29c3fd70a737e86083eecae58ef394db1cb56bc122d06cff63742aa89a8e868730c64
2021-12-10 10:02:12 +01:00
fanquake
7908772244
Merge bitcoin/bitcoin#23703: scripted-diff: Use named args in RPC docs
fa9aaf8694 scripted-diff: Use named args in RPC docs (MarcoFalke)

Pull request description:

  Incorrect named args are source of bugs, like #22979.

  To allow them being checked by `clang-tidy`, use a format it can understand.

ACKs for top commit:
  fanquake:
    ACK fa9aaf8694 - checked `clang-tidy` and it's fine here, (but throwing errors in other files. i.e `wallet/test/wallet_tests.cpp`).

Tree-SHA512: e09dae8ee999a5c4819e6f848c12139593ca0e915e645c8fabeb97c379188fb9104d286c02c71f590abc64cdec125f78026735f83e016111976baa49d588a9bc
2021-12-09 17:14:26 +08:00
MarcoFalke
faa0833c43
doc: Normalize RPC description whitespace 2021-12-08 19:43:24 +01:00
Andrew Chow
c0405ee27f rpc: Document that DEFAULT is for Taproot, ALL for everything else 2021-12-08 09:43:30 -05:00
Andrew Chow
eb9a1a2c59 psbt: Make sighash_type std::optional<int>
It is better to ues an optional to determine whether the sighash type
is set rather than using 0 as a magic number.
2021-12-08 09:43:26 -05:00
MarcoFalke
fa9aaf8694
scripted-diff: Use named args in RPC docs
-BEGIN VERIFY SCRIPT-
 sed -i -e 's|, /\* optional \*/ true,|, /*optional=*/true,|g' $( git grep -l ', /\* optional \*/ true,' )
-END VERIFY SCRIPT-
2021-12-08 11:54:12 +01:00
MarcoFalke
aaaa34e34d
doc: Add missing optional to getblockfrompeer 2021-12-08 11:39:31 +01:00
MarcoFalke
f6013265b7
Merge bitcoin/bitcoin#20295: rpc: getblockfrompeer
dce8c4c381 rpc: getblockfrompeer (Sjors Provoost)
b884ababc2 rpc: move Ensure* helpers to server_util.h (Sjors Provoost)

Pull request description:

  This adds an RPC method to fetch a block directly from a peer. This can used to fetch stale blocks with lower proof of work that are normally ignored by the node (`headers-only` in `getchaintips`).

  Usage:
  ```
  bitcoin-cli getblockfrompeer HASH peer_n
  ```

  Closes #20155

  Limitations:
  * you have to specify which peer to fetch the block from
  * the node must already have the header

ACKs for top commit:
  jnewbery:
    ACK dce8c4c381
  fjahr:
     re-ACK dce8c4c381

Tree-SHA512: 843ba2b7a308f640770d624d0aa3265fdc5c6ea48e8db32269b96a082b7420f7953d1d8d1ef2e6529392c7172dded9d15639fbc9c24e7bfa5cfb79e13a5498c8
2021-12-08 10:39:37 +01:00
Samuel Dobson
d1a9742623 Add warnings field to createmultisig to warn about uncompressed keys 2021-12-08 17:11:46 +13:00
MarcoFalke
63c63b5533
Merge bitcoin/bitcoin#14707: [RPC] Include coinbase transactions in receivedby RPCs
1dcba996d3 Coinbase receivedby rpcs release notes (Andrew Toth)
b5696750a9 Test including coinbase transactions in receivedby wallet rpcs (Andrew Toth)
bce20c34d6 Include coinbase transactions in receivedby wallet rpcs (Andrew Toth)

Pull request description:

  The current `*receivedby*` RPCs filter out coinbase transactions. This doesn't seem correct since an output to your address in a coinbase transaction *is* receiving those coins.

  This PR corrects this behaviour. Also, a new option `include_immature_coinbase` is added (default=`false`) that includes immature coinbase transactions when set to true.

  However, since this is potentially a breaking change this PR introduces a hidden configuration option `-deprecatedrpc=exclude_coinbase`. This can be set to revert to previous behaviour. If no reports of broken workflow are received, then this option can be removed in a future release.

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

ACKs for top commit:
  jnewbery:
    reACK 1dcba996d3

Tree-SHA512: bfc43b81279fea5b6770a4620b196f6bc7c818d221b228623e9f535ec75a2406bc440e3df911608a3680f11ab64c5a4103917162114f5ff7c4ca8ab07bb9d3df
2021-12-07 20:52:13 +01:00
Carl Dong
15f2e33bb3 validation: VerifyDB only needs Consensus::Params
Previously we were passing in CChainParams, when VerifyDB only needed
the Consensus::Params subset.
2021-12-07 14:48:49 -05:00
MarcoFalke
fa1571b156
doc: Add missing optional to MempoolEntryDescription 2021-12-07 15:48:04 +01:00
MarcoFalke
4fd0ce75c5
Merge bitcoin/bitcoin#22689: rpc: deprecate top-level fee fields in getmempool RPCs
2f9515f37a rpc: move fees object to match help (josibake)
07ade7db8f doc: add release note for fee field deprecation (josibake)
2ee406ce3e test: add functional test for deprecatedrpc=fees (josibake)
35d928c632 rpc: deprecate fee fields from mempool entries (josibake)

Pull request description:

  per #22682 , top level fee fields for mempool entries have been deprecated since 0.17 but are still returned. this PR properly deprecates them so that they are no longer returned unless `-deprecatedrpc=fees` is passed.

  the first commit takes care of deprecation and also updates `test/functional/mempool_packages.py` to only use the `fees` object. the second commit adds a new functional test for `-deprecatedrpc=fees`

  closes #22682

  ## questions for the reviewer

  * `-deprecatedrpc=fees` made the most sense to me, but happy to change if there is a name that makes more sense
  * #22682 seems to indicate that after some period of time, the fields will be removed all together. if we have a rough idea of when this will be, i can add a `TODO: fully remove in vXX` comment to `entryToJSON`

  ## testing
  to get started on testing, compile, run the tests, and start your node with the deprecated rpcs flag:

  ```bash
  ./src/bitcoind -daemon -deprecatedrpc=fees
  ```
  you should see entries with the deprecated fields like so:
  ```json
  {
    "<txid>": {
      "fees": {
        "base": 0.00000671,
        "modified": 0.00000671,
        "ancestor": 0.00000671,
        "descendant": 0.00000671
      },
      "fee": 0.00000671,
      "modifiedfee": 0.00000671,
      "descendantfees": 671,
      "ancestorfees": 671,
      "vsize": 144,
      "weight": 573,
     ...
    },
  ```
  you can also check `getmempoolentry` using any of the txid's from the output above.

  next start the node without the deprecated flag, repeat the commands from above and verify that the deprecated fields are no longer present at the top level, but present in the "fees" object

ACKs for top commit:
  jnewbery:
    reACK 2f9515f37a
  glozow:
    utACK 2f9515f37a

Tree-SHA512: b175f4d39d26d96dc5bae26717d3ccfa5842d98ab402065880bfdcf4921b14ca692a8919fe4e9969acbb5c4d6e6d07dd6462a7e0a0a7342556279b381e1a004e
2021-12-07 15:26:06 +01:00
W. J. van der Laan
6ac8c4f700
Merge bitcoin/bitcoin#23634: rpc: add missing scantxoutset examples
1ed5681407 rpc: add missing scantxoutset examples (Sebastian Falbesoner)

Pull request description:

  The scantxoutset RPC and its help text was at last improved in #16285, but it's still missing examples (see https://github.com/bitcoin/bitcoin/pull/16285#issuecomment-529313781).

  ~Note that the example descriptor used doesn't follow the developer guideline of using invalid bech32 addresses, as the RPC is not wallet-related and it's use-case is merely to look up state information (i.e. there is no danger of sending funds to a wrong address).~ For the sake of simplicity, the raw descriptor for an early coinbase payout address (block 9) is taken, i.e. it yields results even at an early stage of IBD. Happy to change that though if there are other suggestions.

ACKs for top commit:
  shaavan:
    reACK 1ed5681407

Tree-SHA512: 057ad9ac0d019035bee2332440128de0ef08580bbeae80182ff74771beead3555c4bf7008071a97bbb6a8d85fb85d0f0754fb7941db2c5b755eae1ac9aa65318
2021-12-07 12:38:23 +01:00
MarcoFalke
084c81c8b6
Merge bitcoin/bitcoin#23547: Bugfix: RPC/mining: Fail properly in estimatesmartfee if smart fee data is unavailable
cd8d156354 Bugfix: RPC/mining: Fail properly in estimatesmartfee if smart fee data is unavailable (Luke Dashjr)

Pull request description:

  Fixes a regression introduced by #22722

  (Not entirely sure on the solution)

ACKs for top commit:
  prayank23:
    crACK cd8d156354
  darosior:
    utACK cd8d156354
  kristapsk:
    utACK cd8d156354

Tree-SHA512: eb4aa3cc345c69c44ffd5733b51b90eefe1d7854b7a2855e8cbb98268db24d43b7d0ae9fbb0eccf9b6dc01da644d19433cc77fec52ff67bf890be1fc53a67fc4
2021-12-07 10:16:19 +01:00
Andrew Toth
bce20c34d6
Include coinbase transactions in receivedby wallet rpcs 2021-12-06 12:30:58 -05:00
MarcoFalke
fadc0c80ae
p2p: Make timeout mockable and type safe, speed up test 2021-12-06 10:47:52 +01:00
MarcoFalke
99993425af
rpc: Only allow specific types to be P2(W)SH wrapped in decodescript 2021-12-06 10:33:12 +01:00
MarcoFalke
8b1de78577
Merge bitcoin/bitcoin#23413: Replace MakeSpan helper with Span deduction guide
11daf6ceb1 More Span simplifications (Pieter Wuille)
568dd2f839 Replace MakeSpan helper with Span deduction guide (Pieter Wuille)

Pull request description:

  C++17 supports [user-defined deduction guides](https://en.cppreference.com/w/cpp/language/class_template_argument_deduction), allowing class constructors to be invoked without specifying class template arguments. Instead, the code can contain rules to infer the template arguments from the constructor argument types.

  This alleviates the need for the `MakeSpan` helper. Convert the existing MakeSpan rules into deduction rules for `Span` itself, and replace all invocations of `MakeSpan` with just `Span` ones.

ACKs for top commit:
  MarcoFalke:
    re-ACK 11daf6ceb1 Only change is removing a hunk in the tests 🌕

Tree-SHA512: 10f3e82e4338f39d9b7b407cd11aac7ebe1e9191b58e3d7f4e5e338a4636c0e126b4a1d912127c7446f57ba356c8d6544482e47f97901efea6a54fffbfd7895f
2021-12-03 10:44:37 +01:00
MarcoFalke
fab6c43b40
doc: Document optional result fields in validateaddress
Can be reviewed with --ignore-all-space --word-diff-regex=.
2021-12-02 19:22:35 +01:00
MarcoFalke
faee2656a8
doc: Document optional result fields in getpeerinfo
Can be reviewed with --ignore-all-space --word-diff-regex=.
2021-12-02 17:38:18 +01:00
MarcoFalke
26a1147ce5
Merge bitcoin/bitcoin#23636: Remove GetAdjustedTime from init.cpp
fa551b3bdd Remove GetAdjustedTime from init.cpp (MarcoFalke)
fa815f8473 Replace addrman.h include with forward decl in net.h (MarcoFalke)

Pull request description:

  It seems confusing to call `GetAdjustedTime` there, because no offset could have been retrieved from the network at this point. Even if connman was started, `timedata` needs at least 5 peer connections to calculate an offset.

  Fix the confusion by replacing `GetAdjustedTime` with `GetTime`, which does not change behavior.

  Also:
  * Replace magic number with `MAX_FUTURE_BLOCK_TIME` to clarify the context
  * Add test, which passes both on current master and this pull request
  * An unrelated refactoring commit, happy to drop

ACKs for top commit:
  dongcarl:
    Code Review ACK fa551b3bdd, noticed the exact same thing here: e073634c37
  mzumsande:
    Code Review ACK fa551b3bdd
  jnewbery:
    Code review ACK fa551b3bdd
  shaavan:
    ACK fa551b3bdd
  theStack:
    Code-review ACK fa551b3bdd

Tree-SHA512: 15807a0e943e3e8d8c5250c8f6d7b56afb26002b1e290bf93636a2c747f27e78f01f1de04ce1a83d6339e27284c69c43e077a8467545c4078746f4c1ecb1164d
2021-12-02 15:24:55 +01:00
Sebastian Falbesoner
1ed5681407 rpc: add missing scantxoutset examples 2021-12-02 14:30:29 +01:00
MarcoFalke
0bd7ca9a03
Merge bitcoin/bitcoin#23642: refactor: Call type-solver earlier in decodescript
3333070208 refactor: Call type-solver earlier in decodescript (MarcoFalke)
fab0d998f4 style: Remove whitespace (MarcoFalke)

Pull request description:

  The current logic is a bit confusing. First creating the `UniValue` return dict, then parsing it again to get the type as `std::string`.

  Clean this up by using a strong type `TxoutType`. Also, remove whitespace.

ACKs for top commit:
  shaavan:
    ACK 3333070208
  theStack:
    Code-review ACK 3333070208

Tree-SHA512: 49db7bc614d2491cd3ec0177d21ad1e9924dbece1eb5635290cd7fd18cb30adf4711b891daf522e7c4f6baab3033b66393bbfcd1d4726f24f90a433124f925d6
2021-12-02 13:56:57 +01:00
W. J. van der Laan
6acda4b00b
Merge bitcoin/bitcoin#23155: rpc: various fixups for dumptxoutset
ffd09281fe rpc: various fixups for dumptxoutset (James O'Beirne)

Pull request description:

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

  ---

  A few fixes to make this RPC actually useful when generating snapshots.

  - Generate an assumeutxo hash and display it (sort of a bugfix)
  - Add nchaintx to output (necessary for use in chainparams entry)
  - Add path of serialized UTXO file to output

ACKs for top commit:
  laanwj:
    Code review ACK ffd09281fe

Tree-SHA512: b0b5fd5138dea0e21258b1b18ab75bf3fd1628522cc1dbafa81af9cb9fa96562a1c39124fdb31057f256bfc560f462f907e9fe5e209b577b3f57afae2b7be826
2021-12-02 10:35:02 +01:00
MarcoFalke
a377510e3a
Merge bitcoin/bitcoin#23643: rpc: remove info about mallocinfo needing glibc 2.10+
9a09d307e9 rpc: remove info about mallocinfo needing glibc 2.10+ (fanquake)

Pull request description:

  We require glibc 2.18+.

ACKs for top commit:
  jarolrod:
    ACK 9a09d307e9
  shaavan:
    ACK 9a09d307e9

Tree-SHA512: 61312e48fda4cb4c788d44be0f0c626e753b0a18a8f36bca813ce838f8e619e73c00306bb716e9863a077c09b5bcdec7dec134d75c5ace719a5c0a05cf75ed8a
2021-12-02 08:31:51 +01:00
Sjors Provoost
dce8c4c381
rpc: getblockfrompeer
Co-authored-by: John Newbery <john@johnnewbery.com>
2021-12-02 13:16:18 +07:00
Sjors Provoost
b884ababc2
rpc: move Ensure* helpers to server_util.h 2021-12-02 13:15:32 +07:00
fanquake
36dc5bb8cb
doc: Extract CreateTxDoc in rawtransaction
For the fields: inputs, outputs, locktime, replaceable
2021-12-01 19:24:03 +08:00
fanquake
9a09d307e9
rpc: remove info about mallocinfo needing glibc 2.10+
We require glibc 2.18+.
2021-12-01 19:18:26 +08:00
MarcoFalke
3333070208
refactor: Call type-solver earlier in decodescript
Also, remove std::string type.
2021-12-01 10:07:51 +01:00
MarcoFalke
fab0d998f4
style: Remove whitespace
Can be reviewed via --ignore-all-space
2021-12-01 10:07:20 +01:00
James O'Beirne
ffd09281fe
rpc: various fixups for dumptxoutset
- Actually generate an assumeutxo hash and display it
- Add nchaintx to output (necessary for use in chainparams entry)
- Add path of serialized UTXO file to output
2021-11-30 11:19:26 -05:00
MarcoFalke
fa815f8473
Replace addrman.h include with forward decl in net.h
Also, add missing addrman.h includes
2021-11-30 14:46:16 +01:00
Kiminuo
059f88b6a9 Add RPC help for getblock verbosity level 3 2021-11-30 10:39:44 +01:00
Pieter Wuille
11daf6ceb1 More Span simplifications
Based on suggestions by MarcoFalke <falke.marco@gmail.com>
2021-11-29 17:59:44 -05:00
W. J. van der Laan
383d350bd5
Merge bitcoin/bitcoin#22513: rpc: Allow walletprocesspsbt to sign without finalizing
a99ed89865 psbt: sign without finalizing (Andrew Chow)

Pull request description:

  It can be useful to sign an input with `walletprocesspsbt` but not finalize that input if it is complete. This PR adds another option to `walletprocesspsbt` to be able to do that. We will still finalize by default.

  This does not materially change the PSBT workflow since `finalizepsbt` needs to be called in order to extract the tx for broadcast.

ACKs for top commit:
  meshcollider:
    utACK a99ed89865
  Sjors:
    utACK a99ed89

Tree-SHA512: c88e5d3222109c5f4e763b1b9d97ce4655f68f2985a4509caab2d4e7f5bac5047328fd69696e82a330f5c5a333e0312568ae293515689b77a4747ca2f17caca6
2021-11-29 17:20:20 +01:00
glozow
e12fafda2d [validation] de-duplicate package transactions already in mempool
As node operators are free to set their mempool policies however they
please, it's possible for package transaction(s) to already be in the
mempool. We definitely don't want to reject the entire package in that
case (as that could be a censorship vector).

We should still return the successful result to the caller, so add
another result type to MempoolAcceptResult.
2021-11-29 15:46:48 +00:00
MarcoFalke
16d698cdcf
Merge bitcoin/bitcoin#23517: scripted-diff: Move miner to src/node
fa4e09924b refactor: Replace validation.h include with forward-decl in miner.h (MarcoFalke)
fa0739a7d3 style: Sort file list after rename (MarcoFalke)
fa53e3a58c scripted-diff: Move miner to src/node (MarcoFalke)

Pull request description:

  It is impossible to run the miner without a node (validation, chainstate, mempool, rpc, ...). Also, the module is in the node library. Thus, it should be moved to `src/node`.

  Also, replace the `validation.h` include in the header with a forward-declaration.

ACKs for top commit:
  theStack:
    Code-review ACK fa4e09924b

Tree-SHA512: 791e6caa5839d8dc83b0f58f3f49bc0a7e3c1710822e8a44dede254c87b6f7531a0586fb95e8a067c181457a3895ad6041718aa2a2fac64cfc136bf04bb851d5
2021-11-26 09:03:39 +01:00
W. J. van der Laan
95d19f8c1a
Merge bitcoin/bitcoin#16807: Let validateaddress locate error in Bech32 address
88cc481092 Modify copyright header on Bech32 code (Samuel Dobson)
5599813b80 Add lots of comments to Bech32 (Samuel Dobson)
2eb5792ec7 Add release notes for validateaddress Bech32 error detection (MeshCollider)
42d6a029e5 Refactor and add more tests for validateaddress (Samuel Dobson)
c4979f77c1 Add boost tests for bech32 error detection (MeshCollider)
02a7bdee42 Add error_locations to validateaddress RPC (Samuel Dobson)
b62b67e06c Add Bech32 error location function (Samuel Dobson)
0b06e720c0 More detailed error checking for base58 addresses (Samuel Dobson)

Pull request description:

  Addresses (partially) #16779 - no GUI change in this PR

  Adds a LocateError function the bech32 library, which is then called by `validateaddress` RPC, (and then eventually from a GUI tool too, future work). I think modifying validateaddress is nicer than adding a separate RPC for this.
  Includes tests.

  Based on https://github.com/sipa/bech32/blob/master/ecc/javascript/bech32_ecc.js
  Credit to sipa for that code

ACKs for top commit:
  laanwj:
    Code review and manually tested ACK 88cc481092
  ryanofsky:
    Code review ACK 88cc481092 with caveat that I only checked the new `LocateErrors` code to try to verify it didn't have unsafe or unexpected operations or loop forever or crash. Did not try to verify behavior corresponds to the spec. In the worst case bugs here should just affect error messages not actual decoding of addresses so this seemed ok.
  w0xlt:
    tACK 88cc481

Tree-SHA512: 9c7fe9745bc7527f80a30bd4c1e3034e16b96a02cc7f6c268f91bfad08a6965a8064fe44230aa3f87e4fa3c938f662ff4446bc682c83cb48c1a3f95cf4186688
2021-11-22 13:26:01 +01:00
Luke Dashjr
cd8d156354 Bugfix: RPC/mining: Fail properly in estimatesmartfee if smart fee data is unavailable 2021-11-18 18:35:45 +00:00
MarcoFalke
fac49470ca
doc: Fix incorrect C++ named args 2021-11-17 09:25:14 +01:00
MarcoFalke
fa6d5a238d
scripted-diff: Rename m_last_send and m_last_recv
-BEGIN VERIFY SCRIPT-

ren() { sed -i "s/\<$1\>/$2/g" $(git grep -l "$1" ./src) ; }

ren nLastSend m_last_send
ren nLastRecv m_last_recv

-END VERIFY SCRIPT-
2021-11-17 08:45:17 +01:00
MarcoFalke
fa0739a7d3
style: Sort file list after rename 2021-11-16 10:05:21 +01:00
MarcoFalke
fa53e3a58c
scripted-diff: Move miner to src/node
-BEGIN VERIFY SCRIPT-
 # Move module
 git mv src/miner.cpp src/node/
 git mv src/miner.h   src/node/
 # Replacements
 sed -i 's:miner\.h:node/miner.h:g'     $(git grep -l miner)
 sed -i 's:miner\.cpp:node/miner.cpp:g' $(git grep -l miner)
 sed -i 's:MINER_H:NODE_MINER_H:g'      $(git grep -l MINER_H)
-END VERIFY SCRIPT-
2021-11-16 10:04:55 +01:00
MarcoFalke
fa44237d76
doc: Fix typos in endif header comments 2021-11-16 09:56:45 +01:00
MarcoFalke
38b2a0a3f9
Merge bitcoin/bitcoin#23173: Add ChainstateManager::ProcessTransaction
0fdb619aaf [validation] Always call mempool.check() after processing a new transaction (John Newbery)
2c64270bbe [refactor] Don't call AcceptToMemoryPool() from outside validation.cpp (John Newbery)
92a3aeecf6 [validation] Add CChainState::ProcessTransaction() (John Newbery)
36167faea9 [logging/documentation] Remove reference to AcceptToMemoryPool from error string (John Newbery)
4c24142b1e [validation] Remove comment about AcceptToMemoryPool() (John Newbery)
5759fd12b8 [test] Don't set bypass_limits to true in txvalidation_tests.cpp (John Newbery)
497c9e2964 [test] Don't set bypass_limits to true in txvalidationcache_tests.cpp (John Newbery)

Pull request description:

  Similarly to how #18698 added `ProcessNewBlock()` and `ProcessNewBlockHeaders()` methods to the `ChainstateManager` class, this PR adds a new `ProcessTransaction()` method. Code outside validation no longer calls `AcceptToMemoryPool()` directly, but calls through the higher-level `ProcessTransaction()` method. Advantages:

  - The interface is simplified. Calling code no longer needs to know about the active chainstate or mempool object, since `AcceptToMemoryPool()` can only ever be called for the active chainstate, and that chainstate knows which mempool it's using. We can also remove the `bypass_limits` argument, since that can only be used internally in validation.
  - responsibility for calling `CTxMemPool::check()` is removed from the callers, and run automatically by `ChainstateManager` every time `ProcessTransaction()` is called.

ACKs for top commit:
  lsilva01:
    tACK 0fdb619 on Ubuntu 20.04
  theStack:
    Code-review ACK 0fdb619aaf
  ryanofsky:
    Code review ACK 0fdb619aaf. Only changes since last review: splitting & joining commits, adding more explanations to commit messages, tweaking MEMPOOL_ERROR string, fixing up argument name comments.

Tree-SHA512: 0b395c2e3ef242f0d41d47174b1646b0a73aeece38f1fe29349837e6fb832f4bf8d57e1a1eaed82a97c635cfd59015a7e07f824e0d7c00b2bee4144e80608172
2021-11-10 14:35:22 +01:00
W. J. van der Laan
ed479497bd
Merge bitcoin/bitcoin#23398: rpc: add return message to savemempool RPC
aa1a4c9204 Add file validation to savemempool RPC test (lsilva01)
871e64d22f Add filename to savemempool RPC result (lsilva01)

Pull request description:

  Currently, if the user calls the `savemempool` RPC method, there is no way to know
  where the file was created (unless the user knows internal implementation details).

  This PR adds a return message stating the file name and path where the mempool was saved and changes `mempool_persist.py` to validate this new return message.

ACKs for top commit:
  laanwj:
    Code review ACK aa1a4c9204

Tree-SHA512: e8b1dd0a8976e5eb15f7476c9651e492d2c621a67e0b726721fa7a2ae0ddd272ee28b87a2d0c650bd635e07fa96bdefe77bece4deb6486ef3ee9a4f83423a840
2021-11-10 14:20:28 +01:00
lsilva01
871e64d22f Add filename to savemempool RPC result 2021-11-09 12:47:32 -03:00
W. J. van der Laan
e70fb87a4f
Merge bitcoin/bitcoin#23381: validation/refactor: refactoring for package submission
14cd7bf793 [test] call CheckPackage for package sanitization checks (glozow)
6876378365 MOVEONLY: move package unit tests to their own file (glozow)
c9b1439ca9 MOVEONLY: mempool checks to their own functions (glozow)
9e910d8152 scripted-diff: clean up MemPoolAccept aliases (glozow)
fd92b0c398 document workspace members (glozow)
3d3e4598b6 [validation] cache iterators to mempool conflicts (glozow)
36a8441912 [validation/rpc] cache + use vsize calculated in PreChecks (glozow)
8fa2936b34 [validation] re-introduce bool for whether a transaction is RBF (glozow)
cbb3598b5c [validation/refactor] store precomputed txdata in workspace (glozow)
0a79eaba72 [validation] case-based constructors for ATMPArgs (glozow)

Pull request description:

  This contains the refactors and moves within #22674. There are no behavior changes, so it should be simpler to review.

ACKs for top commit:
  ariard:
    Code Review ACK 14cd7bf
  jnewbery:
    Code review ACK 14cd7bf793
  laanwj:
    Code review ACK 14cd7bf793, thanks for adding documentation and clarifying the code
  t-bast:
    Code Review ACK 14cd7bf793

Tree-SHA512: 580ed48b43713a3f9d81cd9b573ef6ac44efe5df2fc7b7b7036c232b52952b04bf5ea92940cf73739f4fbd54ecf980cef58032e8a2efe05229ad0b3c639de8a0
2021-11-09 16:46:23 +01:00
John Newbery
2c64270bbe [refactor] Don't call AcceptToMemoryPool() from outside validation.cpp 2021-11-03 14:34:41 +00:00
Vasil Dimov
e6890fcb44
net: don't skip CJDNS from GetNetworksInfo() 2021-11-03 14:58:49 +01:00
josibake
2f9515f37a
rpc: move fees object to match help 2021-11-02 10:05:46 +01:00
josibake
35d928c632
rpc: deprecate fee fields from mempool entries
Unless `-deprecatedrpc=fees` is passed, top level
fee fields are no longer returned for mempool entries.

Add instructions to field help on how to access
deprecated fields, update help text for readability,
and include units. This is important to help
avoid any confusion as users move from deprecated
fields to the fee fields object (credit: jonatack).

This affects `getmempoolentry`, `getrawmempool`,
`getmempoolancestors`, and `getmempooldescendants`

Modify `test/functional/mempool_packages.py` and
`test/functional/rpc_fundrawtransaction.py` tests
to no longer use deprecated fields.

Co-authored-by: jonatack <jon@atack.com>
2021-11-02 10:05:37 +01:00
glozow
36a8441912 [validation/rpc] cache + use vsize calculated in PreChecks
This is not only cleaner but also helps make sure we are always using
the virtual size measure that includes the sigop weight heuristic (which
is the vsize the mempool would return).
2021-10-28 15:58:54 +01:00
glozow
4307849256 [mempool] delete exists(uint256) function
Allowing callers to pass in a uint256 (which could be txid or wtxid)
but then always assuming that it's a txid is a footgunny interface.
2021-10-21 16:26:59 +01:00
Kiminuo
1bdd5f6322 Address review comments from #22918
* fix English in release notes
* Simplify `switch` to `if`.
2021-10-20 10:51:01 +02:00
W. J. van der Laan
986003aff9
Merge bitcoin/bitcoin#22918: rpc: Add level 3 verbosity to getblock RPC call (#21245 modified)
5c34507ecb core_write: Rename calculate_fee to have_undo for clarity (fyquah)
8edf6204a8 release-notes: Add release note about getblock verbosity level 3. (fyquah)
459104b2aa rest: Add test for prevout fields in getblock (fyquah)
4330af6f72 rpc: Add test for level 3 verbosity getblock rpc call. (fyquah)
51dbc167e9 rpc: Add level 3 verbosity to getblock RPC call. (fyquah)
3cc95345ca rpc: Replace boolean argument for tx details with enum class. (fyquah)

Pull request description:

  Author of #21245 expressed [time issues](https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-902332088) in the original PR. Given that #21245 has received a lot of review*, I have decided to open this new pull request with [modifications required to get ACK from luke-jr ](https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-905150806) and a few nits of mine.

  ### Original PR description

  > Display the prevout in transaction inputs when calling getblock level 3 verbosity. This PR affects the existing `/rest/block` API by adding a `prevout` fields to tx inputs. This is mentioned in the change to the release notes.
  >
  > I added some functional tests that
  >
  >     * checks that the RPC call still works when TxUndo can't be found
  >
  >     * Doesn't display the "value" or "scriptPubKey" of the previous output when at a lower verbosity level
  >
  >
  > This "completes" the issue #18771

  ### Possible improvements

  * b0bf4f255f - I can include even this commit to this PR if deemed useful or I can leave it for a follow-up PR. See https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-894853784 for more context.

  ### Examples

  Examples of the `getblock` output with various verbose levels. Note that `000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5` contains only 2 transactions.

  #### Verbose level 0

  ```bash
  ./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 0
  ```

  ##### Verbose level 1

  ```bash
  ./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 1
  ```

  ##### Verbose level 2

  ```bash
  ./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 2
  ```

  ##### Verbose level 3

  ```bash
  ./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 3
  ```

  #### REST

  ```bash
  curl -H "content-type:text/plain;" http://127.0.0.1:18332/rest/block/000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5.json
  ```

  <sub>* ... and my everyday obsessive checking of my email inbox whether the PR moves forward.</sub>

  Edit laanwj: Removed at symbol from message, and large example output to prevent it from all ending up in the commit message.

ACKs for top commit:
  0xB10C:
    ACK 5c34507ecb
  meshcollider:
    utACK 5c34507ecb
  theStack:
    ACK 5c34507ecb 👘
  promag:
    Concept ACK 5c34507ecb

Tree-SHA512: bbff120d8fd76e617b723b102b0c606e0d8eb27f21c631d5f4cdab0892137c4bc7c65b1df144993405f942c91be47a26e80480102af55bff22621c19f518aea3
2021-10-19 15:47:53 +02:00
Samuel Dobson
02a7bdee42 Add error_locations to validateaddress RPC 2021-10-12 12:03:14 +13:00
Russell Yanofsky
6544ea5035 refactor: Block unsafe fs::path std::string conversion calls
There is no change in behavior. This just helps prepare for the
transition from boost::filesystem to std::filesystem by avoiding calls
to methods which will be unsafe after the transaction to std::filesystem
to due lack of a boost::filesystem::path::imbue equivalent and inability
to set a predictable locale.

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Co-authored-by: Kiminuo <kiminuo@protonmail.com>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2021-10-05 11:10:47 -04:00
fyquah
51dbc167e9 rpc: Add level 3 verbosity to getblock RPC call.
Display the prevout in transaction inputs when calling getblock level 3
verbosity.

Co-authored-by: Luke Dashjr <luke_github1@dashjr.org>
Co-authored-by: 0xB10C <19157360+0xB10C@users.noreply.github.com>
2021-10-05 10:42:34 +02:00
fyquah
3cc95345ca rpc: Replace boolean argument for tx details with enum class.
Co-authored-by: Luke Dashjr <luke_github1@dashjr.org>
Co-authored-by: 0xB10C <19157360+0xB10C@users.noreply.github.com>
2021-10-05 10:42:10 +02:00
MarcoFalke
816e15ee81
Merge bitcoin/bitcoin#22951: consensus: move amount.h into consensus
9d0379cea6 consensus: use <cstdint> over <stdint.h> in amount.h (fanquake)
863e52fe63 consensus: make COIN & MAX_MONEY constexpr (fanquake)
d09071da5b [MOVEONLY] consensus: move amount.h into consensus (fanquake)

Pull request description:

  A first step (of a few) towards some source code reorganization, as well as making libbitcoinconsensus slightly more self contained.

  Related to #15732.

ACKs for top commit:
  MarcoFalke:
    concept ACK 9d0379cea6 🏝

Tree-SHA512: 97fc79262dcb8c00996852a288fee69ddf8398ae2c95700bba5b326f1f38ffcfaf8fa66e29d0cb446d9b3f4e608a96525fae0c2ad9cd531ad98ad2a4a687cd6a
2021-10-05 09:43:23 +02:00
W. J. van der Laan
9e530c6352
Merge bitcoin/bitcoin#20487: Add syscall sandboxing using seccomp-bpf (Linux secure computing mode)
4747da3a5b Add syscall sandboxing (seccomp-bpf) (practicalswift)

Pull request description:

  Add experimental syscall sandboxing using seccomp-bpf (Linux secure computing mode).

  Enable filtering of system calls using seccomp-bpf: allow only explicitly allowlisted (expected) syscalls to be called.

  The syscall sandboxing implemented in this PR is an experimental feature currently available only under Linux x86-64.

  To enable the experimental syscall sandbox the `-sandbox=<mode>` option must be passed to `bitcoind`:

  ```
    -sandbox=<mode>
         Use the experimental syscall sandbox in the specified mode
         (-sandbox=log-and-abort or -sandbox=abort). Allow only expected
         syscalls to be used by bitcoind. Note that this is an
         experimental new feature that may cause bitcoind to exit or crash
         unexpectedly: use with caution. In the "log-and-abort" mode the
         invocation of an unexpected syscall results in a debug handler
         being invoked which will log the incident and terminate the
         program (without executing the unexpected syscall). In the
         "abort" mode the invocation of an unexpected syscall results in
         the entire process being killed immediately by the kernel without
         executing the unexpected syscall.
  ```

  The allowed syscalls are defined on a per thread basis.

  I've used this feature since summer 2020 and I find it to be a helpful testing/debugging addition which makes it much easier to reason about the actual capabilities required of each type of thread in Bitcoin Core.

  ---

  Quick start guide:

  ```
  $ ./configure
  $ src/bitcoind -regtest -debug=util -sandbox=log-and-abort
  …
  2021-06-09T12:34:56Z Experimental syscall sandbox enabled (-sandbox=log-and-abort): bitcoind will terminate if an unexpected (not allowlisted) syscall is invoked.
  …
  2021-06-09T12:34:56Z Syscall filter installed for thread "addcon"
  2021-06-09T12:34:56Z Syscall filter installed for thread "dnsseed"
  2021-06-09T12:34:56Z Syscall filter installed for thread "net"
  2021-06-09T12:34:56Z Syscall filter installed for thread "msghand"
  2021-06-09T12:34:56Z Syscall filter installed for thread "opencon"
  2021-06-09T12:34:56Z Syscall filter installed for thread "init"
  …
  # A simulated execve call to show the sandbox in action:
  2021-06-09T12:34:56Z ERROR: The syscall "execve" (syscall number 59) is not allowed by the syscall sandbox in thread "msghand". Please report.
  …
  Aborted (core dumped)
  $
  ```

  ---

  [About seccomp and seccomp-bpf](https://en.wikipedia.org/wiki/Seccomp):

  > In computer security, seccomp (short for secure computing mode) is a facility in the Linux kernel. seccomp allows a process to make a one-way transition into a "secure" state where it cannot make any system calls except exit(), sigreturn(), and read() and write() to already-open file descriptors. Should it attempt any other system calls, the kernel will terminate the process with SIGKILL or SIGSYS. In this sense, it does not virtualize the system's resources but isolates the process from them entirely.
  >
  > […]
  >
  > seccomp-bpf is an extension to seccomp that allows filtering of system calls using a configurable policy implemented using Berkeley Packet Filter rules. It is used by OpenSSH and vsftpd as well as the Google Chrome/Chromium web browsers on Chrome OS and Linux. (In this regard seccomp-bpf achieves similar functionality, but with more flexibility and higher performance, to the older systrace—which seems to be no longer supported for Linux.)

ACKs for top commit:
  laanwj:
    Code review and lightly tested ACK 4747da3a5b

Tree-SHA512: e1c28e323eb4409a46157b7cc0fc29a057ba58d1ee2de268962e2ade28ebd4421b5c2536c64a3af6e9bd3f54016600fec88d016adb49864b63edea51ad838e17
2021-10-04 22:45:43 +02:00
practicalswift
4747da3a5b Add syscall sandboxing (seccomp-bpf) 2021-10-01 13:51:10 +00:00
practicalswift
4343f114cc Replace use of locale dependent atoi(…) with locale-independent std::from_chars(…) (C++17)
test: Add test cases for LocaleIndependentAtoi

fuzz: Assert legacy atoi(s) == LocaleIndependentAtoi<int>(s)

fuzz: Assert legacy atoi64(s) == LocaleIndependentAtoi<int64_t>(s)
2021-09-30 14:21:17 +00:00
fanquake
d09071da5b
[MOVEONLY] consensus: move amount.h into consensus
Move amount.h to consensus/amount.h.
Renames, adds missing and removes uneeded includes.
2021-09-30 07:41:57 +08:00
Andrew Chow
a99ed89865 psbt: sign without finalizing
We don't always want to finalize after signing, so make it possible to
do that.
2021-09-28 19:13:42 -04:00
Samuel Dobson
b55232a337
Merge bitcoin/bitcoin#22722: rpc: update estimatesmartfee to return max of CBlockPolicyEstimator::estimateSmartFee, mempoolMinFee and minRelayTxFee
ea31caf6b4 update estimatesmartfee rpc to return max of estimateSmartFee, mempoolMinFee and minRelayTxFee. (pranabp-bit)

Pull request description:

  This PR is in response to the issue [#19699](https://github.com/bitcoin/bitcoin/issues/19699).

  Based on the discussion in the comments of PR [#22673](https://github.com/bitcoin/bitcoin/pull/22673) changes have been made in the `estimatesmartfee` itself such that it takes into account `mempoolMinFee` and `relayMinFee` . Hence it provides a fee estimate that is most likely to be paid by the user in an actual transaction, preventing issues such as [#16072](https://github.com/bitcoin/bitcoin/issues/16072).

  The test file test/functional/feature_fee_estimation.py has also been updated to check this functionality.

ACKs for top commit:
  meshcollider:
    re-utACK ea31caf6b4

Tree-SHA512: 8f36153a07cbd552c5c13d11d9c6e987a7a555ea4cc83f2573c0c92dd97c706d90c30a7248671437c2f3a836d3272f8fad53d15a5fa6efaa0409ae8009b0a18d
2021-09-29 10:55:29 +13:00
Samuel Dobson
d6492d4ed0
Merge bitcoin/bitcoin#22650: Remove -deprecatedrpc=addresses flag and corresponding code/logic
43cd6b8af9 doc: add release notes for removal of the -deprecatedrpc=addresses flag (Michael Dietz)
2b1fdc2c6c refactor: minor styling, prefer snake case and same line if (Michael Dietz)
d64deac7b8 refactor: share logic between ScriptPubKeyToUniv and ScriptToUniv (Michael Dietz)
8721638daa rpc: remove deprecated addresses and reqSigs from rpc outputs (Michael Dietz)

Pull request description:

  Resolves #21797 now that we've branched-off to v23 ("addresses" and "reqSigs" deprecated) "ExtractDestinations" should be removed.

   `-deprecatedrpc=addresses` was initially added in this PR #20286 (which resolved the original issue #20102).

  Some chunks of code and logic are no longer used/necessary with the removal of this, and therefore some minor refactoring is done in this PR as well (separated commits)

ACKs for top commit:
  MarcoFalke:
    re-ACK 43cd6b8af9 🐉
  meshcollider:
    Code review ACK 43cd6b8af9
  jonatack:
    ACK 43cd6b8af9 per `git range-diff a9d0cec 92dc5e9 43cd6b8`, also rebased to latest master, debug built + quick re-review of each commit to bring back context, and ran tests locally at the final commit

Tree-SHA512: fba83495e396d3c06f0dcf49292f14f4aa6b68fa758f0503941fade1a6e7271cda8378e2734af1faea550d1b43c85a36c52ebcc9dec0732936f9233b4b97901c
2021-09-29 10:41:30 +13:00
pranabp-bit
ea31caf6b4 update estimatesmartfee rpc to return max of estimateSmartFee, mempoolMinFee and minRelayTxFee.
This will provide better estimates which would be closer to fee paid in actual
transactions.
The test has also been changed such that when the node is restarted with a
high mempoolMinFee, the estimatesmartfee still returns a feeRate greater
than or equal to the mempoolMinFee, minRelayTxFee.(just like the feeRate of actual transactions)
2021-09-28 18:36:38 +05:30
Michael Dietz
2b1fdc2c6c
refactor: minor styling, prefer snake case and same line if 2021-09-24 14:22:53 -05:00
Michael Dietz
d64deac7b8
refactor: share logic between ScriptPubKeyToUniv and ScriptToUniv 2021-09-24 14:22:53 -05:00
Michael Dietz
8721638daa
rpc: remove deprecated addresses and reqSigs from rpc outputs 2021-09-24 14:22:49 -05:00
John Newbery
0220b834b1 [test] Add testing for outbound feeler connections
Extend the addconnection RPC method to allow creating outbound
feeler connections. Extend the test framework to accept those
feeler connections.
2021-09-22 16:12:14 +01:00
MarcoFalke
fa10fbc665
doc: Fix RPC result documentation 2021-09-21 11:13:29 +02:00
merge-script
223ad2fd0d
Merge bitcoin/bitcoin#22831: test: add addpeeraddress "tried", test addrman checks on restart with asmap
cdaab90662 Add test for addrman consistency check on restart with asmap (Jon Atack)
869f136816 Add test for rpc addpeeraddress with "tried" argument (Jon Atack)
ef242f5213 Allow passing "tried" to rpc addpeeraddress to call CAddrMan::Good() (Jon Atack)

Pull request description:

  This pull adds a `tried` argument to RPC addpeeraddress and a regression test for the recent addrman/asmap changes and issue.

  PR #22697 introduced a reproducible bug in commit 181a1207 that fails addrman consistency checks and causes it to significantly lose peer entries when the `-asmap` configuration option is used.

  The issue occurs upon bitcoind restart due to an initialization order change in `src/init.cpp` in that commit, whereby CAddrman asmap is set after deserializing `peers.dat`, rather than before.

  Issue reported on the `#bitcoin-core-dev` IRC channel starting at https://www.erisian.com.au/bitcoin-core-dev/log-2021-08-23.html#l-263.

  ```
  addrman lost 22813 new and 2 tried addresses due to collisions or invalid addresses
  ADDRMAN CONSISTENCY CHECK FAILED!!! err=-17 bitcoind: ./addrman.h:707: void CAddrMan::Check() const: Assertion `false' failed. Aborted
  ```

  How to reproduce:

  - `git checkout 181a1207`, build, and launch bitcoind with the `-asmap` and `-checkaddrman=1` configuration options enabled
  - restart bitcoind
  - bitcoind aborts on the second call to the addrman consistency checks in `CAddrMan::Check()`

  How to test this pull:

  - `git checkout 181a1207`, cherry pick the first commit of this branch, build, git checkout this branch, run `test/functional/rpc_net.py`, which should pass, and then run `test/functional/feature_asmap.py`, which should fail with the following output:

  ```
  AssertionError: Unexpected stderr bitcoind: ./addrman.h:739: void CAddrMan::Check() const: Assertion `false' failed.
  ```

ACKs for top commit:
  jnewbery:
    utACK cdaab90662
  mzumsande:
    re-ACK cdaab90662 (based on code review of diff to d586817)
  vasild:
    ACK cdaab90662

Tree-SHA512: 0251a18fea629b62486fc907d7ab0e96c6df6fadb9e4d62cff018bc681afb6ac31e0e7258809c0a88f91e4a36c4fb0b16ed294ce47ef30585217de89c3342399
2021-09-21 09:34:28 +02:00
fanquake
528e08119f
Merge bitcoin/bitcoin#22219: multiprocess: Start using init makeNode, makeChain, etc methods
e4709c7b56 Start using init makeNode, makeChain, etc methods (Russell Yanofsky)

Pull request description:

  Use `interfaces::Init::make*` methods instead of `interfaces::Make*` functions, so interfaces can be constructed differently in different executable without having to change any code. (So for example `bitcoin-gui` can make an `interfaces::Node` pointer that communicates with a `bitcoin-node` subprocess, while `bitcoin-qt` can make an `interfaces::Node` pointer that controls node code in the same process.)

  ---

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102.

ACKs for top commit:
  jamesob:
    reACK e4709c7b56
  achow101:
    ACK e4709c7b56
  benthecarman:
    utACK e4709c7b56

Tree-SHA512: 580c1979dbb2ef444157c8e53041e70d15ddeee77e5cbdb34f70b6d228cc2d2fe3843825f172da84e506200c58f7e0932f7cd4c006bb5058c1f4e43259394834
2021-09-16 08:47:38 +08:00
Jon Atack
ef242f5213
Allow passing "tried" to rpc addpeeraddress to call CAddrMan::Good()
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
Co-authored-by: John Newbery <john@johnnewbery.com>
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2021-09-15 15:28:36 +02:00
Russell Yanofsky
93b9800fec scripted-diff: Rename overloaded int GetArg to GetIntArg
Improve readability of code, simplify future scripted diff cleanup PRs, and be
more consistent with naming for GetBoolArg.

This will also be useful for replacing runtime settings type checking
with compile time checking.

-BEGIN VERIFY SCRIPT-
git grep -l GetArg | xargs sed -i 's/GetArg(\([^)]*\( [0-9]\+\|-1\|port\|BaseParams().RPCPort()\|Params().GetDefaultPort()\|_TIMEOUT\|Height\|_WORKQUEUE\|_THREADS\|_CONNECTIONS\|LIMIT\|SigOp\|Bytes\|_VERSION\|_AGE\|_CHECKS\|Checks() ? 1 : 0\|_BANTIME\|Cache\|BLOCKS\|LEVEL\|Weight\|Version\|BUFFER\|TARGET\|WEIGHT\|TXN\|TRANSACTIONS\|ADJUSTMENT\|i64\|Size\|nDefault\|_EXPIRY\|HEIGHT\|SIZE\|SNDHWM\|_TIME_MS\)\))/GetIntArg(\1)/g'
-END VERIFY SCRIPT-

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2021-09-27 06:57:20 -04:00
W. J. van der Laan
09cb5ec6c8
Merge bitcoin/bitcoin#23065: Allow UTXO locks to be written to wallet DB
d96b000e94 Make GUI UTXO lock/unlock persistent (Samuel Dobson)
077154fe69 Add release note for lockunspent change (Samuel Dobson)
719ae927dc Update lockunspent tests for lock persistence (Samuel Dobson)
f13fc16295 Allow lockunspent to store the lock in the wallet DB (Samuel Dobson)
c52789365e Allow locked UTXOs to be store in the wallet database (Samuel Dobson)

Pull request description:

  Addresses and closes #22368

  As per that issue (and its predecessor #14907), there seems to be some interest in allowing unspent outputs to be locked persistently. This PR does so by adding a flag to lockunspent to store the change in the wallet database. Defaults to false, so there is no change in default behaviour.

  Edit: GUI commit changes default behaviour. UTXOs locked/unlocked via the GUI are now persistent.

ACKs for top commit:
  achow101:
    ACK d96b000e94
  kristapsk:
    ACK d96b000e94
  lsilva01:
    Tested ACK d96b000e94 on Ubuntu 20.04
  prayank23:
    ACK d96b000e94

Tree-SHA512: 957a5bbfe7f763036796906ccb1598feb6c14c5975838be1ba24a198840bf59e83233165cb112cebae909b6b25bf27275a4d7fa425923ef6c788ff671d7a89a8
2021-09-26 11:30:18 +02:00
Samuel Dobson
f13fc16295 Allow lockunspent to store the lock in the wallet DB 2021-09-25 23:50:06 +12:00
MarcoFalke
dd097c42df
Merge bitcoin/bitcoin#18448: rpc: fix/add missing RPCExamples for "Util" RPCs
ea98d9c2ef rpc: fix/add missing RPCExamples for "Util" RPCs (Sebastian Falbesoner)

Pull request description:

  Similar to https://github.com/bitcoin/bitcoin/pull/18398, this PR gives the RPCExamples in the RPC category "Util" (that currently contains `createmultisig`, `deriveaddresses`, `estimatesmartfee`, `getdescriptorinfo`, `signmessagewithprivkey`, `validateaddress`, `verifymessage`) some love by fixing one broken and adding three missing examples:
  - fixed `HelpExampleRpc` for `createmultisig` (disturbing escape characters and quotation marks)
  - added missing `HelpExampleRpc` for
      -  `deriveaddresses` (also put descriptor in a new string constant)
      - `estimatesmartfee`
      - `getdescriptorinfo` (also put descriptor in a new string constant)

  Output for `createmultisig` example on the master branch:
  ```
  $ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, "[\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\",\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\"]"]}' -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 array as expected"},"id":"curltest"}
  ```

  Output for `createmultisig` example on the PR branch:
  ```
  $ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, ["03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd","03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626"]]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
  Enter host password for user '__cookie__':
  {"result":{"address":"3QsFXpFJf2ZY6GLWVoNFFd2xSDwdS713qX","redeemScript":"522103789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd2103dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a6162652ae","descriptor":"sh(multi(2,03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd,03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626))#4djp057k"},"error":null,"id":"curltest"}
  ```

ACKs for top commit:
  jonatack:
    ACK ea98d9c2ef looked at the code, rebased to master, ran the helps, did not try running the added json-rpc examples

Tree-SHA512: d6ecb6da66f19517065453357d210102e2cc9f1f8037aeb6a9177ff036d0c21773dddf5e0acdbc71edbbde3026e4d1e7ce7c0935cd3e023c60f34e1b173b3299
2021-09-02 18:04:54 +02:00
MarcoFalke
fa9eade142
Remove GetAddrName
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html#c131-avoid-trivial-getters-and-setters
2021-08-26 10:44:26 +02:00
Russell Yanofsky
e4709c7b56 Start using init makeNode, makeChain, etc methods
Use interfaces::Init::make* methods instead of interfaces::Make*
functions, so interfaces can be constructed differently in different
executables without having to change any code. (So for example
bitcoin-gui can make an interfaces::Node pointer that communicates with
a bitcoin-node subprocess, while bitcoin-qt can make an interfaces::Node
pointer that starts node code in the same process.)
2021-08-17 03:05:15 -05:00
Amiti Uttarwar
60e0cbdd57 [addrman] Merge the two Add() functions
Merge the two definitions of this overloaded function to reduce code
duplication.
2021-08-15 22:35:13 -07:00
Samuel Dobson
502d22ceed
Merge bitcoin/bitcoin#22541: Add a new RPC command: restorewallet
5fe8100ff3 Change the wallet_backup.py test to use the restorewallet RPC command instead of restoring wallets manually. (lsilva01)
ae23faba6f Add a new RPC command: restorewallet (lsilva01)

Pull request description:

  As far as I know, there is no command to restore the wallet from a backup file.
  The only way to do this is to replace the `wallet.dat` of a newly created wallet with the backup file, which is hardly an intuitive way.

  This PR implements the `restorewallet` RPC command which restores the wallet from the backup file.

  To test:
  First create a backup file:
  `$ bitcoin-cli -rpcwallet="wallet-01" backupwallet /home/Backups/wallet-01.bak`

  Then restore it in another wallet:
  `$ bitcoin-cli  restorewallet "restored-wallet-01" /home/Backups/wallet-01.bak`

ACKs for top commit:
  achow101:
    re-ACK 5fe8100ff3
  prayank23:
    tACK 5fe8100ff3
  meshcollider:
    utACK 5fe8100ff3

Tree-SHA512: 9639df4d8ad32f255f5b868320dc69878bd9aceb3b471b49dfad500b67681e2d354292b5410982fbf18e25a44ed0c06fd4a0dd010e82807c2e00ff32e84047a1
2021-08-15 16:58:05 +12:00
fanquake
4c87665707
Merge bitcoin/bitcoin#22604: p2p, rpc, test: address rate-limiting follow-ups
d930c7f5b0 p2p, rpc, test: address rate-limiting follow-ups (Jon Atack)

Pull request description:

  Incorporates review feedback in #22387.

  Edit, could be considered separately: should a release note (or two) be added for 22.0? e.g. the new getpeerinfo fields in `Updated RPCs` and the rate-limiting itself in `P2P and network changes`.

ACKs for top commit:
  MarcoFalke:
    review ACK d930c7f5b0
  theStack:
    re-ACK d930c7f5b0 🌮
  Zero-1729:
    crACK d930c7f

Tree-SHA512: b2101cad87f59c238603f38bd8e8df7a4d48929794e4de9e0e0ff2afa935a68475c2d369aa669d124a0bec2f50280fb47e8b980bde6ad812db08cf67b71c066a
2021-08-14 13:33:47 +08:00
fanquake
c3545a7396
Merge bitcoin/bitcoin#22653: refactor: Rename JoinErrors and re-use it
bb56486a17 refactor: Reuse MakeUnorderedList where possible (Hennadii Stepanov)
77a90f03ac refactor: Move MakeUnorderedList into util/string.h to make it reusable (Hennadii Stepanov)
6a5ccd65c7 scripted-diff: Rename JoinErrors in more general MakeUnorderedList (Hennadii Stepanov)

Pull request description:

  A nice `JoinErrors` utility function was introduced in https://github.com/bitcoin-core/gui/pull/379 by Russell Yanofsky.

  This PR renames this function and re-uses it across the code base.

ACKs for top commit:
  Zero-1729:
    Concept ACK bb56486a17
  theStack:
    Code-review ACK bb56486a17
  Talkless:
    utACK bb56486a17
  ryanofsky:
    Code review ACK bb56486a17. Nice deduping, thanks for this!

Tree-SHA512: 6bdbfa61f2ffa69e075f46b733f247c6d5b8486779a1dac064285a199a4bb8bc5ef44eaee37086305646b5c88eb6a11990883219a4a9140a5117ee21ed529bb9
2021-08-11 09:56:34 +08:00
lsilva01
ae23faba6f Add a new RPC command: restorewallet 2021-08-10 22:44:54 -03:00
fanquake
21438d55d5
Merge bitcoin/bitcoin#21800: mempool/validation: mempool ancestor/descendant limits for packages
accf3d5868 [test] mempool package ancestor/descendant limits (glozow)
2b6b26e57c [test] parameterizable fee for make_chain and create_child_with_parents (glozow)
313c09f7b7 [test] helper function to increase transaction weight (glozow)
f8253d69d6 extract/rename helper functions from rpc_packages.py (glozow)
3cd663a5d3 [policy] ancestor/descendant limits for packages (glozow)
c6e016aa13 [mempool] check ancestor/descendant limits for packages (glozow)
f551841d3e [refactor] pass size/count instead of entry to CalculateAncestorsAndCheckLimits (glozow)
97dd1c729d MOVEONLY: add helper function for calculating ancestors and checking limits (glozow)
f95bbf58aa misc package validation doc improvements (glozow)

Pull request description:

  This PR implements a function to calculate mempool ancestors for a package and enforces ancestor/descendant limits on them as a whole. It reuses a portion of `CalculateMemPoolAncestors()`; there's also a small refactor to move the reused code into a generic helper function. Instead of calculating ancestors and descendants on every single transaction in the package and their ancestors, we use a "worst case" heuristic, treating every transaction in the package as each other's ancestor and descendant. This may overestimate everyone's counts, but is still pretty accurate in the our main package use cases, in which at least one of the transactions in the package is directly related to all the others (e.g. 1 parent + 1 child, multiple parents with 1 child, or chains).

  Note on Terminology: While "package" is often used to describe groups of related transactions _within_ the mempool, here, I only use package to mean the group of not-in-mempool transactions we are currently validating.

  #### Motivation

  It would be a potential DoS vector to allow submission of packages to mempool without a proper guard for mempool ancestors/descendants. In general, the purpose of mempool ancestor/descendant limits is to limit the computational complexity of dealing with families during removals and additions. We want to be able to validate multiple transactions on top of the mempool, but also avoid these scenarios:

  - We underestimate the ancestors/descendants during package validation and end up with extremely complex families in our mempool (potentially a DoS vector).
  - We expend an unreasonable amount of resources calculating everyone's ancestors and descendants during package validation.

ACKs for top commit:
  JeremyRubin:
    utACK accf3d5
  ariard:
    ACK accf3d5.

Tree-SHA512: 0d18ce4b77398fe872e0b7c2cc66d3aac2135e561b64029584339e1f4de2a6a16ebab3dd5784f376e119cbafc4d50168b28d3bd95d0b3d01158714ade2e3624d
2021-08-09 12:23:39 +08:00
Samuel Dobson
b1a672d158
Merge bitcoin/bitcoin#22337: wallet: Use bilingual_str for errors
92993aa5cf Change SignTransaction's input_errors to use bilingual_str (Andrew Chow)
171366e89b Use bilingual_str for address fetching functions (Andrew Chow)
9571c69b51 Add bilingual_str::clear() (Andrew Chow)

Pull request description:

  In a couple of places in the wallet, errors are `std::string`. In order for these errors to be translated, change them to use `bilingual_str`.

ACKs for top commit:
  hebasto:
    re-ACK 92993aa5cf, only rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/22337#pullrequestreview-694542729) review, verified with
  klementtan:
    Code review ACK 92993aa5cf
  meshcollider:
    Code review ACK 92993aa5cf

Tree-SHA512: 5400e419dd87db8c49b67ed0964de2d44b58010a566ca246f2f0760ed9ef6a9b6f6df7a6adcb211b315b74c727bfe8c7d07eb5690b5922fa5828ceef4c83461f
2021-08-09 14:45:12 +12:00
Samuel Dobson
8fa03c4ddf
Merge bitcoin/bitcoin#21500: wallet, rpc: add an option to list private descriptors
bb822a7af8 wallet, rpc: add listdescriptors private option (S3RK)

Pull request description:

  Rationale: make it possible to backup your wallet with `listdescriptors` command

  * The default behaviour is still to show public version
  * For private version only the root xprv is returned

  Example use-case:
  ```
  > bitcoin-cli -regtest -named createwallet wallet_name=old descriptors=true
  > bitcoin-cli -regtest -rpcwallet=old listdescriptors true | jq '.descriptors' > descriptors.txt

  > bitcoin-cli -regtest -named createwallet wallet_name=new descriptors=true blank=true
  > bitcoin-cli -regtest -rpcwallet=new importdescriptors "$(cat descriptors.txt)"
  ```

  In case of watch-only wallet without private keys there will be following output:
  ```
  error code: -4
  error message:
  Can't get descriptor string.
  ```

ACKs for top commit:
  achow101:
    re-ACK bb822a7af8
  Rspigler:
    tACK bb822a7af8
  jonatack:
    ACK bb822a7af8 per `git diff 2854ddc bb822a7`
  prayank23:
    tACK bb822a7af8
  meshcollider:
    Code review ACK bb822a7af8

Tree-SHA512: f6dddc72a74e5667071ccd77f8dce578382e8e29e7ed6a0834ac2e114a6d3918b59c2f194f4079b3259e13d9ba3b4f405619940c3ecb7a1a0344615aed47c43d
2021-08-09 14:09:07 +12:00
Hennadii Stepanov
bb56486a17
refactor: Reuse MakeUnorderedList where possible 2021-08-06 22:08:26 +03:00