π¬ luke-jr commented on pull request "ThreadSanitizer: Fix #29767":
(https://github.com/bitcoin/bitcoin/pull/29776#issuecomment-2068164464)
>It seems like the race condition between m_synced = true and committing has been around since 4368384 from #14121, and the race condition accessing the m_next_filter_pos variable existed then too.
Until #28955, `cs_main` was held across all this to prevent a race.
Not going to look at it right now, but I suspect this fix just creates a different race instead.
(https://github.com/bitcoin/bitcoin/pull/29776#issuecomment-2068164464)
>It seems like the race condition between m_synced = true and committing has been around since 4368384 from #14121, and the race condition accessing the m_next_filter_pos variable existed then too.
Until #28955, `cs_main` was held across all this to prevent a race.
Not going to look at it right now, but I suspect this fix just creates a different race instead.
β οΈ asctime opened an issue: "ReadAnchor throws exception on second run"
(https://github.com/bitcoin/bitcoin/issues/29931)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
```
std::vector<CAddress> ReadAnchors(const fs::path& anchors_db_path)
{
std::vector<CAddress> anchors;
try {
DeserializeFileDB(anchors_db_path, CAddress::V2_DISK(anchors));
LogPrintf("Loaded %i addresses from %s\n", anchors.size(), fs::quoted(fs::PathToString(anchors_db_path.filename())));
} catch (const std::exception&) {
anchors.clear();
...
(https://github.com/bitcoin/bitcoin/issues/29931)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
```
std::vector<CAddress> ReadAnchors(const fs::path& anchors_db_path)
{
std::vector<CAddress> anchors;
try {
DeserializeFileDB(anchors_db_path, CAddress::V2_DISK(anchors));
LogPrintf("Loaded %i addresses from %s\n", anchors.size(), fs::quoted(fs::PathToString(anchors_db_path.filename())));
} catch (const std::exception&) {
anchors.clear();
...
π¬ 1440000bytes commented on issue "Crash on fail ~CCheckQueue() destructor?":
(https://github.com/bitcoin/bitcoin/issues/29930#issuecomment-2068178196)
> Open settings in bitcoin-qt, adjust and save.
I am not able to reproduce this. What exactly did you adjust in settings before clicking on ok?
(https://github.com/bitcoin/bitcoin/issues/29930#issuecomment-2068178196)
> Open settings in bitcoin-qt, adjust and save.
I am not able to reproduce this. What exactly did you adjust in settings before clicking on ok?
π¬ luke-jr commented on pull request "ThreadSanitizer: Fix #29767":
(https://github.com/bitcoin/bitcoin/pull/29776#issuecomment-2068179144)
Looks like furszy already got this in #29867
(https://github.com/bitcoin/bitcoin/pull/29776#issuecomment-2068179144)
Looks like furszy already got this in #29867
π¬ paplorinc commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573899433)
Thanks for adding me as a co-author, if you force push again, please change the email to `LΕrinc <pap.lorinc@gmail.com>`
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573899433)
Thanks for adding me as a co-author, if you force push again, please change the email to `LΕrinc <pap.lorinc@gmail.com>`
π¬ fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2068180640)
I have added the behavior change in a separate commit.
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2068180640)
I have added the behavior change in a separate commit.
π¬ fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573899901)
The behavior change is now re-introduced but explicitly documented as such in a separate commit.
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573899901)
The behavior change is now re-introduced but explicitly documented as such in a separate commit.
π¬ paplorinc commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573900516)
would it make sense to mention the `&` as well?
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573900516)
would it make sense to mention the `&` as well?
π¬ fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573902309)
Are these commonly decoded as a space as well? I couldn't find evidence for this but I didn't spend much time on it.
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573902309)
Are these commonly decoded as a space as well? I couldn't find evidence for this but I didn't spend much time on it.
π¬ fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573902893)
> Thanks for adding me as a co-author, if you force push again, please change the email to `LΕrinc <pap.lorinc@gmail.com>`
fixed
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573902893)
> Thanks for adding me as a co-author, if you force push again, please change the email to `LΕrinc <pap.lorinc@gmail.com>`
fixed
π¬ paplorinc commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573902969)
Not exactly: https://github.com/libevent/libevent/blob/release-2.1.12-stable/http.c#L3186-L3188
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573902969)
Not exactly: https://github.com/libevent/libevent/blob/release-2.1.12-stable/http.c#L3186-L3188
π¬ fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573904156)
Hm, the code you are linking to is part of how libevent handles the `+`. Or did you want to refer to `?`? I think if it's not very common for `&` to be decoded as space then we don't need to mention it.
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573904156)
Hm, the code you are linking to is part of how libevent handles the `+`. Or did you want to refer to `?`? I think if it's not very common for `&` to be decoded as space then we don't need to mention it.
π¬ paplorinc commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573905184)
? seems to be a control char, `If -1, when true we transform plus to space only after we've seen a ?. -1 is deprecated.`, no need to mention it, thanks for checking.
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573905184)
? seems to be a control char, `If -1, when true we transform plus to space only after we've seen a ?. -1 is deprecated.`, no need to mention it, thanks for checking.
π¬ asctime commented on issue "Crash on fail ~CCheckQueue() destructor?":
(https://github.com/bitcoin/bitcoin/issues/29930#issuecomment-2068196611)
Any setting, or just clicking ok (cancel is fine). It's probably something slightly different in MinGW pthread support.
so assert(m_worker_threads.empty()); causing a crash still analyzing.
So I understand assert will tell us what part of the code crashed, but it doesn't really tell us anything about the worker that wont stop, (thread id etc) so far it's only the settings box -> everything else seems to work after #29931 fingers crossed. gui syncing now.
(https://github.com/bitcoin/bitcoin/issues/29930#issuecomment-2068196611)
Any setting, or just clicking ok (cancel is fine). It's probably something slightly different in MinGW pthread support.
so assert(m_worker_threads.empty()); causing a crash still analyzing.
So I understand assert will tell us what part of the code crashed, but it doesn't really tell us anything about the worker that wont stop, (thread id etc) so far it's only the settings box -> everything else seems to work after #29931 fingers crossed. gui syncing now.
π¬ ismaelsadeeq commented on pull request "test: Refactor fee calculation to remove satoshi_round function":
(https://github.com/bitcoin/bitcoin/pull/29566#issuecomment-2068218318)
The C.I failure here is due to unused import @naiyoma
```
{'-zmqpubhashblock', '-zmqpubsequencehwm', '-zmqpubsequence', '-zmqpubrawblock', '-zmqpubhashtxhwm', '-zmqpubhashblockhwm', '-includeconf', '-zmqpubhashtx', '-zmqpubrawtx', '-testdatadir', '-zmqpubrawblockhwm', '-zmqpubrawtxhwm'}
test/functional/test_framework/util.py:8:1: F401 'decimal.ROUND_DOWN' imported but unused
^---- failure generated from lint-python.py
^---- β οΈ Failure generated from lint-*.py scripts!
```
(https://github.com/bitcoin/bitcoin/pull/29566#issuecomment-2068218318)
The C.I failure here is due to unused import @naiyoma
```
{'-zmqpubhashblock', '-zmqpubsequencehwm', '-zmqpubsequence', '-zmqpubrawblock', '-zmqpubhashtxhwm', '-zmqpubhashblockhwm', '-includeconf', '-zmqpubhashtx', '-zmqpubrawtx', '-testdatadir', '-zmqpubrawblockhwm', '-zmqpubrawtxhwm'}
test/functional/test_framework/util.py:8:1: F401 'decimal.ROUND_DOWN' imported but unused
^---- failure generated from lint-python.py
^---- β οΈ Failure generated from lint-*.py scripts!
```
π¬ luke-jr commented on pull request "Don't permit port in proxy IP option":
(https://github.com/bitcoin-core/gui/pull/813#issuecomment-2068230372)
@jarolrod Isn't that a bug independent from the validation one?
(https://github.com/bitcoin-core/gui/pull/813#issuecomment-2068230372)
@jarolrod Isn't that a bug independent from the validation one?
π¬ fjahr commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943040)
done
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943040)
done
π¬ fjahr commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943357)
I added a counter and check it at the end
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943357)
I added a counter and check it at the end
π¬ fjahr commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943413)
fixed
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943413)
fixed
π¬ fjahr commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943494)
renamed
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1573943494)
renamed