Commit graph

1592 commits

Author SHA1 Message Date
MarcoFalke
fa0aa87071
rpc: Return wtxid from testmempoolaccept 2021-01-12 18:43:43 +01:00
Sebastian Falbesoner
fc726e0138 doc, rpc: add missing signet mentions in network name lists 2021-01-12 18:43:31 +01:00
Wladimir J. van der Laan
7b975639ef
Merge : signet mining utility
595a34dbea contrib/signet: Document miner script in README.md (Anthony Towns)
ff7dbdc08a contrib/signet: Add script for generating a signet chain (Anthony Towns)
13762bcc96 Add bitcoin-util command line utility (Anthony Towns)
95d5d5e625 rpc: allow getblocktemplate for test chains when unconnected or in IBD (Anthony Towns)
81c54dec20 rpc: update getblocktemplate with signet rule, include signet_challenge (Anthony Towns)

Pull request description:

  Adds `contrib/signet/miner` for mining signet blocks.

  Adds `bitcoin-util` cli utility, with the idea being it can provide bitcoin related functionality that does not rely on the ability to access a running node. Only subcommand currently is "grind" which takes a hex-encoded header and grinds its nonce until its nBits is satisfied.

  Updates `getblocktemplate` to include `signet_challenge` field, and makes `getblocktemplate` require the signet rule when invoked on the signet change. Removes connectivity and IBD checks from `getblocktemplate` when applied to a test chain (regtest, testnet, signet).

ACKs for top commit:
  laanwj:
    code review ACK 595a34dbea

Tree-SHA512: 8d43297710fdc1edc58acd9b53e1bd1671e5724f7097b40ab73653715dc8becc70534c4496cbba9290f4dd6538a7a3d5830eb85f83391ea31a3bb5b9d3378cc3
2021-01-12 12:53:45 +01:00
MarcoFalke
6af013792f
Merge : [tests] Allow outbound & block-relay-only connections in functional tests.
b4dd2ef800 [test] Test the add_outbound_p2p_connection functionality (Amiti Uttarwar)
602e69e427 [test] P2PBlocksOnly - Test block-relay-only connections. (Amiti Uttarwar)
8bb6beacb1 [test/refactor] P2PBlocksOnly - Extract transaction violation test into helper. (Amiti Uttarwar)
99791e7560 [test/refactor] P2PBlocksOnly - simplify transaction creation using blocktool helper. (Amiti Uttarwar)
3997ab9154 [test] Add test framework support to create outbound connections. (Amiti Uttarwar)
5bc04e8837 [rpc/net] Introduce addconnection to test outbounds & blockrelay (Amiti Uttarwar)

Pull request description:

  The existing functional test framework uses the `addnode` RPC to spin up manual connections between bitcoind nodes. This limits our ability to add integration tests for our networking code, which often executes different code paths for different connection types.

  **This PR enables creating `outbound` & `block-relay-only` P2P connections in the functional tests.** This allows us to increase our p2p test coverage, since we can now verify expectations around these connection types.

  This builds out the [prototype](https://github.com/bitcoin/bitcoin/issues/14210#issuecomment-527421978) proposed by ajtowns in . 🙌🏽

  An overview of this branch:
  - introduces a new test-only RPC function `addconnection` which initiates opening an `outbound` or `block-relay-only` connection. (conceptually similar to `addnode` but for different connection types & restricted to regtest)
  - adds `test_framework` support so a mininode can open an `outbound`/`block-relay-only` connection to a `P2PInterface`/`P2PConnection`.
  - updates `p2p_blocksonly` tests to create a `block-relay-only` connection & verify expectations around transaction relay.
  - introduces `p2p_add_connections` test that checks the behaviors of the newly introduced `add_outbound_p2p_connection` test framework function.

  With these changes, there are many more behaviors that we can add integration tests for. The blocksonly updates is just one example.

  Huge props to ajtowns for conceiving the approach & providing me feedback as I've built out this branch. Also thank you to jnewbery for lots of thoughtful input along the way.

ACKs for top commit:
  troygiorshev:
    reACK b4dd2ef800
  jnewbery:
    utACK b4dd2ef800
  MarcoFalke:
    Approach ACK b4dd2ef800 🍢

Tree-SHA512: d1cba768c19c9c80e6a38b1c340cc86a90701b14772c4a0791c458f9097f6a4574b4a4acc7d13d6790c7b1f1f197e2c3d87996270f177402145f084ef8519a6b
2021-01-11 21:06:57 +01:00
fanquake
6d81d7aa87
Merge : Use C++17 std::array deduction for OUTPUT_TYPES, ALL_FEE_ESTIMATE_HORIZONS
aaaa987840 refactor: Use C++17 std::array deduction for ALL_FEE_ESTIMATE_HORIZONS (MarcoFalke)
fa39cdd072 refactor: Use C++17 std::array deduction for OUTPUT_TYPES (MarcoFalke)

Pull request description:

  With the new C++17 array deduction rules, an array encompassing all values in an enum can be specified in the same header file that specifies the enum. This is useful to avoid having to repeatedly enumerate all enum values in the code. E.g. the RPC code, but also the fuzz code.

ACKs for top commit:
  theStack:
    cr ACK aaaa987840 ⚙️
  fanquake:
    ACK aaaa987840

Tree-SHA512: b71bd98f3ca07ddfec385735538ce89a4952e418b52dc990fb160187ccef1fc7ebc139d42988b6f7b48df24823af61f803b83d47fb7a3b82475f0c0b109bffb7
2021-01-11 21:46:09 +08:00
fanquake
bd6af53e1f
Merge : Replace boost::variant with std::variant
faa8f68943 Replace boost::variant with std::variant (MarcoFalke)

Pull request description:

  Now that we can use std::variant from the vanilla standard library, drop the third-party boost variant dependency

ACKs for top commit:
  fjahr:
    Code review ACK faa8f68943
  fanquake:
    ACK faa8f68943

Tree-SHA512: 6e3aecd33b00c2e31a763f999247944d5b2ce5e3018f1965c516c1000cd08ff6703a8d50fb0be64883153da2925ae72986b8a6b96586db74057bd05d6f4986e6
2021-01-11 12:05:46 +08:00
Jon Atack
06ba9b3008
rpc: move getpeerinfo connection_type help to correct place
per review feedback
2021-01-09 13:50:00 +01:00
Amiti Uttarwar
5bc04e8837 [rpc/net] Introduce addconnection to test outbounds & blockrelay
Add a new RPC endpoint to enable opening outbound connections from
the tests. The functional test framework currently uses the addnode RPC, which
has different behavior than general outbound peers. These changes enable
creating both full-relay and block-relay-only connections. The new RPC
endpoint calls through to a newly introduced AddConnection method on
CConnman that ensures we stay within the allocated max.
2021-01-07 10:15:56 -08:00
Jon Atack
faecb74562
Expose integral m_conn_type in CNodeStats, remove m_conn_type_string 2021-01-07 15:18:44 +01:00
MarcoFalke
f13e03cda2
Merge : Declare de facto const reference variables/member functions as const
31b136e580 Don't declare de facto const reference variables as non-const (practicalswift)
1c65c075ee Don't declare de facto const member functions as non-const (practicalswift)

Pull request description:

  _Meta: This is the second and final part of the `const` refactoring series (part one: ). **I promise: no more refactoring PRs from me in a while! :)** I'll now go back to focusing on fuzzing/hardening!_

  Changes in this PR:
  * Don't declare de facto const member functions as non-const
  * Don't declare de facto const reference variables as non-const

  Awards for finding candidates for the above changes go to:
  * `clang-tidy`'s [`readability-make-member-function-const`](https://clang.llvm.org/extra/clang-tidy/checks/readability-make-member-function-const.html)  check ([list of `clang-tidy` checks](https://clang.llvm.org/extra/clang-tidy/checks/list.html))
  * `cppcheck`'s `constVariable` check ([list of `cppcheck` checks](https://sourceforge.net/p/cppcheck/wiki/ListOfChecks/))

  See  for instructions on how to analyse Bitcoin Core using Clang Static Analysis, `clang-tidy` and `cppcheck`.

ACKs for top commit:
  ajtowns:
    ACK 31b136e580
  jonatack:
    ACK 31b136e580
  theStack:
    ACK 31b136e580 ❄️

Tree-SHA512: f58f8f00744219426874379e9f3e9331132b9b48e954d24f3a85cbb858fdcc98009ed42ef7e7b4619ae8af9fc240a6d8bfc1c438db2e97b0ecd722a80dcfeffe
2021-01-07 09:05:09 +01:00
MarcoFalke
faa8f68943
Replace boost::variant with std::variant 2021-01-05 10:10:50 +01:00
MarcoFalke
aaaa987840
refactor: Use C++17 std::array deduction for ALL_FEE_ESTIMATE_HORIZONS 2021-01-03 18:38:31 +01:00
MarcoFalke
fa749fbea3
rpc: Replace boost::variant with std::variant for RPCArg.m_fallback 2021-01-01 15:08:07 +01:00
MarcoFalke
fa0074e2d8
scripted-diff: Bump copyright headers
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
2020-12-31 09:45:41 +01:00
MarcoFalke
fa29272459
Remove redundant MakeUCharSpan wrappers 2020-12-31 09:06:00 +01:00
Wladimir J. van der Laan
d875bcc8f9
Merge : Add network to peers window and peer details
e262a19b0b gui: display network in peer details (Jon Atack)
9136953073 gui: rename peer tab column headers, initialize in .h (Hennadii Stepanov)
05c08c696a gui: add network column in peers tab/window (Jon Atack)
e0e55060bf gui: fix broken doxygen formatting in src/qt/guiutil.h (Jon Atack)
0d5613f9de gui: create GUIUtil::NetworkToQString() utility function (Jon Atack)
af9103cc79 net, rpc: change CNodeStats::m_network from string to Network (Jon Atack)

Pull request description:

  and rename peers window column headers from NodeId and Node/Service to Peer Id and Address.

  ![Screenshot from 2020-12-27 14-45-31](https://user-images.githubusercontent.com/2415484/103172228-efec8600-4849-11eb-8cee-04a3d2ab1273.png)

ACKs for top commit:
  laanwj:
    ACK e262a19b0b

Tree-SHA512: 709c2a805c109c2dd033aca7b6b6dc94ebe2ce7a0168c71249e1e661c9c57d1f1c781a5b9ccf3b776bedeb83ae2fb5c505637337c45b1eb9a418cb1693a89761
2020-12-28 23:56:23 +01:00
Jon Atack
af9103cc79
net, rpc: change CNodeStats::m_network from string to Network 2020-12-27 14:28:31 +01:00
Amiti Uttarwar
b1a936d4ae [rpc] Remove deprecated "whitelisted" field from getpeerinfo 2020-12-26 13:30:54 -08:00
Amiti Uttarwar
094c3beaa4 [rpc] Remove deprecated "banscore" field from getpeerinfo 2020-12-26 13:30:08 -08:00
Amiti Uttarwar
537053336f [rpc] Remove deprecated "addnode" field from getpeerinfo 2020-12-26 13:30:08 -08:00
MarcoFalke
02cf20b9f5
Merge : [doc] Add missing field (permissions) to the getpeerinfo help
667d203687 [doc] Add permissions to the getpeerinfo help. (Amiti Uttarwar)

Pull request description:

  This field was previously being returned, but missing from the RPCHelpMan. This PR uses the existing `NET_PERMISSIONS_DOC` to inform RPC users about this field.

  ```
     "permissions" : [                 (json array) Any special permissions that have been granted to this peer
        "str",                          (string) bloomfilter (allow requesting BIP37 filtered blocks and transactions),
                                        noban (do not ban for misbehavior; implies download),
                                        forcerelay (relay transactions that are already in the mempool; implies relay),
                                        relay (relay even in -blocksonly mode, and unlimited transaction announcements),
                                        mempool (allow requesting BIP35 mempool contents),
                                        download (allow getheaders during IBD, no disconnect after maxuploadtarget limit),
                                        addr (responses to GETADDR avoid hitting the cache and contain random records with the most up-to-date info).

        ...
      ],

  ```

ACKs for top commit:
  Sjors:
    tACK 667d203687

Tree-SHA512: 973631b41d35d6333e3cb06b35277de869110f6ad6498c7e74f00c75202e8de1788a48755c21ac964903e5e6050a5e769a63866211aec9004cd665a727a54a3c
2020-12-26 21:26:12 +01:00
MarcoFalke
f656165e9c
Merge : rpc: calculate fees in getblock using BlockUndo data
66d012ad7f test: RPC: getblock fee calculations (Elliott Jin)
bf7d6e31b1 RPC: getblock: tx fee calculation for verbosity 2 via Undo data (Elliott Jin)

Pull request description:

  This change is progress towards  .  It adapts the fee calculation part of  and addresses some feedback.  The additional "verbosity level 3" features are planned for a future PR.

  **Original PR description:**

  > Using block undo data (like in ) we can now show fee information for each transaction in a block without the need for additional -txindex and/or a ton of costly lookups. For a start we'll add transaction fee information to getblock verbosity level 2. This comes at a negligible speed penalty (<1%).

ACKs for top commit:
  luke-jr:
    tACK 66d012ad7f
  fjahr:
    tACK 66d012ad7f
  MarcoFalke:
    review ACK 66d012ad7f 🗜

Tree-SHA512: be1fe4b866946a8dc36427f7dc72a20e10860e320a28fa49bc85bd2a93a0d699768179be29fa52e18b2ed8505d3ec272e586753ef2239b4230e0aefd233acaa2
2020-12-24 15:32:10 +01:00
Amiti Uttarwar
667d203687 [doc] Add permissions to the getpeerinfo help.
This field was already being returned, but the RPCHelpMan did not indicate
this. So, this PR updates the help text to match.
2020-12-23 11:38:59 -08:00
MarcoFalke
df127ecede
Merge : net processing: Move block inventory state to net_processing
3002b4af2b [net processing] Guard m_continuation_block with m_block_inv_mutex (John Newbery)
184557e8e0 [net processing] Move hashContinue to net processing (John Newbery)
c853ef002e scripted-diff: rename vBlockHashesToAnnounce and vInventoryBlockToSend (John Newbery)
53b7ac1b7d [net processing] Move block inventory data to Peer (John Newbery)
78040f9168 [net processing] Rename nStartingHeight to m_starting_height (John Newbery)
77a2c2f8f9 [net processing] Move nStartingHeight to Peer (John Newbery)
717a374e74 [net processing] Improve documentation for Peer destruction/locking (John Newbery)

Pull request description:

  This continues the work of moving application layer data into net_processing, by moving all block inventory state into the new Peer object added in .

  For motivation, see .

ACKs for top commit:
  jonatack:
    re-ACK 3002b4af2b per `git diff 9aad3e4 3002b4a`
  Sjors:
    Code review re-ACK 3002b4af2b
  MarcoFalke:
    re-ACK 3002b4af2b 🌓

Tree-SHA512: eb2b474b73b025791ee3e6e41809926b332b48468763219f31638ca390f427632f05902dfc6a2c6bdc1ce47b215782f67874ddbf05b97d77d5897b7e2abfe4d9
2020-12-22 12:44:19 +01:00
MarcoFalke
2556a973ed
Merge : rpc: Add missing description of vout in getrawtransaction help text
b23349b880 rpc: Add missing description of vout in getrawtransaction help text (Ben Carman)

Pull request description:

  In `getrawtransaction` the vout did not have a description. I gave it the same description as the one used in `decoderawtransaction`.

ACKs for top commit:
  MarcoFalke:
    ACK b23349b880 🏯

Tree-SHA512: 3833b97c82a46dfeb7ac825d4b2514b4b05ce54ac41f2144a8e2f2093b3411fe1d090c1e5b0c3d09200a2ea164c8d17ece12cdb43bbaeaeccc51a9da6dd7b7a3
2020-12-21 19:57:38 +01:00
Ben Carman
b23349b880
rpc: Add missing description of vout in getrawtransaction help text 2020-12-21 09:57:06 -06:00
John Newbery
78040f9168 [net processing] Rename nStartingHeight to m_starting_height
Not done as a scripted diff to avoid misnaming the local variable in
ProcessMessage().
2020-12-20 10:01:46 +00:00
John Newbery
77a2c2f8f9 [net processing] Move nStartingHeight to Peer 2020-12-20 10:01:26 +00:00
Anthony Towns
95d5d5e625 rpc: allow getblocktemplate for test chains when unconnected or in IBD 2020-12-14 18:32:57 +10:00
Anthony Towns
81c54dec20 rpc: update getblocktemplate with signet rule, include signet_challenge 2020-12-14 18:32:57 +10:00
fanquake
0475c8ba4d
net: use std::chrono throughout maxOutbound logic 2020-12-13 11:41:33 +08:00
MarcoFalke
22f13c1e08
Merge : net, rpc: expose high bandwidth mode state via getpeerinfo
343dc4760f test: add test for high-bandwidth mode states in getpeerinfo (Sebastian Falbesoner)
dab6583307 doc: release note for new getpeerinfo fields "bip152_hb_{from,to}" (Sebastian Falbesoner)
a7ed00f8bb rpc: expose high-bandwidth mode states via getpeerinfo (Sebastian Falbesoner)
30bc8fab68 net: save high-bandwidth mode states in CNodeStats (Sebastian Falbesoner)

Pull request description:

  Fixes , "_For every peer expose through getpeerinfo RPC whether or not we selected them as HB peers, and whether or not they selected us as HB peers._" See [BIP152](https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki), in particular the [protocol flow diagram](https://github.com/bitcoin/bips/raw/master/bip-0152/protocol-flow.png).  The newly introduced states are changed on the following places in the code:
  * on reception of a `SENDCMPCT` message with valid version, the field `m_highbandwidth_from` is changed depending on the first integer parameter in the message (1=high bandwidth, 0=low bandwidth), i.e. it just mirrors the field `CNodeState.fPreferHeaderAndIDs`.
  * after adding a `SENDCMPCT` message to the send queue, the field `m_highbandwidth_to` is changed depending on how the first integer parameter is set (same as above)

  Note that after receiving `VERACK`, the node also sends `SENDCMPCT`, but that is only to announce the preferred version and never selects high-bandwidth mode, hence there is no need to change the state variables there, which are initialized to `false` anyways.

ACKs for top commit:
  naumenkogs:
    reACK 343dc4760f
  jonatack:
    re-ACK 343dc4760f per `git range-diff 7ea6499 4df1d12 343dc47`

Tree-SHA512: f4999e6a935266812c2259a9b5dc459710037d3c9e938006d282557cc225e56128f72965faffb207fc60c6531fab1206db976dd8729a69e8ca29d4835317b99f
2020-12-10 08:21:36 +01:00
John Newbery
f3f61d0eb9 [net processing] Add IgnoresIncomingTxs() function to PeerManager 2020-12-09 18:10:38 +00:00
fanquake
795afe6e63
Merge : net processing: Move peer_map to PeerManager
3025ca9e77 [net processing] Add RemovePeer() (John Newbery)
a20ab22786 [net processing] Make GetPeerRef const (John Newbery)
ed7e469cee [net_processing] Move peer_map to PeerManager (John Newbery)
a529fd3e3f [net processing] Move GetNodeStateStats into PeerManager (John Newbery)

Pull request description:

  This moves `g_peer_map` from a global in net_processing.cpp's unnamed namespace to being a member `m_peer_map` of `PeerManager`.

ACKs for top commit:
  theuni:
    Re-ACK 3025ca9e77.
  dongcarl:
    Re-ACK 3025ca9
  hebasto:
    re-ACK 3025ca9e77, since my [previous](https://github.com/bitcoin/bitcoin/pull/19910#pullrequestreview-545574237) review only reverted the change that introduced NRVO in `PeerManager::GetPeerRef`, and comments are fixed in the proper commits.

Tree-SHA512: 6369eb3c688ac5b84f89f7674115f78ff02edbed76063ac2ebb1759894c9e973883e10821a35dab92bd3d738280acc095bd5368f552a060b83cd309330387d47
2020-12-09 21:56:17 +08:00
Wladimir J. van der Laan
5c4911e7e7
Merge : doc: Use FeeModes doc helper in estimatesmartfee
fa8abdc995 rpc: Use FeeModes doc helper in estimatesmartfee (MarcoFalke)

Pull request description:

  Not sure why this doesn't use the doc helper, probably an oversight?

ACKs for top commit:
  laanwj:
    Code review ACK fa8abdc995

Tree-SHA512: 1f2dc8356e3476ddcf9cafafa7f9865ad95bed1e3067c0edab8e3c483e374bdbdbecc066167554b4a1b479e28f6a52c4ae6a75a70c67ee4e1ff4f3ba36b04001
2020-12-07 14:09:53 +01:00
MarcoFalke
03b1db6114
Merge : Disable fee estimation in blocksonly mode (by removing the fee estimates global)
4e28753f60 feestimator: encapsulate estimation file logic (Antoine Poinsot)
e8ea6ad9c1 init: don't create a CBlockPolicyEstimator if we don't relay transactions (Antoine Poinsot)
86ff2cf202 Remove the remaining fee estimation globals (Antoine Poinsot)
03bfeee957 interface: remove unused estimateSmartFee method from node (Antoine Poinsot)

Pull request description:

  If the `blocksonly` mode is turned on after running with transaction
  relay enabled for a while, the fee estimation will serve outdated data
  to both the internal wallet and to external applications that might be
  feerate-sensitive and make use of `estimatesmartfee` (for example a
  Lightning Network node).

  This has already caused issues (for example https://github.com/bitcoin/bitcoin/issues/16840 (C-lightning), or https://github.com/lightningnetwork/lnd/issues/2562 (LND)) and it seems prudent to fail rather than to give inaccurate values.

  This fixes , and closes  which tried to fix the symptoms (RPC) but not the cause as mentioned by sdaftuar :
  > If this is a substantial problem, then I would think we should take action to protect our own wallet users as well (rather than hide the results of what our fee estimation would do!).

ACKs for top commit:
  MarcoFalke:
    re-ACK 4e28753f60 👋
  jnewbery:
    utACK 4e28753f60

Tree-SHA512: c869cf03b86d8194002970bbc84662dae76874967949b9be0d9a4511a1eabcb1627c38aca3154da9dcece1a4c49ec02bd4f9fcca2ec310986e07904559e63ba8
2020-12-07 12:59:48 +01:00
MarcoFalke
fa8abdc995
rpc: Use FeeModes doc helper in estimatesmartfee
Can be reviewed with --ignore-all-space
2020-12-07 09:28:47 +01:00
Jonas Schnelli
eab63b971d
Merge : rpc, refactor: Avoid duplicate set lookup in gettxoutproof
52fc39917f rpc: Reject empty txids in gettxoutproof (João Barbosa)
73dc19a330 rpc, refactor: Avoid duplicate set lookup in gettxoutproof (João Barbosa)

Pull request description:

ACKs for top commit:
  jonasschnelli:
    code review ACK 52fc39917f

Tree-SHA512: 76b18e5235e8b2d394685515a4a60335666eeb0f6b31c1d397f7db2fbe681bc817b8cd3e8f6708b9dacd6113e4e1d94837072cae27834b8a1a22d2717db8191e
2020-12-07 09:17:08 +01:00
practicalswift
31b136e580 Don't declare de facto const reference variables as non-const 2020-12-06 18:44:31 +00:00
practicalswift
12dcdaaa54 Don't make "in" parameters look like "out"/"in-out" parameters: pass by ref to const instead of ref to non-const 2020-12-06 00:22:40 +00:00
João Barbosa
52fc39917f rpc: Reject empty txids in gettxoutproof 2020-12-04 18:38:23 +00:00
João Barbosa
73dc19a330 rpc, refactor: Avoid duplicate set lookup in gettxoutproof 2020-12-04 18:38:23 +00:00
John Newbery
a529fd3e3f [net processing] Move GetNodeStateStats into PeerManager 2020-12-04 11:37:45 +00:00
Antoine Poinsot
86ff2cf202
Remove the remaining fee estimation globals
This moves the CBlockPolicyEstimator to the NodeContext, which get rids
of two globals and allows us to conditionally create the
CBlockPolicyEstimator (and to remove a circular dep).

Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2020-12-03 12:56:37 +01:00
Fabian Jahr
1e62350ca2
refactor: Improve use of explicit keyword 2020-12-01 18:36:39 +01:00
MarcoFalke
71d068db40
Merge : rpc: remove deprecated CRPCCommand constructor
faaf9c58e4 remove CRPCCommand constructor that takes rpcfn_type function pointer (MarcoFalke)
fa19bb2cd8 remove dead rpc code (MarcoFalke)

Pull request description:

  Remove the CRPCCommand arguments, now that they are asserted to be equal and thus redundant

  ### Future work

  > Here or follow up, makes sense to also assert type of returned UniValue?

  Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including:

  * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table
  * Auto-formatting and sanity checking the RPCExamples with RPCMan
  * Checking passed-in json in self-check. Removing redundant checks
  * Checking returned json against documentation to avoid regressions or false documentation
  * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static

  ### Bugs found

  * The assert identified issue 
  * The changes itself fixed bug 

ACKs for top commit:
  fjahr:
    tested ACK faaf9c58e4
  promag:
    Tested ACK faaf9c58e4.
  ryanofsky:
    Code review ACK faaf9c58e4. Two obviously good simplifications.

Tree-SHA512: 5de3b440f7b2ed2c3e86655d4f0e2e5df9c67e8ce3c7817d5ea5311d1a38690f2f3e28fab41aad6936be9fc884326d037e5f19e85d4d2fe281474dada13911ee
2020-11-19 14:19:05 +01:00
Jon Atack
449b730579
wallet: provide valid values if invalid estimate mode passed
Co-authored-by: Murch <murch@murch.one>
2020-11-12 11:43:17 +01:00
Jon Atack
a0d4957473
wallet: introduce fee_rate (sat/vB) param/option
Create a fee_rate (sat/vB) RPC param and replace overloading the conf_target and
estimate_mode params in the following 6 RPCs with it:

- sendtoaddress
- sendmany
- send
- fundrawtransaction
- walletcreatefundedpsbt
- bumpfee

In RPC bumpfee, the previously existing fee_rate remains but the unit is changed
from BTC/kvB to sat/vB. This is a breaking change, but it should not be an
overly risky one, as the units change by a factor of 1e5 and any fees specified
in BTC/kvB after this commit will either be too low and raise an error or be 1
sat/vB and can be RBFed.

Update the test coverage for each RPC.

Co-authored-by: Murch <murch@murch.one>
2020-11-11 15:55:59 +01:00
Wladimir J. van der Laan
79a3b59cc7
Merge : net, rpc, test, bugfix: update GetNetworkName, GetNetworksInfo, regression tests
7b5bd3102e test: add getnetworkinfo network name regression tests (Jon Atack)
9a75e1e569 rpc: update GetNetworksInfo() to not return unsupported networks (Jon Atack)
ba8997fb2e net: update GetNetworkName() with all enum Network cases (Jon Atack)

Pull request description:

  Following up on the BIP155 addrv2 changes, and starting with 7be6ff6 in , RPC getnetworkinfo began returning networks with empty names.

  <details><summary><code>getnetworkinfo</code> on current master</summary><p>

  ```
    "networks": [
      {
        "name": "ipv4",
        "limited": false,
        "reachable": true,
        "proxy": "",
        "proxy_randomize_credentials": false
      },
      {
        "name": "ipv6",
        "limited": false,
        "reachable": true,
        "proxy": "",
        "proxy_randomize_credentials": false
      },
      {
        "name": "onion",
        "limited": false,
        "reachable": true,
        "proxy": "127.0.0.1:9050",
        "proxy_randomize_credentials": true
      },
      {
        "name": "",
        "limited": false,
        "reachable": true,
        "proxy": "",
        "proxy_randomize_credentials": false
      },
      {
        "name": "",
        "limited": false,
        "reachable": true,
        "proxy": "",
        "proxy_randomize_credentials": false
      }
    ],
  ```
  </p></details>

  <details><summary><code>getnetworkinfo</code> on this branch</summary><p>

  ```
    "networks": [
      {
        "name": "ipv4",
        "limited": false,
        "reachable": true,
        "proxy": "",
        "proxy_randomize_credentials": false
      },
      {
        "name": "ipv6",
        "limited": false,
        "reachable": true,
        "proxy": "",
        "proxy_randomize_credentials": false
      },
      {
        "name": "onion",
        "limited": false,
        "reachable": true,
        "proxy": "127.0.0.1:9050",
        "proxy_randomize_credentials": true
      }
    ],
  ```
  </p></details>

  This patch:
  - updates `GetNetworkName()` to the current Network enum
  - updates `getNetworksInfo()` to ignore as-yet unsupported networks
  - adds regression tests

ACKs for top commit:
  hebasto:
    re-ACK 7b5bd3102e
  vasild:
    ACK 7b5bd3102

Tree-SHA512: 8f12363eb430e6f45e59e3b1d69c2f2eb5ead254ce7a67547d116c3b70138d763157335a3c85b51f684a3b3b502c6aace4f6fa60ac3b988cf7a9475a7423c4d7
2020-11-09 17:07:23 +01:00
Samuel Dobson
5d32009f1a
Merge : wallet, rpc: explicit fee rate follow-ups/fixes for 0.21
0be29000c0 rpc: update conf_target helps for correctness/consistency (Jon Atack)
778b9be406 wallet, rpc: fix send subtract_fee_from_outputs help (Jon Atack)
603c005083 wallet: add rpc send explicit fee rate coverage (Jon Atack)
dd341e602d wallet: add sendtoaddress/sendmany explicit fee rate coverage (Jon Atack)
44e7bfa603 wallet: add walletcreatefundedpsbt explicit fee rate coverage (Jon Atack)
6e1ea4273e test: refactor for walletcreatefundedpsbt fee rate coverage (Jon Atack)
3ac7b0c6f1 wallet: fundrawtx fee rate coverage, fixup ParseConfirmTarget() (Jon Atack)
2d8eba8f84 wallet: combine redundant bumpfee invalid params and args tests (Jon Atack)
1697a40b6f wallet: improve bumpfee error/help, add explicit fee rate coverage (Jon Atack)
fc5721723d wallet: fix SetFeeEstimateMode() error message (Jon Atack)
052427eef1 wallet, bugfix: fix bumpfee with explicit fee rate modes (Jon Atack)

Pull request description:

  Follow-up to  providing a base to build on for :

  - bugfix for `bumpfee` raising a JSON error with explicit feerates, fixes issue 
  - adds explicit feerate test coverage for `bumpfee`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send`, `sendtoaddress`, and `sendmany`
  - improves a few related RPC error messages and `ParseConfirmTarget()` / error message
  - fixes/improves the explicit fee rate information in the 6 RPC helps, of which 2 were also missing `conf_target` sat/B units

  This provides a spec and regression coverage for the potential next step of a universal `sat/vB` feerate argument (see ), as well as immediate coverage and minimum fixes for 0.21.

ACKs for top commit:
  kallewoof:
    Concept/Tested ACK 0be29000c0
  meshcollider:
    Code review + functional test run ACK 0be29000c0

Tree-SHA512: efd965003e991cba51d4504e2940f06ab3d742e34022e96a673606b44fad85596aa03a8c1809f06df7ebcf21a38e18a891e54392fe3d6fb4d120bbe4ea0cf5e0
2020-11-04 16:35:23 +13:00