💬 murchandamus commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2276736020)
@fjahr: Having thought a bit more about the attack scenario @zawy12 describes, it might be useful to additionally require that the first block of a difficulty period N has a lower timestamp than the last block of the difficulty period N.
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2276736020)
@fjahr: Having thought a bit more about the attack scenario @zawy12 describes, it might be useful to additionally require that the first block of a difficulty period N has a lower timestamp than the last block of the difficulty period N.
💬 fjahr commented on pull request "assumeutxo: Drop block height from metadata":
(https://github.com/bitcoin/bitcoin/pull/30598#discussion_r1710346824)
done
(https://github.com/bitcoin/bitcoin/pull/30598#discussion_r1710346824)
done
💬 fjahr commented on pull request "assumeutxo: Drop block height from metadata":
(https://github.com/bitcoin/bitcoin/pull/30598#issuecomment-2276736901)
> The block height should also be removed from the `SnapshotMetadata` constructor and the `utxo_snapshot` fuzz target.
Thanks, I fixed that.
(https://github.com/bitcoin/bitcoin/pull/30598#issuecomment-2276736901)
> The block height should also be removed from the `SnapshotMetadata` constructor and the `utxo_snapshot` fuzz target.
Thanks, I fixed that.
💬 ryanofsky commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2276739868)
The test failure https://cirrus-ci.com/task/5102088831631360 in the new test which tests binding and connecting on [multiple sockets](https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1707288885) turns out to reveal a corner case bug in libmultiprocess. The bug does not happen locally for me, but it does happen if I run the CI scripts inside a container, I think maybe because the container is 32 bit.
Basically what happens is the test disconnects and reconnects immediately, which caus
...
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2276739868)
The test failure https://cirrus-ci.com/task/5102088831631360 in the new test which tests binding and connecting on [multiple sockets](https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1707288885) turns out to reveal a corner case bug in libmultiprocess. The bug does not happen locally for me, but it does happen if I run the CI scripts inside a container, I think maybe because the container is 32 bit.
Basically what happens is the test disconnects and reconnects immediately, which caus
...
🤔 tdb3 reviewed a pull request: "doc, chainparams: 29775 release notes and follow-ups"
(https://github.com/bitcoin/bitcoin/pull/30604#pullrequestreview-2228943873)
Approach ACK
The commit message of f2f3d802c9dbc096808dca8924501ca90081d813 can be updated to remove "and mention v30 target"
(https://github.com/bitcoin/bitcoin/pull/30604#pullrequestreview-2228943873)
Approach ACK
The commit message of f2f3d802c9dbc096808dca8924501ca90081d813 can be updated to remove "and mention v30 target"
💬 fjahr commented on pull request "doc, chainparams: 29775 release notes and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/30604#issuecomment-2276750217)
> The commit message of [f2f3d80](https://github.com/bitcoin/bitcoin/commit/f2f3d802c9dbc096808dca8924501ca90081d813) can be updated to remove "and mention v30 target"
Fixed
(https://github.com/bitcoin/bitcoin/pull/30604#issuecomment-2276750217)
> The commit message of [f2f3d80](https://github.com/bitcoin/bitcoin/commit/f2f3d802c9dbc096808dca8924501ca90081d813) can be updated to remove "and mention v30 target"
Fixed
👍 tdb3 approved a pull request: "doc, chainparams: 29775 release notes and follow-ups"
(https://github.com/bitcoin/bitcoin/pull/30604#pullrequestreview-2228949658)
ACK 35ad61a6bde14a74cc07dc8ecf3bb1cd49590e46
(https://github.com/bitcoin/bitcoin/pull/30604#pullrequestreview-2228949658)
ACK 35ad61a6bde14a74cc07dc8ecf3bb1cd49590e46
💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1710368478)
@gmaxwell I think you're right that these ought to be added to the limits in the tests, though I don't actually see any failures with a quick test. The sqrt(2^k)+1 limit is only hit for very small clusters (say, 3 transactions) though, for larger ones the worst I've seen is around 70%-80% of this presumed limit, which may explain not seeing failures.
It does make me wonder if perhaps `Linearize` shouldn't bother with trying to do CPU time management, and should just expose a `bool do_search`
...
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1710368478)
@gmaxwell I think you're right that these ought to be added to the limits in the tests, though I don't actually see any failures with a quick test. The sqrt(2^k)+1 limit is only hit for very small clusters (say, 3 transactions) though, for larger ones the worst I've seen is around 70%-80% of this presumed limit, which may explain not seeing failures.
It does make me wonder if perhaps `Linearize` shouldn't bother with trying to do CPU time management, and should just expose a `bool do_search`
...
💬 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.