📝 theStack opened a pull request: "test: check for output to stdout in `TestShell` test"
(https://github.com/bitcoin/bitcoin/pull/33951)
This is a small follow-up PR to the recently added `TestShell` test (#33546), verifying the stdout message "TestShell is already running!" when trying to instantiate a second instance.
(https://github.com/bitcoin/bitcoin/pull/33951)
This is a small follow-up PR to the recently added `TestShell` test (#33546), verifying the stdout message "TestShell is already running!" when trying to instantiate a second instance.
💬 m3dwards commented on pull request "ci: clear out space on CentOS, depends, gui GHA job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3582022147)
ACK e07e57368e9fab8ecfc140d44aef7db9b23c7ce0
Like that it runs in the background, dislike that it hides any errors. It's simple enough that I don't think it's worth being more complex to catch any problems.
As an [experiment](https://github.com/m3dwards/bitcoin/commit/129850417f09cd9bbc819794b2626d82c4de39b8) I removed CMakeFiles directories after cmake ran which deletes a few GBs in under a second which was just enough to get my fork building again. I prefer the solution in this PR howeve
...
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3582022147)
ACK e07e57368e9fab8ecfc140d44aef7db9b23c7ce0
Like that it runs in the background, dislike that it hides any errors. It's simple enough that I don't think it's worth being more complex to catch any problems.
As an [experiment](https://github.com/m3dwards/bitcoin/commit/129850417f09cd9bbc819794b2626d82c4de39b8) I removed CMakeFiles directories after cmake ran which deletes a few GBs in under a second which was just enough to get my fork building again. I prefer the solution in this PR howeve
...
💬 Eunovo commented on pull request "fix assumevalid is ignored during reindex":
(https://github.com/bitcoin/bitcoin/pull/33854#issuecomment-3582057691)
Thanks for the reviews.
@l0rinc I added your test as an automated test in https://github.com/bitcoin/bitcoin/pull/33854/commits/729bf4d627f3bc3414af22990fcf67af2d50d4cf
@hodlinator I added a useful log message in https://github.com/bitcoin/bitcoin/pull/33854/commits/729bf4d627f3bc3414af22990fcf67af2d50d4cf
(https://github.com/bitcoin/bitcoin/pull/33854#issuecomment-3582057691)
Thanks for the reviews.
@l0rinc I added your test as an automated test in https://github.com/bitcoin/bitcoin/pull/33854/commits/729bf4d627f3bc3414af22990fcf67af2d50d4cf
@hodlinator I added a useful log message in https://github.com/bitcoin/bitcoin/pull/33854/commits/729bf4d627f3bc3414af22990fcf67af2d50d4cf
👍 hodlinator approved a pull request: "contrib: Count entry differences in asmap-tool diff summary"
(https://github.com/bitcoin/bitcoin/pull/33939#pullrequestreview-3511709141)
ACK fd4ce55121e7b0fe0e5b1ecf648dc3178ed37fd8
Tested by diffing 2 previously recorded[^1] and encoded[^2] Kartograf outputs:
```shell
./contrib/asmap/asmap-tool.py diff 1762865730.dat 1762878275.dat
```
#### master
```
...
# 96634298 (2^26.53) IPv4 addresses changed; 2116571614109896006051667873103669 (2^110.71) IPv6 addresses changed
```
#### fd4ce55121e7b0fe0e5b1ecf648dc3178ed37fd8
```
...
# Summary
IPv4: 35383 entries with 96634298 (2^26.53) addresses changed
IPv6: 37688 entries with 211
...
(https://github.com/bitcoin/bitcoin/pull/33939#pullrequestreview-3511709141)
ACK fd4ce55121e7b0fe0e5b1ecf648dc3178ed37fd8
Tested by diffing 2 previously recorded[^1] and encoded[^2] Kartograf outputs:
```shell
./contrib/asmap/asmap-tool.py diff 1762865730.dat 1762878275.dat
```
#### master
```
...
# 96634298 (2^26.53) IPv4 addresses changed; 2116571614109896006051667873103669 (2^110.71) IPv6 addresses changed
```
#### fd4ce55121e7b0fe0e5b1ecf648dc3178ed37fd8
```
...
# Summary
IPv4: 35383 entries with 96634298 (2^26.53) addresses changed
IPv6: 37688 entries with 211
...
💬 hodlinator commented on pull request "contrib: Count entry differences in asmap-tool diff summary":
(https://github.com/bitcoin/bitcoin/pull/33939#discussion_r2565613005)
suggestion: Would be nice to output how many entries there are in total, something like
```
Total entries - {args.infile1.name}: {len(state1.to_entries(overlapping=False))}, {args.infile2.name}: {len(state2.to_entries(overlapping=False))}
->
Total entries - 1762865730.dat: 706503, 1762878275.dat: 698844
```
But adding that makes it run 7 seconds slower on top of the ~29 seconds I got otherwise. Maybe there's some other way to get an approximate value.
(https://github.com/bitcoin/bitcoin/pull/33939#discussion_r2565613005)
suggestion: Would be nice to output how many entries there are in total, something like
```
Total entries - {args.infile1.name}: {len(state1.to_entries(overlapping=False))}, {args.infile2.name}: {len(state2.to_entries(overlapping=False))}
->
Total entries - 1762865730.dat: 706503, 1762878275.dat: 698844
```
But adding that makes it run 7 seconds slower on top of the ~29 seconds I got otherwise. Maybe there's some other way to get an approximate value.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565643372)
This code is outdated. Anyway, there are still calls `NodeClock::now()`. I think passing the present time as parameter would be too cumbersome and unnecessary at the call sites. There is mock time to use from tests. I prefer to keep the `now()` call internal to the methods. The world outside of the class does not even know it deals with time internally.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565643372)
This code is outdated. Anyway, there are still calls `NodeClock::now()`. I think passing the present time as parameter would be too cumbersome and unnecessary at the call sites. There is mock time to use from tests. I prefer to keep the `now()` call internal to the methods. The world outside of the class does not even know it deals with time internally.
💬 brunoerg commented on pull request "fuzz: wallet: add target for `TransactionCanBeBumped`":
(https://github.com/bitcoin/bitcoin/pull/33916#discussion_r2565667203)
Done
(https://github.com/bitcoin/bitcoin/pull/33916#discussion_r2565667203)
Done
💬 brunoerg commented on pull request "fuzz: wallet: add target for `TransactionCanBeBumped`":
(https://github.com/bitcoin/bitcoin/pull/33916#discussion_r2565667562)
Done
(https://github.com/bitcoin/bitcoin/pull/33916#discussion_r2565667562)
Done
💬 brunoerg commented on pull request "fuzz: wallet: add target for `TransactionCanBeBumped`":
(https://github.com/bitcoin/bitcoin/pull/33916#issuecomment-3582150488)
Force-pushed 2df9f10755ea7de94ecc9a17e6dd6dc89de0a637..52101d8809062d524393ebfde20c926d36fe5bf8:
- Rebased - and now that we have cluster mempool, I simplified the target by mocking `HasDescendants`.
- Addressed https://github.com/bitcoin/bitcoin/pull/33916#discussion_r2564296509 and https://github.com/bitcoin/bitcoin/pull/33916#discussion_r2564297647
(https://github.com/bitcoin/bitcoin/pull/33916#issuecomment-3582150488)
Force-pushed 2df9f10755ea7de94ecc9a17e6dd6dc89de0a637..52101d8809062d524393ebfde20c926d36fe5bf8:
- Rebased - and now that we have cluster mempool, I simplified the target by mocking `HasDescendants`.
- Addressed https://github.com/bitcoin/bitcoin/pull/33916#discussion_r2564296509 and https://github.com/bitcoin/bitcoin/pull/33916#discussion_r2564297647
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565696859)
I prefer a programming style where things are rather on separate lines instead of combining things into one line. But `try_emplace()` is a clear winner here over `emplace()`, taken.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565696859)
I prefer a programming style where things are rather on separate lines instead of combining things into one line. But `try_emplace()` is a clear winner here over `emplace()`, taken.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565698576)
Reworded.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565698576)
Reworded.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565701315)
Outdated, no more after using `max_element()`.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565701315)
Outdated, no more after using `max_element()`.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565703911)
Outdated, no more after using `max_element()`.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565703911)
Outdated, no more after using `max_element()`.
📝 fanquake opened a pull request: "depends: update freetype and document remaining `bitcoin-qt` runtime libs"
(https://github.com/bitcoin/bitcoin/pull/33952)
Update freetype to `2.11.1`.
Updating fontconfig (currently `2.12.6`) to `2.13.1` requires what looks like a hard dep on gperf; leaving that as-is for now. If someone wants to investigate,
Document expectations in `symbol-check.py`.
Closes #29977 (and changes based on discussion there).
(https://github.com/bitcoin/bitcoin/pull/33952)
Update freetype to `2.11.1`.
Updating fontconfig (currently `2.12.6`) to `2.13.1` requires what looks like a hard dep on gperf; leaving that as-is for now. If someone wants to investigate,
Document expectations in `symbol-check.py`.
Closes #29977 (and changes based on discussion there).
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565710172)
Rough overview, but reworded as suggested.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565710172)
Rough overview, but reworded as suggested.
💬 fanquake commented on issue "depends: Freetype and xcbproto version in depends are too new":
(https://github.com/bitcoin/bitcoin/issues/29977#issuecomment-3582195905)
See #33952.
(https://github.com/bitcoin/bitcoin/issues/29977#issuecomment-3582195905)
See #33952.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565716163)
Yes. Then the `last_confirmed` in the priority will default to the epoch time.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2565716163)
Yes. Then the `last_confirmed` in the priority will default to the epoch time.
📝 Ataraxia009 opened a pull request: "init: clean up loadchainstate function"
(https://github.com/bitcoin/bitcoin/pull/33953)
`LoadChainState` had some redundant code, more specifically:
- unnecessarily setting the `m_chain` variable
- recalling `GetBestBlock()` unnecessarily
(https://github.com/bitcoin/bitcoin/pull/33953)
`LoadChainState` had some redundant code, more specifically:
- unnecessarily setting the `m_chain` variable
- recalling `GetBestBlock()` unnecessarily
✅ Ataraxia009 closed a pull request: "init: clean up loadchainstate function"
(https://github.com/bitcoin/bitcoin/pull/33953)
(https://github.com/bitcoin/bitcoin/pull/33953)
💬 fanquake commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3582235905)
Backported to `30.x` in #33609.
(https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3582235905)
Backported to `30.x` in #33609.