aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2021-04-13wallet: remove CWallet::GetExternalSigner()fanquake
2021-04-13external_signer: remove ignore_errors from Enumerate()fanquake
This is undocumented and unused.
2021-04-13refactor: unify external wallet runtime errorsfanquake
Rather than 3 different messages that are confusing / leak implementation details, use a single message, that is similar to other wallet related messages. i.e: "Compiled without sqlite support (required for descriptor wallets)".
2021-04-13refactor: add missing includes to external signer codefanquake
2021-04-13refactor: move all signer code inside ENABLE_EXTERNAL_SIGNER #ifdefsfanquake
2021-04-13Merge #21653: ci: Fix previous releases cache orderMarcoFalke
fa4f0b301ba5c07f1271fa7129d07683014d169b ci: Fix previous releases cache order (MarcoFalke) Pull request description: ACKs for top commit: hebasto: ACK fa4f0b301ba5c07f1271fa7129d07683014d169b Tree-SHA512: 997e46e5432abb1f24c0762dab6366e173a8afd13a02d655691dbe8d6f532f4c5748e0874a7d158d7e171b2991ed2ce9cfc1982a5d9cd30d1dbb30f43452025d
2021-04-13Merge #21467: Move external signer out of wallet modulefanquake
88d4d5ff2f5c71a9a2f4c78c2b2e2fd00568cfee rpc: add help for enumeratesigners and walletdisplayaddress (Sjors Provoost) b0db187e5b30a491c9f95685430a82a1e35e921d ci: use --enable-external-signer instead of --with-boost-process (Sjors Provoost) b54b2e7b1a171203404bd41853372c73f2c64532 Move external signer out of wallet module (Sjors Provoost) Pull request description: In addition, this PR enables external signer testing on CI. This PR moves the ExternalSigner class and RPC methods out of the wallet module. The `enumeratesigners` RPC can be used without a wallet since #21417. With additional modifications external signers could be used without a wallet in general, e.g. via `signrawtransaction`. The `signerdisplayaddress` RPC is ranamed to `walletdisplayaddress` because it requires wallet context. A future `displayaddress` RPC call without wallet context could take a descriptor argument. This commit fixes a `rpc_help.py` failure when configured with `--disable-wallet`. ACKs for top commit: ryanofsky: Code review ACK 88d4d5ff2f5c71a9a2f4c78c2b2e2fd00568cfee fanquake: ACK 88d4d5ff2f5c71a9a2f4c78c2b2e2fd00568cfee Tree-SHA512: 3242a24e22313aed97eee32a520bfcb1c17495ba32a2b8e06a5e151e2611320e2da5ef35b572d84623af0a49a210d2f9377a2531250868d1a0ccf3e144352a97
2021-04-13Merge #21631: i2p: always check the return value of Sock::Wait()MarcoFalke
1c1467f51b6dda92dec974eb59026c2c5ba79ed6 i2p: cancel the Accept() method if waiting on the socket errors (Vasil Dimov) Pull request description: If `Sock::Wait()` fails, then cancel the `Accept()` method. Not checking the return value may cause an uninitialized read a few lines below when we read the `occurred` variable. [Spotted](https://github.com/bitcoin/bitcoin/pull/21630#issuecomment-814765659) by MarcoFalke, thanks! ACKs for top commit: laanwj: Code review ACK 1c1467f51b6dda92dec974eb59026c2c5ba79ed6 practicalswift: cr ACK 1c1467f51b6dda92dec974eb59026c2c5ba79ed6: patch looks correct and agree with laanwj that `[[nodiscard]]` can be taken in a follow-up PR :) Tree-SHA512: 57fa8a03a4e055999e23121cd9ed1566a585ece0cf68b74223d8c902804cb6890218c9356d60e0560ccacc6c8542a526356c226ebd48e7b299b4572be312d49b
2021-04-13Merge #21330: Deal with missing data in signature hashes more consistentlyfanquake
725d7ae0494d4a45f5a840bbbd19c008a7363965 Use PrecomputedTransactionData in signet check (Pieter Wuille) 497718b467330b2c6bb0d44786020c55f1aa75f9 Treat amount<0 also as missing data for P2WPKH/P2WSH (Pieter Wuille) 3820090bd619ac85ab35eff376c03136fe4a9f04 Make all SignatureChecker explicit about missing data (Pieter Wuille) b77b0cc507bdc716e5236b1d9880e648147e0af9 Add MissingDataBehavior and make TransactionSignatureChecker handle it (Pieter Wuille) Pull request description: Currently we have 2 levels of potentially-missing data in the transaction signature hashes: * P2WPKH/P2WSH hashes need the spent amount * P2TR hashes need all spent outputs (amount + scriptPubKey) Missing amounts are treated as -1 (thus leading to unexpected signature failures), while missing outputs in P2TR validation cause assertion failure. This is hard to extend for signing support, and also quite ugly in general. In this PR, an explicit configuration option to {Mutable,}TransactionSignatureChecker is added (MissingDataBehavior enum class) to either select ASSERT_FAIL or FAIL. Validation code passes ASSERT_FAIL (as at validation time all data should always be passed, and anything else is a serious bug in the code), while signing code uses FAIL. The existence of the ASSERT_FAIL option is really just an abundance of caution. Always using FAIL should be just fine, but if there were for some reason a code path in consensus code was introduced that misses certain data, I think we prefer as assertion failure over silently introducing a consensus change. Potentially useful follow-ups (not for this PR, in my preference): * Having an explicit script validation error code for missing data. * Having a MissingDataBehavior::SUCCEED option as well, for use in script/sign.cpp DataFromTransaction (if a signature is present in a witness, and we don't have enough data to fully validate it, we should probably treat it as valid and not touch it). ACKs for top commit: sanket1729: reACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 Sjors: ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 achow101: re-ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 benthecarman: ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 fjahr: Code review ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 Tree-SHA512: d67dc51bae9ca7ef6eb9acccefd682529f397830f77d74cd305500a081ef55aede0e9fa380648c3a8dd4857aa7eeb1ab54fe808979d79db0784ac94ceb31b657
2021-04-13Merge #21661: doc: Fix name of script guix-buildfanquake
08151e19d97f0407e99826007df3ba5b257129b4 doc: Fix name of script guix-build (Stephan Oeste) Pull request description: ACKs for top commit: hebasto: ACK 08151e19d97f0407e99826007df3ba5b257129b4 jarolrod: ACK 08151e19d97f0407e99826007df3ba5b257129b4 Tree-SHA512: 50e52f91b489db6616b5c9a993474bc1b8c196c3cac4fd5ded7c8fece5a7d72f85d9f566ee6a3df56a132a22a91dd72801ce849ec5e430a7850ff05abcab6b37
2021-04-12Merge #21663: ci: Fix macOS brew install commandW. J. van der Laan
b7381552cd4f965c45f1560d9cfc2fb09dbfcc1d ci: Fix macOS brew install command (Hennadii Stepanov) Pull request description: A solution for https://bintray.com shutdown. Details: https://github.com/Homebrew/discussions/discussions/691. ACKs for top commit: jamesob: ACK https://github.com/bitcoin/bitcoin/pull/21663/commits/b7381552cd4f965c45f1560d9cfc2fb09dbfcc1d Tree-SHA512: 4570c297421ae66043348acbe2f1e50b000e700406b38bc813c80714d4f109c6cadecff571ecbd8c2f0094ebdc959486fc55022ba6545bd592683a421e9c0655
2021-04-12ci: Fix macOS brew install commandHennadii Stepanov
Details: https://github.com/Homebrew/discussions/discussions/691
2021-04-12doc: Fix name of script guix-buildStephan Oeste
2021-04-11ci: Fix previous releases cache orderMarcoFalke
The order was broken in commit ffff4e7373f7c1260e6a8347b4ea1a99db4fff76
2021-04-11Merge #21602: rpc: add additional ban time fields to listbannedMarcoFalke
d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec doc: release notes for new listbanned fields (Jarol Rodriguez) 60290d3f5ec8e7e3b8cb1ebae02d5d72f6005184 test: increase listbanned unit test coverage (Jon Atack) 3e978d1a5dbd43f85bd03e759984ab1f209d6e34 rpc: add time_remaining field to listbanned (Jarol Rodriguez) 5456b345312857981cb426712f0665800c682e09 rpc: add ban_duration field to listbanned (Jarol Rodriguez) c95c61657afd058b46549fb3d65633d7c736f5fc doc: improve listbanned help (Jarol Rodriguez) dd3c8eaa3399b28dc78a883ff78cbe7cc5c31b5b rpc: swap position of banned_until and ban_created fields (Jarol Rodriguez) Pull request description: This PR adds a `ban_duration` and `time_remaining` field to the `listbanned` RPC command. Thanks to jonatack, this PR also expands the `listbanned` test coverage to include these new fields It's useful to keep track of `ban_duration` as this is another data point on which to sort banned peers. I found this helpful in adding additional context columns to the GUI `bantablemodel` as part of a follow-up PR. As [suggested](https://github.com/bitcoin/bitcoin/pull/21602#issuecomment-813486134) by jonatack, `time_remaining` is another useful user-centric data point. Since a ban always expires after its created, the `ban_created` field is now placed before the `banned_until` field. This new ordering is more logical. This PR also improves the `help listbanned` output by providing additional context to the descriptions of the `address`, `ban_created`, and `banned_until` fields. **Master: listbanned** ``` [ { "address": "1.2.3.4/32", "banned_until": 1617691101, "ban_created": 1617604701 }, { "address": "135.181.41.129/32", "banned_until": 1649140716, "ban_created": 1617604716 } ] ``` **PR: listbanned** ``` [ { "address": "1.2.3.4/32", "ban_created": 1617775773, "banned_until": 1617862173, "ban_duration": 86400, "time_remaining": 86392 }, { "address": "3.114.211.172/32", "ban_created": 1617753165, "banned_until": 1618357965, "ban_duration": 604800, "time_remaining": 582184 } ] ``` ACKs for top commit: jonatack: re-ACK d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec hebasto: ACK d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec, tested on Linux Mint 20.1 (x86_64). MarcoFalke: review ACK d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec 🕙 Tree-SHA512: 5b83ed2483344e546d57e43adc8a1ed7a1fff292124b14c86ca3a1aa2aec8b0f7198212fabff2c5145e7f726ca04ae567fe667b141254c7519df290cf63774e5
2021-04-11Merge #21619: ci: Run self-hosted ciMarcoFalke
fa41a917356a7f5e0f3286b227ced7f2e6797e67 ci: Run self-hosted ci (MarcoFalke) fa52a40f0ebb7aa33af210a18e525eec7bb6887f ci: Make cirrus cache folders relative to cirrus base dir (MarcoFalke) fa278412a023ac150da764a465567f2970348449 ci: Restart docker before run (MarcoFalke) fad4f48e0763d111369656dea9575a789d2016e8 ci: [refactor] Create setting for ephemeral config in .cirrus.yml (MarcoFalke) Pull request description: Due to our heavy use of the Cirrus CI community cluster, some tasks may take a long time to get scheduled. While it is possible to use "Compute Credits" to get immediately scheduled on the cluster, I couldn't find a sponsor that'd be willing to cover the total cost, if all tasks were paid for with credits. However, it is also possible to bring our own runners to Cirrus CI. For testing purposes, a single task will be transformed to run on the DrahtBot infrastructure. If all goes well, the other tasks can be moved, too. ACKs for top commit: hebasto: ACK fa41a917356a7f5e0f3286b227ced7f2e6797e67, I have reviewed the code and it looks OK. Tree-SHA512: 513738daec36da8cd48a8f11e687ff0b7dfaba1ae4ed2fa77e7b043f88fd52bf5c0dbad2768e13df88518323917f08348cb62be6376a423142921f8d2c59a938
2021-04-11Merge #21643: Ignore guix buildsfanquake
8e84c1872c67b9c0e82b66146ecdb845a49aee9e Ignore guix builds (Hennadii Stepanov) Pull request description: This PR is a #21375 follow up. ACKs for top commit: sipa: ACK 8e84c1872c67b9c0e82b66146ecdb845a49aee9e Tree-SHA512: 71d1c5acac3382f232074a025445d6d6660cb99e53233fade9ab29ad95b56da44e4e42e44411cfef175a0a8f3800633779ad1d24c2cfdcbc1a36239d38d5b8c3
2021-04-09Merge #21606: fuzz: Extend psbt fuzz target a bitMarcoFalke
faaf3954e2f0089b6c6b9965f15e7f9af09c6fb0 fuzz: Extend psbt fuzz target a bit (MarcoFalke) Pull request description: Previously it only merged the psbt with itself, now it tries to merge another. ACKs for top commit: practicalswift: Tested ACK faaf3954e2f0089b6c6b9965f15e7f9af09c6fb0 Tree-SHA512: e1b1d31a47d35e1767285bc2fda176c79cb0550d6d383fe467104272e61e1c83f6cbc0c7d6bbc0c3027729eec13ae1f289f8950117ee91e0fb3703e66d5e6918
2021-04-09Ignore guix buildsHennadii Stepanov
2021-04-09ci: Run self-hosted ciMarcoFalke
2021-04-09ci: Make cirrus cache folders relative to cirrus base dirMarcoFalke
2021-04-09ci: Restart docker before runMarcoFalke
Also, add setting for persistent worker in .cirrus.yml
2021-04-09ci: [refactor] Create setting for ephemeral config in .cirrus.ymlMarcoFalke
This allows easier switching between self-hosted runners and the community cluster. Also, named variables can be documented better.
2021-04-09fuzz: Extend psbt fuzz target a bitMarcoFalke
2021-04-09Merge #21592: test: Remove option to make TestChain100Setup non-deterministicMarcoFalke
fa6183d7761eef8bb815aa888f0396e92e275f05 test: Remove option to make TestChain100Setup non-deterministic (MarcoFalke) fa732bccb3cf9bc2cdc444975286df0e799917a2 test: Use compressed keys in TestChain100Setup (MarcoFalke) Pull request description: Seems odd to have an option for non-deterministic tests when the goal should be for all tests to be deterministic. ACKs for top commit: jamesob: ACK https://github.com/bitcoin/bitcoin/pull/21592/commits/fa6183d7761eef8bb815aa888f0396e92e275f05 practicalswift: cr ACK fa6183d7761eef8bb815aa888f0396e92e275f05: patch looks deterministic! Tree-SHA512: 6897a9f36e0dfb7d63b25dd6984414b3ee8a62458ad232cb21ed5077184fdb0bc626996e4ac84ef0bdd452b9f17c54aac75a71575b8e723b84cac07c9f9d5611
2021-04-09Merge #21445: cirrus: Use SSD cluster for speedupfanquake
fa212391ce0bb04669bc5205553a71e653f5ad48 cirrus: Use SSD cluster for speedup (MarcoFalke) Pull request description: Haven't tested, but this might be faster: https://twitter.com/fedor/status/1354505744293502980 ACKs for top commit: fanquake: ACK fa212391ce0bb04669bc5205553a71e653f5ad48 - going to merge this now given there is a speedup, and it's enough to fix the fuzz task the is continually timing out. Tree-SHA512: b24161ba2ed16fcf23ac6098100b04f7f6eb8936bb312a35fcd8e632568b877bfc09a1e522836fe298a2cd87a53a91e3b501a7f2e1c81cc0edb57c59aecffb0d
2021-04-08Merge #21304: guix: Add guix-clean script + establish gc-root for container ↵W. J. van der Laan
profiles 867a5e172a23899a4a70eca4a396c64f1951745e guix: Register garbage collector root for containers (Carl Dong) 8f8b96fb542701b7717683caa3848390b24f77ab guix: Update hint messages to mention guix-clean (Carl Dong) 44f6d4f56b16e1dc5e8a23318b8e7aad0665f178 guix: Record precious directories and add guix-clean (Carl Dong) 84912d4b24382ae022da3a863bd6caa2b8948d94 build: Remove spaces from variable-printing rules (Carl Dong) Pull request description: ``` guix: Record precious directories and add guix-clean Many users have reported problems that stem from having an unclean working tree. To that end, I've written a guix-clean script which should help reset the working tree while respecting user-specified precious directories. Precious directories, such as: - SOURCES_PATH - BASE_CACHE - SDK_PATH - OUTDIR Should be preserved when cleaning the working tree, and are thus recorded in ./contrib/guix/var/precious_dirs. The ./contrib/guix/guix-clean script is able to parse that file and make sure to avoid them when cleaning out the working tree. ``` ACKs for top commit: laanwj: ACK 867a5e172a23899a4a70eca4a396c64f1951745e Tree-SHA512: c498fad781ff5e6406639df2b91b687fc528273fdf266bcdba8f6eec3b3b37ecce544b6da0252f0b9c6717f9d88e844e4c7b72d1877bdbabfc6871ddd0172af5
2021-04-08doc: release notes for new listbanned fieldsJarol Rodriguez
2021-04-08rpc: add help for enumeratesigners and walletdisplayaddressSjors Provoost
2021-04-08ci: use --enable-external-signer instead of --with-boost-processSjors Provoost
An earlier version of #16546 used both --with-boost-process and --enable-external-signer, which was simplified to only use the latter. However I forgot to update CI, so the external signer tests were not run.
2021-04-08Move external signer out of wallet moduleSjors Provoost
This commit moves the ExternalSigner class and RPC methods out of the wallet module. The enumeratesigners RPC can be used without a wallet since #21417. With additional modifications external signers could be used without a wallet in general, e.g. via signrawtransaction. The signerdisplayaddress RPC is ranamed to walletdisplayaddress because it requires wallet context. A future displayaddress RPC call without wallet context could take a descriptor argument. This commit fixes a rpc_help.py failure when configured with --disable-wallet.
2021-04-08i2p: cancel the Accept() method if waiting on the socket errorsVasil Dimov
2021-04-08cirrus: Use SSD cluster for speedupMarcoFalke
2021-04-08Merge #21574: Drop JSONRPCRequest constructors after #21366MarcoFalke
9044522ef76f880760165d98fab024802ccfc062 Drop JSONRPCRequest constructors after #21366 (Russell Yanofsky) Pull request description: This just makes an additional simplification after #21366 replaced util::Ref with std::any. It was originally suggested https://github.com/bitcoin/bitcoin/pull/21366#issuecomment-792044351 but delayed for a followup. It would have prevented usage bug https://github.com/bitcoin/bitcoin/pull/21572. ACKs for top commit: promag: ACK 9044522ef76f880760165d98fab024802ccfc062, fixed conflict in src/wallet/interfaces.cpp. Tree-SHA512: e909411b8f75013620b94e1a609296befb832fdcb574cd2e6689bfe3c636b03cd4ac1ccb2b32b532daf0f2131bb043464024966310fffc7e3cad77713d4bd0ef
2021-04-08test: Remove option to make TestChain100Setup non-deterministicMarcoFalke
Seems odd to have an option for non-deterministic tests when the goal should be for all tests to be deterministic. Can be reviewed with `--ignore-all-space`.
2021-04-08test: Use compressed keys in TestChain100SetupMarcoFalke
coinbaseKey.MakeNewKey(true); creates a compressed key and there is no reason for the deterministic setup to use uncompressed ones.
2021-04-08Merge #21626: doc: Fix typos from codespellfanquake
94c7dd9ac810a60f9c818c494273ad798cbac34b doc: Fix typos from codespell lint (Yerzhan Mazhkenov) Pull request description: Typos from codespell linter: https://cirrus-ci.com/task/6677401661865984?logs=lint#L856 - txrequest.cpp: `annoucements` ==> `announcements` - contrib/guix/README.md:298: `stil` ==> `still` - contrib/guix/guix-build:18: `invokable` ==> `invocable` - contrib/guix/libexec/prelude.bash:12: `invokable` ==> `invocable` - src/test/fuzz/tx_pool.cpp:37: `acess` ==> `access` - src/txorphanage.h:29: `orginating` ==> `originating` ACKs for top commit: practicalswift: cr ACK 94c7dd9ac810a60f9c818c494273ad798cbac34b: thnaks fro fiixng tpyos! jarolrod: ACK 94c7dd9ac810a60f9c818c494273ad798cbac34b Tree-SHA512: e0fac462a2f9e68b6a161c9f5d95b4d0648ce5c618fd7cd243d57db8f0256138b8823b166ea406b21e95586eae43047df1ef0df04616858082a39c1d1eb13a86
2021-04-07guix: Register garbage collector root for containersCarl Dong
By registering the container profiles as garbage collector roots, it will prevent `guix gc` from garbage collecting derivations which our container needs and inconvieniencing the user with a rebuild.
2021-04-07guix: Update hint messages to mention guix-cleanCarl Dong
2021-04-07doc: Fix typos from codespell lintYerzhan Mazhkenov
2021-04-07Merge #21594: rpc: add network field to getnodeaddressesW. J. van der Laan
5c446784b10b168a6f649469a6627ac231eb1de2 rpc: improve getnodeaddresses help (Jon Atack) 1b9189866af26ed0003c1afe8dd5652ebe9b2e4a rpc: simplify/constify getnodeaddresses code (Jon Atack) 3bb6e7b6555f3c8743a697cb9d509620714dc483 rpc: add network field to rpc getnodeaddresses (Jon Atack) Pull request description: This patch adds a network field to RPC `getnodeaddresses`, which is useful on its own, particularly with the addition of new networks like I2P and others in the future, and which I also found helpful for adding a new CLI command as a follow-up to this pull that calls `getnodeaddresses` and needs to know the network of each address. While here, also improve the `getnodeaddresses` code and help. ``` $ bitcoin-cli -signet getnodeaddresses 3 [ { "time": 1611564659, "services": 1033, "address": "2600:1702:3c30:734f:8f2e:744b:2a51:dfa5", "port": 38333, "network": "ipv6" }, { "time": 1617531931, "services": 1033, "address": "153.126.143.201", "port": 38333, "network": "ipv4" }, { "time": 1617473058, "services": 1033, "address": "nsgyo7begau4yecc46ljfecaykyzszcseapxmtu6adrfagfrrzrlngyd.onion", "port": 38333, "network": "onion" } ] $ bitcoin-cli help getnodeaddresses getnodeaddresses ( count ) Return known addresses, which can potentially be used to find new nodes in the network. Arguments: 1. count (numeric, optional, default=1) The maximum number of addresses to return. Specify 0 to return all known addresses. Result: [ (json array) { (json object) "time" : xxx, (numeric) The UNIX epoch time when the node was last seen "services" : n, (numeric) The services offered by the node "address" : "str", (string) The address of the node "port" : n, (numeric) The port number of the node "network" : "str" (string) The network (ipv4, ipv6, onion, i2p) the node connected through }, ... ] ``` Future idea: allow passing `getnodeaddresses` a network (or networks) as an argument to return only addresses in that network. ACKs for top commit: laanwj: Tested ACK 5c446784b10b168a6f649469a6627ac231eb1de2 jarolrod: re-ACK 5c446784b10b168a6f649469a6627ac231eb1de2 promag: Code review ACK 5c446784b10b168a6f649469a6627ac231eb1de2. Tree-SHA512: ab0101f50c76d98c3204133b9f2ab6b7b17193ada31455ef706ad11afbf48f472fa3deb33e96028682369b35710ccd07d81863d2fd55c1485f32432f2b75efa8
2021-04-07rpc: improve getnodeaddresses helpJon Atack
2021-04-07rpc: simplify/constify getnodeaddresses codeJon Atack
2021-04-07rpc: add network field to rpc getnodeaddressesJon Atack
2021-04-07Merge #21572: Fix wrong wallet RPC context set after #21366MarcoFalke
937fd4a66f048780bffc5e714d0c800de987ce93 Fix wrong wallet RPC context set after #21366 (Russell Yanofsky) Pull request description: This bug doesn't have any effects currently because it only affects external signer RPCs which aren't currently using the wallet context, but it does cause an appveyor failure in a upcoming PR: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/38512882 This bug is subtle and could have been avoided if JSONRPCRequest didn't have constructors that were so loose with type checking. Suggested change https://github.com/bitcoin/bitcoin/pull/21366#issuecomment-792044351 eliminates these and would be a good followup for a future PR. This PR just implements the simplest possible fix. ACKs for top commit: theStack: Code-review ACK 937fd4a66f048780bffc5e714d0c800de987ce93 meshcollider: Code review ACK 937fd4a66f048780bffc5e714d0c800de987ce93 Tree-SHA512: 53e6265ed6c7abb47d2b3e77d1604edfeb993c3a2440f0c19679cfeb23516965e6707ff486196a0acfbeff21c79a9a08b5cd33bae9a232d33d0134bca1bd0ff3
2021-04-07Drop JSONRPCRequest constructors after #21366Russell Yanofsky
This just makes an additional simplification after #21366 replaced util::Ref with std::any. It was originally suggested https://github.com/bitcoin/bitcoin/pull/21366#issuecomment-792044351 but delayed for a followup. It would have prevented usage bug https://github.com/bitcoin/bitcoin/pull/21572.
2021-04-07Merge #21613: build: enable -Wdocumentationfanquake
a4e970adb6de8425025ae3f62fb89d9e27a8ab1f build: enable -Wdocumentation if suppressing external warnings (fanquake) 3b0078f958c46e94b468c829522ba965f5549f11 doc: fixup -Wdocumentation issues (fanquake) c6edcf1c710e4aaf1cafdbf8e86fe209b57bdeb8 build: suppress libevent warnings if supressing external warnings (fanquake) Pull request description: Enable `-Wdocumentation` by taking advantage of our `--enable-suppress-external-warnings` flag. Most of the CIs are using this flag now, so any regressions should be caught. This also required modifying libevents flags when suppressing warnings, as depending on the version being built against, that could generate a large number of warnings. i.e: ```bash In file included from httpserver.cpp:34: In file included from ./support/events.h:12: /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:464:11: warning: parameter 'req' not found in the function declaration [-Wdocumentation] @param req a request object ^~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:465:11: warning: parameter 'databuf' not found in the function declaration [-Wdocumentation] @param databuf the data chunk to send as part of the reply. ^~~~~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:467:11: warning: parameter 'call' not found in the function declaration [-Wdocumentation] @param call back's argument. ^~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:939:4: warning: declaration is marked with '@deprecated' command but does not have a deprecation attribute [-Wdocumentation-deprecated-sync] @deprecated This function is deprecated; you probably want to use ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:946:1: note: add a deprecation attribute to the declaration to silence this warning char *evhttp_decode_uri(const char *uri); ^ __AVAILABILITY_INTERNAL_DEPRECATED /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:979:5: warning: declaration is marked with '@deprecated' command but does not have a deprecation attribute [-Wdocumentation-deprecated-sync] @deprecated This function is deprecated as of Libevent 2.0.9. Use ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:987:1: note: add a deprecation attribute to the declaration to silence this warning int evhttp_parse_query(const char *uri, struct evkeyvalq *headers); ^ __AVAILABILITY_INTERNAL_DEPRECATED /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:1002:11: warning: parameter 'query_parse' not found in the function declaration [-Wdocumentation] @param query_parse the query portion of the URI ^~~~~~~~~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:1002:11: note: did you mean 'uri'? @param query_parse the query portion of the URI ^~~~~~~~~~~ uri 69 warnings generated. ``` Note that a lot of these have already been fixed upstream. ACKs for top commit: laanwj: Concept and code review ACK a4e970adb6de8425025ae3f62fb89d9e27a8ab1f practicalswift: cr ACK a4e970adb6de8425025ae3f62fb89d9e27a8ab1f: automatic compiler feedback comes sooner and is more reliable than manual reviewer feedback jonatack: Light ACK a4e970adb6de8425025ae3f62fb89d9e27a8ab1f skimmed the changes, clang 11 build is clean with the change, verified -Wdocumentation build warnings with this change when a doc fix was reverted Tree-SHA512: 57a1e30cffcc8bcceee72d85f58ebe29eae525861c70acb237541bd480c51ede89875c033042c0af376fdbb49fb7f588ef9282a47c6e78f9d4501c41f1b21eb6
2021-04-07Merge #21617: fuzz: Fix uninitialized read in i2p testMarcoFalke
33333755f2edcbe88fcd136f6fef81f94819002e fuzz: Fix uninitialized read in test (MarcoFalke) Pull request description: Can be tested with: ``` ./test/fuzz/test_runner.py -l DEBUG --valgrind ../btc_qa_assets/fuzz_seed_corpus/ i2p ``` ``` ==22582== Conditional jump or move depends on uninitialised value(s) ==22582== at 0x6BB2D8: __sanitizer_cov_trace_const_cmp1 (in /tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz) ==22582== by 0xB305DB: ConnectSocketDirectly(CService const&, Sock const&, int, bool) (netbase.cpp:570) ==22582== by 0x8AAA5D: i2p::sam::Session::Hello() const (i2p.cpp:284) ==22582== by 0x8A6FA0: i2p::sam::Session::CreateIfNotCreatedAlready() (i2p.cpp:352) ==22582== by 0x8A6742: i2p::sam::Session::Listen(i2p::Connection&) (i2p.cpp:134) ==22582== by 0x7A6C42: i2p_fuzz_target(Span<unsigned char const>) (i2p.cpp:37) ACKs for top commit: sipa: utACK 33333755f2edcbe88fcd136f6fef81f94819002e vasild: ACK 33333755f2edcbe88fcd136f6fef81f94819002e Tree-SHA512: 36073582b26b541324b3e55f3fd4a44abf89cb3081f36d361525daf8c27602fbc25f736510ec30df7cb4ca0c4e395e8d8a60f531bf6af358b5a3e65dbabf72c0
2021-04-07Merge #21540: wallet: refactor: dedup sqlite statement preparations/deletionsfanquake
ea19cc844e780b29825b26aee321204be981a3ae wallet: refactor: dedup sqlite statement deletions (Sebastian Falbesoner) 9a3670930eaf6b495f81ef9c5f6e68883a3a2750 wallet: refactor: dedup sqlite statement preparations (Sebastian Falbesoner) Pull request description: This refactoring PR deduplicates repeated SQLite statement preparation calls (`sqlite3_prepare_v2(...)`) / deletions (`sqlite3_finalize(...)`) and its surrounding logic by putting each prepared statement and its corresponding text representation into a ~std::map~ ~`std::array`~ `std::vector`. This should be more readable and less error-prone, e.g. in case an additional statement needs to be added in the future or the error handling has to be adapted. ACKs for top commit: achow101: ACK ea19cc844e780b29825b26aee321204be981a3ae meshcollider: utACK ea19cc844e780b29825b26aee321204be981a3ae Tree-SHA512: ced89869b2147e088e7a4cda2acbbdd4a806f66dbc2d6999953d0d702c0655aa53c0eb699cc7e5e3732f2d24206d577a9d9e1b5de7f439100dead2696ade1092
2021-04-07test: increase listbanned unit test coverageJon Atack
Add test coverage for the new ban_duration and time_remaining fields. While here, some code improvements.