💬 hodlinator commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2364576723)
At first I thought there was some special property with `re.search(..., re.MULTILINE)`, but digging deeper I think not. Looking at the PR which added it, I found https://github.com/bitcoin/bitcoin/pull/14024#discussion_r212662890 which indicates that the original user of the function was passing in a search string with symbols that could be interpreted as a regex but shouldn't be. #13006 which spawned the issue mentioned regexps, so that's probably why it's in this unfortunate nerfed state.
I
...
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2364576723)
At first I thought there was some special property with `re.search(..., re.MULTILINE)`, but digging deeper I think not. Looking at the PR which added it, I found https://github.com/bitcoin/bitcoin/pull/14024#discussion_r212662890 which indicates that the original user of the function was passing in a search string with symbols that could be interpreted as a regex but shouldn't be. #13006 which spawned the issue mentioned regexps, so that's probably why it's in this unfortunate nerfed state.
I
...
👍 hodlinator approved a pull request: "log: always print initial script verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3249291256)
re-ACK 010cf81407c0df8de766fd2a116463d180f25f33
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3249291256)
re-ACK 010cf81407c0df8de766fd2a116463d180f25f33
💬 hodlinator commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2365841816)
Warp speed engaged, thank you!
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2365841816)
Warp speed engaged, thank you!
💬 enirox001 commented on pull request "rpc: Fix dumptxoutset rollback with competing forks":
(https://github.com/bitcoin/bitcoin/pull/33444#issuecomment-3315294250)
@fjahr @mzumsande Would appreciate your feedback on this implementation when you get a chance. Thanks!
(https://github.com/bitcoin/bitcoin/pull/33444#issuecomment-3315294250)
@fjahr @mzumsande Would appreciate your feedback on this implementation when you get a chance. Thanks!
🤔 pablomartin4btc reviewed a pull request: "wallet: reduce unconditional logging during load"
(https://github.com/bitcoin/bitcoin/pull/33299#pullrequestreview-3249356226)
tested 862faf3fa7a42238017f8d673b0c656531c688dc
This version doesn't work properly and produces the same issue it's trying to fix as every call to `MakeWalletDatabase()` within conditionals in `VerifyWallets()` (_`src/wallet/load.cpp`_), ends up also calling the SQLiteDatabase::Cleanup() in the destroyer which does `--g_sqlite_count`.
<details>
<summary><i>(tiny nit: only if you have to re-touch, there's a typo in commit message body)</i></summary>
<img width="742" height="253" alt="
...
(https://github.com/bitcoin/bitcoin/pull/33299#pullrequestreview-3249356226)
tested 862faf3fa7a42238017f8d673b0c656531c688dc
This version doesn't work properly and produces the same issue it's trying to fix as every call to `MakeWalletDatabase()` within conditionals in `VerifyWallets()` (_`src/wallet/load.cpp`_), ends up also calling the SQLiteDatabase::Cleanup() in the destroyer which does `--g_sqlite_count`.
<details>
<summary><i>(tiny nit: only if you have to re-touch, there's a typo in commit message body)</i></summary>
<img width="742" height="253" alt="
...
💬 l0rinc commented on pull request "wallet: reduce unconditional logging during load":
(https://github.com/bitcoin/bitcoin/pull/33299#issuecomment-3315461001)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33299#issuecomment-3315461001)
Concept ACK
💬 pablomartin4btc commented on pull request "wallet: reduce unconditional logging during load":
(https://github.com/bitcoin/bitcoin/pull/33299#discussion_r2365920111)
what about (and no longer need to check `g_sqlite_count`)...
```suggestion
static bool sqlite_version_logged = false;
if (!sqlite_version_logged) {
LogInfo("Using SQLite Version %s", SQLiteDatabaseVersion());
sqlite_version_logged = true;
}
```
(https://github.com/bitcoin/bitcoin/pull/33299#discussion_r2365920111)
what about (and no longer need to check `g_sqlite_count`)...
```suggestion
static bool sqlite_version_logged = false;
if (!sqlite_version_logged) {
LogInfo("Using SQLite Version %s", SQLiteDatabaseVersion());
sqlite_version_logged = true;
}
```
💬 monlovesmango commented on issue "30.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/33369#issuecomment-3315487388)
Testing guide looks great! Some feedback below.
### 6.1 TRUC transactions
- Perhaps make the pull request number (listed at the end of the intro paragraph) linkable like other sections?
- This guide isn't the right place for this feedback item but was hoping for guidance on whether it is expected, and if not where to report this. If I create a tx3 that is a child of tx2 (all as v3 txs) the `send` command fails silently. It failing is the correct behavior because the package size shouldn't be gr
...
(https://github.com/bitcoin/bitcoin/issues/33369#issuecomment-3315487388)
Testing guide looks great! Some feedback below.
### 6.1 TRUC transactions
- Perhaps make the pull request number (listed at the end of the intro paragraph) linkable like other sections?
- This guide isn't the right place for this feedback item but was hoping for guidance on whether it is expected, and if not where to report this. If I create a tx3 that is a child of tx2 (all as v3 txs) the `send` command fails silently. It failing is the correct behavior because the package size shouldn't be gr
...
👋 l0rinc's pull request is ready for review: "log: don't rate limit "rolling forward" messages"
(https://github.com/bitcoin/bitcoin/pull/33443)
(https://github.com/bitcoin/bitcoin/pull/33443)
💬 ajtowns commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3315535475)
> On a conceptual level, this seems like a strong fingerprinting vector (tor/clearnet response would be identical if requests happen roughly at the same time and the same template is used). I'm not sure if we have given up on that, considering that there seems to be no lack of other existing fingerprinting methods.
This might be a little better than it seems at first glance, because the top 2MvB of txs is likely to be fairly common across nodes running similar mempool acceptance policies, and
...
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3315535475)
> On a conceptual level, this seems like a strong fingerprinting vector (tor/clearnet response would be identical if requests happen roughly at the same time and the same template is used). I'm not sure if we have given up on that, considering that there seems to be no lack of other existing fingerprinting methods.
This might be a little better than it seems at first glance, because the top 2MvB of txs is likely to be fairly common across nodes running similar mempool acceptance policies, and
...
💬 ajtowns commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#discussion_r2365971591)
Not really. When only generating the templates and not requesting them it simplifies the locking slightly, and it takes a cs_main lock to generate the block, so most other processing can't continue anyway.
(https://github.com/bitcoin/bitcoin/pull/33191#discussion_r2365971591)
Not really. When only generating the templates and not requesting them it simplifies the locking slightly, and it takes a cs_main lock to generate the block, so most other processing can't continue anyway.
💬 ajtowns commented on pull request "log: don't rate limit "rolling forward" messages":
(https://github.com/bitcoin/bitcoin/pull/33443#issuecomment-3315626106)
Should call this `LogEssential()` or similar, rather than calling internal logging functions. cf https://github.com/ajtowns/bitcoin/commits/202509-logessential/
Is this information actually very useful though, or is it only relevant for debugging? There's already the `Replaying blocks...` notification which should show progress. Would something like this make more sense?
```c++
// Roll forward from the forking point to the new tip.
int nForkHeight = pindexFork ? pindexFork->nHe
...
(https://github.com/bitcoin/bitcoin/pull/33443#issuecomment-3315626106)
Should call this `LogEssential()` or similar, rather than calling internal logging functions. cf https://github.com/ajtowns/bitcoin/commits/202509-logessential/
Is this information actually very useful though, or is it only relevant for debugging? There's already the `Replaying blocks...` notification which should show progress. Would something like this make more sense?
```c++
// Roll forward from the forking point to the new tip.
int nForkHeight = pindexFork ? pindexFork->nHe
...
⚠️ Horlabrainmoore opened an issue: "python3 -m venv venv && source venv/bin/activate pip install pandas matplotlib PyPDF2 python-dateutil # (optional) pip install pdfminer.six if PyPDF2 extraction fails"
(https://github.com/bitcoin/bitcoin/issues/33447)
[# File: integrated_wallet_and_whitepaper_report.py
# Usage: adjust CSV_PATH and PDF_PATH then run: python integrated_wallet_and_whitepaper_report.py
from pathlib import Path
import re
import sys
import json
from collections import Counter, defaultdict
import pandas as pd
import matplotlib.pyplot as plt
from dateutil import parser as dateparser
# PDF extraction
try:
from PyPDF2 import PdfReader
except Exception:
PdfReader = None
# -------- CONFIG --------
CSV_PATH = Path("/mnt/data/W
...
(https://github.com/bitcoin/bitcoin/issues/33447)
[# File: integrated_wallet_and_whitepaper_report.py
# Usage: adjust CSV_PATH and PDF_PATH then run: python integrated_wallet_and_whitepaper_report.py
from pathlib import Path
import re
import sys
import json
from collections import Counter, defaultdict
import pandas as pd
import matplotlib.pyplot as plt
from dateutil import parser as dateparser
# PDF extraction
try:
from PyPDF2 import PdfReader
except Exception:
PdfReader = None
# -------- CONFIG --------
CSV_PATH = Path("/mnt/data/W
...
✅ fanquake closed an issue: "python3 -m venv venv && source venv/bin/activate pip install pandas matplotlib PyPDF2 python-dateutil # (optional) pip install pdfminer.six if PyPDF2 extraction fails"
(https://github.com/bitcoin/bitcoin/issues/33447)
(https://github.com/bitcoin/bitcoin/issues/33447)
💬 fanquake commented on pull request "ci: run native_fuzz_with_msan":
(https://github.com/bitcoin/bitcoin/pull/33425#issuecomment-3315724310)
Runtime currently 25m, which is on par with ASAN, and faster than both Windows CIs. Slowest targets:
```bash
addrman_serdeser: 1019
descriptor_parse: 875
ephemeral_package_eval: 863
tx_pool_standard: 829s
mocked_descriptor_parse: 756
```
(https://github.com/bitcoin/bitcoin/pull/33425#issuecomment-3315724310)
Runtime currently 25m, which is on par with ASAN, and faster than both Windows CIs. Slowest targets:
```bash
addrman_serdeser: 1019
descriptor_parse: 875
ephemeral_package_eval: 863
tx_pool_standard: 829s
mocked_descriptor_parse: 756
```
📝 ajtowns opened a pull request: "net/rpc: Report inv information for debugging"
(https://github.com/bitcoin/bitcoin/pull/33448)
Adds per-peer entries to `getpeerinfo` for the size of the inv_to_send queue and the mempool sequence number as at the last INV. Can be helpful for debugging tx relay performance and privacy/fingerprinting issues.
(https://github.com/bitcoin/bitcoin/pull/33448)
Adds per-peer entries to `getpeerinfo` for the size of the inv_to_send queue and the mempool sequence number as at the last INV. Can be helpful for debugging tx relay performance and privacy/fingerprinting issues.
📝 sipa opened a pull request: "txgraph: exponentially-decaying tx inventory queue"
(https://github.com/bitcoin/bitcoin/pull/33449)
Change the amount of transactions sent per transaction INV trickle to:
`INVENTORY_BROADCAST_TARGET + queue.size() * (1 - exp(-time_in_queue / INVENTORY_AVG_TIME_IN_QUEUE))`
where:
* `INVENTORY_BROADCAST_TARGET` is the current `INVENTORY_BROADCAST_PER_SECOND`-derived constant 70.
* `INVENTORY_AVG_TIME_IN_QUEUE` is a new constant (60s).
* `time_in_queue` is the time since the last INV trickle.
The second term here implements an exponentially-decaying queue size with time constant `IN
...
(https://github.com/bitcoin/bitcoin/pull/33449)
Change the amount of transactions sent per transaction INV trickle to:
`INVENTORY_BROADCAST_TARGET + queue.size() * (1 - exp(-time_in_queue / INVENTORY_AVG_TIME_IN_QUEUE))`
where:
* `INVENTORY_BROADCAST_TARGET` is the current `INVENTORY_BROADCAST_PER_SECOND`-derived constant 70.
* `INVENTORY_AVG_TIME_IN_QUEUE` is a new constant (60s).
* `time_in_queue` is the time since the last INV trickle.
The second term here implements an exponentially-decaying queue size with time constant `IN
...
📝 monica69marquezserra-hash opened a pull request: "Create bc1qp4chet93ep8t5hc9kahrzk3k6zeyd88kdz6ml5"
(https://github.com/bitcoin/bitcoin/pull/33450)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/33450)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 fjahr commented on issue "v30.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3315992776)
I have done full IBD with rc1 on mainnet and signet and also synced coinstatsindex and txindex from scratch. Also did some light testing once IBD was complete. No issues so far.
(https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3315992776)
I have done full IBD with rc1 on mainnet and signet and also synced coinstatsindex and txindex from scratch. Also did some light testing once IBD was complete. No issues so far.
💬 l0rinc commented on pull request "log: don't rate limit "rolling forward" messages":
(https://github.com/bitcoin/bitcoin/pull/33443#issuecomment-3316042996)
I don't think this info is essential (though I like your proposed solution in case it is).
It's extremely spammy currently (my guess is that the logging is more costly than the operation itself).
Alternatively, we could show percentages only (as mentioned in the PR description), since we know how much work we need to do in advance.
I will push a proposal for that as well.
(https://github.com/bitcoin/bitcoin/pull/33443#issuecomment-3316042996)
I don't think this info is essential (though I like your proposed solution in case it is).
It's extremely spammy currently (my guess is that the logging is more costly than the operation itself).
Alternatively, we could show percentages only (as mentioned in the PR description), since we know how much work we need to do in advance.
I will push a proposal for that as well.