Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 hodlinator commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019968139)
Thanks! Fixed.
💬 l0rinc commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019968258)
commit message still contains `seen_multipath`
💬 l0rinc commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019968705)
this implicit empty constructor was always weird to me, a more explicit and uniform alternative could be:
```suggestion
substitutes = {};
```
💬 hodlinator commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019970351)
To me `.emplace()` appears more specific. Assigning `{}` seems to make it `std::nullopt`, leading to test failures.
💬 hodlinator commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019969499)
It also contains:

> Rename it to seen_substitutes to better describe what it does, now that the context implies its involved in multipath.
💬 l0rinc commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019971150)
ok, if that's deliberate, please resolve this
💬 l0rinc commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2019971282)
My mistake, thanks for checking
👍 l0rinc approved a pull request: "descriptors: Multipath/PR 22838 follow-ups"
(https://github.com/bitcoin/bitcoin/pull/32134#pullrequestreview-2727715824)
utACK 56f271e9b9c82f40054d63d4b638584bd2faef00
🤔 Prabhat1308 reviewed a pull request: "fuzz: Make partially_downloaded_block more deterministic"
(https://github.com/bitcoin/bitcoin/pull/32158#pullrequestreview-2727721222)
tACK [`fa1e299`](https://github.com/bitcoin/bitcoin/pull/32158/commits/fa1e2995d9996641e79f92549e99a6b37e36d140)

Tested 10 runs with each 32 and 128 parallel threads on MacOS.
Steps followed
```
cmake -B build -DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" \
-DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" \
-DCMAKE_C_FLAGS="-fprofile-instr-generate -fcoverage-mapping" \
-DCMAKE_CXX_FLAGS="-fprofile-instr-generate -fcoverage-mapping" \
-DBUILD_FOR_FUZZING=ON
...
💬 hodlinator commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2764238498)
Also seeing warnings regarding locale (https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2762968613, https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2763240357). (Which one would hope for reproducible builds).

Guix (x86_64 NixOS):
```
6ea4a76be3383337e57d6a12450bd589776ebb3fd0d9161347766ef845241e13 guix-build-c4861570e468/output/aarch64-linux-gnu/SHA256SUMS.part
3eb7656483dfe47fa6b7cf40bceb3decda73474c813edb224d42840adb8b49d6 guix-build-c4861570e468/output/aarch64-lin
...
💬 laanwj commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2764240416)
i built with the following patch to set a UTF-8 locale check if there would be a difference:
```patch
diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh
index 26180bae24..47366c7e11 100755
--- a/contrib/guix/libexec/build.sh
+++ b/contrib/guix/libexec/build.sh
@@ -2,7 +2,8 @@
# Copyright (c) 2019-2022 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.p
...
👍 TheCharlatan approved a pull request: "validation: stricter internal handling of invalid blocks"
(https://github.com/bitcoin/bitcoin/pull/31405#pullrequestreview-2727393482)
crACK 7b6fe5a2196d53a3024dfe7f11b316a50acc3cb7

This reduces complexity overall and I am happy that the slightly verbose block in `AcceptBlockHeader` and the `m_dirty_blockindex` are gone. It is not really straight forward to review though and I hope I did not miss any edge cases or real performance drawbacks. Might be good to get some more eyes on this. Having a better abstraction on the BlockMap / block tree data structure that just handles all of this internally could make things easier in
...
💬 TheCharlatan commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2019820910)
In commit fbe9e2a391d5f6e14998776b03d4ffbbf43f33c8:

Nit: Should this change really be in this commit? It does make sense in the next commit. I don't think it would be a bug per se to never erase, but it might be less efficient.
📝 olaristo109 opened a pull request: "Grammatical typos in benchmarking.md"
(https://github.com/bitcoin/bitcoin/pull/32163)
docs: Corrected grammatical and stylistic errors in benchmarking.md

This commit addresses several grammatical typos and improves the clarity and readability of the benchmarking.md documentation. It also standardizes terminology
# On branch grammatical_typos_in_benchmarking.md
# Changes to be committed:
# modified: doc/benchmarking.md
#
pinheadmz closed a pull request: "Grammatical typos in benchmarking.md"
(https://github.com/bitcoin/bitcoin/pull/32163)
💬 pinheadmz commented on pull request "Grammatical typos in benchmarking.md":
(https://github.com/bitcoin/bitcoin/pull/32163#issuecomment-2764255685)
Typo fixes are generally not accepted from new contributors because they are a burden on regular reviewers. I encourage you to find a more significant way to contribute to this code base and open a new PR.

See contributing and developer-notes in our docs.
📝 doug369shirey opened a pull request: "Create aws.yml"
(https://github.com/bitcoin/bitcoin/pull/32164)
<!--
UN commissioner of cryptocurrency IHL Humanitarian BeinExchange involvement since 2008 sashatoshi mr pool Mr forbes always Beinhauer born Shirey living one enternity.
*** 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
...
🤔 doug369shirey reviewed a pull request: "Create aws.yml"
(https://github.com/bitcoin/bitcoin/pull/32164#pullrequestreview-2727736154)
Green

> ``
💬 mabu44 commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#issuecomment-2764269830)
tACK 56f271e9b9c82f40054d63d4b638584bd2faef00

<details>
<summary>Testing procedure</summary>

```bash
./bitcoind -regtest
```
```bash
./bitcoin-cli createwallet pr32134 false true
```
```bash
./bitcoin-cli importdescriptors '[{"desc":"wpkh(tprv8ZgxMBicQKsPdGJed5SjAaeKBLjKoYKbp4E8855hsG3HNiFuaHNprjb4UmuovVsvAQ7Hwa2MdBsrgyhePCyVgMPnTgQdigFFZLCJTkNm7CK/<0;1>/*)#p5mvj4c3","timestamp":0,"range":[0,999],"active":true}]'
```
Output:
```bash
[
{
"success": true
}
]
```
```
...
hebasto closed a pull request: "Create aws.yml"
(https://github.com/bitcoin/bitcoin/pull/32164)