💬 achow101 commented on pull request "contrib/signet/miner updates":
(https://github.com/bitcoin/bitcoin/pull/28417#discussion_r1710394839)
In 338a266a9a08e47bc6dd02175c8fa649f701515d "signet/miner: add support for a poolnum/poolid tag in mined blocks"
Using a mutually exclusive group here and below will let the argument parser enforce that these are mutually exclusive rather than having to enforce it in `get_poolid`.
```suggestion
poolid_group = genpsbt.add_mutually_exclusive_group()
poolid_group.add_argument("--poolnum", default=None, type=int, help="Identify blocks that you mine")
poolid_group.add_argument("-
...
(https://github.com/bitcoin/bitcoin/pull/28417#discussion_r1710394839)
In 338a266a9a08e47bc6dd02175c8fa649f701515d "signet/miner: add support for a poolnum/poolid tag in mined blocks"
Using a mutually exclusive group here and below will let the argument parser enforce that these are mutually exclusive rather than having to enforce it in `get_poolid`.
```suggestion
poolid_group = genpsbt.add_mutually_exclusive_group()
poolid_group.add_argument("--poolnum", default=None, type=int, help="Identify blocks that you mine")
poolid_group.add_argument("-
...
💬 achow101 commented on pull request "contrib/signet/miner updates":
(https://github.com/bitcoin/bitcoin/pull/28417#discussion_r1710395799)
In 85c5c0bea9d45e93a9fb20988457480798d68637 "signet/miner: add Generate.next_block_time function"
Why change the time delta to use `self.INTERVAL` instead of using `next_block_delta` as previously?
(https://github.com/bitcoin/bitcoin/pull/28417#discussion_r1710395799)
In 85c5c0bea9d45e93a9fb20988457480798d68637 "signet/miner: add Generate.next_block_time function"
Why change the time delta to use `self.INTERVAL` instead of using `next_block_delta` as previously?
💬 zawy12 commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2276866163)
As @murchandamus just said, preventing nActualtimespan from going negative could stop my attack above, but now I believe a slight variation to get around that will work just as good.
The fix I'll propose is this: **no** block can be more than 2 hours and 80 minutes before its parent block. The 80 minutes is to provide protection from a dual Sybil attack on the 40 minute allowable error in peer time.
An attacker could "stack up" of a sequence of I think up to 5 of these "max allowable n
...
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2276866163)
As @murchandamus just said, preventing nActualtimespan from going negative could stop my attack above, but now I believe a slight variation to get around that will work just as good.
The fix I'll propose is this: **no** block can be more than 2 hours and 80 minutes before its parent block. The 80 minutes is to provide protection from a dual Sybil attack on the 40 minute allowable error in peer time.
An attacker could "stack up" of a sequence of I think up to 5 of these "max allowable n
...
⚠️ 9Zu2LfCn opened an issue: "[!MICROSOFT] Office 2021 Download fully Activated and functional [[*UPDATED]] Latest Version"
(https://github.com/bitcoin/bitcoin/issues/30613)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
[!MICROSOFT] Office 2021 Download fully Activated and functional [[*UPDATED]] Latest Version
Obtaining the latest version of Microsoft Office is a straightforward process, but it's crucial to ensure you download it from official and trusted sources to avoid potential security risks or compatibility issues. The first step is to visit the official Microsoft website or the Microsoft Store, w
...
(https://github.com/bitcoin/bitcoin/issues/30613)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
[!MICROSOFT] Office 2021 Download fully Activated and functional [[*UPDATED]] Latest Version
Obtaining the latest version of Microsoft Office is a straightforward process, but it's crucial to ensure you download it from official and trusted sources to avoid potential security risks or compatibility issues. The first step is to visit the official Microsoft website or the Microsoft Store, w
...
💬 gmaxwell commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1710491169)
Yeah, I assumed that the specific limit used in the tests right how happened to evade failing. I don't have a strong opinion. The adjustments seemed fine to me but maybe just making some function for the adjustments and having the test use that to derive its test limits might make sense-- so that further updates to those adhoc adjustments don't accidentally break test limits that might be hit infrequently (or after changing the amount of work the test does).
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1710491169)
Yeah, I assumed that the specific limit used in the tests right how happened to evade failing. I don't have a strong opinion. The adjustments seemed fine to me but maybe just making some function for the adjustments and having the test use that to derive its test limits might make sense-- so that further updates to those adhoc adjustments don't accidentally break test limits that might be hit infrequently (or after changing the amount of work the test does).
👍 tdb3 approved a pull request: "contrib: support reading XORed blocks in linearize-data.py script"
(https://github.com/bitcoin/bitcoin/pull/30607#pullrequestreview-2229129698)
ACK 77ff0ec1f185b818b30877de2bedc1750319e6c4
Light code review. In addition to running unit/functionals locally, performed some sanity checks:
1) Created a regtest chain (~11k transactions in 411 blocks)
2) Confirmed that xor.dat had a non-zero key, created hashlist, then stopped bitcoind
3) Created a bootstrap.dat with `linearize-data.py` (successful)
4) Deleted the regtest datadir
5) Started bitcoind with `-loadblock=/path/to/bootstrap.dat -blocksxor=0` (successful restore), then stop
...
(https://github.com/bitcoin/bitcoin/pull/30607#pullrequestreview-2229129698)
ACK 77ff0ec1f185b818b30877de2bedc1750319e6c4
Light code review. In addition to running unit/functionals locally, performed some sanity checks:
1) Created a regtest chain (~11k transactions in 411 blocks)
2) Confirmed that xor.dat had a non-zero key, created hashlist, then stopped bitcoind
3) Created a bootstrap.dat with `linearize-data.py` (successful)
4) Deleted the regtest datadir
5) Started bitcoind with `-loadblock=/path/to/bootstrap.dat -blocksxor=0` (successful restore), then stop
...
💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1710554112)
I've simplified the change a bit (now much closer to older deserialization code) and moved it to a separate commit (together with the introduction of the `DepGraph` reordering constructor).
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1710554112)
I've simplified the change a bit (now much closer to older deserialization code) and moved it to a separate commit (together with the introduction of the `DepGraph` reordering constructor).
💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2277019007)
Unsure where this should live, but I have a Python reimplementation of the depgraph serialization code + mermaid formatter: https://gist.github.com/sipa/822f60db6608a26bb4aae75fd31bcb12
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2277019007)
Unsure where this should live, but I have a Python reimplementation of the depgraph serialization code + mermaid formatter: https://gist.github.com/sipa/822f60db6608a26bb4aae75fd31bcb12
💬 ajtowns commented on pull request "contrib/signet/miner updates":
(https://github.com/bitcoin/bitcoin/pull/28417#discussion_r1710716168)
`next_block_delta` can be somewhat small if you have a high difficulty target (ie 2.5m instead of 10m) and if `--poisson` is specified will be smaller again (the deterministic random calculation used against the genesis block reduces the target by ~73%). The `--poisson` result alone is already annoying -- it will generally only take 25 blocks before it's caught up to real time; but even without that if you switch from mining with a high nbits target to a minimal nbits target, then even without `
...
(https://github.com/bitcoin/bitcoin/pull/28417#discussion_r1710716168)
`next_block_delta` can be somewhat small if you have a high difficulty target (ie 2.5m instead of 10m) and if `--poisson` is specified will be smaller again (the deterministic random calculation used against the genesis block reduces the target by ~73%). The `--poisson` result alone is already annoying -- it will generally only take 25 blocks before it's caught up to real time; but even without that if you switch from mining with a high nbits target to a minimal nbits target, then even without `
...
💬 maflcko commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2277190642)
(The drahtbot guix build failed due to a silent merge conflict, I presume)
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2277190642)
(The drahtbot guix build failed due to a silent merge conflict, I presume)
💬 maflcko commented on pull request "assumeutxo: Drop block height from metadata":
(https://github.com/bitcoin/bitcoin/pull/30598#issuecomment-2277205856)
The CI failure is known and can be ignored (https://github.com/bitcoin/bitcoin/actions/runs/10310090865/job/28541115356?pr=30598#step:27:229)
Only change since my last review is a small style-only cleanup to remove the unused parameter from the constructor.
re-ACK 00618e8745192d209c23e3ae873c077e58168957 🎌
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_fi
...
(https://github.com/bitcoin/bitcoin/pull/30598#issuecomment-2277205856)
The CI failure is known and can be ignored (https://github.com/bitcoin/bitcoin/actions/runs/10310090865/job/28541115356?pr=30598#step:27:229)
Only change since my last review is a small style-only cleanup to remove the unused parameter from the constructor.
re-ACK 00618e8745192d209c23e3ae873c077e58168957 🎌
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_fi
...
💬 maflcko commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710808023)
This will also drop the "All checks passed!" on the success case (https://cirrus-ci.com/task/4866693351079936?logs=lint#L656), which I did not like when writing this.
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710808023)
This will also drop the "All checks passed!" on the success case (https://cirrus-ci.com/task/4866693351079936?logs=lint#L656), which I did not like when writing this.
💬 maflcko commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710809136)
I copy-pasted this from the mlc linter, so maybe this can be done in a follow-up for both. Otherwise, it seems inconsistent.
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710809136)
I copy-pasted this from the mlc linter, so maybe this can be done in a follow-up for both. Otherwise, it seems inconsistent.
💬 maflcko commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710811261)
I am trying to preserve the behavior of the previous check based on `git grep --extended-regexp`, which didn't exclude subtrees either.
However, I'll apply your suggestion in a follow-up.
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710811261)
I am trying to preserve the behavior of the previous check based on `git grep --extended-regexp`, which didn't exclude subtrees either.
However, I'll apply your suggestion in a follow-up.
💬 maflcko commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710817315)
Thanks, fixed typo (in another way)
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710817315)
Thanks, fixed typo (in another way)
🤔 maflcko reviewed a pull request: "Fixes for GCC 15 compatibility"
(https://github.com/bitcoin/bitcoin/pull/30612#pullrequestreview-2229402700)
> * Two are related to missing includes. You can't use `uint16_t` et al. without including `<cstdint>`.
Probably a good time to enable iwyu. :sweat_smile:
> This manifests as the following compile-time mess:
Are you sure this is not a GCC bug?
(https://github.com/bitcoin/bitcoin/pull/30612#pullrequestreview-2229402700)
> * Two are related to missing includes. You can't use `uint16_t` et al. without including `<cstdint>`.
Probably a good time to enable iwyu. :sweat_smile:
> This manifests as the following compile-time mess:
Are you sure this is not a GCC bug?
💬 maflcko commented on pull request "Fixes for GCC 15 compatibility":
(https://github.com/bitcoin/bitcoin/pull/30612#discussion_r1710836374)
```suggestion
template<std::integral I> // Disallow silent float -> int conversion
```
Please keep the comment
(https://github.com/bitcoin/bitcoin/pull/30612#discussion_r1710836374)
```suggestion
template<std::integral I> // Disallow silent float -> int conversion
```
Please keep the comment
💬 maflcko commented on pull request "Fixes for GCC 15 compatibility":
(https://github.com/bitcoin/bitcoin/pull/30612#discussion_r1710833459)
iwyu:
```
node/interface_ui.h should add these lines:
#include <vector> // for vector
node/interface_ui.h should remove these lines:
- #include <memory> // lines 11-11
(https://github.com/bitcoin/bitcoin/pull/30612#discussion_r1710833459)
iwyu:
```
node/interface_ui.h should add these lines:
#include <vector> // for vector
node/interface_ui.h should remove these lines:
- #include <memory> // lines 11-11
💬 whitslack commented on pull request "Fixes for GCC 15 compatibility":
(https://github.com/bitcoin/bitcoin/pull/30612#discussion_r1710848701)
With all due respect, that's a semantically void comment. `std::integral I` already means "disallow floats."
(https://github.com/bitcoin/bitcoin/pull/30612#discussion_r1710848701)
With all due respect, that's a semantically void comment. `std::integral I` already means "disallow floats."
💬 whitslack commented on pull request "Fixes for GCC 15 compatibility":
(https://github.com/bitcoin/bitcoin/pull/30612#issuecomment-2277260451)
> Are you sure this is not a GCC bug?
The compile-time error is of a class of errors that is not specific to GCC. Maybe the stdlibc++ in GCC 15 changed the implementation of `std::optional` so that the previously valid code in `feerate.h` is no longer valid. In general, if, in the course of checking a type constraint, the compiler instantiates a template that invokes the same type constraint, then there is an infinite recursion, and the compiler will reject the code with an error saying that
...
(https://github.com/bitcoin/bitcoin/pull/30612#issuecomment-2277260451)
> Are you sure this is not a GCC bug?
The compile-time error is of a class of errors that is not specific to GCC. Maybe the stdlibc++ in GCC 15 changed the implementation of `std::optional` so that the previously valid code in `feerate.h` is no longer valid. In general, if, in the course of checking a type constraint, the compiler instantiates a template that invokes the same type constraint, then there is an infinite recursion, and the compiler will reject the code with an error saying that
...