🚀 fanquake merged a pull request: "rpc: doc: Added `longpollid` and `data` params to `template_request`"
(https://github.com/bitcoin/bitcoin/pull/28056)
(https://github.com/bitcoin/bitcoin/pull/28056)
✅ fanquake closed an issue: "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate"
(https://github.com/bitcoin/bitcoin/issues/27998)
(https://github.com/bitcoin/bitcoin/issues/27998)
📝 MarcoFalke opened a pull request: "doc: Clarify that -fstack-reuse=all bugs exist on all versions of GCC"
(https://github.com/bitcoin/bitcoin/pull/28105)
This is a follow-up to commit 7b850bc2a1cd8547a2dbb5a18173f53439601220. While the test case no longer reproduces, the general class of `-fstack-reuse` bugs still exists in all versions of GCC. The workaround can never be removed, unless the whole class of bugs is fixed.
(https://github.com/bitcoin/bitcoin/pull/28105)
This is a follow-up to commit 7b850bc2a1cd8547a2dbb5a18173f53439601220. While the test case no longer reproduces, the general class of `-fstack-reuse` bugs still exists in all versions of GCC. The workaround can never be removed, unless the whole class of bugs is fixed.
🚀 fanquake merged a pull request: "test: remove race in the user-agent reception check"
(https://github.com/bitcoin/bitcoin/pull/27986)
(https://github.com/bitcoin/bitcoin/pull/27986)
👍 fanquake approved a pull request: "test: Add missing `set -ex` to ci/lint/06_script.sh"
(https://github.com/bitcoin/bitcoin/pull/28103#pullrequestreview-1536890199)
ACK ffff4b5dc57c32bf759b705530c1368de4aa787e
(https://github.com/bitcoin/bitcoin/pull/28103#pullrequestreview-1536890199)
ACK ffff4b5dc57c32bf759b705530c1368de4aa787e
🚀 fanquake merged a pull request: "depends: xcb-proto 1.15.2"
(https://github.com/bitcoin/bitcoin/pull/28097)
(https://github.com/bitcoin/bitcoin/pull/28097)
💬 jonatack commented on pull request "test: move remaining rand code from util/setup_common to util/random":
(https://github.com/bitcoin/bitcoin/pull/27425#issuecomment-1641941150)
> My suggestion, for future reference:
Thank you, will have a look.
(https://github.com/bitcoin/bitcoin/pull/27425#issuecomment-1641941150)
> My suggestion, for future reference:
Thank you, will have a look.
📝 MarcoFalke converted_to_draft a pull request: "ci: Use qemu-user through container engine"
(https://github.com/bitcoin/bitcoin/pull/28087)
Currently the CI containers always run on the host architecture, and only wrap `bitcoind` into `qemu-user` when needed. This has many issues:
* The `i386` tasks can not be run on non-x86 hosts.
* `config.guess` isn't present when building the CI image, which is fine. But it prints a warning, see https://github.com/bitcoin/bitcoin/pull/27739#pullrequestreview-1446580353
* The python tests are run on the host architecture, making it harder to find architecture specific bugs. See for example h
...
(https://github.com/bitcoin/bitcoin/pull/28087)
Currently the CI containers always run on the host architecture, and only wrap `bitcoind` into `qemu-user` when needed. This has many issues:
* The `i386` tasks can not be run on non-x86 hosts.
* `config.guess` isn't present when building the CI image, which is fine. But it prints a warning, see https://github.com/bitcoin/bitcoin/pull/27739#pullrequestreview-1446580353
* The python tests are run on the host architecture, making it harder to find architecture specific bugs. See for example h
...
💬 pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1267962992)
thanks, that is leftover from older version
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1267962992)
thanks, that is leftover from older version
💬 pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1267964112)
sure thanks
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1267964112)
sure thanks
💬 jonatack commented on pull request "I2P: also sleep after errors in Accept() & destroy the session if we get "Session was closed"":
(https://github.com/bitcoin/bitcoin/pull/28077#issuecomment-1641960701)
> this unexpected behavior causes Bitcoin Core to log way too many messages.
The log flooding rate is very high, but perhaps the more severe issue fixed here is that the I2P connections remain down until the router is manually stopped and then restarted, during which time the log flooding continues, and for anyone running `onlynet=i2p`, it would mean their node is effectively offline until they notice it and take action.
With this fix, the connections recover on their own, like they do for
...
(https://github.com/bitcoin/bitcoin/pull/28077#issuecomment-1641960701)
> this unexpected behavior causes Bitcoin Core to log way too many messages.
The log flooding rate is very high, but perhaps the more severe issue fixed here is that the I2P connections remain down until the router is manually stopped and then restarted, during which time the log flooding continues, and for anyone running `onlynet=i2p`, it would mean their node is effectively offline until they notice it and take action.
With this fix, the connections recover on their own, like they do for
...
💬 jonatack commented on pull request "validation: use noexcept instead of deprecated throw()":
(https://github.com/bitcoin/bitcoin/pull/28090#issuecomment-1641966462)
Post-merge ack
(https://github.com/bitcoin/bitcoin/pull/28090#issuecomment-1641966462)
Post-merge ack
💬 pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1267981369)
done thanks. agreed about the smell. lets lock down the expected behavior with tests and then we can clean up in future PR.
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1267981369)
done thanks. agreed about the smell. lets lock down the expected behavior with tests and then we can clean up in future PR.
💬 stickies-v commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1267987502)
As discussed offline, what I meant with "leave it as is" is to not move banman in this PR, i.e. reverting the change in this PR.
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1267987502)
As discussed offline, what I meant with "leave it as is" is to not move banman in this PR, i.e. reverting the change in this PR.
💬 darosior commented on pull request "Wallet: estimate the size of signed inputs using descriptors":
(https://github.com/bitcoin/bitcoin/pull/26567#issuecomment-1641982667)
Rebased.
(https://github.com/bitcoin/bitcoin/pull/26567#issuecomment-1641982667)
Rebased.
💬 stickies-v commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1267996774)
To summarize offline discussion: `CNode` [uses](https://github.com/bitcoin/bitcoin/blob/78a983f597af224e017f522443112cec81422fe6/src/net.h#L556) move semantics for this, but as you pointed out we construct many more `CNode` instances than we do `PeerManager`s. I'm not sure if the decreased readability trade-off is worth the benefits/sticking to best practices, so happy to have this marked as resolved.
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1267996774)
To summarize offline discussion: `CNode` [uses](https://github.com/bitcoin/bitcoin/blob/78a983f597af224e017f522443112cec81422fe6/src/net.h#L556) move semantics for this, but as you pointed out we construct many more `CNode` instances than we do `PeerManager`s. I'm not sure if the decreased readability trade-off is worth the benefits/sticking to best practices, so happy to have this marked as resolved.
💬 dergoegge commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268004838)
Removed banman from the options on latest push
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268004838)
Removed banman from the options on latest push
💬 jonatack commented on pull request "I2P: also sleep after errors in Accept() & destroy the session if we get "Session was closed"":
(https://github.com/bitcoin/bitcoin/pull/28077#issuecomment-1641997568)
https://geti2p.net/en/download has just updated their available macOS version from 1.9 to 2.3 -- will retest.
(https://github.com/bitcoin/bitcoin/pull/28077#issuecomment-1641997568)
https://geti2p.net/en/download has just updated their available macOS version from 1.9 to 2.3 -- will retest.
💬 stickies-v commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268013582)
What's the rationale for using `m_node.banman.get()` instead of `nullptr` or instead of the locally constructed `banman.get()`? This comment applies to all 4 changes in this file.
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268013582)
What's the rationale for using `m_node.banman.get()` instead of `nullptr` or instead of the locally constructed `banman.get()`? This comment applies to all 4 changes in this file.
💬 stickies-v commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268014256)
Probably no need to switch the parameter placement of `banman`?
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268014256)
Probably no need to switch the parameter placement of `banman`?