Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 glozow commented on pull request "Fix field name styling in `submitpackage` RPC results":
(https://github.com/bitcoin/bitcoin/pull/31900#issuecomment-2668959139)
> Alright, good to know. It might be noteworthy that there are other projects that directly forward Core's JSON (e.g. the Esplora implementations over at https://github.com/mempool/electrs/pull/105 and https://github.com/Blockstream/electrs/pull/119) that would also be affected by the API breakage

Ah right, I forgot about that. And https://github.com/spesmilo/electrumx/pull/288 and https://github.com/spesmilo/electrum-protocol/pull/1. I think this would inconvenience a few people.

Ok... ev
...
💬 l0rinc commented on pull request "refactor: modernize outdated trait patterns using helper aliases (C++14/C++17)":
(https://github.com/bitcoin/bitcoin/pull/31904#discussion_r1961881831)
Haven't noticed the header, will push a PR to https://github.com/llvm/llvm-project/blob/main/compiler-rt/include/fuzzer/FuzzedDataProvider.h#L206

Should I remove it from here in the meantime?
💬 Sjors commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1961882621)
Good catch, that's also consistent with `waitforblock`.
maflcko closed an issue: "ERROR: AcceptBlockHeader: prev block not found"
(https://github.com/bitcoin/bitcoin/issues/31905)
💬 maflcko commented on issue "ERROR: AcceptBlockHeader: prev block not found":
(https://github.com/bitcoin/bitcoin/issues/31905#issuecomment-2668974005)
Your are running an EOL version of Bitcoin Core.

Is this still an issue with a recent version of Bitcoin Core?
💬 glozow commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1961889923)
Another option would be to revert this.
💬 yancyribbens commented on pull request "refactor: Remove redundant and confusing calls to IsArgSet":
(https://github.com/bitcoin/bitcoin/pull/31896#discussion_r1961892017)
I agree the TODO isn't very clear and possibly no longer applicable. Maybe it could be rounded up with other unclear comments and TODOs in a separate PR. A PR to just remove this one TODO line seems silly.
🤔 ryanofsky reviewed a pull request: "multiprocess: Add libmultiprocess git subtree"
(https://github.com/bitcoin/bitcoin/pull/31741#pullrequestreview-2627160751)
Updated 66c318abe1b7df3e2ff1035376bc5f4b2059626a -> 828c440b0dea29ab41ffc32d88969176d1286655 ([`pr/subtree.17`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.17) -> [`pr/subtree.18`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.18), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.17..pr/subtree.18)) fixing cmake whitespace.
💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1961881829)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1961422233

Good catch, fixed indentation and spacing. I don't think it should be too much of a burden to enforce a consistent style for cmake code given how little cmake code there is relatively. There also seems to be a [cmake-lint](https://cmake-format.readthedocs.io/en/latest/lint-usage.html) tool that could maybe help.
💬 hodlinator commented on issue "qa: timeout in StopHTTPServer()":
(https://github.com/bitcoin/bitcoin/issues/31894#issuecomment-2668999318)
I'm working on making the code a bit more robust:
- Impose a timeout on HTTP connections shutting down - Maybe related to this issue.
- Fix an issue where we were not always processing the event to free eventHTTP - Would probably just leak otherwise, shutdown completes regardless of this in my testing.
```diff
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 88e640c377..ce2534fbd0 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -197,10 +197,15 @@ public:
return WIT
...
💬 Sjors commented on pull request "rpc: add optional blockhash to waitfornewblock, unhide in help":
(https://github.com/bitcoin/bitcoin/pull/30635#issuecomment-2668999401)
This is now based on #31785, and we now unhide all three wait methods.
📝 darosior opened a pull request: "Revert merge of PR #31826"
(https://github.com/bitcoin/bitcoin/pull/31908)
PR #31826 was merged [despite the code not compiling](https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1961315638).

#31902 was opened to fix the code but since this code is only targeting a not officially supported platform, we don't have a CI in place to compile and run tests on this platform, neither apparently reviewers do (nor does the author?), don't take more risk right before 29 and revert the original broken PR.
👍 pablomartin4btc approved a pull request: "cmake: Exclude generated sources from translation"
(https://github.com/bitcoin/bitcoin/pull/31899#pullrequestreview-2627202385)
tACK ff4ddd3d2e3b08156fd0a0aeb954fde0a5f4cb03

Tested in both Ubuntu and macOS.
💬 mzumsande commented on issue "wallet: wrong balance and crash after reorg and unclean shutdown":
(https://github.com/bitcoin/bitcoin/issues/31824#issuecomment-2669011985)
> Is this not where the chainstate related changes are flushed to disk?
> https://github.com/bitcoin/bitcoin/blob/28.x/src/validation.cpp#L3069

The call uses `FlushStateMode::IF_NEEDED`, which means it usually won't do anything - it will only flush if it's necessary because the cache has grown to a critical size.
💬 darosior commented on pull request "random: move VerifyRNDRRS above InitHardwareRand":
(https://github.com/bitcoin/bitcoin/pull/31902#issuecomment-2669021440)
I think for now it's safer to just revert the original PR https://github.com/bitcoin/bitcoin/pull/31908. The changes can be re-considered and re-reviewed after the 29.0 branch-off.
tnull closed a pull request: "Fix field name styling in `submitpackage` RPC results"
(https://github.com/bitcoin/bitcoin/pull/31900)
💬 tnull commented on pull request "Fix field name styling in `submitpackage` RPC results":
(https://github.com/bitcoin/bitcoin/pull/31900#issuecomment-2669032754)
> Ok... even though it's marked as experimental, I think it's too late to break `submitpackage` purely for style reasons. Fwiw I did think this was worthwhile before (and I pinged @tnull after seeing [comment](https://github.com/spesmilo/electrum-protocol/issues/4#issuecomment-2665116596)), but I'm leaning towards nack now. My apologies!

Alright, no worries! Going ahead and closing this.
💬 glozow commented on pull request "Revert merge of PR #31826":
(https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2669053075)
Up to reviewers to decide whether this or #31902 is more appropriate, but added to milestone so we do this before branch off.
💬 maflcko commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2669069081)
So the issue is internal to the functional tests themselves. See https://cirrus-ci.com/task/5346278168592384?logs=ci#L4358 (commit https://github.com/bitcoin/bitcoin/commit/fa780b31ab54febd7e56a0fc165683e056fc2500) which prints:

```
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
python3 50589 root 11u IPv4 179599797 0t0 TCP localhost:60000->localhost:19216 (ESTABLISHED)
bitcoind 50594 root 21u IPv4 179704903 0t0 TCP localhost:19216->localhost:60000 (ESTABLISHED)
...
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1961955650)
`clang-format` doesn't seem to touch this. However once I added some line breaks myself, it suddenly had an opinion. So I'll commit that.