💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1217942753)
Looks like this doesn't work some of the time:
* https://cirrus-ci.com/task/5798762795237376?logs=ci#L267
* https://cirrus-ci.com/task/4640353173635072?logs=ci#L266
```
++ echo 'Restart docker before run to stop and clear all containers started with --rm'
Restart docker before run to stop and clear all containers started with --rm
++ podman container kill --all
++ echo 'Prune all dangling images'
Prune all dangling images
++ docker image prune --force
Emulate Docker CLI using podma
...
(https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1217942753)
Looks like this doesn't work some of the time:
* https://cirrus-ci.com/task/5798762795237376?logs=ci#L267
* https://cirrus-ci.com/task/4640353173635072?logs=ci#L266
```
++ echo 'Restart docker before run to stop and clear all containers started with --rm'
Restart docker before run to stop and clear all containers started with --rm
++ podman container kill --all
++ echo 'Prune all dangling images'
Prune all dangling images
++ docker image prune --force
Emulate Docker CLI using podma
...
📝 Sjors opened a pull request: "validation: log which peer sent us a header"
(https://github.com/bitcoin/bitcoin/pull/27826)
Fixes #27744
Since #27278 we log received headers. For compact blocks we also log which peer sent it (e5ce8576349d404c466b2f4cab1ca7bf920904b2), but not for regular headers. That required an additional refactor, which this PR provides.
Move the logging from validation to net_processing.
This also reduces the number of log entries (under default configuration) per compact block header from 3 to 2: one for the header and one for the connected tip.
The PR introduces a new helper method
...
(https://github.com/bitcoin/bitcoin/pull/27826)
Fixes #27744
Since #27278 we log received headers. For compact blocks we also log which peer sent it (e5ce8576349d404c466b2f4cab1ca7bf920904b2), but not for regular headers. That required an additional refactor, which this PR provides.
Move the logging from validation to net_processing.
This also reduces the number of log entries (under default configuration) per compact block header from 3 to 2: one for the header and one for the connected tip.
The PR introduces a new helper method
...
💬 hebasto commented on pull request "ci: enable AArch64 target in MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27824#discussion_r1217967152)
> Would be nice to just detect and build for the current CPU, if that is possible?
What about not providing the value at all, and using the default one?
(https://github.com/bitcoin/bitcoin/pull/27824#discussion_r1217967152)
> Would be nice to just detect and build for the current CPU, if that is possible?
What about not providing the value at all, and using the default one?
💬 Sjors commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1576652503)
cc @jamesob, @instagibbs, @mzumsande
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1576652503)
cc @jamesob, @instagibbs, @mzumsande
💬 Sjors commented on pull request "contrib: verify torrents with verify-binary.py":
(https://github.com/bitcoin/bitcoin/pull/27762#issuecomment-1576661757)
In the case of Github, I believe you can get individual raw files too.
But there's something to be said for not being dependent on having Git installed, and also have the option to not download if you used a torrent before. So maybe we can have a `--signatures-from-repo` like option in that case? For non-torrent imo this should be `true` by default if git is installed. For torrents the option could be suggested.
(https://github.com/bitcoin/bitcoin/pull/27762#issuecomment-1576661757)
In the case of Github, I believe you can get individual raw files too.
But there's something to be said for not being dependent on having Git installed, and also have the option to not download if you used a torrent before. So maybe we can have a `--signatures-from-repo` like option in that case? For non-torrent imo this should be `true` by default if git is installed. For torrents the option could be suggested.
💬 hebasto commented on pull request "refactor (tidy): Fixes after enable-debug configure":
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1576664795)
re: https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1538515538
> or I think we could just close this?
Having https://github.com/bitcoin/bitcoin/pull/27724 been merged, I agree to close this PR.
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1576664795)
re: https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1538515538
> or I think we could just close this?
Having https://github.com/bitcoin/bitcoin/pull/27724 been merged, I agree to close this PR.
✅ glozow closed a pull request: "refactor (tidy): Fixes after enable-debug configure"
(https://github.com/bitcoin/bitcoin/pull/27353)
(https://github.com/bitcoin/bitcoin/pull/27353)
📝 Sjors converted_to_draft a pull request: "validation: log which peer sent us a header"
(https://github.com/bitcoin/bitcoin/pull/27826)
Fixes #27744
Since #27278 we log received headers. For compact blocks we also log which peer sent it (e5ce8576349d404c466b2f4cab1ca7bf920904b2), but not for regular headers. That required an additional refactor, which this PR provides.
Move the logging from validation to net_processing.
This also reduces the number of log entries (under default configuration) per compact block header from 3 to 2: one for the header and one for the connected tip. That is, in the best case, in practice we
...
(https://github.com/bitcoin/bitcoin/pull/27826)
Fixes #27744
Since #27278 we log received headers. For compact blocks we also log which peer sent it (e5ce8576349d404c466b2f4cab1ca7bf920904b2), but not for regular headers. That required an additional refactor, which this PR provides.
Move the logging from validation to net_processing.
This also reduces the number of log entries (under default configuration) per compact block header from 3 to 2: one for the header and one for the connected tip. That is, in the best case, in practice we
...
📝 josibake opened a pull request: "Silent Payments"
(https://github.com/bitcoin/bitcoin/pull/27827)
(https://github.com/bitcoin/bitcoin/pull/27827)
💬 MarcoFalke commented on pull request "[WIP] ci: Use DEBUG=1 in depends for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27495#issuecomment-1576676178)
Looks like this is blocked on https://github.com/bitcoin/bitcoin/issues/27222
(https://github.com/bitcoin/bitcoin/pull/27495#issuecomment-1576676178)
Looks like this is blocked on https://github.com/bitcoin/bitcoin/issues/27222
💬 Sjors commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1576678050)
There's various ways in which `AcceptBlockHeader` can return `true` or `false` early without setting `state`. This happens before the place where we used to log stuff. That in turn causes f25b8712c368ecfa250a6878af66f3fea3a4bcb0 to potentially log more stuff than it should. Moving to draft while I give it more thought.
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1576678050)
There's various ways in which `AcceptBlockHeader` can return `true` or `false` early without setting `state`. This happens before the place where we used to log stuff. That in turn causes f25b8712c368ecfa250a6878af66f3fea3a4bcb0 to potentially log more stuff than it should. Moving to draft while I give it more thought.
💬 Sjors commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1576703050)
Added a check to ensure the header was valid and newly seen before we log it. That matches the behaviour from before this PR.
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1576703050)
Added a check to ensure the header was valid and newly seen before we log it. That matches the behaviour from before this PR.
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1218012577)
For `GETDATA`, this only applies to transactions which we have not `INV`ed to the peer. Otherwise `INV`ed transactions will be sent via:
https://github.com/bitcoin/bitcoin/blob/f4a8269dfc144cc918570bdb870aa5143a11c1fe/src/net_processing.cpp#L2307
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1218012577)
For `GETDATA`, this only applies to transactions which we have not `INV`ed to the peer. Otherwise `INV`ed transactions will be sent via:
https://github.com/bitcoin/bitcoin/blob/f4a8269dfc144cc918570bdb870aa5143a11c1fe/src/net_processing.cpp#L2307
💬 MatthewLM commented on issue "Remove Ambiguity of Script ASM Hex and Decimal Integer Representations":
(https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1576709225)
> Edit: Curious if anyone knows the original rationale for displaying small values as decimal, it was carved out [by satoshi](https://github.com/bitcoin/bitcoin/blob/4405b78d6059e536c36974088a8ed4d9f0f29898/script.h#L298-L305).
It is because any numerical/arithmetic operations only allow 32-bit values, so the ASM displays 32-bit values as decimal and longer values as hex. This makes sense as humans generally better comprehend decimal values when thinking arithmetically, but the values need to
...
(https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1576709225)
> Edit: Curious if anyone knows the original rationale for displaying small values as decimal, it was carved out [by satoshi](https://github.com/bitcoin/bitcoin/blob/4405b78d6059e536c36974088a8ed4d9f0f29898/script.h#L298-L305).
It is because any numerical/arithmetic operations only allow 32-bit values, so the ASM displays 32-bit values as decimal and longer values as hex. This makes sense as humans generally better comprehend decimal values when thinking arithmetically, but the values need to
...
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1218020772)
TLDR: @instagibbs, right! :)
First, the problem with the user-agent string is that it can be customized which will totally reveal the identity of the sender. Next idea is to use the non-customized user-agent even if `-uacomment=` is configured. But then, why even reveal the version - whether it is 26.0 or 27.0? Right after a new release there will be few nodes that run the just-released version and thus the anonymity set will be small. So this string better be constant - either empty string o
...
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1218020772)
TLDR: @instagibbs, right! :)
First, the problem with the user-agent string is that it can be customized which will totally reveal the identity of the sender. Next idea is to use the non-customized user-agent even if `-uacomment=` is configured. But then, why even reveal the version - whether it is 26.0 or 27.0? Right after a new release there will be few nodes that run the just-released version and thus the anonymity set will be small. So this string better be constant - either empty string o
...
💬 hebasto commented on pull request "kernel: Remove args, settings, chainparams, chainparamsbase from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1218041412)
Side note: Removing of `common/config.cpp` is correct but not directly related to this PR changes as it can be done even on the current master branch.
(https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1218041412)
Side note: Removing of `common/config.cpp` is correct but not directly related to this PR changes as it can be done even on the current master branch.
👍 hebasto approved a pull request: "kernel: Remove args, settings, chainparams, chainparamsbase from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27576#pullrequestreview-1462486263)
ACK db77f87c6365cb5f414036d6bfb1a12705772028, I have reviewed the code and it looks OK.
(https://github.com/bitcoin/bitcoin/pull/27576#pullrequestreview-1462486263)
ACK db77f87c6365cb5f414036d6bfb1a12705772028, I have reviewed the code and it looks OK.
💬 furszy commented on pull request "init: return error when block index is non-contiguous, fix feature_init.py file perturbation":
(https://github.com/bitcoin/bitcoin/pull/27823#discussion_r1218054540)
Couldn't `vSortedByHeight` contain blocks from different chains at the same height?
(https://github.com/bitcoin/bitcoin/pull/27823#discussion_r1218054540)
Couldn't `vSortedByHeight` contain blocks from different chains at the same height?
💬 josibake commented on pull request "[Draft / POC] Silent Payments":
(https://github.com/bitcoin/bitcoin/pull/24897#issuecomment-1576783417)
I've rebased and created a slimmed-down version of this PR in https://github.com/bitcoin/bitcoin/pull/27827. Most notably, I took out labels and removed some of the RPC support, and made several updates as this silent payments spec has changed a bit since this PR was last updated. For anyone reviewing this PR, I'd appreciate your feedback on #27827.
Big thanks to @w0xlt for moving this draft along as far as they did! My plan is to keep cherry-picking commits from this draft for follow-up PRs
...
(https://github.com/bitcoin/bitcoin/pull/24897#issuecomment-1576783417)
I've rebased and created a slimmed-down version of this PR in https://github.com/bitcoin/bitcoin/pull/27827. Most notably, I took out labels and removed some of the RPC support, and made several updates as this silent payments spec has changed a bit since this PR was last updated. For anyone reviewing this PR, I'd appreciate your feedback on #27827.
Big thanks to @w0xlt for moving this draft along as far as they did! My plan is to keep cherry-picking commits from this draft for follow-up PRs
...
💬 instagibbs commented on pull request "test: avoid sporadic MINIMALDATA failure in feature_taproot.py (fixes #27595)":
(https://github.com/bitcoin/bitcoin/pull/27631#issuecomment-1576787374)
ACK https://github.com/bitcoin/bitcoin/pull/27631/commits/54877253c807dac7a3720b2c3d1d989c410259a7
(https://github.com/bitcoin/bitcoin/pull/27631#issuecomment-1576787374)
ACK https://github.com/bitcoin/bitcoin/pull/27631/commits/54877253c807dac7a3720b2c3d1d989c410259a7