Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 maflcko commented on issue "ci: failure in `rpc_scanblocks.py`":
(https://github.com/bitcoin/bitcoin/issues/29831#issuecomment-2071791387)
https://cirrus-ci.com/task/4813688014635008?logs=ci#L3947


```
node1 2024-04-22T20:51:07.549912Z [scheduler] [index/base.cpp:293] [BlockConnected] BlockConnected: WARNING: Block 20c19368f5c214888708d959d40752a61a0c5e2fb5b331876693773c113e5da1 does not connect to an ancestor of known best chain (tip=39455d83324a3dbbb15712ab63dd2f96b45a30f271617aa16c12483a561f721c); not updating index
...
node0 2024-04-22T20:51:08.867727Z [httpworker.2] [rpc/request.cpp:187] [parse] [rpc] ThreadRPCServer
...
📝 9167674641 opened a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/29937)
Mmmmm

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests th
...
willcl-ark closed a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/29937)
📝 fanquake locked a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/29937)
Mmmmm

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests th
...
📝 hanmz opened a pull request: "Fix typos in description.md"
(https://github.com/bitcoin/bitcoin/pull/29938)
Fix typos in description.md.
`digestable` => `digestible`
💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#discussion_r1575956670)
Sgtm, will address it.
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1575987644)
Done. Also refactored orphanage_tests to be more readable.
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1575989359)
Edited the comment to say this is a probably uncommon case
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1575996002)
I definitely it's awkward and doesn't have much benefit. I'll leave the assume/handling here, and maybe in a followup we can change it to non-optional.
💬 maflcko commented on pull request "Fix typos in description.md":
(https://github.com/bitcoin/bitcoin/pull/29938#issuecomment-2071925121)
Also:

```
test/functional/test_framework/wallet_util.py:168: lenghts ==> lengths
💬 laanwj commented on pull request "depends: Remove Qt build-time dependencies":
(https://github.com/bitcoin/bitcoin/pull/29923#issuecomment-2071948004)
i've added some review notes for the Qt patch here: https://github.com/laanwj/qtsowrap?tab=readme-ov-file#patching-qt
🤔 hebasto reviewed a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2016884640)
Approach ACK b7a3f8e375b87e4f3ccfa7994905dfbf46a23bf2.
💬 hebasto commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576040769)
nit: For new source files we can use a copyright header with HTTPS link and future-proof dating:
```suggestion
// Copyright (c) 2024-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or https://opensource.org/license/mit/.
```
💬 hebasto commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576041988)
nit: It seems no symbols from that header are used in the source file.
💬 tdb3 commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2071989207)
> The rebase is wrong.

Thank you. Looks like I had overlooked the addition of `wallet_util` (`TEST_FRAMEWORK_MODULES` being moved to feature_framework_unit_tests.py made it less obvious). Will rebase/push soon.
📝 theStack opened a pull request: "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`"
(https://github.com/bitcoin/bitcoin/pull/29939)
Different MiniWallet instances using the same mode (either ADDRESS_OP_TRUE, RAW_OP_TRUE or RAW_P2PK) currently always create and spend UTXOs with identical output scripts, which can can cause unintentional tx dependencies (see e.g. the discussion in https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565443465). In order to avoid mixing of UTXOs between instances, this PR introduces the possibility to provide a MiniWallet tag name, that is used to derive a different internal key for the t
...
👍 stickies-v approved a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2016873417)
ACK b7a3f8e375b87e4f3ccfa7994905dfbf46a23bf2

Good to remove this unnecessary libevent dependency, and nice work on restructuring the commits etc. Easy to review PR. Left some nits and happy to quickly review if you decide to address them.

---

Not essential for this PR, but I noticed we're doing two unnecessary string copies (through `std::string::substr()`) in code touched by this PR:

<details>
<summary>git diff on b7a3f8e375</summary>

```diff
diff --git a/src/common/url.cpp b/s
...
💬 stickies-v commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576038708)
nit: `std::errc` [is part of `system_error`](https://en.cppreference.com/w/cpp/error/errc)
💬 stickies-v commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576033526)
nit: this should move up a bit:

<details>
<summary>git diff on b7a3f8e375</summary>

```diff
diff --git a/src/Makefile.am b/src/Makefile.am
index 239113a056..7387eb1eaa 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -679,6 +679,7 @@ libbitcoin_common_a_SOURCES = \
common/run_command.cpp \
common/settings.cpp \
common/system.cpp \
+ common/url.cpp \
compressor.cpp \
core_read.cpp \
core_write.cpp \
@@ -711,8 +712,6 @@ libbitcoin_common_a_SOURCES = \

...
💬 stickies-v commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576047415)
typo nit
```suggestion
BOOST_AUTO_TEST_CASE(decode_internal_nulls_test) {
```