Bitcoin Core Github
42 subscribers
125K links
Download Telegram
💬 virtu commented on pull request "[WIP] seeds: Add additional seed source and bump uptime requirements for Onion and I2P nodes":
(https://github.com/bitcoin/bitcoin/pull/30695#discussion_r1732113690)
nice, added!
💬 maflcko commented on pull request "rpc: add `revelant_blocks` to `scanblocks status`":
(https://github.com/bitcoin/bitcoin/pull/30713#discussion_r1732132002)
That'd introduce a race, where the relevant blocks are cleared before the reserver is destructed (and thus the status call may return an empty list where previously it didn't for the same call)

Not sure if it matters, or worthy to address.
💬 virtu commented on pull request "[WIP] seeds: Add additional seed source and bump uptime requirements for Onion and I2P nodes":
(https://github.com/bitcoin/bitcoin/pull/30695#issuecomment-2311586771)
> I was manually curating I2P nodes based on trusted colleagues (akin to addnode peer selection), filtered by connection reliability and regularly seeing blocks from them. (Edit: I now see that you have ones that I'd recommend, so seems good. There were a couple that were missing, but I see that they, like me, began pruning.)

Good to know the automatic process is getting all of cjdns nodes you were tracking (modulo pruning).

Data from @luke-jr's seed is now included as well. As before: 512
...
💬 maflcko commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2311591619)
> **Notable**: I removed the case for "-342 Service unavailable, RPC server started but is shutting down due to error". Seems like we should be failing if the server is shutting down instead of _silently ignoring it_.

That'd revert 62c304ea481d474bc87d950e21907b8b05134fe7 and thus break existing tests, no?
💬 maflcko commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2311611155)
For reference: You can use `git log -S "unique_code_blob"` to see when (and hopefully why) a piece of code was added. In this case it would be something like:

```
$ git log -S ' != -342'
commit 62c304ea481d474bc87d950e21907b8b05134fe7
Date: Sat Oct 6 13:42:11 2018 +0800

tests: Allow closed http server in assert_start_raises_init_error
💬 virtu commented on pull request "seeds: Pull additional nodes from my seeder and update fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/30008#issuecomment-2311628787)
just rebased #30695 since #30008 got merged and noticed it accidentally removes all hardcoded onion and i2p seeds
💬 Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2311682864)
re-utACK 46c8d75d24d3355ec468f7f608effe94636e16db

#30690 just modified the script this PR deletes.
💬 maflcko commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2311690611)
Other than that the patch

> Seen recently here (`wallet_bumpfee.py --descriptors`) https://github.com/bitcoin/bitcoin/actions/runs/9863555844/job/27236587622#step:27:12496:

This one looks different, because `node1` never actually starts to start up. It doesn't even get to emit an `10061` error.
💬 maflcko commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2311697202)
As for the ignored_errors counter, I presume it will just count `errno.ECONNREFUSED` for WinError 10061?
💬 maflcko commented on pull request "bench: [refactor] iwyu":
(https://github.com/bitcoin/bitcoin/pull/30716#issuecomment-2311698582)
> I still see IWYU warnings

Thanks, fixed.
🤔 mzumsande reviewed a pull request: "chainparams: Remove seed.bitcoinstats.com"
(https://github.com/bitcoin/bitcoin/pull/30720#pullrequestreview-2262327212)
ACK c88a7dc53e3be7489605c3326cf768df5437393a

It can be re-added when it's fixed.
💬 mzumsande commented on pull request "validation: improve m_best_header tracking":
(https://github.com/bitcoin/bitcoin/pull/30666#issuecomment-2311729974)
> You mean if you invalidateblock, then reconsiderblock, the rest of the child blocks also need to be reconsidered individually?

That's already happening, reconsiderblock already goes over the entire block index db on master, looking at each index individually, see `Chainstate::ResetBlockFailureFlags` (this also has a bug, #30479, but that is unrelated to the failure flags).
💬 maflcko commented on pull request "doc: Drop description of LogError messages as fatal":
(https://github.com/bitcoin/bitcoin/pull/30361#issuecomment-2311750723)
> I think I might not have enough information to understand the weak nack

It is the line prior: "Seems odd to change something twice, when it can be changed once? weak NACK for now."

Now that the other pull is closed, I guess this is fine.
💬 maflcko commented on pull request "doc: Drop description of LogError messages as fatal":
(https://github.com/bitcoin/bitcoin/pull/30361#issuecomment-2311759659)
lgtm ACK b7aae361b273f2f439d3b278214b7e37908c8cb0

Seems fine to change the documentation here to be a description of what the code does right now. Also, it seems fine for this description to be applied going forward, because both an "Error" or "Warning" should be considered an Alert for the node admin to investigate (and possibly dismiss) (with or without the node shutting down).

There are probably cases where an Error can be "downgraded" to a Warning in the code, but they can be done in f
...
💬 hodlinator commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2311773405)
Thanks, I should have searched for why the -342 check was introduced before I suggested removing it. :facepalm: And my git-fu could do with some leveling up, that's impressive how it can single out the right commit.

Updated my [suggested diff](https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2311170636), bringing back the -342 check.

> As for the ignored_errors counter, I presume it will just count errno.ECONNREFUSED for WinError 10061?

Yes, that's what [ConnectionRefusedErr
...
💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1732338260)
I added it just to make the `updateRwSetting` method easier to read. I will update when their is need to retouch.
💬 hodlinator commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2311831456)
> Other than that the patch
>
> > Seen recently here (`wallet_bumpfee.py --descriptors`) https://github.com/bitcoin/bitcoin/actions/runs/9863555844/job/27236587622#step:27:12496:
>
> This one looks different, because `node1` never actually starts to start up. It doesn't even get to emit an `10061` error.

The linked log has:
```
node1 2024-07-09T20:27:21.198248Z [shutoff] [D:\a\bitcoin\bitcoin\src\init.cpp:386] [Shutdown] Shutdown: done
test 2024-07-09T20:27:21.233000Z TestFramewo
...
💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#issuecomment-2311832660)
> Any reason not to just lock the mutex across the three steps?

Yes the reasons where highlighted in https://github.com/bitcoin/bitcoin/pull/30697#pullrequestreview-2258765600 and https://github.com/bitcoin/bitcoin/pull/30697#pullrequestreview-2260433636 comments