π¬ andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1749277789)
Removed the dates altogether.
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1749277789)
Removed the dates altogether.
π¬ andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1749277815)
Split into 2 commits.
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1749277815)
Split into 2 commits.
π¬ andrewtoth commented on pull request "Drop -dbcache limit":
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-2336737880)
If we merge https://github.com/bitcoin/bitcoin/pull/30611, then write outs will be more evenly distributed throughout the sync and will not all happen on shutdown with a high dbcache.
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-2336737880)
If we merge https://github.com/bitcoin/bitcoin/pull/30611, then write outs will be more evenly distributed throughout the sync and will not all happen on shutdown with a high dbcache.
π¬ andrewtoth commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1749283536)
Are you suggesting to refactor direct flags access with `SetDirty` and `SetDirtyAndFresh` first? If we do that first, we would still need a `SetFresh` setter since we still have a case here with setting FRESH-but-not-DIRTY. It's kind of a chicken-and-egg problem.
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1749283536)
Are you suggesting to refactor direct flags access with `SetDirty` and `SetDirtyAndFresh` first? If we do that first, we would still need a `SetFresh` setter since we still have a case here with setting FRESH-but-not-DIRTY. It's kind of a chicken-and-egg problem.
π¬ itornaza commented on pull request "docs: Updated debug build instructions for cmake":
(https://github.com/bitcoin/bitcoin/pull/30840#issuecomment-2336760996)
Rebased to address the CI lint test error that was due to documentation lines terminating with whitespace characters.
(https://github.com/bitcoin/bitcoin/pull/30840#issuecomment-2336760996)
Rebased to address the CI lint test error that was due to documentation lines terminating with whitespace characters.
π tdb3 approved a pull request: "docs: Updated debug build instructions for cmake"
(https://github.com/bitcoin/bitcoin/pull/30840#pullrequestreview-2288575391)
ACK 0b003e1ff7e09b56cd013b15f1998495994b7211
(https://github.com/bitcoin/bitcoin/pull/30840#pullrequestreview-2288575391)
ACK 0b003e1ff7e09b56cd013b15f1998495994b7211
π¬ l0rinc commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1748096184)
since this always return false now for missing values, can https://github.com/bitcoin/bitcoin/pull/30673/files#diff-a8f78513bc27f9bf679eead54819a8e5be720401c6ae40858da226a66ca002e2R258 still happen?
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1748096184)
since this always return false now for missing values, can https://github.com/bitcoin/bitcoin/pull/30673/files#diff-a8f78513bc27f9bf679eead54819a8e5be720401c6ae40858da226a66ca002e2R258 still happen?
π¬ l0rinc commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1748087322)
I found this very confusing - a coin getter checking the cache, and if it's *not* found, it clears the coin that it was supposed to return?
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1748087322)
I found this very confusing - a coin getter checking the cache, and if it's *not* found, it clears the coin that it was supposed to return?
π¬ l0rinc commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1749317722)
I suggest we make our tests reflect reality before we do this change, since currently if we add asserts about the internal consistency of the code, the tests seem to constantly violate them.
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1749317722)
I suggest we make our tests reflect reality before we do this change, since currently if we add asserts about the internal consistency of the code, the tests seem to constantly violate them.
β οΈ kingston26 opened an issue: "Atomic "
(https://github.com/bitcoin/bitcoin/issues/30848)
0x853c3185a0793Ff452f244C734a7639439aD73aB
(https://github.com/bitcoin/bitcoin/issues/30848)
0x853c3185a0793Ff452f244C734a7639439aD73aB
π¬ l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2336783216)
Running
```bash
hyperfine \
--runs 5 \
--export-json /mnt/ibd_full.json \
--parameter-list COMMIT 8aabdfa6,1b448f5b \
--prepare 'git checkout {COMMIT} && git clean -fxd && git reset --hard && cmake -B build && cmake --build build -j8 && rm -rf /mnt/BitcoinData/*' \
'COMMIT={COMMIT} ./build/src/bitcoind -datadir=/mnt/BitcoinData -stopatheight=820000 -dbcache=16384 -printtoconsole=0'
```
Resulted in:
```python
Benchmark 1: COMMIT=8aabdfa6 ./build/src/bitcoind -datadir=/mnt/BitcoinData
...
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2336783216)
Running
```bash
hyperfine \
--runs 5 \
--export-json /mnt/ibd_full.json \
--parameter-list COMMIT 8aabdfa6,1b448f5b \
--prepare 'git checkout {COMMIT} && git clean -fxd && git reset --hard && cmake -B build && cmake --build build -j8 && rm -rf /mnt/BitcoinData/*' \
'COMMIT={COMMIT} ./build/src/bitcoind -datadir=/mnt/BitcoinData -stopatheight=820000 -dbcache=16384 -printtoconsole=0'
```
Resulted in:
```python
Benchmark 1: COMMIT=8aabdfa6 ./build/src/bitcoind -datadir=/mnt/BitcoinData
...
β
willcl-ark closed an issue: "Atomic "
(https://github.com/bitcoin/bitcoin/issues/30848)
(https://github.com/bitcoin/bitcoin/issues/30848)
π¬ l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1749324715)
you can resolve this comment
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1749324715)
you can resolve this comment
π¬ l0rinc commented on pull request "Drop -dbcache limit":
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-2336790306)
Concept ACK, I'll run some tests later
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-2336790306)
Concept ACK, I'll run some tests later
π¬ ismaelsadeeq commented on pull request "interfaces: #30697 follow ups":
(https://github.com/bitcoin/bitcoin/pull/30828#discussion_r1749332974)
There are two suggestions here:
1. **@furszy suggested to fail when the value is null**: I agree with not checking for null values, for the reasons outlined above.
2. **@ryanofsky suggested removing the null value check**: This approach would overwrite the previous settings with a null value.
> Setting a null value should consistently delete the setting, rather than having different behavior in different functions.
However, if we donβt delete the key-value pair, the setting sti
...
(https://github.com/bitcoin/bitcoin/pull/30828#discussion_r1749332974)
There are two suggestions here:
1. **@furszy suggested to fail when the value is null**: I agree with not checking for null values, for the reasons outlined above.
2. **@ryanofsky suggested removing the null value check**: This approach would overwrite the previous settings with a null value.
> Setting a null value should consistently delete the setting, rather than having different behavior in different functions.
However, if we donβt delete the key-value pair, the setting sti
...
π¬ ismaelsadeeq commented on pull request "interfaces: #30697 follow ups":
(https://github.com/bitcoin/bitcoin/pull/30828#discussion_r1749339659)
I guess you are trying to avoid a situation where we would need to write:
```
return action == interfaces::SettingsAction::SKIP_WRITE || action == interfaces::SettingsAction::NEW_ACTION || args().WriteSettingsFile();
```
In some cases we would still need to modify this to handle the new action if it has additional effects beyond just writing, such as writing and logging, for example.
---
I've updated to your suggestion, as it is more maintainable.
(https://github.com/bitcoin/bitcoin/pull/30828#discussion_r1749339659)
I guess you are trying to avoid a situation where we would need to write:
```
return action == interfaces::SettingsAction::SKIP_WRITE || action == interfaces::SettingsAction::NEW_ACTION || args().WriteSettingsFile();
```
In some cases we would still need to modify this to handle the new action if it has additional effects beyond just writing, such as writing and logging, for example.
---
I've updated to your suggestion, as it is more maintainable.
π¬ sipsorcery commented on pull request "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`":
(https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2336808242)
> @sipsorcery What do you think?
The overall build time for me on Win11 msvc reduced by 44s (only one test run each of the master and PR builds).
**master**:
```
PS C:\Dev\github\bitcoin> Measure-Command { cmake --build build --config Release }
TotalSeconds : 637.2643505
```
**pr 30842**:
```
PS C:\Dev\github\bitcoin> Measure-Command { cmake --build build --config Release }
TotalSeconds : 553.3287435
```
IMHO it's always beneficial to improve CI times so +1 for me
...
(https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2336808242)
> @sipsorcery What do you think?
The overall build time for me on Win11 msvc reduced by 44s (only one test run each of the master and PR builds).
**master**:
```
PS C:\Dev\github\bitcoin> Measure-Command { cmake --build build --config Release }
TotalSeconds : 637.2643505
```
**pr 30842**:
```
PS C:\Dev\github\bitcoin> Measure-Command { cmake --build build --config Release }
TotalSeconds : 553.3287435
```
IMHO it's always beneficial to improve CI times so +1 for me
...
π¬ l0rinc commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1749350258)
Did this change require any other test adjustment?
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1749350258)
Did this change require any other test adjustment?
π¬ sipsorcery commented on pull request "build: Minor build system fixes and amendments":
(https://github.com/bitcoin/bitcoin/pull/30803#issuecomment-2336816423)
tACK 1cc93fe7b40f10a7d1d1189058af98a2bce31381 (win11 msvc).
(https://github.com/bitcoin/bitcoin/pull/30803#issuecomment-2336816423)
tACK 1cc93fe7b40f10a7d1d1189058af98a2bce31381 (win11 msvc).
π l0rinc opened a pull request: "refactor: Remove unrealistic simulation state"
(https://github.com/bitcoin/bitcoin/pull/30849)
While reviewing the PR regarding [the removal of impossible Coin cache logic](https://github.com/bitcoin/bitcoin/pull/30673/files#r1740154464), we've noticed that the related tests often [reflect impossible states](https://github.com/bitcoin/bitcoin/pull/30673/files#r1740154464).
Some of these states are possible [because of the design](https://github.com/bitcoin/bitcoin/pull/30673/files#r1748087322) of the `GetCoin` method.
Browsing the Coin cache refactoring history revealed that migrating
...
(https://github.com/bitcoin/bitcoin/pull/30849)
While reviewing the PR regarding [the removal of impossible Coin cache logic](https://github.com/bitcoin/bitcoin/pull/30673/files#r1740154464), we've noticed that the related tests often [reflect impossible states](https://github.com/bitcoin/bitcoin/pull/30673/files#r1740154464).
Some of these states are possible [because of the design](https://github.com/bitcoin/bitcoin/pull/30673/files#r1748087322) of the `GetCoin` method.
Browsing the Coin cache refactoring history revealed that migrating
...