💬 TheCharlatan commented on pull request "guix: use python-minimal (3.9)":
(https://github.com/bitcoin/bitcoin/pull/27361#issuecomment-1488878073)
Concept ACK
The `python-minimal` derivation is defined in https://github.com/guix-mirror/guix/blob/v1.4.0/gnu/packages/python.scm. The `python-minimal` package inherits from the `python-3`, which in turn inherits from `python-2.7`. If I am reading this correctly, the baseline `python` package requires the following input dependencies: `bzip2, expat, gdbm, libffi, sqlite, openssl, readline, zlib, tcl, tk`, while the `python-minimal` package requires `expat, libffi, openssl, zlib`. Seems like a
...
(https://github.com/bitcoin/bitcoin/pull/27361#issuecomment-1488878073)
Concept ACK
The `python-minimal` derivation is defined in https://github.com/guix-mirror/guix/blob/v1.4.0/gnu/packages/python.scm. The `python-minimal` package inherits from the `python-3`, which in turn inherits from `python-2.7`. If I am reading this correctly, the baseline `python` package requires the following input dependencies: `bzip2, expat, gdbm, libffi, sqlite, openssl, readline, zlib, tcl, tk`, while the `python-minimal` package requires `expat, libffi, openssl, zlib`. Seems like a
...
💬 dergoegge commented on pull request "refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer":
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1152170209)
Yes that is fine because `m_best_header` is not used by `UpdateBlockAvailability` afaict.
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1152170209)
Yes that is fine because `m_best_header` is not used by `UpdateBlockAvailability` afaict.
📝 fanquake opened a pull request: "ci: set docker run --ulimit to workaround Valgrind assertion"
(https://github.com/bitcoin/bitcoin/pull/27364)
Running the `native_fuzz_with_valgrind_job`, on aarch64 (Fedora 37), I've seen the following:
```bash
Run addr_info_deserialize with args ['valgrind', '--quiet', '--error-exitcode=1', '/home/fedora/ci_scratch/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz', '-runs=1', '/home/fedora/ci_scratch/ci/scratch/qa-assets/fuzz_seed_corpus/addr_info_deserialize']
valgrind: m_libcfile.c:66 (vgPlain_safe_fd): Assertion 'newfd >= VG_(fd_hard_limit)' failed.
valgrind: m_libcfile
...
(https://github.com/bitcoin/bitcoin/pull/27364)
Running the `native_fuzz_with_valgrind_job`, on aarch64 (Fedora 37), I've seen the following:
```bash
Run addr_info_deserialize with args ['valgrind', '--quiet', '--error-exitcode=1', '/home/fedora/ci_scratch/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz', '-runs=1', '/home/fedora/ci_scratch/ci/scratch/qa-assets/fuzz_seed_corpus/addr_info_deserialize']
valgrind: m_libcfile.c:66 (vgPlain_safe_fd): Assertion 'newfd >= VG_(fd_hard_limit)' failed.
valgrind: m_libcfile
...
💬 glozow commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1488942459)
> @glozow want to take a look at the mempool tests?
LGTM, makes sense to generate 1 to clear mempool after subtests. changing who calls `generate` shouldn't impact mempool_package_limits.
I haven't taken the time to verify that this fixes the test intermittent failure.
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1488942459)
> @glozow want to take a look at the mempool tests?
LGTM, makes sense to generate 1 to clear mempool after subtests. changing who calls `generate` shouldn't impact mempool_package_limits.
I haven't taken the time to verify that this fixes the test intermittent failure.
💬 Tracachang commented on issue "Export a watch wallet only (with descriptors and without private keys) for an air gap setup":
(https://github.com/bitcoin/bitcoin/issues/24829#issuecomment-1488988174)
@pinheadmz
I have followed all the steps using the bash commands from your answer and I am unable to create a receiving taproot address from the GUI (it does not show bech2m but it shows both base58 (legacy & p2sh-segwit and bech32) if I select the watch_wallet, however if I select the "original_wallet" it appears in GUI all 4 including bech32m.
In any case, from console I can `getnewaddress "test" "bech32m"` and it works but for some reason not on GUI.
(https://github.com/bitcoin/bitcoin/issues/24829#issuecomment-1488988174)
@pinheadmz
I have followed all the steps using the bash commands from your answer and I am unable to create a receiving taproot address from the GUI (it does not show bech2m but it shows both base58 (legacy & p2sh-segwit and bech32) if I select the watch_wallet, however if I select the "original_wallet" it appears in GUI all 4 including bech32m.
In any case, from console I can `getnewaddress "test" "bech32m"` and it works but for some reason not on GUI.
💬 TheCharlatan commented on pull request "guix: use python-minimal (3.9)":
(https://github.com/bitcoin/bitcoin/pull/27361#issuecomment-1489001205)
ACK d0e571ebb187
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
53aa12b7fe92d92eaf81a51a8f5d534f92a2f18f0218f29fdf3221d817a53c04 guix-build-d0e571ebb187/output/aarch64-linux-gnu/SHA256SUMS.part
01467530b06b90114561e86968710250e3648fe4aba5c228781ccf9ac72394c2 guix-build-d0e571ebb187/output/aarch64-linux-gnu/bitcoin-d0e571ebb187-aarch64-linux-gnu-debug.tar.gz
fdf6dfc901e371b4e11a7f69eb2ff9b36a3ac393255862bad1dcd45b
...
(https://github.com/bitcoin/bitcoin/pull/27361#issuecomment-1489001205)
ACK d0e571ebb187
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
53aa12b7fe92d92eaf81a51a8f5d534f92a2f18f0218f29fdf3221d817a53c04 guix-build-d0e571ebb187/output/aarch64-linux-gnu/SHA256SUMS.part
01467530b06b90114561e86968710250e3648fe4aba5c228781ccf9ac72394c2 guix-build-d0e571ebb187/output/aarch64-linux-gnu/bitcoin-d0e571ebb187-aarch64-linux-gnu-debug.tar.gz
fdf6dfc901e371b4e11a7f69eb2ff9b36a3ac393255862bad1dcd45b
...
💬 jonatack commented on pull request "rpc, test: remove newline escape sequence from wallet warning fields":
(https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1489034354)
> I found a few other similar mechanisms in other modules, dunno if this change should apply to them as well
Circling back for completeness to cover these two questions.
> https://github.com/bitcoin/bitcoin/blob/82793f1984911774b111117f2e81d5f3b0bbec68/src/validation.cpp#L2565-L2569
This code appends warning messages to the `UpdateTipLog()` logging and looks correct.
> https://github.com/bitcoin/bitcoin/blob/82793f1984911774b111117f2e81d5f3b0bbec68/src/warnings.cpp#L55-L57
The `<h
...
(https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1489034354)
> I found a few other similar mechanisms in other modules, dunno if this change should apply to them as well
Circling back for completeness to cover these two questions.
> https://github.com/bitcoin/bitcoin/blob/82793f1984911774b111117f2e81d5f3b0bbec68/src/validation.cpp#L2565-L2569
This code appends warning messages to the `UpdateTipLog()` logging and looks correct.
> https://github.com/bitcoin/bitcoin/blob/82793f1984911774b111117f2e81d5f3b0bbec68/src/warnings.cpp#L55-L57
The `<h
...
👍 Ayush170-Future approved a pull request: "ci: cleanup of CI_EXEC & CI_EXEC_ROOT"
(https://github.com/bitcoin/bitcoin/pull/27333)
(https://github.com/bitcoin/bitcoin/pull/27333)
💬 furszy commented on pull request "wallet, tests: Expand and test when the blank wallet flag should be un/set":
(https://github.com/bitcoin/bitcoin/pull/25634#issuecomment-1489236967)
Before the text; this is not a blocking comment. Purely about adding some ideas.
> I'm still slightly hesitating though. One question that still bothers me is whether we should expose `blank` flag at all. It's an implementation detail and we only need it for the functional tests. It doesn't seem like there is any observable by the user behaviour of the flag, so why test it with functional tests? Unit tests are better fit for testing internal invariants.
Hmm, I also agree that exposing an i
...
(https://github.com/bitcoin/bitcoin/pull/25634#issuecomment-1489236967)
Before the text; this is not a blocking comment. Purely about adding some ideas.
> I'm still slightly hesitating though. One question that still bothers me is whether we should expose `blank` flag at all. It's an implementation detail and we only need it for the functional tests. It doesn't seem like there is any observable by the user behaviour of the flag, so why test it with functional tests? Unit tests are better fit for testing internal invariants.
Hmm, I also agree that exposing an i
...
💬 arnabnandikgp commented on issue "gen-manpages output depends on build options, so needs to check them":
(https://github.com/bitcoin/bitcoin/issues/17506#issuecomment-1489328037)
wanted to know whether this issue is still active as i am interested in this issue..
(https://github.com/bitcoin/bitcoin/issues/17506#issuecomment-1489328037)
wanted to know whether this issue is still active as i am interested in this issue..
💬 ProofOfKeags commented on issue "Manual-pruning cursor rewind":
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1489330250)
> serving old blocks to bootstrapping nodes
a node should just serve the blocks it has. The cursor rewind should cause it to download earlier blocks as if it was doing IBD only it doesn't have to do any consensus validation except that the block data matches the expected blockhash
> wallet rescans
A rescan has to go back to the wallet birthday irrespective of the pruning cursor.
One other thing to note. I no longer personally need this feature, but I'm simply describing the need as I
...
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1489330250)
> serving old blocks to bootstrapping nodes
a node should just serve the blocks it has. The cursor rewind should cause it to download earlier blocks as if it was doing IBD only it doesn't have to do any consensus validation except that the block data matches the expected blockhash
> wallet rescans
A rescan has to go back to the wallet birthday irrespective of the pruning cursor.
One other thing to note. I no longer personally need this feature, but I'm simply describing the need as I
...
💬 EthanHeilman commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#issuecomment-1489476165)
@hebasto Thanks for the help for getting the CI/CD fixed. The fact that the version of libevent was being pinned to the earlier version explains why testing didn't catch this.
(https://github.com/bitcoin/bitcoin/pull/27335#issuecomment-1489476165)
@hebasto Thanks for the help for getting the CI/CD fixed. The fact that the version of libevent was being pinned to the earlier version explains why testing didn't catch this.
💬 EthanHeilman commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1152592590)
Using the latest version of libevent in CI is now fixes the build.
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1152592590)
Using the latest version of libevent in CI is now fixes the build.
💬 jonatack commented on pull request "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet":
(https://github.com/bitcoin/bitcoin/pull/27279#discussion_r1152609626)
Done, thanks.
(https://github.com/bitcoin/bitcoin/pull/27279#discussion_r1152609626)
Done, thanks.
💬 ajtowns commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1489587649)
My opinion only, but it seems like it would be simpler to have the RPC not take any arguments and always output an object:
```
$ bitcoin-cli getaddrinfo
{
"ipv4": {
"new": 5,
"tried": 3
},
...
}
```
If you want a specific network, you can use jq: `bitcoin-cli getaddrinfo | jq .ipv4` ? You can also do sums this way if desired: `| jq .ipv4.new + ipv6.new`. (Having an object rather than an array makes it easy to get at the values rather than an array and h
...
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1489587649)
My opinion only, but it seems like it would be simpler to have the RPC not take any arguments and always output an object:
```
$ bitcoin-cli getaddrinfo
{
"ipv4": {
"new": 5,
"tried": 3
},
...
}
```
If you want a specific network, you can use jq: `bitcoin-cli getaddrinfo | jq .ipv4` ? You can also do sums this way if desired: `| jq .ipv4.new + ipv6.new`. (Having an object rather than an array makes it easy to get at the values rather than an array and h
...
💬 nopara73 commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1489662232)
@sipa sorry for the late reply, at first I was like "yes, that makes perfect sense," however an argument came to my mind - this morning during shower :) - in favor of it, so I'd like to share that.
Can't we assume miners are seeking the highest fee transactions at all times? In other words: maximize their income from fees. If we could, then adjusting `estimatesmartfee` results based on the possibility of block acceptance of the current mempool state of the node does not seem gameable to me, b
...
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1489662232)
@sipa sorry for the late reply, at first I was like "yes, that makes perfect sense," however an argument came to my mind - this morning during shower :) - in favor of it, so I'd like to share that.
Can't we assume miners are seeking the highest fee transactions at all times? In other words: maximize their income from fees. If we could, then adjusting `estimatesmartfee` results based on the possibility of block acceptance of the current mempool state of the node does not seem gameable to me, b
...
📝 Doodoobrown23 opened a pull request: "Merge bitcoin/bitcoin#27247: [24.x] Bump version to v24.1rc1"
(https://github.com/bitcoin/bitcoin/pull/27367)
932a609312cb7b72bf5f23112763ab933a54389c doc: add initial release notes for v24.1 (fanquake)
cc4e3158c9e7840151fe7b6209ba5f7ea5f103c3 doc: update manual pages for v24.1rc1 (fanquake)
787affb9ea04651a682acf8d09342d6e843fdd0f doc: update version in bips.md to v24.1 (fanquake)
5077e02263cf47749583f22dc2bc609878d8290a build: bump version to v24.1rc1 (fanquake)
Pull request description:
Bump the version number to v24.1rc1.
Regenerate the man pages.
Update the version number in bips.md.
Move
...
(https://github.com/bitcoin/bitcoin/pull/27367)
932a609312cb7b72bf5f23112763ab933a54389c doc: add initial release notes for v24.1 (fanquake)
cc4e3158c9e7840151fe7b6209ba5f7ea5f103c3 doc: update manual pages for v24.1rc1 (fanquake)
787affb9ea04651a682acf8d09342d6e843fdd0f doc: update version in bips.md to v24.1 (fanquake)
5077e02263cf47749583f22dc2bc609878d8290a build: bump version to v24.1rc1 (fanquake)
Pull request description:
Bump the version number to v24.1rc1.
Regenerate the man pages.
Update the version number in bips.md.
Move
...
✅ fanquake closed a pull request: "Merge bitcoin/bitcoin#27247: [24.x] Bump version to v24.1rc1"
(https://github.com/bitcoin/bitcoin/pull/27367)
(https://github.com/bitcoin/bitcoin/pull/27367)
📝 fanquake locked a pull request: "Merge bitcoin/bitcoin#27247: [24.x] Bump version to v24.1rc1"
(https://github.com/bitcoin/bitcoin/pull/27367)
932a609312cb7b72bf5f23112763ab933a54389c doc: add initial release notes for v24.1 (fanquake)
cc4e3158c9e7840151fe7b6209ba5f7ea5f103c3 doc: update manual pages for v24.1rc1 (fanquake)
787affb9ea04651a682acf8d09342d6e843fdd0f doc: update version in bips.md to v24.1 (fanquake)
5077e02263cf47749583f22dc2bc609878d8290a build: bump version to v24.1rc1 (fanquake)
Pull request description:
Bump the version number to v24.1rc1.
Regenerate the man pages.
Update the version number in bips.md.
Move
...
(https://github.com/bitcoin/bitcoin/pull/27367)
932a609312cb7b72bf5f23112763ab933a54389c doc: add initial release notes for v24.1 (fanquake)
cc4e3158c9e7840151fe7b6209ba5f7ea5f103c3 doc: update manual pages for v24.1rc1 (fanquake)
787affb9ea04651a682acf8d09342d6e843fdd0f doc: update version in bips.md to v24.1 (fanquake)
5077e02263cf47749583f22dc2bc609878d8290a build: bump version to v24.1rc1 (fanquake)
Pull request description:
Bump the version number to v24.1rc1.
Regenerate the man pages.
Update the version number in bips.md.
Move
...
💬 Mob1le commented on pull request "fuzz: Remove legacy int parse fuzz tests":
(https://github.com/bitcoin/bitcoin/pull/27344#discussion_r1152829942)
WTF
(https://github.com/bitcoin/bitcoin/pull/27344#discussion_r1152829942)
WTF