👍 willcl-ark approved a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/31648#pullrequestreview-2661414388)
ACK e57359c7dd75283363b5f82675531af167742da5
Now with nits addressed.
(https://github.com/bitcoin/bitcoin/pull/31648#pullrequestreview-2661414388)
ACK e57359c7dd75283363b5f82675531af167742da5
Now with nits addressed.
📝 fanquake opened a pull request: "depends: patch around PlacementNew issue in capnp"
(https://github.com/bitcoin/bitcoin/pull/31998)
See #31772 and https://github.com/capnproto/capnproto/pull/2235.
Given there isn't agreement in #29796, pulled this out so it could be merged separately, and it's easier to run different test configurations externally.
Closes #31772.
(https://github.com/bitcoin/bitcoin/pull/31998)
See #31772 and https://github.com/capnproto/capnproto/pull/2235.
Given there isn't agreement in #29796, pulled this out so it could be merged separately, and it's easier to run different test configurations externally.
Closes #31772.
✅ glozow closed an issue: "Intermittent failures in interface_usdt_mempool.py"
(https://github.com/bitcoin/bitcoin/issues/27380)
(https://github.com/bitcoin/bitcoin/issues/27380)
🚀 glozow merged a pull request: "test, tracing: don't use problematic `bpf_usdt_readarg_p()`"
(https://github.com/bitcoin/bitcoin/pull/31848)
(https://github.com/bitcoin/bitcoin/pull/31848)
⚠️ fanquake opened an issue: "build: document `BITCOIN_GENBUILD_NO_GIT` environment variable?"
(https://github.com/bitcoin/bitcoin/issues/31999)
This was ported from Autotools, however it's entirely undocumented, so it's unclear what we are trying to support, or why (and if we can just drop this entirely). It'd be good if it was documented, even just inline in the source: https://github.com/bitcoin/bitcoin/blob/36d4bd7fe326ba69e9372f8ddb3b12a3b5b36a3d/cmake/script/GenerateBuildInfo.cmake#L33 so it's more clear what the use-case is.
Note that as-of `29.x`, the ways we distribute our source code will be via git clone, or using a tarball t
...
(https://github.com/bitcoin/bitcoin/issues/31999)
This was ported from Autotools, however it's entirely undocumented, so it's unclear what we are trying to support, or why (and if we can just drop this entirely). It'd be good if it was documented, even just inline in the source: https://github.com/bitcoin/bitcoin/blob/36d4bd7fe326ba69e9372f8ddb3b12a3b5b36a3d/cmake/script/GenerateBuildInfo.cmake#L33 so it's more clear what the use-case is.
Note that as-of `29.x`, the ways we distribute our source code will be via git clone, or using a tarball t
...
💬 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
...