💬 fanquake commented on issue "build: document `BITCOIN_GENBUILD_NO_GIT` environment variable?":
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2701263841)
cc @hebasto
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2701263841)
cc @hebasto
💬 laanwj commented on pull request "test, tracing: don't use problematic `bpf_usdt_readarg_p()`":
(https://github.com/bitcoin/bitcoin/pull/31848#issuecomment-2701278673)
Post-merge code review ACK a0b66b4bffaa6bc354c293785b785c2da2cef4de
(https://github.com/bitcoin/bitcoin/pull/31848#issuecomment-2701278673)
Post-merge code review ACK a0b66b4bffaa6bc354c293785b785c2da2cef4de
💬 melvincarvalho commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)":
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2701297653)
@1440000bytes wrote:
> not on this pull request
Discussions related to this GitHub PR should remain on GitHub and the mailing list, where Bitcoin development has historically been debated and recorded. Moving discussions to multiple external platforms with different registration requirements, user bases, and change control mechanisms creates fragmentation and makes it harder to maintain a clear, consistent record of technical decisions.
For example, your [tweet from Jan 26](https://x.co
...
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2701297653)
@1440000bytes wrote:
> not on this pull request
Discussions related to this GitHub PR should remain on GitHub and the mailing list, where Bitcoin development has historically been debated and recorded. Moving discussions to multiple external platforms with different registration requirements, user bases, and change control mechanisms creates fragmentation and makes it harder to maintain a clear, consistent record of technical decisions.
For example, your [tweet from Jan 26](https://x.co
...
💬 pinheadmz commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)":
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2701307917)
@melvincarvalho please review the moderation policy: https://github.com/bitcoin-core/meta/blob/main/MODERATION-GUIDELINES.md
Github pull request review comments are restricted exclusively for PULL REQUEST REVIEW COMMENTS
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2701307917)
@melvincarvalho please review the moderation policy: https://github.com/bitcoin-core/meta/blob/main/MODERATION-GUIDELINES.md
Github pull request review comments are restricted exclusively for PULL REQUEST REVIEW COMMENTS
💬 fjahr commented on pull request "test: chain reorg for coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/31885#issuecomment-2701314632)
We are usually using a combination of functional and unit tests and for user facing features like the index which directly changes how `gettxoutsetinfo` works and what results it returns, we definitely need functional tests to check that end to end. Where we draw the line and if it would be appropriate to have some duplication between unit and functional tests is a bigger question and I think there should probably an issue to discuss this with a reference to this PR as an example. I think people
...
(https://github.com/bitcoin/bitcoin/pull/31885#issuecomment-2701314632)
We are usually using a combination of functional and unit tests and for user facing features like the index which directly changes how `gettxoutsetinfo` works and what results it returns, we definitely need functional tests to check that end to end. Where we draw the line and if it would be appropriate to have some duplication between unit and functional tests is a bigger question and I think there should probably an issue to discuss this with a reference to this PR as an example. I think people
...
📝 fanquake opened a pull request: "Update minisketch subtree to d1e6bb8bbf8ef104b9dd002cab14a71b91061177"
(https://github.com/bitcoin/bitcoin/pull/32000)
Includes:
* https://github.com/bitcoin-core/minisketch/pull/92
(https://github.com/bitcoin/bitcoin/pull/32000)
Includes:
* https://github.com/bitcoin-core/minisketch/pull/92
💬 furszy commented on pull request "wallet: fix crash on double block disconnection":
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1981686931)
> Aren't you garunteeing the shutdown was not clean on L124 by asserting the "invalid" block is still the tip?
I was talking about the wallet best block locator inside the parentheses. My wording was really not the best.
Updated, thanks. Hopefully it is more understandable now.
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1981686931)
> Aren't you garunteeing the shutdown was not clean on L124 by asserting the "invalid" block is still the tip?
I was talking about the wallet best block locator inside the parentheses. My wording was really not the best.
Updated, thanks. Hopefully it is more understandable now.
💬 furszy commented on pull request "wallet: fix crash on double block disconnection":
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1981689826)
Updated per feedback, thanks!. Hopefully the wording is better now.
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1981689826)
Updated per feedback, thanks!. Hopefully the wording is better now.
⚠️ fanquake opened an issue: "ci: `native_fuzz_with_*` jobs are broken"
(https://github.com/bitcoin/bitcoin/issues/32001)
Noticed in https://github.com/bitcoin-core/qa-assets/pull/219.
https://cirrus-ci.com/task/4864208556261376?logs=ci#L7169:
```bash
[100%] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/__/__/wallet/test/fuzz/notifications.cpp.o
[100%] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/__/__/wallet/test/fuzz/scriptpubkeyman.cpp.o
[100%] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/__/__/wallet/test/fuzz/spend.cpp.o
[100%] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/__
...
(https://github.com/bitcoin/bitcoin/issues/32001)
Noticed in https://github.com/bitcoin-core/qa-assets/pull/219.
https://cirrus-ci.com/task/4864208556261376?logs=ci#L7169:
```bash
[100%] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/__/__/wallet/test/fuzz/notifications.cpp.o
[100%] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/__/__/wallet/test/fuzz/scriptpubkeyman.cpp.o
[100%] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/__/__/wallet/test/fuzz/spend.cpp.o
[100%] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/__
...
✅ fanquake closed an issue: "Manually Banning Peers Results in Crash"
(https://github.com/bitcoin/bitcoin/issues/29916)
(https://github.com/bitcoin/bitcoin/issues/29916)
💬 fanquake commented on issue "Manually Banning Peers Results in Crash":
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2701345537)
Closing for now. Could be re-opened / followed up with if we get some concrete steps to repro.
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2701345537)
Closing for now. Could be re-opened / followed up with if we get some concrete steps to repro.
💬 furszy commented on pull request "wallet: fix crash on double block disconnection":
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1981697362)
> However, I don't understand this comment. Hypothetically, if the state were to be flushed before abrupt node kill, wouldn't this assertion here fail?
I was talking about the wallet best locator, which is different to the node's best block (`getwalletinfo()['lastprocessedblock']` vs `getbestblockhash()`).
Just updated the test to reflect this in a more understandable manner.
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1981697362)
> However, I don't understand this comment. Hypothetically, if the state were to be flushed before abrupt node kill, wouldn't this assertion here fail?
I was talking about the wallet best locator, which is different to the node's best block (`getwalletinfo()['lastprocessedblock']` vs `getbestblockhash()`).
Just updated the test to reflect this in a more understandable manner.
💬 l0rinc commented on pull request "RFC: Add `operator""_uint256` compile-time user-defined literal":
(https://github.com/bitcoin/bitcoin/pull/31991#discussion_r1981715292)
Done, thanks
(https://github.com/bitcoin/bitcoin/pull/31991#discussion_r1981715292)
Done, thanks
💬 fanquake commented on issue "guix: re-enable exported symbol checking for RISC-V":
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-2701377875)
> Holding this off until https://github.com/bitcoin/bitcoin/pull/29987 is.merged.
This has gone in, and I've confirmed that this is still an issue with current master (0391d7e4c24e49ed186215e9fa375903c19af86e). @laanwj any chance you still wanted to investigate?
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-2701377875)
> Holding this off until https://github.com/bitcoin/bitcoin/pull/29987 is.merged.
This has gone in, and I've confirmed that this is still an issue with current master (0391d7e4c24e49ed186215e9fa375903c19af86e). @laanwj any chance you still wanted to investigate?
💬 ryanofsky commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2701441513)
re: https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2701132902
> @ryanofsky what would be a good way for a consumer of `waitNext()`, directly or via IPC, to abort a wait? Currently only a node shutdown can interrupt it, but in the context of a Template Provider, a stratum client disconnect should probably also abort the wait.
Could add a cancel method and call it from the destructor, which will get called automatically when a connection is broken. Maybe like:
<details><summar
...
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2701441513)
re: https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2701132902
> @ryanofsky what would be a good way for a consumer of `waitNext()`, directly or via IPC, to abort a wait? Currently only a node shutdown can interrupt it, but in the context of a Template Provider, a stratum client disconnect should probably also abort the wait.
Could add a cancel method and call it from the destructor, which will get called automatically when a connection is broken. Maybe like:
<details><summar
...
📝 fanquake opened a pull request: "doc: add note to Windows build about stripping bins"
(https://github.com/bitcoin/bitcoin/pull/32002)
The Windows binaries are particularly big when they contain debug info, closing in on 500mb. Add a note to the Windows build instructions about using `install/strip`. It seems that `--prefix` cannot be used with `--target install/strip`, so the instructions are re-ordered such that any install prefix is handled during configure.
I haven't tested this. If we don't want to add this note, in favour of [user-presents or similar](https://github.com/bitcoin/bitcoin/issues/30593#issuecomment-2271304
...
(https://github.com/bitcoin/bitcoin/pull/32002)
The Windows binaries are particularly big when they contain debug info, closing in on 500mb. Add a note to the Windows build instructions about using `install/strip`. It seems that `--prefix` cannot be used with `--target install/strip`, so the instructions are re-ordered such that any install prefix is handled during configure.
I haven't tested this. If we don't want to add this note, in favour of [user-presents or similar](https://github.com/bitcoin/bitcoin/issues/30593#issuecomment-2271304
...
💬 dongcarl commented on issue "guix: Unable to reproduce macOS SDK tarball on Fedora 40":
(https://github.com/bitcoin/bitcoin/issues/31873#issuecomment-2701538499)
This is likely because python >= 3.11 delegates `gzip.compress` to `zlib.compress`, which then delegates to the `zlib` specified at configure-time for python.
On Ubuntu 24.04, this is `zlib`/`zlib1g`: https://packages.ubuntu.com/noble/python3.12-minimal
On Fedora 40, this is `zlib-ng-compat`: https://packages.fedoraproject.org/pkgs/python3.11/python3.11-libs/fedora-40.html
(https://github.com/bitcoin/bitcoin/issues/31873#issuecomment-2701538499)
This is likely because python >= 3.11 delegates `gzip.compress` to `zlib.compress`, which then delegates to the `zlib` specified at configure-time for python.
On Ubuntu 24.04, this is `zlib`/`zlib1g`: https://packages.ubuntu.com/noble/python3.12-minimal
On Fedora 40, this is `zlib-ng-compat`: https://packages.fedoraproject.org/pkgs/python3.11/python3.11-libs/fedora-40.html
💬 achow101 commented on issue "`DEFAULT_TRANSACTION_MAXFEE` is 0.1 ₿":
(https://github.com/bitcoin/bitcoin/issues/31716#issuecomment-2701543605)
> any thoughts here?
We should probably fix this at some point.
(https://github.com/bitcoin/bitcoin/issues/31716#issuecomment-2701543605)
> any thoughts here?
We should probably fix this at some point.
🤔 pablomartin4btc reviewed a pull request: "doc: update fuzz instructions when on macOS"
(https://github.com/bitcoin/bitcoin/pull/31954#pullrequestreview-2661800122)
tACK 75486c8ed87a480b9f0c4dc7a10f3cd4eee87b12
Managed to reproduce the issue in `master` (before merge) and this PR fixes it.
(https://github.com/bitcoin/bitcoin/pull/31954#pullrequestreview-2661800122)
tACK 75486c8ed87a480b9f0c4dc7a10f3cd4eee87b12
Managed to reproduce the issue in `master` (before merge) and this PR fixes it.
💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2701564492)
> > I don't think they are. @davidgumberg did you compare this behavior against master?
>
> [#31407 (comment)](https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2699746082).
There's no comparison against master in that comment.
Just tried opening the 28.1 unsigned app and it gives the exact same error.
Furthermore, this PR does not touch unsigned binaries at all. If there is a regression there, it's caused by something else.
> Can you elaborate on why this is this expecte
...
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2701564492)
> > I don't think they are. @davidgumberg did you compare this behavior against master?
>
> [#31407 (comment)](https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2699746082).
There's no comparison against master in that comment.
Just tried opening the 28.1 unsigned app and it gives the exact same error.
Furthermore, this PR does not touch unsigned binaries at all. If there is a regression there, it's caused by something else.
> Can you elaborate on why this is this expecte
...