Age | Commit message (Collapse) | Author |
|
314b49bd50906c03911d2b17a21a34685a60b3c8 gui: Fix regression in GUI console (Hennadii Stepanov)
Pull request description:
The regression was introduced in #19056: if the GUI is running without `-server=1`, the `*txoutset*` call in the console returns "Shutting down".
Fix #19255.
ACKs for top commit:
ryanofsky:
Code review ACK 314b49bd50906c03911d2b17a21a34685a60b3c8. Only change since last review is rebase
Tree-SHA512: 8ff85641a5c249858fecb1ab69c7a1b2850af651ff2a94aa41ce352b5b5bc95bc45c41e1767e871b51e647612d09e4d54ede3e20c313488afef5678826c51b62
|
|
abstract class
b82f0ca4d5465b36debb6c57f335bdccf4899c49 walletdb: Add MakeBatch function to BerkeleyDatabase and use it (Andrew Chow)
eac9200814fa01da6522625be01dded730b26751 walletdb: Refactor DatabaseBatch abstract class from BerkeleyBatch (Andrew Chow)
Pull request description:
In order to support alternative database systems, we need to have a generic `Batch` class. This PR adds a `DatabaseBatch` abstract class which is implemented by `BerkeleyBatch`. `DatabaseBatch` is now the class that is used by `WalletBatch` to interact with the database. To be able to get the correct type of `DatabaseBatch`, `BerkeleyDatabase` now has a `MakeBatch` function which returns a newly constructed `std::unique_ptr<DatabaseBatch>`. For `BerkeleyDatabase`, that will be `std::unique_ptr<BerkeleyBatch>`.
The `Read`, `Write`, `Erase`, and `Exists` template functions are moved from `BerkeleyBatch`.
Part of #18971
Requires #19308 and #19324
ACKs for top commit:
Sjors:
re-utACK b82f0ca4d5465b36debb6c57f335bdccf4899c49
MarcoFalke:
ACK b82f0ca4d5465b36debb6c57f335bdccf4899c49 🌘
meshcollider:
LGTM, utACK b82f0ca4d5465b36debb6c57f335bdccf4899c49
Tree-SHA512: 6d2d41631c0983391dbecd702e881c6775b155c90b275df97f7157e42608ed251744f9d7ce5173d02a6c5cc38d90b611880fac7fa635d3d8c4d590681f56ac6a
|
|
fa9f20b6477a206adf5089398803b45d1a114b6f log: Properly log txs rejected from mempool (MarcoFalke)
Pull request description:
Currently `CheckTxInputs` rejections from the mempool are the only rejections that log directly and unconditionally to debug.log instead of leaving it to the caller. This has multiple issues:
* A rejected RPC transaction will log a redundant failure reason to debug log. All other failures are merely reported to the RPC user.
* A rejected p2p transaction will log the failure twice. Once with the `MEMPOOLREJ` flag, and once unconditionally.
* A rejected orphan transaction will log no failure.
Fix all issues by simply returning the state to the caller, like it is done for all other rejections.
The patch includes whitespace fixups to highlight relevant parts of the codebase and simplify review.
ACKs for top commit:
naumenkogs:
utACK fa9f20b6477a206adf5089398803b45d1a114b6f
rajarshimaitra:
Concept ACK. Compiled and ran tests. `fa9f20b`
jnewbery:
code review ACK fa9f20b6477a206adf5089398803b45d1a114b6f
Tree-SHA512: 86cc17b2a9239c01c4fc3f254ad48ee1d3883266966b9811030176338b9ac3deaea7ea5babfb8bbf739d7440154e30011fede8f9313175f199d4a062af6494f7
|
|
ca3585a483ca5f6fc4cc54fd1530f89d13e5b7b0 [net/net processing] check banman pointer before dereferencing (John Newbery)
Pull request description:
Although we currently don't do this, it should be possible to create a
CConnman or PeerLogicValidation without a Banman instance. Therefore
always check that banman exists before dereferencing the pointer.
Also add comments to the m_banman members of CConnman and
PeerLogicValidation to document that these may be nullptr.
ACKs for top commit:
jonatack:
ACK ca3585a
theStack:
ACK https://github.com/bitcoin/bitcoin/commit/ca3585a483ca5f6fc4cc54fd1530f89d13e5b7b0
Tree-SHA512: 726401c8921b9a502029ead34ae797473a1bc359d6e4e58dcbe3e25b70dde40bb100723be467fd3e2bf418892c493911998226de19c9d529d72034e3be26be48
|
|
Although we currently don't do this, it should be possible to create a
CConnman or PeerLogicValidation without a Banman instance. Therefore
always check that banman exists before dereferencing the pointer.
Also add comments to the m_banman members of CConnman and
PeerLogicValidation to document that these may be nullptr.
|
|
d842e6ac965b528f0d704f54aceb91eae84085fb doc: Add non-thread-safe note to FeeFilterRounder::round() (Hennadii Stepanov)
Pull request description:
The `FastRandomContext` class is documented as not thread-safe.
This PR adds a relevant note to the `FeeFilterRounder::round()` function declaration.
Close #19254
ACKs for top commit:
MarcoFalke:
self ACK d842e6ac965b528f0d704f54aceb91eae84085fb
practicalswift:
ACK d842e6ac965b528f0d704f54aceb91eae84085fb: explicit is better than implicit
naumenkogs:
ACK d842e6a
Tree-SHA512: 538508f24b9cb29baece6a64108e2c5fc3960768c6475c4f2baf48a4a7bdb96dcef1a74d21a4822e1f8635e1375362986da4e3a20f5644129046a354c4b0a8a0
|
|
timeouts
60824b3c3a4221a31c3638008b05fecc040c3df2 ci: Fix configure options for macOS builds (Hennadii Stepanov)
687939e3d251125aea5f6f7202e00a12d60a899f ci: Drop Homebrew caching while using Homebrew addon on Travis (Hennadii Stepanov)
557d3f1cc065702ebbf929d39ac7c5561111ce4c ci: Do not activate Travis ccache caching strategy (Hennadii Stepanov)
2d747428e23c0b7e3e7d0ce0db4226372f5ec0dd ci: Disable functional tests on forked repos to avoid timeouts for macOS (Hennadii Stepanov)
Pull request description:
See: https://github.com/bitcoin-core/gui/issues/5#issuecomment-656819184
Additionally, this PR:
- updates macOS image to the recent 10.15.5 version
- drops Homebrew caching as the Travis Homebrew addon have been used since #18438
My forked repo build: https://travis-ci.org/github/hebasto/bitcoin/jobs/707200431
Top commit has no ACKs.
Tree-SHA512: 398e935f965a04babeb10e7b26d2341562f21a1ef671c2e7cc97c9ec79d5c31643f81ca18561ab7714b5c52e19df2e4bffe4223eadbab984daa9418ffbf8c2a8
|
|
06059b0c2a6c2db70c87a7715f8a344a13400fa1 net: rename DEFAULT_BANSCORE_THRESHOLD to DISCOURAGEMENT_THRESHOLD (Jon Atack)
1d4024bca8086cceff7539dd8c15e0b7fe1cc5ea net: remove -banscore configuration option (Jon Atack)
Pull request description:
per https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652684340, https://github.com/bitcoin/bitcoin/pull/19219#discussion_r443074487 and https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652699592. Edit: now split into 3 straightforward PRs:
- net: remove -banscore configuration option (this PR)
- rpc: deprecate banscore field in getpeerinfo (#19469, *merged*)
- gui: no longer display banscores (TBA in the gui repo)
ACKs for top commit:
MarcoFalke:
review ACK 06059b0c2a6c2db70c87a7715f8a344a13400fa1 📙
vasild:
ACK 06059b0c
Tree-SHA512: 03fad249986e0896697033fbb8ba2cbfaae7d7603b1fb2a38b3d41db697630d238623f4d732b9098c82af249ce5a1767dd432b7ca0fec10544e23d24fbd57c50
|
|
f32c408f3a0b7e597977df2bc2cdc4ae298586e5 Make sure unconfirmed parents are requestable (Pieter Wuille)
c4626bcd211af08c85b6567ef07eeae333edba47 Drop setInventoryTxToSend based filtering (Pieter Wuille)
43f02ccbff9b137d59458da7a8afdb0bf80e127f Only respond to requests for recently announced transactions (Pieter Wuille)
b24a17f03982c9cd8fd6ec665b16e022374c96f0 Introduce constant for mempool-based relay separate from mapRelay caching (Pieter Wuille)
a9bc5638031a29abaa40284273a3507b345c31e9 Swap relay pool and mempool lookup (Pieter Wuille)
Pull request description:
This implements the follow-up suggested here: https://github.com/bitcoin/bitcoin/pull/18861#issuecomment-627630111 . Instead of checking `setInventoryTxToSend`, maintain an explicit bloom filter with the 3500 most recently announced invs, and permit fetching any of these as long as they're in the relay pool or the mempool. In addition, permit relay from the mempool after just 2 minutes instead of 15.
This:
* Fixes the brief opportunity an attacker has to request unannounced invs just after the connection is established (pointed out by naumenkogs, see https://github.com/bitcoin/bitcoin/pull/18861#issuecomment-627627010).
* Guarantees that locally resubmitted invs after `filterInventoryKnown` rolls over can still be requested (pointed out by luke-jr, see https://github.com/bitcoin/bitcoin/pull/18861#discussion_r419695831).
It adds 37 KiB of filter per peer.
This is also a step towards dropping the relay pool entirely and always relaying from the mempool directly (see #17303), but that is still blocked by dealing properly with NOTFOUNDs (see #18238).
ACKs for top commit:
jnewbery:
reACK f32c408f3
jonatack:
re-ACK f32c408 per `git range-diff f7c19e8 2da7ee3 f32c408` and redid the following: code review, thought about motivation, DoS and privacy aspects, debug build to check for warnings after updating Clang from 6 to 11 since last review.
ajtowns:
re-ACK f32c408f3a0b7e597977df2bc2cdc4ae298586e5
Tree-SHA512: aa05b9fd01bad59581c4ec91836a52d7415dc933fa49d4c4adced79aa25aaad51e11166357e8c8b29fbf6021a7401b98c21b850b5d8e8ad773fdb5d6608e1e85
|
|
b03697b68e24bea7a177f84954c93691450d5638 doc: CONTRIBUTING.md improvements (Jon Atack)
Pull request description:
The motivation here was to add a mention of hygienic commits following a discussion today, e.g. something along the lines of:
*Make sure each individual commit is hygienic, building successfully on its own without warnings, errors, or regressions, and that all tests pass.*
While here, made various fixups. They are optional and can be omitted.
ACKs for top commit:
harding:
ACK b03697b68e24bea7a177f84954c93691450d5638 Locally reviewed the word diff.
MarcoFalke:
ACK b03697b68e24bea7a177f84954c93691450d5638 🚌
practicalswift:
ACK b03697b68e24bea7a177f84954c93691450d5638
hebasto:
ACK b03697b68e24bea7a177f84954c93691450d5638, I have reviewed the changes and they look OK, I agree they can be merged.
Tree-SHA512: 6fb56219c311d914ec18fcf5d50fdbe3a51e4743a8cace93e348cb4a10c83b6fce631518f1455a1804d1fc81558b235bef58a8be6ccb1a010f46aa4143b1ebf5
|
|
|
|
|
|
It is sufficient to specify CCACHE_DIR to cache.
Also this change fixes ccache on native macOS build.
|
|
|
|
fa740d4cea92727601d5d9b474e07f7a89afc2db doc: move-only release notes (MarcoFalke)
faa3d2eee1cc52b76f6eda8271691fdbff1baaa7 doc: Remove release notes of backports (MarcoFalke)
Pull request description:
ACKs for top commit:
laanwj:
ACK fa740d4cea92727601d5d9b474e07f7a89afc2db
Tree-SHA512: 86e7077f9050c65b0b33829892aafabbb783a603a1a77932893659bc1071fb2d2a0515662e7e9e6fdfffb59f86bfc41ad74ba6e86d618bf71a6616d9f7a18fa2
|
|
overlay is shown
d0cc1f6df740e03ca0213a3754c3277b01ae2c05 qt: Disable toolbar when overlay is shown (Hennadii Stepanov)
e74cd2083d579b14b0b718aa36796f2bcf679600 qt, refactor: Cleanup ModalOverlay slots (Hennadii Stepanov)
Pull request description:
Keeping the main window toolbar activated while the modal overlay is shown could create the appearance of the non-responsive GUI.
Fixes #22.
---
On master (ca055885c631de8ac0ffe24be6b02835dbcc039d):
![Screenshot from 2020-07-11 13-07-00](https://user-images.githubusercontent.com/32963518/87221791-7504e100-c377-11ea-9689-ddd4b21b98f9.png)
With this PR:
![Screenshot from 2020-07-11 13-07-39](https://user-images.githubusercontent.com/32963518/87221803-8817b100-c377-11ea-92c8-3602dc4d2451.png)
ACKs for top commit:
harding:
Tested ACK d0cc1f6df740e03ca0213a3754c3277b01ae2c05. Tested on Linux/X11 as much as I could given it's a pretty small change; seems like a nice improvement. I'm not experienced in Qt, but I don't see anything obviously problematic about the code.
jonatack:
ACK d0cc1f6 tested on Debian 5.7.6-1 (2020-06-24) x86_64 GNU/Linux
LarryRuane:
ACK d0cc1f6df740e03ca0213a3754c3277b01ae2c05 tested on Ubuntu 18.04.4 LTS
Tree-SHA512: e371b34231c01e77118deb100e0f280ba1cdef54e317f7f7d6ac322598bda811bd1bfe3035e90d87f8267f4f5d2095d34a8136911159db63694fd1b1b11335a1
|
|
`GETHEADERS_VERSION`
7bb6f9bfdbd3b0b5594febc9a0ae2f84e6d7add1 [protocol] Remove unused GETHEADERS_VERSION (John Newbery)
37a934e6b35bea2125732d2c074998d9fe70633e [protocol] Remove unused CADDR_TIME_VERSION (John Newbery)
Pull request description:
These constants are no longer required and can be removed.
Additional code comments are added to explain CAddress serialization.
ACKs for top commit:
MarcoFalke:
ACK 7bb6f9bfdbd3b0b5594febc9a0ae2f84e6d7add1 already an improvement, but maybe getting rid of INIT_PROTO_VERSION here would be an even stronger improvement (can be done later)
jonatack:
ACK 7bb6f9bfdbd3b0b5594febc9a0ae2f84e6d7add1
vasild:
ACK 7bb6f9bf
Tree-SHA512: 5382562c60fd677c86583754eca11aad3719064efe2e5ef4f307d693b583422ca8d385926c2582aaab899f502b151f2eb87a7ac23363b15f4fceaa06296f98e3
|
|
|
|
Review suggestion:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
|
|
|
|
08fc6f6cfc3b06fd170452a766696d7b833113fa [rpc] refactor: consolidate sendmany and sendtoaddress code (Sjors Provoost)
Pull request description:
I consolidated code between these two RPC calls, since `sendtoaddress` is essentially `sendmany` with 1 destination.
Unless I overlooked something, the only behaviour change is that some `sendtoaddress` error codes changed from `-4` to `-6`. The release note mentions this.
Salvaged from #18201.
ACKs for top commit:
fjahr:
Code review ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
jonatack:
ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
meshcollider:
Code review & functional test run ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
Tree-SHA512: 7b66c52fa0444a4d02fc3f81d9c2a386794d447616026a30111eda35fb46510475eea6506a9ceda00bb4e0230ebb758da5d236b3ac05c954c044fa68a1e3e909
|
|
facd7dd3d1f9d51e1133974ff69eeb48f5ae282b wallet: Fix typo in comments; Simplify assert (MarcoFalke)
Pull request description:
Follow up to https://github.com/bitcoin/bitcoin/pull/19046#discussion_r443783101 and https://github.com/bitcoin/bitcoin/pull/19046#discussion_r443793690
ACKs for top commit:
practicalswift:
ACK facd7dd3d1f9d51e1133974ff69eeb48f5ae282b
jonatack:
ACK facd7dd3d1f9d51e1133974ff69eeb48f5ae282b
hebasto:
ACK facd7dd3d1f9d51e1133974ff69eeb48f5ae282b, spelling verified with `test/lint/lint-spelling.sh`: all remaining warnings are false positive.
Tree-SHA512: 2b185d138058840db56726bb6bcc42e5288a954e2a410c49e04806a047fbbdaf0bb2decc70ecf7613c69caa766655705ca44151613e7ea5015b386d1e726d870
|
|
and move it from validation to net processing.
|
|
|
|
1e58bcc9afefcf009653567c6373b4f7facba8f5 wallet: Fix clang build in Mac (Anthony Fieroni)
Pull request description:
Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>
Top commit has no ACKs.
Tree-SHA512: 19312929af14dab97c37cf4547fbd6589a6de960f1a499c2118bb684240639af4b127cf8dc4d201b41d253cfbb645614a0606d4ecce29f300b10c210d38a961b
|
|
Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>
|
|
|
|
entries
a66a7a1a7060bb422eba3b8c214852416c4280d1 walletdb: don't reinitialize desc cache with multiple cache entries (Andrew Chow)
Pull request description:
When loading descriptor caches, we would accidentally reinitialize the descriptor cache when seeing that one already exists. This should have only been initializing the cache when one does not exist. However this code itself is unnecessary as the act of looking up the cache to add to it will initialize it if it didn't already exist.
This issue could be hit by trying to load a wallet that had imported a multisig descriptor. The wallet would fail to load.
A test has been added to wallet_importdescriptors.py to catch this case. Another test case has also been added to check that loading a wallet with only single key descriptors works.
ACKs for top commit:
hugohn:
tACK [a66a7a1](https://github.com/bitcoin/bitcoin/commit/a66a7a1a7060bb422eba3b8c214852416c4280d1)
jonatack:
ACK a66a7a1a706
meshcollider:
Code review ACK a66a7a1a7060bb422eba3b8c214852416c4280d1
Tree-SHA512: 3df746421a008708eaa3bbbdd12b9ddd3e2ec111d54625a212dca7414b971cc1f6e2b1757b3232c31a2f637d1b1ef43bf3ffa4ac4216646cf1e92db5f79954f1
|
|
is off
fa73493930e35850e877725167dc9d42e47015c8 refactor: Use C++11 range-based for loop (MarcoFalke)
fa7b164d62d9f12e9cda79bf28bf435acf2d1e38 wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off (MarcoFalke)
faf8401c195f52470d1ca6e2c94cb3820e57ee41 wallet: Pass unused args to StartWallets (MarcoFalke)
fa6c186436337c8ed7d9e1ab065377f8cda5c0b7 gui tests: Limit life-time of dummy testing setup (MarcoFalke)
fa28a618972911239a119248ab1194702a5c36d8 test: Add smoke test to check that wallets are flushed by default (MarcoFalke)
Pull request description:
User-facing, this is a refactor. Internally, the scheduler does not have to call a mostly empty function every half a second.
ACKs for top commit:
jnewbery:
utACK fa73493930e35850e877725167dc9d42e47015c8
meshcollider:
utACK fa73493930e35850e877725167dc9d42e47015c8
ryanofsky:
Code review ACK fa73493930e35850e877725167dc9d42e47015c8. Just rebased since last review
Tree-SHA512: 99e1fe1b2c22a3f4b19de3e566241d38693f4fd8d5a68ba1838d86740aa6c08e3325c11a072e30fd262a8861af4278bed52eb9374c85179b8f536477f528247c
|
|
variants
3a9aba21a49a6d80bd187940d5e26893937b6832 Split SetWalletFlags into Add/LoadWalletFlags (Andrew Chow)
d9cd095b5965fc20c09f401370e7ba99446663e3 Split SetActiveScriptPubKeyMan into Add/LoadActiveScriptPubKeyMan (Andrew Chow)
0122fbab4c340b23ae56173de6c5ab866ba25ab8 Split SetHDChain into AddHDChain and LoadHDChain (Andrew Chow)
Pull request description:
`SetHDChaiin`, `SetActiveScriptPubKeyMan`, and `SetWalletFlags` have a `memonly` argument which is kind of confusing, as noted in https://github.com/bitcoin/bitcoin/pull/17681#discussion_r427633081. This PR replaces those functions with `Add*` and `Load*` variants so that they follow the pattern used elsewhere in the wallet.
`AddHDChain`, `AddActiveScriptPubKeyMan`, and `AddWalletFlags` both set their respective variables in `CWallet` and writes them to disk. These functions are used by the actions which modify the wallet such as `sethdseed`, `importdescriptors`, and creating a new wallet.
`LoadHDChain`, `LoadActiveScriptPubKeyMan`, and `LoadWalletFlags` just set the `CWallet` variables. These functions are used by `LoadWallet` when loading the wallet from disk.
ACKs for top commit:
jnewbery:
Code review ACK 3a9aba21a49a6d80bd187940d5e26893937b6832
ryanofsky:
Code review ACK 3a9aba21a49a6d80bd187940d5e26893937b6832. Only changes since last review tweaks making m_wallet_flags updates more safe
meshcollider:
utACK 3a9aba21a49a6d80bd187940d5e26893937b6832
Tree-SHA512: 365aeaafc5ba42879c0eb797ec3beb29ab70e27f917dc880763f743420b3be6ddf797240996beed8a9ad70fb212c2590253c6b44c9dc244529c3939d9538983f
|
|
9c59f9c285303659ee1beed7555bbb322e6e6981 Fix ZapSelectTx to sync wallet spends (Anthony Fieroni)
Pull request description:
Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>
ACKs for top commit:
achow101:
ACK 9c59f9c285303659ee1beed7555bbb322e6e6981
ryanofsky:
Code review ACK 9c59f9c285303659ee1beed7555bbb322e6e6981. Only change since last review tweaking the for loop as suggested
jonatack:
ACK 9c59f9c285303659ee1beed7555bbb322e6e6981 tested rebased on current master b33136b6ba9887f7d and the new unit test does indeed fail without the change.
meshcollider:
utACK 9c59f9c285303659ee1beed7555bbb322e6e6981
Tree-SHA512: 71672a5ab0c659550c3a40577614ea896412b79566b5672636ab18765e4c71b9d0a990d94dc6b6e623b03a05737022b04026b5699438809c7c54782d0fd0a5d2
|
|
|
|
97846d7f5b47ef77469b9f961db77f770e8bcc0f tests: Add fuzzing harness for BanMan (practicalswift)
deba199f1c88c2e5f754b0a4ec43b9ef28de8352 tests: Add ConsumeSubNet(...). Move and increase coverage in ConsumeNetAddr(...). (practicalswift)
Pull request description:
Add fuzzing harness for `BanMan`.
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
Top commit has no ACKs.
Tree-SHA512: f4126c15bbb77638833367d73f58193c8f05d16bed0b1d6c33b39387d5b610ff34af78cd721adb51778062ce3ac5e79756d1c3895ef54c6c80c61dcf056e94ff
|
|
|
|
fab558612278909df93bdf88f5727b04f13aef0f doc: Use precise permission flags where possible (MarcoFalke)
Pull request description:
Instead of mentioning the all-encompassing `-whitelist*` settings, change the docs to mention the exact permission flag that will influence the behaviour.
This is needed because in the future, the too-broad `-whitelist*` settings (they either include *all* permission flags or apply to *all* peers) might be deprecated to require the permission flags to be enumerated.
Alternatively, in the future there could be an RPC to set the net permission flags on an existing connection, in which case the `-whitelist*` terminology is of no help.
ACKs for top commit:
jnewbery:
reACK fab558612278909df93bdf88f5727b04f13aef0f
fjahr:
Code review ACK fab558612278909df93bdf88f5727b04f13aef0f
jonatack:
ACK fab558612278909df93bdf88f5727b04f13aef0f
Tree-SHA512: c7dea3e577d90103bb2b0ffab7b7c8640b388932a3a880f69e2b70747fc9213dc1f437085671fd54c902ec2a578458b8a2fae6dbe076642fb88efbf9fa9e679c
|
|
GETHEADERS_VERSION is unused. No need to keep it in the codebase
for just its historic comment.
|
|
Add comments to CAddress serialization code explaining why
it's no longer needed.
|
|
when handling PROXY requests
20d31bdd92cc2ad9b8d26ed80da73bbcd6016144 tests: Avoid fuzzer-specific nullptr dereference in libevent when handling PROXY requests (practicalswift)
Pull request description:
Avoid constructing requests that will be interpreted by libevent as PROXY requests to avoid triggering a `nullptr` dereference. Split out from #19074 as suggested by MarcoFalke.
The dereference (`req->evcon->http_server`) takes place in `evhttp_parse_request_line` and is a consequence of our hacky but necessary use of the internal function `evhttp_parse_firstline_` in the `http_request` fuzzing harness.
The suggested workaround is not aesthetically pleasing, but it successfully avoids the troublesome code path.
`" http:// HTTP/1.1\n"` was a crashing input prior to this workaround.
Before this PR:
```
$ echo " http:// HTTP/1.1" > input
$ src/test/fuzz/http_request input
src/test/fuzz/http_request: Running 1 inputs 1 time(s) each.
Running: input
AddressSanitizer:DEADLYSIGNAL
=================================================================
==27905==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000108 (pc 0x55a169b7e053 bp 0x7ffd452f1160 sp 0x7ffd452f10e0 T0)
==27905==The signal is caused by a READ memory access.
==27905==Hint: address points to the zero page.
#0 0x55a169b7e053 in evhttp_parse_request_line depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:1883:37
#1 0x55a169b7d9ae in evhttp_parse_firstline_ depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:2041:7
#2 0x55a1687f624e in test_one_input(std::vector<unsigned char, std::allocator<unsigned char> > const&) src/test/fuzz/http_request.cpp:51:9
…
$ echo $?
1
```
After this PR:
```
$ echo " http:// HTTP/1.1" > input
$ src/test/fuzz/http_request input
src/test/fuzz/http_request: Running 1 inputs 1 time(s) each.
Running: input
Executed input in 0 ms
***
*** NOTE: fuzzing was not performed, you have only
*** executed the target code on a fixed set of inputs.
***
$ echo $?
0
```
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
Top commit has no ACKs.
Tree-SHA512: 7a6b68e52cbcd6c117487e74e47760fe03566bec09b0bb606afb3b652edfd22186ab8244e8e27c38cef3fd0d4a6c237fe68b2fd22e0970c349e4ab370cf3e304
|
|
f20b359bb9dabc7be11c3e3319e435aa42a8f0f5 cli: reduce DefaultRequestHandler memory allocations (Jon Atack)
Pull request description:
per https://github.com/bitcoin/bitcoin/pull/16439#discussion_r443957125. Simpler code, fewer allocations. No change of behavior. The code has good test coverage in `interface_bitcoin_cli.py`.
ACKs for top commit:
MarcoFalke:
review ACK f20b359bb9dabc7be11c3e3319e435aa42a8f0f5
fjahr:
Code review ACK f20b359
Tree-SHA512: 745eab44dfdcc485ca2bbc1db8b8d364cbd3cf94982e46e033745ce05ab617c15320ee55da7fb930d365f4d26b172049d5f5efcf0b6d3af5b0a28185bdb93ea8
|
|
MIN_PEER_PROTO_VERSION is greater
57b0c0a93a243769beb306c89560d1eda61f54bd Drop CADDR_TIME_VERSION checks now that MIN_PEER_PROTO_VERSION is greater (Ben Woosley)
Pull request description:
We do not connect to peers older than 31800
ACKs for top commit:
sipa:
Code reivew ACK 57b0c0a93a243769beb306c89560d1eda61f54bd
jnewbery:
Code review ACK 57b0c0a93a243769beb306c89560d1eda61f54bd
vasild:
ACK 57b0c0a9
Tree-SHA512: e1ca7c9203cbad83ab7c7a2312777ad07ed6a16119169b256648b8a8738c260a5168acdd4fb33f6e4b17f51ec7e033e110b76bde55b4e3b2d444dc02c01bc2b1
|
|
fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3 net: Use mockable time for ping/pong, add tests (MarcoFalke)
faab4aaf2fa1153c6d76efc8113fa01b06943ece util: Add count_microseconds helper (MarcoFalke)
Pull request description:
Switch `CNode::m_ping_start` and `CNetMessage::m_time` to mockable time, so that tests can be added.
Mockable time is also type-safe, since it uses `std::chrono`
ACKs for top commit:
jonatack:
Code review re-ACK fa33654 re-read code, verified rebase per `git range-diff 4b5c919 fa94d6f fa33654`, previous tested ACKs still valid
troygiorshev:
ACK fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3
Tree-SHA512: 7d632bd6019ce7c882029e71b667a61517e783af82755a85dd979ef09380934e172dec8b8f91d57b200a30a6e096aeaf01f19fee7f3aed0e0e871c72eb44d70e
|
|
|
|
41d55d30579358c805036201664ad6a1c1d48681 doc: getpeerinfo banscore deprecation release note (Jon Atack)
dd54e3796e633cfdf6954af306afd26eadc25116 test: getpeerinfo banscore deprecation test (Jon Atack)
8c7647b3fbbab03ea84071cf3cd2d0d2bf8be255 rpc: deprecate banscore field in rpc getpeerinfo (Jon Atack)
Pull request description:
Per https://github.com/bitcoin/bitcoin/pull/19219#discussion_r443074487 and https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652699592, this PR deprecates returning the `banscore` field in the `getpeerinfo` RPC, updates the help, adds a test, and updates the release notes. Related to #19464.
ACKs for top commit:
fanquake:
ACK 41d55d30579358c805036201664ad6a1c1d48681
Tree-SHA512: 8eca08332581e2fe191a2aafff6ba89ce39413f0491ed0de8b86577739f0ec430b1a8fbff2914b0f3138a229563dfcc1981c0cf5b7dd6061b5c48680a28423bc
|
|
fa6d5ab67444013f9db696cca2257c871e002594 refactor: Remove unused BlockAssembler::pblock member var (MarcoFalke)
Pull request description:
It seems odd to have a confusing and fragile "convenience pointer" member variable to be able to write `pblock->vtx` instead of `pblocktemplate->block.vtx` in a single place.
ACKs for top commit:
promag:
Code review ACK fa6d5ab67444013f9db696cca2257c871e002594.
Tree-SHA512: e9f032b5ab702dbefffd370db3768ebfb95c13acc732972b695281ea34c91d70cd0a1700bc2c6f106dbc9de68e81bc6bb06c68c2afd53c17cba8ebee4f9931b9
|
|
a4a3fc4cd2e6f53cdffcc2962fd152a4e40c7413 doc: improve subtree check instructions (Sjors Provoost)
Pull request description:
Running `git-subtree-check.sh` requires adding the subtree repository as a remote. I learned that several years ago and then forgot again.
This PR also improves the error message if the subtree commit can't be found.
ACKs for top commit:
laanwj:
ACK a4a3fc4cd2e6f53cdffcc2962fd152a4e40c7413
fanquake:
ACK a4a3fc4cd2e6f53cdffcc2962fd152a4e40c7413 - this looks ok.
Tree-SHA512: 959bd923726c172d17f9f97f8a56988bf2df5a94d3131e5152a66150b941394cee9e82fdc6b86e09c0ba91d123a496599f07ca454212168d8d301738394c12c8
|
|
Instead of having WalletBatch construct the BerkeleyBatch, have
BerkeleyDatabase do it and return a std::unique_ptr<BerkeleyBatch>
|
|
|
|
fa0540cd46eaf44d9e1a9f91c3a937986826c4fa net: Extract download permission from noban (MarcoFalke)
Pull request description:
It should be possible to grant nodes in a local network (e.g. home, university, enterprise, ...) permission to download blocks even after the maxuploadtarget is hit.
Currently this is only possible by setting the `noban` permission, which has some adverse effects, especially if the peers can't be fully trusted.
Fix this by extracting a `download` permission from `noban`.
ACKs for top commit:
jonatack:
ACK fa0540c
Sjors:
re-utACK fa0540cd46eaf44d9e1a9f91c3a937986826c4fa
Tree-SHA512: 255566baa43ae925d93f5d0a3aa66b475a556d1590f662a88278a4872f16a1a05739a6119ae48a293011868042e05cb264cffe5822a50fb80db7333bf44376d9
|
|
1cabbddbca615b26aa4510c75f459c28d6fe0afd refactor: Use uint16_t instead of unsigned short (Aaron Hook)
Pull request description:
I wanted to see if the `up for grabs` label works and looked at PR #17822 originally opend by ahook I saw it had many acks for example by jonatack and practicalswift but needed rebasing.
So I checked out the remote branch rebased it resolved three conflicts and continued the rebase.
Hope everything is as expected (:
ACKs for top commit:
sipsorcery:
ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd.
practicalswift:
ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd -- patch looks correct :)
laanwj:
ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd
hebasto:
ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 0e6bf64f274aae5dacb188358b4d5f65ccb207d4f70922f039bc4ed7934709418ddad19f8bfb7462517427837c3d2bb3f86ef284bb40e87119aad2a1e148d9d6
|
|
fc6a637a013daeb14b2f93652d7f494f3b8462aa qt: increase console command max length (10xcryptodev)
Pull request description:
fix #17618
Tested the examples https://github.com/bitcoin/bitcoin/issues/17618#issuecomment-559538070 and works
ACKs for top commit:
MarcoFalke:
Approach ACK fc6a637a013daeb14b2f93652d7f494f3b8462aa
hebasto:
ACK fc6a637a013daeb14b2f93652d7f494f3b8462aa, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 4975d7fa4c13a6b0f50f5754c3e04eb5a42b1411c385dc883d9948b6fc0dee38900ba2a418218a9a30ce39988a27d22f3ff3a02f0fa44f4136f01eef473efeca
|