💬 stickies-v commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1612215147)
Sounds good, I've added release notes to cover that. The way I read the startup option docstring _("Execute command when an alert is raised (%s in cmd is replaced by message)")_, I think the new behaviour aligns better with how I'd expect this option to behave.
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1612215147)
Sounds good, I've added release notes to cover that. The way I read the startup option docstring _("Execute command when an alert is raised (%s in cmd is replaced by message)")_, I think the new behaviour aligns better with how I'd expect this option to behave.
💬 stickies-v commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1612240542)
> I guess this change also affects the order of warnings.
It does indeed. I hadn't really considered this, and I think I agree that the current behaviour is fine - but I'll think about it more and open to suggestions.
> Changing this is probably fine, but it would be good to note the change in the commit message.
I've updated the commit message section on behaviour change to:
<details>
```
Introduces behaviour change:
- the `-alertnotify` command is executed for all
`KernelNotifi
...
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1612240542)
> I guess this change also affects the order of warnings.
It does indeed. I hadn't really considered this, and I think I agree that the current behaviour is fine - but I'll think about it more and open to suggestions.
> Changing this is probably fine, but it would be good to note the change in the commit message.
I've updated the commit message section on behaviour change to:
<details>
```
Introduces behaviour change:
- the `-alertnotify` command is executed for all
`KernelNotifi
...
💬 stickies-v commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1612216154)
Ah makes sense, I've updated that, thanks.
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1612216154)
Ah makes sense, I've updated that, thanks.
💬 stickies-v commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1612194682)
Thanks, fixed.
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1612194682)
Thanks, fixed.
💬 stickies-v commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1612228366)
> Can commit message be updated to say what this change in behavior here is?
I've updated the commit message:
<details>
```
node: update uiInterface whenever warnings updated
This commit introduces slight behaviour change. Previously, the
GUI status bar would be updated for most warnings, namely
UNKNOWN_NEW_RULES_ACTIVATED, CLOCK_OUT_OF_SYNC and
PRE_RELEASE_TEST_BUILD, but not for LARGE_WORK_INVALID_CHAIN
(and not for FATAL_INTERNAL_ERROR, but that is not really
meaningful).
...
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1612228366)
> Can commit message be updated to say what this change in behavior here is?
I've updated the commit message:
<details>
```
node: update uiInterface whenever warnings updated
This commit introduces slight behaviour change. Previously, the
GUI status bar would be updated for most warnings, namely
UNKNOWN_NEW_RULES_ACTIVATED, CLOCK_OUT_OF_SYNC and
PRE_RELEASE_TEST_BUILD, but not for LARGE_WORK_INVALID_CHAIN
(and not for FATAL_INTERNAL_ERROR, but that is not really
meaningful).
...
🤔 jonatack reviewed a pull request: "net: log connections failures via SOCKS5 with less severity"
(https://github.com/bitcoin/bitcoin/pull/30064#pullrequestreview-2075246052)
Concept ACK, [`a0be62c` (#25203)](https://github.com/bitcoin/bitcoin/pull/25203/commits/a0be62c9043caaebf64b3178b9bea080fa200ec7#diff-3499e52d708f04ebd0bfeec799dd26464ca6bd26a802c700460227c4f41ec4b5R543) contains these changes and further ones, perhaps look if other ones there could be done here.
(https://github.com/bitcoin/bitcoin/pull/30064#pullrequestreview-2075246052)
Concept ACK, [`a0be62c` (#25203)](https://github.com/bitcoin/bitcoin/pull/25203/commits/a0be62c9043caaebf64b3178b9bea080fa200ec7#diff-3499e52d708f04ebd0bfeec799dd26464ca6bd26a802c700460227c4f41ec4b5R543) contains these changes and further ones, perhaps look if other ones there could be done here.
💬 jonatack commented on pull request "net: log connections failures via SOCKS5 with less severity":
(https://github.com/bitcoin/bitcoin/pull/30064#discussion_r1612411821)
In this case, I had added `Manual` and considered it an error (or perhaps warning), what do you think: https://github.com/bitcoin/bitcoin/pull/25203/commits/a0be62c9043caaebf64b3178b9bea080fa200ec7#diff-3499e52d708f04ebd0bfeec799dd26464ca6bd26a802c700460227c4f41ec4b5R541
```suggestion
LogPrintLevel(BCLog::NET, BCLog::Level::Error, "Manual %s\n", error_message);
```
(https://github.com/bitcoin/bitcoin/pull/30064#discussion_r1612411821)
In this case, I had added `Manual` and considered it an error (or perhaps warning), what do you think: https://github.com/bitcoin/bitcoin/pull/25203/commits/a0be62c9043caaebf64b3178b9bea080fa200ec7#diff-3499e52d708f04ebd0bfeec799dd26464ca6bd26a802c700460227c4f41ec4b5R541
```suggestion
LogPrintLevel(BCLog::NET, BCLog::Level::Error, "Manual %s\n", error_message);
```
💬 hebasto commented on pull request "[PoC] ci: Add FreeBSD GitHub Actions job":
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2128162922)
cc @vasild
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2128162922)
cc @vasild
💬 luke-jr commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2128176766)
Somehow you introduced a typo in the commit message >_<
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2128176766)
Somehow you introduced a typo in the commit message >_<
💬 epiccurious commented on pull request "doc: update mention of generating bitcoin.conf":
(https://github.com/bitcoin/bitcoin/pull/30154#issuecomment-2128181002)
> squash this into a single commit and update the PR description
Squashed into 9013e2b97e8f50d2be63ce740c42d0b0e0b9b7f2 and updated PR description.
(https://github.com/bitcoin/bitcoin/pull/30154#issuecomment-2128181002)
> squash this into a single commit and update the PR description
Squashed into 9013e2b97e8f50d2be63ce740c42d0b0e0b9b7f2 and updated PR description.
💬 apulsifer commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2128231134)
Yes, all the checksum errors are in numerically-named NNNNNN.ldb files in /bdata/bitcoin-data/chainstate/
I had no luck with gettxoutsetinfo muhash:
bitcoin/bin/bitcoin-cli -rpcwaittimeout=0 -conf=/home/ec2-user/bitcoin.conf getblockcount
844826
bitcoin/bin/bitcoin-cli -rpcwaittimeout=0 -conf=/home/ec2-user/bitcoin.conf gettxoutsetinfo muhash
error: timeout on transient error: Could not connect to the server 127.0.0.1:8332 (error code 0 - "timeout reached")
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2128231134)
Yes, all the checksum errors are in numerically-named NNNNNN.ldb files in /bdata/bitcoin-data/chainstate/
I had no luck with gettxoutsetinfo muhash:
bitcoin/bin/bitcoin-cli -rpcwaittimeout=0 -conf=/home/ec2-user/bitcoin.conf getblockcount
844826
bitcoin/bin/bitcoin-cli -rpcwaittimeout=0 -conf=/home/ec2-user/bitcoin.conf gettxoutsetinfo muhash
error: timeout on transient error: Could not connect to the server 127.0.0.1:8332 (error code 0 - "timeout reached")
💬 ajtowns commented on pull request "contrib/signet/miner: increase miner search space":
(https://github.com/bitcoin/bitcoin/pull/30130#discussion_r1612527980)
I think you also would want to change:
```
if tmpl["mintime"] > mine_time:
logging.info("Updating block time from %d to %d", mine_time, tmpl["mintime"])
mine_time = tmpl["mintime"]
```
to be
```
mintime = tmpl["mintime"] + retries
if mine_time < mintime:
logging.info(...)
mine_time = mintime
```
(https://github.com/bitcoin/bitcoin/pull/30130#discussion_r1612527980)
I think you also would want to change:
```
if tmpl["mintime"] > mine_time:
logging.info("Updating block time from %d to %d", mine_time, tmpl["mintime"])
mine_time = tmpl["mintime"]
```
to be
```
mintime = tmpl["mintime"] + retries
if mine_time < mintime:
logging.info(...)
mine_time = mintime
```
💬 edilmedeiros commented on pull request "doc: update mention of generating bitcoin.conf":
(https://github.com/bitcoin/bitcoin/pull/30154#issuecomment-2128296962)
ACK 9013e2b97e8f50d2be63ce740c42d0b0e0b9b7f2
(https://github.com/bitcoin/bitcoin/pull/30154#issuecomment-2128296962)
ACK 9013e2b97e8f50d2be63ce740c42d0b0e0b9b7f2
⚠️ lcharles123 opened an issue: ""netinfo" doesn't show IPv6 "Local addresses""
(https://github.com/bitcoin/bitcoin/issues/30165)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
"Local addresses" section of "netinfo" does not show the IPv6 addresses added using "externalip" config option. (Probably there is no announce of this addresses).
(These are not my real addresses).
root@debianpc:~# bitcoin-cli -netinfo
` Bitcoin Core client v27.0.0 - server 70016/Satoshi:27.0.0/
ipv4 onion i2p npr total block
in 0 1
...
(https://github.com/bitcoin/bitcoin/issues/30165)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
"Local addresses" section of "netinfo" does not show the IPv6 addresses added using "externalip" config option. (Probably there is no announce of this addresses).
(These are not my real addresses).
root@debianpc:~# bitcoin-cli -netinfo
` Bitcoin Core client v27.0.0 - server 70016/Satoshi:27.0.0/
ipv4 onion i2p npr total block
in 0 1
...
💬 jonatack commented on issue ""netinfo" doesn't show IPv6 "Local addresses"":
(https://github.com/bitcoin/bitcoin/issues/30165#issuecomment-2128360266)
Thank you for highlighting this. This is an edge case that I've been meaning to open a fix for since a long time, along with a couple other ones. I'll do that shortly.
(https://github.com/bitcoin/bitcoin/issues/30165#issuecomment-2128360266)
Thank you for highlighting this. This is an edge case that I've been meaning to open a fix for since a long time, along with a couple other ones. I'll do that shortly.
💬 jonatack commented on issue ""netinfo" doesn't show IPv6 "Local addresses"":
(https://github.com/bitcoin/bitcoin/issues/30165#issuecomment-2128362673)
Can you post the result of running `./src/bitcoin-cli getnetworkinfo` on your machine (with modified IPs)
(https://github.com/bitcoin/bitcoin/issues/30165#issuecomment-2128362673)
Can you post the result of running `./src/bitcoin-cli getnetworkinfo` on your machine (with modified IPs)
💬 lcharles123 commented on issue ""netinfo" doesn't show IPv6 "Local addresses"":
(https://github.com/bitcoin/bitcoin/issues/30165#issuecomment-2128367476)
root@debianpc:~# bcli getnetworkinfo
{
"version": 270000,
"subversion": "/Satoshi:27.0.0/",
"protocolversion": 70016,
"localservices": "0000000000000c09",
"localservicesnames": [
"NETWORK",
"WITNESS",
"NETWORK_LIMITED",
"P2P_V2"
],
"localrelay": true,
"timeoffset": -8,
"networkactive": true,
"connections": 68,
"connections_in": 58,
"connections_out": 10,
"networks": [
{
"name": "ipv4",
"limited": false,
"reacha
...
(https://github.com/bitcoin/bitcoin/issues/30165#issuecomment-2128367476)
root@debianpc:~# bcli getnetworkinfo
{
"version": 270000,
"subversion": "/Satoshi:27.0.0/",
"protocolversion": 70016,
"localservices": "0000000000000c09",
"localservicesnames": [
"NETWORK",
"WITNESS",
"NETWORK_LIMITED",
"P2P_V2"
],
"localrelay": true,
"timeoffset": -8,
"networkactive": true,
"connections": 68,
"connections_in": 58,
"connections_out": 10,
"networks": [
{
"name": "ipv4",
"limited": false,
"reacha
...
💬 kristapsk commented on pull request "[PoC] ci: Add FreeBSD GitHub Actions job":
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2128466549)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2128466549)
Concept ACK
💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2128638490)
> Somehow you introduced a typo in the commit message >_<
Oh.. sorry. fixed and added a spellchecker.
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2128638490)
> Somehow you introduced a typo in the commit message >_<
Oh.. sorry. fixed and added a spellchecker.
💬 maflcko commented on pull request "[PoC] ci: Add FreeBSD GitHub Actions job":
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2128649877)
> IMPORTANT. The `vmactions/*` needs to be added to the "Actions permissions" section of the repository settings.
Not sure. What was the conclusion when GHA was added to the repo about enabling third party actions? IIRC it was unclear whether malicious or backdoored actions could compromise the repo, so the conclusion was to refrain from enabling them?
No objection to merging this, but I'd say unless it is clear that the repo can not be compromised, the action should not be enabled in thi
...
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2128649877)
> IMPORTANT. The `vmactions/*` needs to be added to the "Actions permissions" section of the repository settings.
Not sure. What was the conclusion when GHA was added to the repo about enabling third party actions? IIRC it was unclear whether malicious or backdoored actions could compromise the repo, so the conclusion was to refrain from enabling them?
No objection to merging this, but I'd say unless it is clear that the repo can not be compromised, the action should not be enabled in thi
...