🚀 glozow merged a pull request: "26.2 final changes"
(https://github.com/bitcoin/bitcoin/pull/30376)
(https://github.com/bitcoin/bitcoin/pull/30376)
💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2206679066)
@luke-jr May I kindly ask for a review on this PR?
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2206679066)
@luke-jr May I kindly ask for a review on this PR?
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664435059)
I think the equivalent assert would be
```
BOOST_CHECK(n2.second.IsFresh() && !n2.second.IsDirty());
```
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664435059)
I think the equivalent assert would be
```
BOOST_CHECK(n2.second.IsFresh() && !n2.second.IsDirty());
```
📝 ajtowns opened a pull request: "Early logging improvements"
(https://github.com/bitcoin/bitcoin/pull/30386)
In order to cope gracefully with `Log*()` calls that are invoked prior to logging being fully configured (indicated by calling `StartLogging()` we buffer early log messages in `m_msgs_before_open`. This has a couple of minor issues:
* if there are many such log messages the buffer can become arbitrarily large; this can be a problem for users of libkernel that might not wish to worry about logging at all, and as a result never invoke `StartLogging()`
* early log messages are formatted befor
...
(https://github.com/bitcoin/bitcoin/pull/30386)
In order to cope gracefully with `Log*()` calls that are invoked prior to logging being fully configured (indicated by calling `StartLogging()` we buffer early log messages in `m_msgs_before_open`. This has a couple of minor issues:
* if there are many such log messages the buffer can become arbitrarily large; this can be a problem for users of libkernel that might not wish to worry about logging at all, and as a result never invoke `StartLogging()`
* early log messages are formatted befor
...
💬 theStack commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2206760235)
Suggestion for adding corresponding `decodescript` test coverage:
<details>
<summary>Patch</summary>
<br>
```diff
diff --git a/test/functional/rpc_decodescript.py b/test/functional/rpc_decodescript.py
index f37e61ab50..a105b3d3b4 100755
--- a/test/functional/rpc_decodescript.py
+++ b/test/functional/rpc_decodescript.py
@@ -187,6 +187,16 @@ class DecodeScriptTest(BitcoinTestFramework):
assert_equal('1 ' + xonly_public_key, rpc_result['asm'])
assert 'segwit' not in
...
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2206760235)
Suggestion for adding corresponding `decodescript` test coverage:
<details>
<summary>Patch</summary>
<br>
```diff
diff --git a/test/functional/rpc_decodescript.py b/test/functional/rpc_decodescript.py
index f37e61ab50..a105b3d3b4 100755
--- a/test/functional/rpc_decodescript.py
+++ b/test/functional/rpc_decodescript.py
@@ -187,6 +187,16 @@ class DecodeScriptTest(BitcoinTestFramework):
assert_equal('1 ' + xonly_public_key, rpc_result['asm'])
assert 'segwit' not in
...
💬 ajtowns commented on pull request "RFC: Instanced logs":
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2206789244)
> Re [#30338 (comment)](https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2197683031)
>
> > Have a look at [ajtowns#9](https://github.com/ajtowns/bitcoin/pull/9) and see what you think;
>
> This looks good to me, thank you for implementing.
Okay, see #30386 for the more serious take. I have another patch that I could add on top that would also buffer early debug/trace logs, but that seemed a bit unnecessary (LMK if it would be useful).
> Regarding disabling the buffering, I
...
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2206789244)
> Re [#30338 (comment)](https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2197683031)
>
> > Have a look at [ajtowns#9](https://github.com/ajtowns/bitcoin/pull/9) and see what you think;
>
> This looks good to me, thank you for implementing.
Okay, see #30386 for the more serious take. I have another patch that I could add on top that would also buffer early debug/trace logs, but that seemed a bit unnecessary (LMK if it would be useful).
> Regarding disabling the buffering, I
...
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664508903)
I guess `head` could be though of as a sentinel, which always points to the head of the linked list? I suppose I misnamed that then.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664508903)
I guess `head` could be though of as a sentinel, which always points to the head of the linked list? I suppose I misnamed that then.
💬 ajtowns commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#issuecomment-2206858748)
Easiest way to test is to add some log lines into init.cpp `InitLogging()`, eg:
```c++
for (int i = 0; i < 10000; ++i) {
LogInfo("log spam %d...\n", i);
}
LogWarning("awoogah\n");
LogError("internal bug not detected\nhello, world!\nwas that fun?\n");
```
Running `bitcoind -regtest -logtimemicros` should give a decent indication of the differences.
If you want to modify bitcoin-chainstate to see what the logs would look like, remove the `DisableLogging` call
...
(https://github.com/bitcoin/bitcoin/pull/30386#issuecomment-2206858748)
Easiest way to test is to add some log lines into init.cpp `InitLogging()`, eg:
```c++
for (int i = 0; i < 10000; ++i) {
LogInfo("log spam %d...\n", i);
}
LogWarning("awoogah\n");
LogError("internal bug not detected\nhello, world!\nwas that fun?\n");
```
Running `bitcoind -regtest -logtimemicros` should give a decent indication of the differences.
If you want to modify bitcoin-chainstate to see what the logs would look like, remove the `DisableLogging` call
...
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664539266)
the exact equivalent, yes, but you might not want to test the dirtiness part here - up to you, I don't mind either way.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664539266)
the exact equivalent, yes, but you might not want to test the dirtiness part here - up to you, I don't mind either way.
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664542828)
hmmm, why did this become `=` instead of `|=`?
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664542828)
hmmm, why did this become `=` instead of `|=`?
✅ ajtowns closed a pull request: "logging: Simplify edge cases in logging configuration"
(https://github.com/bitcoin/bitcoin/pull/30384)
(https://github.com/bitcoin/bitcoin/pull/30384)
💬 ajtowns commented on pull request "Logging cleanup":
(https://github.com/bitcoin/bitcoin/pull/29798#issuecomment-2206882030)
Consider also updating the help text for `-debug` and `-debugexclude`:
```diff
--- a/src/init/common.cpp
+++ b/src/init/common.cpp
@@ -27,9 +27,9 @@ void AddLoggingArgs(ArgsManager& argsman)
{
argsman.AddArg("-debuglogfile=<file>", strprintf("Specify location of debug log file (default: %s). Relative paths will be prefixed by a net-specific datadir location. Pass -nodebuglogfile to disable writing the log to a file.", DEFAULT_DEBUGLOGFILE), ArgsManager::ALLOW_ANY, OptionsCategory::
...
(https://github.com/bitcoin/bitcoin/pull/29798#issuecomment-2206882030)
Consider also updating the help text for `-debug` and `-debugexclude`:
```diff
--- a/src/init/common.cpp
+++ b/src/init/common.cpp
@@ -27,9 +27,9 @@ void AddLoggingArgs(ArgsManager& argsman)
{
argsman.AddArg("-debuglogfile=<file>", strprintf("Specify location of debug log file (default: %s). Relative paths will be prefixed by a net-specific datadir location. Pass -nodebuglogfile to disable writing the log to a file.", DEFAULT_DEBUGLOGFILE), ArgsManager::ALLOW_ANY, OptionsCategory::
...
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664555939)
Whoops bad rebase, thanks for catching!
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664555939)
Whoops bad rebase, thanks for catching!
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664558253)
But since these are unit tests we are kind of testing the internals, so it would be good to catch if any other flags sneak in. With `IsFresh` it just checks that the FRESH flag is set, but any other flag could be set as well.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664558253)
But since these are unit tests we are kind of testing the internals, so it would be good to catch if any other flags sneak in. With `IsFresh` it just checks that the FRESH flag is set, but any other flag could be set as well.
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664558577)
Added `noexcept` everywhere.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664558577)
Added `noexcept` everywhere.
💬 ajtowns commented on pull request "Logging cleanup":
(https://github.com/bitcoin/bitcoin/pull/29798#issuecomment-2206898641)
ACK df2422c5f91e5b1b42d6b718cc2527dc413712fc ; looks like it behaves correctly to me
(https://github.com/bitcoin/bitcoin/pull/29798#issuecomment-2206898641)
ACK df2422c5f91e5b1b42d6b718cc2527dc413712fc ; looks like it behaves correctly to me
💬 stickies-v commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1664572723)
Is there a reason this can't be simplified to:
```suggestion
if (nHeight % consensusParams.DifficultyAdjustmentInterval() == 0) {
```
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1664572723)
Is there a reason this can't be simplified to:
```suggestion
if (nHeight % consensusParams.DifficultyAdjustmentInterval() == 0) {
```
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664545575)
what happens if this ends up throwing, it's not a trivial function (e.g. `m_prev` could become nullable in the future and cause havoc)?
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664545575)
what happens if this ends up throwing, it's not a trivial function (e.g. `m_prev` could become nullable in the future and cause havoc)?
🤔 paplorinc reviewed a pull request: "Don't empty dbcache on prune flushes: >30% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/28280#pullrequestreview-2157030164)
I ran the benchmarks without prune (on the previous push), with just a lazy `-reindex` or `-reindex-chainstate`, but couldn't measure any difference - do I need more than 200k blocks? Or is prune critical here? Or is lazy reindexing not enough?
```bash
sudo ./src/bitcoind -stopatheight=200000
sudo hyperfine \
--warmup 1 --runs 5 \
--show-output \
--parameter-list commit master,670084adc53e3d661ea5b4a19743659609a42d5e \
--prepare 'git checkout {commit} && git reset --hard && make -j10
...
(https://github.com/bitcoin/bitcoin/pull/28280#pullrequestreview-2157030164)
I ran the benchmarks without prune (on the previous push), with just a lazy `-reindex` or `-reindex-chainstate`, but couldn't measure any difference - do I need more than 200k blocks? Or is prune critical here? Or is lazy reindexing not enough?
```bash
sudo ./src/bitcoind -stopatheight=200000
sudo hyperfine \
--warmup 1 --runs 5 \
--show-output \
--parameter-list commit master,670084adc53e3d661ea5b4a19743659609a42d5e \
--prepare 'git checkout {commit} && git reset --hard && make -j10
...
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664560161)
maybe it could make sense to document these rare conditions with something like `} else if ([[unlikely]] fOk) {`
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1664560161)
maybe it could make sense to document these rare conditions with something like `} else if ([[unlikely]] fOk) {`