💬 paplorinc commented on pull request "optimization: Reduce cache lookups in CCoinsViewCache::FetchCoin":
(https://github.com/bitcoin/bitcoin/pull/30326#issuecomment-2276636577)
Rebased after https://github.com/bitcoin/bitcoin/pull/28280
(https://github.com/bitcoin/bitcoin/pull/30326#issuecomment-2276636577)
Rebased after https://github.com/bitcoin/bitcoin/pull/28280
💬 davidgumberg commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710291798)
Feel free to disregard nit: I'm not 100% sure if it makes sense for this check to just warn when the binary is not found. A lint check returning Err() or Ok() only really matters for CI checking the return value of the runner, so not a big deal either way, but it would make this slightly shorter:
```suggestion
Err(e) if e.kind() == ErrorKind::NotFound => Err(format!(
"`{}` was not found in $PATH, skipping this check.",
bin_name
)),
```
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710291798)
Feel free to disregard nit: I'm not 100% sure if it makes sense for this check to just warn when the binary is not found. A lint check returning Err() or Ok() only really matters for CI checking the return value of the runner, so not a big deal either way, but it would make this slightly shorter:
```suggestion
Err(e) if e.kind() == ErrorKind::NotFound => Err(format!(
"`{}` was not found in $PATH, skipping this check.",
bin_name
)),
```
💬 davidgumberg commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710328780)
nit:
```suggestion
cmd.arg("check").args(checks).args(files.lines()).arg("--quiet");
```
This would silence unhelpful messages about fixes not being available.
(https://github.com/bitcoin/bitcoin/pull/30553#discussion_r1710328780)
nit:
```suggestion
cmd.arg("check").args(checks).args(files.lines()).arg("--quiet");
```
This would silence unhelpful messages about fixes not being available.
💬 davidgumberg commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#issuecomment-2276694620)
Left some nits, tested enforcement of `B008` by reverting `ec5e294e4b830766dcc4a80add0613d3705c1794`, and `B006` by inserting into one of the functional tests:
```python
def b006_violator(x, mutable_default=[]):
mutable_default.append(x)
```
The lint check added here complains.
<details>
<summary>Test output</summary>
```console
$ git diff
diff --git a/test/functional/example_test.py b/test/functional/example_test.py
index 39cea2962f..a44ec61b03 100755
--- a/test/func
...
(https://github.com/bitcoin/bitcoin/pull/30553#issuecomment-2276694620)
Left some nits, tested enforcement of `B008` by reverting `ec5e294e4b830766dcc4a80add0613d3705c1794`, and `B006` by inserting into one of the functional tests:
```python
def b006_violator(x, mutable_default=[]):
mutable_default.append(x)
```
The lint check added here complains.
<details>
<summary>Test output</summary>
```console
$ git diff
diff --git a/test/functional/example_test.py b/test/functional/example_test.py
index 39cea2962f..a44ec61b03 100755
--- a/test/func
...
💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2276699135)
> Thanks for all the explanations @ryanofsky , I'll take some time to digest all of this, and read some more libmultiprocess code.
Thanks! Feel free to ping me if you have any questions. it will help me learn what things to work on documentation for.
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2276699135)
> Thanks for all the explanations @ryanofsky , I'll take some time to digest all of this, and read some more libmultiprocess code.
Thanks! Feel free to ping me if you have any questions. it will help me learn what things to work on documentation for.
💬 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).