aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-08-18-maxapsfee: follow-up fixesKarl-Johan Alm
Co-authored-by: Jon Atack <jon@atack.com> Co-authored-by: Samuel Dobson <dobsonsa68@gmail.com>
2020-08-18doc: release notes for -maxapsfeeKarl-Johan Alm
2020-08-18Merge #19756: tests: add sync_all to fix race condition in wallet groups testMarcoFalke
72ae20fc142457a200278cb2fedc5e32a3766b58 tests: add sync_all to fix race condition in wallet groups test (Karl-Johan Alm) Pull request description: This most likely fixes #19749, the intermittent CI issues with wallet_groups. This fix is also included in #19743. Top commit has no ACKs. Tree-SHA512: dd6ef7f89829483e2278191c21fe0912b51fd2187c10a0fa158339c5ab9f22d93b733ae10f17ef25d8b64f44e596e66dba8d7db5c009343472f422ce4cd67d8f
2020-08-18Merge #19015: build: Enable some commonly enabled compiler diagnosticsfanquake
2f8a4c9a06ace680b3dff7cd7d5e33204fe45909 build: Enable some commonly enabled compiler diagnostics (practicalswift) Pull request description: Enable some commonly enabled compiler diagnostics as discussed in #17344. | Compiler diagnostic | no# of emitted unique GCC warnings in `master` | no# of emitted unique Clang warnings in `master` | | ------------- | ------------- | ------------- | | `-Wduplicated-branches`: Warn if `if`/`else` branches have duplicated code | 0 | Not supported | | `-Wduplicated-cond`: Warn if `if`/`else` chain has duplicated conditions | 0 | Not supported | | `-Wlogical-op`: Warn about logical operations being used where bitwise were probably wanted | 0 | Not supported | | `-Woverloaded-virtual`: Warn if you overload (not `override`) a virtual function | 0 | 0 | | ~~`-Wunused-member-function`: Warn on unused member function~~ | Not supported | 2 | | ~~`-Wunused-template`: Warn on unused template~~ | Not supported | 1 | There is a large overlap between this list and [Jason Turner's list of recommended compiler diagnostics in the Collaborative Collection of C++ Best Practices (`cppbestpractices`) project](https://github.com/lefticus/cppbestpractices/blob/master/02-Use_the_Tools_Available.md#gcc--clang). There is also an overlap with the recommendations given in the [C++ Core Guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) (with editors Bjarne Stroustrup and Herb Sutter). Closes #17344. ACKs for top commit: jonatack: ACK 2f8a4c9a06ace6 no warnings for me with these locally on debian 5.7.10-1 (2020-07-26) x86_64 with gcc 10 and clang 12 fanquake: ACK 2f8a4c9a06ace680b3dff7cd7d5e33204fe45909 - no-longer seeing any obvious issues with doing this. hebasto: ACK 2f8a4c9a06ace680b3dff7cd7d5e33204fe45909, no new warnings in Travis jobs. Tree-SHA512: f669ea22b31263a555f999eff6a9d65750662e95831b188c3192a2cf0127fb7b5136deb762a6b0b7bbdfb0dc6a40caf48251a62b164fffb81dd562bdd15ec3c8
2020-08-18Merge #19721: p2p: comment out unused MSG_FILTERED_WITNESS_BLOCKfanquake
4792cad88c5c3c93e639a051df779230ee817396 doc: comment out and add annotation to unused MSG_FILTERED_WITNESS_BLOCK (Adam Jonas) Pull request description: Commenting out and adding a note to unused `MSG_FILTERED_WITNESS_BLOCK` [defined in BIP144](https://github.com/bitcoin/bips/blob/master/bip-0144.mediawiki#relay). There was an attempt to make use of this in https://github.com/bitcoin/bitcoin/pull/10350, but it was closed due to lack of support. (h/t sdaftuar for pointing to the PR and jnewbery for the idea) ACKs for top commit: jnewbery: Obvious ACK 4792cad88c5c3c93e639a051df779230ee817396 theStack: ACK https://github.com/bitcoin/bitcoin/pull/19721/commits/4792cad88c5c3c93e639a051df779230ee817396 📜 MarcoFalke: cr ACK 4792cad88c5c3c93e639a051df779230ee817396 good to keep it around in a comment to avoid accidental future re-assignment practicalswift: ACK 4792cad88c5c3c93e639a051df779230ee817396 Tree-SHA512: 22327ddded643ae50fdb529e4529a9b464f74e90620d0d2079a11070eaa8afe8363f6e14cca52f3bec2c9f87ee13e318edc6c5193761c94b8ae77be353a8da1f
2020-08-18Merge #19719: build: Add Werror=range-loop-analysisfanquake
fa55c1d5fdd88c4bc4d361da231cd63b20255b50 build: Add Werror=range-loop-analysis (MarcoFalke) Pull request description: The warning is implicitly enabled for Bitcoin Core. Also explicitly since commit d92204c900d. To avoid "fix range loop" follow-up refactors, we have two options: * Disable the warning, so that issues never appear * Enable it as an error, so that issues are either caught locally or by ci ACKs for top commit: fanquake: ACK fa55c1d5fdd88c4bc4d361da231cd63b20255b50 practicalswift: ACK fa55c1d5fdd88c4bc4d361da231cd63b20255b50 -- pre-review fix-up is better than post-review fix-up hebasto: re-ACK fa55c1d5fdd88c4bc4d361da231cd63b20255b50 Tree-SHA512: 019aa133f254af8882c1d5d10c420d9882305db0fc2aa9dad7d285168e2556306c3eedcc03bd30e63f11eae4cc82b648d83fb6e9179d6a6364651fb602d70134
2020-08-18tests: add sync_all to fix race condition in wallet groups testKarl-Johan Alm
2020-08-17Merge #19734: Move CDiskTxPos to its own fileMarcoFalke
7668db3b08531a590089d66cc5c91f1fb3afbfcc Move only: Move CDiskTxPos to its own file (Marcin Jachymiak) Pull request description: Moves `CDiskTxPos` it its own file so it can be used without the `txindex.h` include elsewhere. Originally part of #14053. ACKs for top commit: jnewbery: utACK 7668db3b08531a590089d66cc5c91f1fb3afbfcc promag: ACK 7668db3b08531a590089d66cc5c91f1fb3afbfcc. Tree-SHA512: b108e980ad04e43d1323410c3683a82bed70aee7795f5d8a2afbaf32a07ba598571f00b047bdde15048124b17178bcbd10654c48461beac988e9643cb2df664c
2020-08-17Merge #14582: wallet: always do avoid partial spends if fees are within a ↵Samuel Dobson
specified range 7f13dfb587dd6a7a5b7dfbfe689ae0ce818fe5c9 test: test the implicit avoid partial spends functionality (Karl-Johan Alm) b82067bf696c53f22536f9ca2e51949c164f6b06 wallet: try -avoidpartialspends mode and use its result if fees are below threshold (Karl-Johan Alm) Pull request description: The `-avoidpartialspends` feature is normally disabled, as it may affect the optimal fee for payments. This PR introduces a new parameter `-maxapsfee` (max avoid partial spends fee) which acts on the following values: * -1: disable partial spend avoidance completely (do not even try it) * 0: only do partial spend avoidance if fees are the same or better as the regular coin selection * 1..∞: use APS variant if the absolute fee difference is less than or equal to the max APS fee For values other than -1, the code will now try partial spend avoidance once, and if that gives a value within the accepted range, it will use that. Example: -maxapsfee=0.00001000 means the wallet will do regular coin select, APS coin select, and then pick AKS iff the absolute fee difference is <= 1000 satoshi. Edit: updated this to reflect the fact we are now using a max fee. ACKs for top commit: fjahr: tested ACK 7f13dfb587dd6a7a5b7dfbfe689ae0ce818fe5c9 achow101: ACK 7f13dfb587dd6a7a5b7dfbfe689ae0ce818fe5c9 jonatack: ACK 7f13dfb58, code review, debug build, verified the test fails with `AssertionError: not(2 == 1)` for the number of vouts when `-maxapsfee=0.0001` is changed to 0, and verified the new logging with an added assertion. meshcollider: Code review ACK 7f13dfb587dd6a7a5b7dfbfe689ae0ce818fe5c9 Tree-SHA512: 475929df57f6191bb4e36bfbcad5a280a64bb0ecd8767b76cb2e44e2301235d0eb294a3f2fac5bbf15d35d7ecfba47acb2285feadb883c9ce31c08377e3afb3c
2020-08-16Merge #19705: Shrink CAddress from 48 to 40 bytes on x64Wladimir J. van der Laan
767073fb9645f5cb0976a14288c03fe71912b569 Shrink CAddress from 48 to 40 bytes on x64 (Vasil Dimov) Pull request description: `CAddress` inherits `CService` which is 28 bytes (on 64 bit machines). `CAddress` then adds two member variables - one that requires 4 byte alignment (`nTime`) and one that requires 8 byte alignment (`nServices`). Declare the smaller one first so that it fits in bytes 29..32. On 32 bit machines this change has no effect and `CAddress` remains 40 bytes. ACKs for top commit: laanwj: ACK 767073fb9645f5cb0976a14288c03fe71912b569 theStack: ACK https://github.com/bitcoin/bitcoin/commit/767073fb9645f5cb0976a14288c03fe71912b569 Tree-SHA512: 73d6a4fcfa2687b4076950801871252e369510ecf09f820576dbeca9ee3ee94d14672e7d5596cb45fedd9e4b973dd0716a2ea3f13fc3058b4b697d036a7c9db0
2020-08-16Merge #19564: test: p2p_feefilter improvements (logging, refactoring, speedup)MarcoFalke
9e7894357e296dbe0be775e1527654729dbb71b0 test: speedup p2p_feefilter.py by whitelisting peers (immediate tx relay) (Sebastian Falbesoner) fe3f0cc44ec6301a86de48cdc3883377932e44eb test: use wait_until for invs matching in p2p_feefilter.py (Sebastian Falbesoner) 6d941923c318ce9cb2380d3e41ffb760636656a2 test: add logging for p2p_feefilter.py (Sebastian Falbesoner) Pull request description: This PR gives some love to the functional test `p2p_feefilter.py` by introducing the following changes: * add missing log messages for the `test_feefilter` subtest (the main one) * deduplicate code by using the utility function `wait_until` (already using the [recently introduced version](https://github.com/bitcoin/bitcoin/commit/12410b1feb80189061eb4a2b43421e53cbb758a8)) instead of a manual condition checking loop with `time.sleep` * improve naming of the function `matchAllInvs` (more expressive name, snake_case) and moving it from global namespace to the connection class `FeefilterConn` * speeding up the test significantly by the good ol' method of activating immediate tx relay (as seen on e.g. https://github.com/bitcoin/bitcoin/pull/17121, https://github.com/bitcoin/bitcoin/pull/17124, https://github.com/bitcoin/bitcoin/pull/17340, https://github.com/bitcoin/bitcoin/pull/17362, ...): ``` master branch: $ time ./p2p_feefilter.py ... real 0m39.367s user 0m1.227s sys 0m0.571s PR branch: $ time ./p2p_feefilter.py ... real 0m9.386s user 0m1.120s sys 0m0.577s ``` ACKs for top commit: instagibbs: code review ACK https://github.com/bitcoin/bitcoin/pull/19564/commits/9e7894357e296dbe0be775e1527654729dbb71b0 jonatack: re-ACK 9e78943 per `git range-diff 3ab2582 ea74a3c 9e78943` Tree-SHA512: fe21c1c5413df9165fea916b5d5f609d3ba33e7b5c3364b38eb824fcc55d9e6abddf27116cbc0b325913d451a73c44542040fb916aec9c46f805c6e12f6f10cf
2020-08-15Move only: Move CDiskTxPos to its own fileMarcin Jachymiak
2020-08-15Merge #19730: ci: Set increased --timeout-factor by defaultMarcoFalke
fa330ec2fe5f5ba68a8d43fff0b19584c0b1ff39 test: Remove confusing and broken use of wait_until global (MarcoFalke) fa6583c30bf7d82cf7ffdae995f8f16524ad2c0d ci: Set increased --timeout-factor by default (MarcoFalke) Pull request description: Assuming that tests don't have a logic error or race, setting a high timeout should not cause any issues. The tests will still pass just as fast in the fastest case, but it allows for some buffer in case of slow disks or otherwise starved ci machines. Fixes #19729 ACKs for top commit: hebasto: ACK fa330ec2fe5f5ba68a8d43fff0b19584c0b1ff39, I have reviewed the code, and it looks OK, I agree it can be merged. Tree-SHA512: 3da80ee008c7b08bab5fdaf7804d57c79d6fed49a7d37b9c54fc89756659fcb9981fd10afc4d07bd90d93c1699fd410a0110a3cd34d016873759d114ce3cd82d
2020-08-15test: Remove confusing and broken use of wait_until globalMarcoFalke
2020-08-15ci: Set increased --timeout-factor by defaultMarcoFalke
2020-08-15Merge #19718: build: Add missed gcov files to 'make clean'fanquake
6cb8771173d835bdeb89c76b1d53191d3c896968 build: Add missed gcov files to 'make clean' (Hennadii Stepanov) Pull request description: On master (b4d0366b47dd9b8fe29cc9a100dcdf6ca1d3cabf): ``` $ ./autogen.sh $ ./configure --enable-lcov $ make && make cov $ make clean $ find . -name '*.gcno' ./src/rpc/libbitcoin_server_a-blockchain.gcno ./src/rpc/libbitcoin_common_a-util.gcno ./src/rpc/libbitcoin_common_a-rawtransaction_util.gcno ./src/rpc/libbitcoin_server_a-rawtransaction.gcno ./src/rpc/libbitcoin_util_a-request.gcno ./src/rpc/libbitcoin_server_a-net.gcno ./src/rpc/libbitcoin_server_a-server.gcno ./src/rpc/libbitcoin_server_a-mining.gcno ./src/rpc/libbitcoin_server_a-misc.gcno ./src/rpc/libbitcoin_cli_a-client.gcno ./src/node/libbitcoin_server_a-coinstats.gcno ./src/node/libbitcoin_server_a-transaction.gcno ./src/node/libbitcoin_server_a-context.gcno ./src/node/libbitcoin_server_a-psbt.gcno ./src/node/libbitcoin_server_a-ui_interface.gcno ./src/node/libbitcoin_server_a-coin.gcno ./src/test/util/libtest_util_a-setup_common.gcno ./src/test/util/libtest_util_a-net.gcno ./src/test/util/libtest_util_a-blockfilter.gcno ./src/test/util/libtest_util_a-mining.gcno ./src/test/util/libtest_util_a-transaction_utils.gcno ./src/test/util/libtest_util_a-wallet.gcno ./src/test/util/libtest_util_a-str.gcno ./src/test/util/libtest_util_a-logging.gcno ./src/index/libbitcoin_server_a-txindex.gcno ./src/index/libbitcoin_server_a-base.gcno ./src/index/libbitcoin_server_a-blockfilterindex.gcno ./src/util/libbitcoin_util_a-error.gcno ./src/util/libbitcoin_util_a-rbf.gcno ./src/util/libbitcoin_util_a-message.gcno ./src/util/libbitcoin_util_a-time.gcno ./src/util/libbitcoin_util_a-moneystr.gcno ./src/util/libbitcoin_util_a-url.gcno ./src/util/libbitcoin_consensus_a-strencodings.gcno ./src/util/libbitcoin_util_a-settings.gcno ./src/util/libbitcoin_util_a-system.gcno ./src/util/libbitcoin_util_a-threadnames.gcno ./src/util/libbitcoin_util_a-fees.gcno ./src/util/libbitcoin_util_a-asmap.gcno ./src/util/libbitcoin_util_a-strencodings.gcno ./src/util/libbitcoin_util_a-string.gcno ./src/util/libbitcoin_util_a-bytevectorhash.gcno ./src/util/libbitcoin_util_a-bip32.gcno ./src/util/libbitcoin_util_a-spanparsing.gcno ./src/util/libbitcoinconsensus_la-strencodings.gcno ./src/interfaces/libbitcoin_wallet_a-wallet.gcno ./src/interfaces/libbitcoin_util_a-handler.gcno ./src/interfaces/libbitcoin_server_a-chain.gcno ./src/interfaces/libbitcoin_server_a-node.gcno ./src/crc32c/src/libcrc32c_a-crc32c_portable.gcno ./src/crc32c/src/libcrc32c_a-crc32c.gcno ./src/crc32c/src/libcrc32c_sse42_a-crc32c_sse42.gcno ``` This PR fixes this issue. ACKs for top commit: practicalswift: ACK 6cb8771173d835bdeb89c76b1d53191d3c896968 -- patch looks correct Tree-SHA512: d331b8fa18f2e0cb2c107de747a39a018f73bcc20b05ed403aa38cf316b5be30b3229e92fb6c85469747571d0048a34b2846432994d0911c8234d207d4e32f1a
2020-08-15Merge #16841: Replace GetScriptForWitness with GetScriptForDestinationfanquake
7966aa424a8b78983f73742cbdb3d11eccaf9f3a Add variables for repeated scripts (MeshCollider) fec8336ad97dc717ea123f84ecfc10d9ee4a11db Remove GetScriptForWitness function (MeshCollider) b887060d06290abf4983a487f8da6b0986b058ab Replace usage of GetScriptForWitness with GetScriptForDestination (MeshCollider) Pull request description: As per this TODO in the code: > TODO: replace calls to GetScriptForWitness with GetScriptForDestination using the various witness-specific CTxDestination subtypes. The commit "Add additional check for P2SH before adding extra wrapper" also adds an additional check that the scriptPubKey is a P2SH before auto-wrapping the witness script. We shouldn't wrap the witness script if not. Note: #16251 is even better than this check, please review that. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/16841/commits/7966aa424a8b78983f73742cbdb3d11eccaf9f3a jonatack: Code review re-ACK 7966aa4 per `git range-diff b4d0366 ed266f7 7966aa4` achow101: re-ACK 7966aa424a8b78983f73742cbdb3d11eccaf9f3a only changes since last is rebase. Tree-SHA512: 3449e0e83bd842acc7c94544a85367da97ac20d859eefc1a618caef0c98204398c266fe8fb9600b78326df5175402e1ae4a132eb766e2c4485e7cda6a2a95c43
2020-08-15Merge #15937: Add loadwallet and createwallet load_on_startup optionsSamuel Dobson
642ad31b418bbf8da06cb3641329b0810e18e55b Add loadwallet and createwallet RPC load_on_startup options (Russell Yanofsky) Pull request description: This maintains a persistent list of wallets stored in settings that will automatically be loaded on startup. Being able to load a wallet automatically on startup will be more useful in the GUI, but it's reasonable to expose this feature by RPC as well. ACKs for top commit: achow101: re-ACK 642ad31b418bbf8da06cb3641329b0810e18e55b Only change is the test meshcollider: re-utACK 642ad31b418bbf8da06cb3641329b0810e18e55b Tree-SHA512: cca0b71bf1a83ad071830e6c459f1cd979b4add7144e899ec560da72b5910dd9bf9426e5c7d125ae96fad8990fbf81a76bc83c0459486c16086ada6cbde5eaa3
2020-08-15Merge #17458: Refactor OutputGroup effective value calculations and ↵Samuel Dobson
filtering to occur within the struct 9adc2f80fc14f11ee2b1f989ee7be71b58481e6f Refactor OutputGroups to handle effective values, fees, and filtering (Andrew Chow) 7d07e864b8846be186648814a5aaf34269f914a3 Use real value when calculating OutputGroup value (Andrew Chow) Pull request description: Currently, the effective values and filtering for positive effective values is done outside of the OutputGroup. We should instead have functions in Outputgroup to do this and call those for each OutputGroup. So this PR does that. This makes future changes for effective values in coin selection much easier. ACKs for top commit: instagibbs: reACK 9adc2f80fc14f11ee2b1f989ee7be71b58481e6f fjahr: re-ACK 9adc2f80fc14f11ee2b1f989ee7be71b58481e6f meshcollider: Light code review ACK 9adc2f80fc14f11ee2b1f989ee7be71b58481e6f Tree-SHA512: 7445c94b7295b45bcd83a6f8a5c8f6961a89453fcc856335192d4b4a66aec7724513616b04e5111588ab208c89b311055399d6279cd9c4ce452aefb85f04b64a
2020-08-14doc: comment out and add annotation to unused MSG_FILTERED_WITNESS_BLOCKAdam Jonas
2020-08-14build: Add Werror=range-loop-analysisMarcoFalke
2020-08-14Merge #19457: wallet: Cleanup wallettool salvage and walletdb extraneous ↵MarcoFalke
declarations 0e279fe4899beae8630264ef1fe420dd71f29d5d walletdb: Remove unused static functions from walletdb.h (Andrew Chow) 9f536d4fe949666f14a0bf5b814522cecde71f56 wallettool: Have RecoverDatabaseFile return errors and warnings (Andrew Chow) 06e263a4e368671ebb4e4a77c1447ebd5104a488 Call RecoverDatabaseFile directly from wallettool (Andrew Chow) Pull request description: Followup to #19324 addressing some comments. Removes the `SalvageWallet` function in wallettool and instead directly calls `RecoverDatabaseFile` as suggested in https://github.com/bitcoin/bitcoin/pull/19324#discussion_r450379596 Removes the `LogPrintf`s and `tfm::format`s in `RecoverDatabaseFile` as noted in https://github.com/bitcoin/bitcoin/pull/19324#discussion_r448027237 Removes the declarations of `VerifyEnvironment` and `VerifyDatabaseFile` that were forgotten in `walletdb.h` as noted in https://github.com/bitcoin/bitcoin/pull/19324#issuecomment-654389079 ACKs for top commit: meshcollider: Code review ACK 0e279fe4899beae8630264ef1fe420dd71f29d5d ryanofsky: Code review ACK 0e279fe4899beae8630264ef1fe420dd71f29d5d, just dropped last commit Tree-SHA512: ffd01f30536c2eab4bf40ba363c3ea916ecef3c8f0c5262040b40498776ffb00f95240204a40e38415d6931800851d0a3fa63ee91efc1d329b60ac317da0363d
2020-08-14Merge #19709: test: Fix 'make cov' with clangMarcoFalke
35cd2da623e32b975fbc485c3605934e4aa8bdc5 test: Fix 'make cov' with clang (Hennadii Stepanov) Pull request description: This is a follow up of #19688. With this PR it is possible to do the following: ``` $ ./autogen.sh $ ./configure --enable-lcov CC=clang CXX=clang++ $ make $ make cov ``` Currently, on master (8a85377cd0b60cb00dae4f595d628d1afbd28bd5), `make cov` fails to `Processing src/test/test_bitcoin-util_tests.gcda`. ACKs for top commit: vasild: ACK 35cd2da Crypt-iQ: ACK 35cd2da Tree-SHA512: aaf56118e2644064e9738a8279889c617db5805c5c804c904469b24c496bd609f9c5fc2aebcf1a422f8a5ed2eb38bd6e76b484680310b55c36d922b73a4c33cf
2020-08-14build: Add missed gcov files to 'make clean'Hennadii Stepanov
2020-08-14Merge #17204: wallet: Do not turn OP_1NEGATE in scriptSig into 0x0181 in ↵Wladimir J. van der Laan
signing code (sipa) dca28634d779c775678cba402a85fe5bb9b3a5a9 test: ensure OP_1NEGATE satisfies BIP62 minimal push rule (Jon Atack) e629d07199b83f4ad313b23a94c9016e3276c52a Do not turn OP_1NEGATE in scriptSig into 0x0181 in signing code (Pieter Wuille) Pull request description: A rebase of #13084 which additionally modifies the test code (unaddressed in the original, assuming sipa is too busy to deal with this at the moment). Relatively simple bugfix so it'd be good to have merged soon. Turning OP_1NEGATE into 0x0181 results in a larger-than-necessary data push instead of just actually using the OP_1NEGATE opcode (0x4f). This fails the minimal push rule of BIP 62 and makes the result non-standard. ACKs for top commit: fjahr: Code review ACK dca28634d779c775678cba402a85fe5bb9b3a5a9 luke-jr: ACK dca28634d77 jonatack: ACK dca28634d779c775678cba402a85fe5bb9b3a5a9 Tree-SHA512: 706d9a2ef20c809dea923e477a873e2fd60db8d0ae64289e510b766a38005c1f31ab0b5883f16b9c7863ff0d3f705e8e413f6121320028ac196b79c3184a4113
2020-08-14Merge #19455: rpc generate: print useful help and error messageWladimir J. van der Laan
f0aa8aeea5a183ea44a877255d12db7732f2e0a8 test: add rpc_generate functional test (Jon Atack) 92d94ffb8d07cc0d2665c901de5903a3a90d5fd0 rpc: print useful help and error message for generate (Jon Atack) 8d32d2011d3f4e1d9e587d6f80dfa4a3e9f9393d test: consider generate covered in _get_uncovered_rpc_commands() (Jon Atack) Pull request description: This was a requested follow-up to #19133 and #17700 to alleviate confusion and head-scratching by people following tutorials that use `generate`. See https://github.com/bitcoin/bitcoin/pull/19455#issuecomment-668172916 below, https://github.com/bitcoin/bitcoin/pull/19133#issuecomment-636860943 and https://github.com/bitcoin/bitcoin/pull/17700#issuecomment-566159096. before ``` $ bitcoin-cli help generate help: unknown command: generate $ bitcoin-cli generate error code: -32601 error message: Method not found ``` after ``` $ bitcoin-cli help generate generate ( nblocks maxtries ) has been replaced by the -generate cli option. Refer to -help for more information. $ bitcoin-cli generate error code: -32601 error message: generate ( nblocks maxtries ) has been replaced by the -generate cli option. Refer to -help for more information. ``` In the general help it remains hidden, as requested by laanwj. ``` $ bitcoin-cli help == Generating == generateblock "output" ["rawtx/txid",...] generatetoaddress nblocks "address" ( maxtries ) generatetodescriptor num_blocks "descriptor" ( maxtries ) ``` ACKs for top commit: adamjonas: utACK f0aa8aeea5a183ea44a877255d12db7732f2e0a8 pinheadmz: ACK f0aa8aeea5a183ea44a877255d12db7732f2e0a8 Tree-SHA512: d083652589ad3e8228c733455245001db22397559c3946e7e573cf9bd01c46e9e88b72d934728ec7f4361436ae4c74adb8f579670b09f479011924357e729af5
2020-08-14test: Fix 'make cov' with clangHennadii Stepanov
2020-08-14Merge #19568: Wallet should not override signing errorsfanquake
e7448d66803f42984018ef8dfa6699027cb9536d wallet: Don't override signing errors (Fabian Jahr) Pull request description: While reviewing #17204 I noticed that the errors in `input_errors` from `::SignTransaction` where being overridden by `CWallet::SignTransaction`. For example, a Script related error led to incomplete signature data which led to `CWallet::SignTransaction` reporting that keys were missing, which was a less precise error than the original one. Additionally, the error `"Input not found or already spent"` is [duplicated in `sign.cpp`](https://github.com/bitcoin/bitcoin/blob/c7b4968552c704f1e2e9a046911e1207f5af5fe0/src/script/sign.cpp#L481), so the error here is redundant at the moment. So technically the whole error block could be removed, I think. However, this code is affected by the ongoing work on the wallet so there might be a reason why these errors are here. But even if there is a reason to keep them, I don't think existing, potentially more precise errors should be overridden here unless we want to hide them from the users. I am looking for feedback if this is a work in progress state where these errors could be more useful in the future or if they can be removed. On testing: even though [the errors in `CWallet` are covered](https://marcofalke.github.io/btc_cov/total.coverage/src/wallet/wallet.cpp.gcov.html), all tests still pass after removing them. I am not sure if there is a desire to cover these specific error messages, tests in `test/functional/rpc_signrawtransaction.py` seem to aim for a more generic approach. ACKs for top commit: achow101: ACK e7448d66803f42984018ef8dfa6699027cb9536d meshcollider: Code review ACK e7448d66803f42984018ef8dfa6699027cb9536d Tree-SHA512: 3e2bc11d05379d2aef87b093a383d1b044787efc70e35955b2f8ecd028b6acef02f386180566af6a1a63193635f5d685466e2f6141c96326c49ffc5c81ca3e23
2020-08-14Merge #19528: rpc: Assert that RPCArg names are equal to CRPCCommand ones (misc)MarcoFalke
fa77de2baa40ee828c850ef4068c76cc3619e87b rpc: Assert that RPCArg names are equal to CRPCCommand ones (misc) (MarcoFalke) fa50bdc755489b2e291ea5ba0e39e44a20c6c6de rpc: Limit echo to 10 args (MarcoFalke) fa89ca9b5bd334813fd7e7edb202c56b35076e8d refactor: Use C++11 range based for loops to simplify rpc code (MarcoFalke) fa459bdc87bbb050ca1c8d469023a96ed798540e rpc: Treat all args after a hidden arg as hidden as well (MarcoFalke) Pull request description: This is split out from #18531 to just touch the RPC methods in misc. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### 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 the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * 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 #18607 * The changes itself fixed bug #19250 ACKs for top commit: laanwj: Code review ACK fa77de2baa40ee828c850ef4068c76cc3619e87b fjahr: tested ACK fa77de2baa40ee828c850ef4068c76cc3619e87b theStack: ACK https://github.com/bitcoin/bitcoin/pull/19528/commits/fa77de2baa40ee828c850ef4068c76cc3619e87b ryanofsky: Code review ACK fa77de2baa40ee828c850ef4068c76cc3619e87b. Pretty straightfoward changes Tree-SHA512: badae1606518c0b55ce2c0bb9025d14f05556532375eb20fd6f3bfadae1e5e6568860bff8599d037e655bf1d23f1f464ca17f4db10a6ab3d502b6e9e61c7b3d3
2020-08-14Merge #19644: rpc: document returned error fields as optional if applicableSamuel Dobson
f110b7c722eb150816a26cab161ac2b8c0f58609 rpc: document returned error fields as optional if applicable (Sebastian Falbesoner) Pull request description: The following RPCs return error fields (named `"error"` or `"errors"`) that are optional, but don't show up as optional in the help text yet: * `analyzepsbt` * `estimatesmartfee` * `signrawtransactionwithkey` * `signrawtransactionwithwallet` The following RPC has the errors field already marked as optional, but doesn't match the usual format in the description (like `"if there are any"` in parantheses): * `estimaterawfee` This PR adds the missing optional flags and adapts the description strings. Inspired by a recent PR #19634 by justinmoon. The instances were found via `git grep "RPCResult.*\"error"`. Note that there is one RPC so far where the return error is not optional (i.e. in case of no error, the field is included in the result, but is just empty), namely `bumpfee`. ACKs for top commit: adaminsky: ACK `f110b7c` laanwj: ACK f110b7c722eb150816a26cab161ac2b8c0f58609, new documentation looks consistent with actual behavior achow101: ACK f110b7c722eb150816a26cab161ac2b8c0f58609 meshcollider: utACK f110b7c722eb150816a26cab161ac2b8c0f58609 Tree-SHA512: 30c00f78a575b60e32b4536496af986d53a25f33e6ebbf553adcdcf825ad21a44f90267f3d1ea53326dac83bcfa9983fdb3dad6d3126e20f97f3c08ce286e188
2020-08-14Add variables for repeated scriptsMeshCollider
2020-08-14Remove GetScriptForWitness functionMeshCollider
2020-08-14Replace usage of GetScriptForWitness with GetScriptForDestinationMeshCollider
2020-08-13Add loadwallet and createwallet RPC load_on_startup optionsRussell Yanofsky
This maintains a persistent list of wallets stored in settings that will automatically be loaded on startup. Being able to load a wallet automatically on startup will be more useful in the GUI when the option to create wallets is added in #15006, but it's reasonable to expose this feature by RPC as well.
2020-08-13Merge #19070: p2p: Signal support for compact block filters with ↵Wladimir J. van der Laan
NODE_COMPACT_FILTERS f5c003d3ead182335252558c5c6c9b9ca8968065 [test] Add test for NODE_COMPACT_FILTER. (Jim Posen) 132b30d9c84f2a8053714a438f227b583a89a9ea [net] Signal NODE_COMPACT_FILTERS if we're serving compact filters. (Jim Posen) b3fbc94d4f2937bb682f2766cc9a8d4fde328a3f Apply cfilters review fixups (John Newbery) Pull request description: If -peerblockfilters is configured, signal the `NODE_COMPACT_FILTERS` service bit to indicate that we are able to serve compact block filters, headers and checkpoints. ACKs for top commit: MarcoFalke: re-review and Concept ACK f5c003d3ead182335252558c5c6c9b9ca8968065 fjahr: Code review ACK f5c003d3ead182335252558c5c6c9b9ca8968065 clarkmoody: Concept ACK f5c003d3ead182335252558c5c6c9b9ca8968065 ariard: Concept and Code Review ACK f5c003d jonatack: ACK f5c003d3e Tree-SHA512: 34d1c153530a0e55d09046fe548c9dc37344b5d6d50e00af1b4e1de1e7b49de770fca8471346a17c151de9fe164776296bb3dd5af331977f0c3ef1e6fc906f85
2020-08-13Merge #19655: rpc: Catch listsinceblock target_confirmations exceeding block ↵Wladimir J. van der Laan
count c133cdcdc3397a734d57e05494682bf9bf6f4c15 Cap listsinceblock target_confirmations param (Adam Stein) Pull request description: This addresses an issue brought up in #19587. Currently, the `target_confirmations` parameter to `listsinceblock` is not checked for being too large. When `target_confirmations` is greater than one more than the current number of blocks, `listsinceblock` fails with error code -1. In comparison, when `target_confirmations` is less than 1, a -8 "Invalid parameter" error code is thrown. This PR fixes the issue by returning a -8 "Invalid parameter" error if the `target_confirmations` value corresponds to a block with more confirmations than the genesis block. This happens if `target_confirmations` exceeds one more than the number of blocks. ACKs for top commit: laanwj: Code review ACK c133cdcdc3397a734d57e05494682bf9bf6f4c15 ryanofsky: Code review ACK c133cdcdc3397a734d57e05494682bf9bf6f4c15. Just suggested changes since last review. Thanks! Tree-SHA512: 02680f4cb937d2c24d5019abd0ebfa188b8a50679a1e64e9c26bfe5c17eef6aea906832e6e2d492ba8a2ea160041bf185d66795ee691e340f6793db03c21b89a
2020-08-13Merge #19011: Reduce cs_main lock accumulation during GUI startupJonas Schnelli
386ec192a57b76492125d691ceda1b4aa832312e Reduce cs_main lock accumulation during GUI startup (Jonas Schnelli) d42cb790687788c78aa2f0c1988238ab52050782 Optionally populate BlockAndHeaderTipInfo during AppInitMain (Jonas Schnelli) b354a1480abbd71fb7fb82c39c81ea0644bbfce4 Add BlockAndHeaderTipInfo to the node interface/appInit (Jonas Schnelli) 25e1d0bf417237caa5d36b4e757f29e6c8be8aad RPCConsole, take initial chaintip data as parameter (Jonas Schnelli) Pull request description: During the GUI startup, there is currently an accumulation of cs_main locks due to setting initial chain state values at multiple locations (in the GUI main thread). This PR tries to cache the initial chain state (tip height, tip time, best header, etc.) short after loading the blockindex. The cached values are then used instead of fetching them again (and thus locking `cs_main`) during setting the client model. This should fix the initial GUI blocking often experienced during or short after the splashscreen. On mac, best tested together with #19007. ACKs for top commit: promag: Code review ACK 386ec192a57b76492125d691ceda1b4aa832312e. ryanofsky: Code review ACK 386ec192a57b76492125d691ceda1b4aa832312e. Just rebased since last review due to conflicts Tree-SHA512: caccca05360e6dc0c3aade5e7ed24be513607821a8bd6612d0337259304ab772799fb2d707a0d7c7e50fbff4bd394354643fd0aeaa3bb55960ccc28562f4763d
2020-08-13Merge #18654: rpc: separate bumpfee's psbt creation function into psbtbumpfeeSamuel Dobson
79d6332e9e4fc01e6418247c31e31b4faa1b3b84 moveonly: Fix indentation in bumpfee RPC (Andrew Chow) 431071c28ae35be8aa012df51233be19067d625c Hide bumpfee's psbt creation behavior behind -deprecatedrpc (Andrew Chow) 4638224f64ba7c8ea7c4fb550ec89c6a6d8c7887 Add psbtbumpfee RPC (Andrew Chow) Pull request description: Adds a new RPC `psbtbumpfee` which always creates a psbt. `bumpfee` will then only be able to create and broadcast fee bumping transactions instead of changing its behavior based on `IsWalletSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)`. Split from #18627 ACKs for top commit: Sjors: re-utACK 79d6332 meshcollider: utACK 79d6332e9e4fc01e6418247c31e31b4faa1b3b84 fjahr: Code review ACK 79d6332e9e4fc01e6418247c31e31b4faa1b3b84 Tree-SHA512: 1c92c4b4461bb30e78be3ee73165f624398ef33996ce36043b61a8931be667030d0fca12fd0b30097b78c56e4e9092c69582b237cbdac51d56f6be23d8c0f1bb
2020-08-12Cap listsinceblock target_confirmations paramAdam Stein
Previously, listsinceblock would fail with error code -1 when the target_confirmations exceeded the number of confirmations of the genesis block. This commit allows target_confirmations to refer to a lastblock hash with more confirmations than exist in the chain by setting the lastblock hash to the genesis hash in this case. This allows for `listsinceblock "" 6` to not fail if the block count is less than 5 which may happen on regtest. Includes update to the functional test for listsinceblock to test for this case.
2020-08-12test: speedup p2p_feefilter.py by whitelisting peers (immediate tx relay)Sebastian Falbesoner
Most of the test time is spent in wait_for_invs() after sending to addresses, i.e. the bottleneck is in relaying transactions. By whitelisting the peers via -whitelist, the inventory is transmissioned immediately rather than on average every 5 seconds, speeding up the test significantly: before: $ time ./p2p_feefilter.py ... real 0m39.367s user 0m1.227s sys 0m0.571s with this commit: $ time ./p2p_feefilter.py ... real 0m9.386s user 0m1.120s sys 0m0.577s
2020-08-12test: use wait_until for invs matching in p2p_feefilter.pySebastian Falbesoner
additionally: -> rename function with snake_case (s/allInvsMatch/wait_for_invs_to_match) -> move it from global namespace to the class FeefilterConn
2020-08-12test: add logging for p2p_feefilter.pySebastian Falbesoner
2020-08-12Merge #19688: build: Add support for llvm-covMarcoFalke
75f9659d7a497ae1f86d905029f83c0193ecfeb2 build: Add missed fuzz.coverage/ directory to .gitignore (Hennadii Stepanov) 8ebc0505e98f68cb51e9d6e2b47bf54eab530693 build: Add missed fuzz_filtered.info to COVERAGE_INFO (Hennadii Stepanov) c71bdf93d7bb2b7aa368a49538edb9c3df2c74f8 build, test: Add support for llvm-cov (Hennadii Stepanov) Pull request description: With this PR it is possible to use `lcov` with clang: ``` $ ./autogen.sh $ ./configure --enable-lcov --enable-fuzz --with-sanitizers=fuzzer CC=clang CXX=clang++ $ make $ make cov_fuzz ``` --- NOTE: Unfortunately, on my system (`clang version 10.0.0-4ubuntu1`) due to unknown for me reasons `make cov` never finishes, trying to `Processing src/test/test_bitcoin-util_tests.gcda` forever (stopped waiting). Closes #12602 ACKs for top commit: Crypt-iQ: Tested ACK 75f9659d7a497ae1f86d905029f83c0193ecfeb2 vasild: ACK 75f9659d7 Tree-SHA512: 4bc31b38fa62d70c21f890f17f0340e64d0509cea3c29ff6ac101e90ae65d2032640abf100a380c31557bea4c3f54301c2acc2b88a00cbc5261d54c01358ce4e
2020-08-12Merge #19696: rpc: Fix addnode remove command errorWladimir J. van der Laan
a51d0ad2de89b9757d158df95ddeba2bfcb23935 rpc: Improve addnode remove command error message (Fabian Jahr) Pull request description: The `addnode` RPC with the `remove` command parameter is used to remove a node from the "added nodes". It did not have test coverage and in case of failure to remove the node it responded with the confusing message "Error: Node has not been added.". This PR adds test coverage and introduces a new error code as well as changes the error message to something that makes sense. ACKs for top commit: laanwj: Code review ACK a51d0ad2de89b9757d158df95ddeba2bfcb23935 theStack: Tested ACK https://github.com/bitcoin/bitcoin/commit/a51d0ad2de Tree-SHA512: 033ef5de0d4d49d58ef4df3759b838c9d19ee9dfb0aff9f814a3a63d124ca231a442c930efa7d343fe1f65727c4b59fc23dd5e26fe6ea69f9e84fda48b5c5cc2
2020-08-12Reduce cs_main lock accumulation during GUI startupJonas Schnelli
2020-08-12Optionally populate BlockAndHeaderTipInfo during AppInitMainJonas Schnelli
2020-08-12Add BlockAndHeaderTipInfo to the node interface/appInitJonas Schnelli
2020-08-12RPCConsole, take initial chaintip data as parameterJonas Schnelli
2020-08-12Shrink CAddress from 48 to 40 bytes on x64Vasil Dimov
`CAddress` inherits `CService` which is 28 bytes (on 64 bit machines). `CAddress` then adds two member variables - one that requires 4 byte alignment (`nTime`) and one that requires 8 byte alignment (`nServices`). Declare the smaller one first so that it fits in bytes 29..32. On 32 bit machines this change has no effect and `CAddress` remains 40 bytes.
2020-08-12Merge #19658: [rpc] Allow RPC to fetch all addrman records and add records ↵Wladimir J. van der Laan
to addrman 37a480e0cd94895b6051abef12d984ff74bdc4a3 [net] Add addpeeraddress RPC method (John Newbery) ae8051bbd8377f2458ff1f167dc30c2d5f83e317 [test] Test that getnodeaddresses() can return all known addresses (John Newbery) f26502e9fc8a669b30717525597e3f468eaecf79 [addrman] Specify max addresses and pct when calling GetAddresses() (John Newbery) Pull request description: Currently addrman only allows a maximum of 1000 records or 23% of all records to be returned in a call to `GetAddr()`. Relax this limit and have the client specify the max records they want. For p2p, behaviour is unchanged (but the rate limiting is set inside net_processing, where it belongs). For RPC, `getnodeaddresses` can now return the complete addrman, which is helpful for testing and monitoring. Also add a test-only RPC `addpeeraddress`, which adds an IP address:port to addrman. This is helpful for testing (eg #18991). ACKs for top commit: naumenkogs: utACK 37a480e0cd94895b6051abef12d984ff74bdc4a3 laanwj: Code review and lightly manually tested ACK 37a480e0cd94895b6051abef12d984ff74bdc4a3 Tree-SHA512: f86dcd410aaebaf6e9ca18ce6f23556e5e4649c1325577213d873aa09967298e65ab2dc19a72670641ae92211a923afda1fe124a82e9d2c1cad73d478ef27fdc
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-20 15:38:51 +0000