Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 l0rinc commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2361375488)
can we do this without mutating the input arg?
```python
remaining = list(expected_msgs)
...
remaining = [m for m in remaining if m not in found]
```
💬 l0rinc commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2361373085)
nit: `print_log` and `log` are both used in the vicinity, if you retouch again, consider different names
nit2: any reason for putting this before the yield?
💬 l0rinc commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2361380997)
we don't actually need this here, can we move it closer to it actual usage? Or if we use a list/set comprehension we don't even need this line
🤔 epcgrs reviewed a pull request: "contrib: Add bash completion for new bitcoin command"
(https://github.com/bitcoin/bitcoin/pull/33385#pullrequestreview-3242636586)
- Well structured;
- Good practices with bash scripting;
- Dynamic search using help;

LGTM
💬 benthecarman commented on issue "v30.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3310324026)
Noticing some weirdness with the gui, switching tabs feels like it closes and reopens the whole app.

This is on ubuntu 24.04

https://github.com/user-attachments/assets/fd7c878f-e9f6-44f2-9a76-33c367c24363
⚠️ benthecarman opened an issue: "v30rc1 Weird GUI windowing behavior"
(https://github.com/bitcoin/bitcoin/issues/33432)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

Changing tabs in the GUI will close and re-open the app, it also will reposition the app

### Expected behaviour

Just switch tab

### Steps to reproduce

https://github.com/user-attachments/assets/86be4b98-de5e-4a4a-a7d4-3eaed7d32491

### Relevant log output

_No response_

### How did you obtain Bitcoin Core

Compiled from source

### What version of Bitcoin Core are you using?

v30rc1


...
📝 luke-jr opened a pull request: "Bugfix: QA: rpc_bind: Skip nonloopback test if no such address is found"
(https://github.com/bitcoin/bitcoin/pull/33433)
Without this, I get:

```
2025-09-19T03:14:05.157000Z TestFramework (INFO): PRNG seed is: 3218602557639511064
2025-09-19T03:14:05.158000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin-test/a
2025-09-19T03:14:05.158000Z TestFramework (INFO): Check for ipv6
2025-09-19T03:14:05.158000Z TestFramework (INFO): Check for non-loopback interface
2025-09-19T03:14:05.158000Z TestFramework (INFO): Bind test for []
2025-09-19T03:14:05.516000Z TestFramework (INFO): Bind test for []
2
...
💬 benthecarman commented on issue "v30rc1 Weird GUI windowing behavior":
(https://github.com/bitcoin/bitcoin/issues/33432#issuecomment-3310407746)
Doing `sudo apt install libxcb-cursor0` seems to have fixed the issue but also removed dark mode for me!
👍 willcl-ark approved a pull request: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33416#pullrequestreview-3243651387)
ACK 78cb0a241786bbda5adabfe30f95ad41205c82bd

Backports are good, accounted for in release notes, and client versions and dates correct.
💬 HowHsu commented on pull request "index: handle case where pindex_prev equals chain tip in NextSyncBlock()":
(https://github.com/bitcoin/bitcoin/pull/32875#discussion_r2362131224)
> I still think this should need a test as well, otherwise it just seems like a random change - especially in a PR that claims to change a comment only

Hi l0rinc,

Just like what I said, revert this commit doesn't cause anything wrong, the work this PR does is just to handle a case explicitly rather than leaving it in `return chain.Next(chain.FindFork(pindex_prev));` which is not for that case, and may cause confusion. So this PR is code clean work.
I've updated the PR title, it isn't abou
...
👍 TheCharlatan approved a pull request: "Update libmultiprocess subtree to fix intermittent mptest hang"
(https://github.com/bitcoin/bitcoin/pull/33412#pullrequestreview-3243996515)
ACK c49a43591f88dc199cc04e76f3cfcb7ba136f1a6
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#issuecomment-3311443431)
`2dcf0c69b8...2427939935`: take https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2359730526
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2362310458)
Done, thanks!
🤔 hodlinator reviewed a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3243827867)
Reviewed 38025edd1978670bfa3c32cf518acd0b01644167
💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2362289291)
nit:
```suggestion
- node5 starts with no -assumevalid parameter. Reindex to deterministically hit
"assumevalid hash not in headers" and "below minimum chainwork".
```
💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2362319419)
nit: Had a back & forth out-of-band about increasing the `with`-block scopes. While it makes the code layout nicer, it makes for slightly sloppier tests. But initializing a p2p connection and then doing some asserts at the end of the block doesn't seem too bad, so this is nit-level.
💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2362210661)
Thanks!
💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2362200979)
Don't see much of a point to change `m_prev_script_checks_logged` types twice within the PR (`atomic_bool` -> `optional<bool>` -> `optional<string>`)?
Would suggest swapping the order of 00194b5c and 0ab8939c and then squashing 0ab8939c together with ed9d573d.
💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2362231753)
Thanks for combining the variables!

Could you please lay down your aversion to using `const char*` here? Benefits I see over `std::string`:
* Allocation-free.
* Quick comparisons (pointer values rather than fetching the two cache lines where the strings are allocated on the heap and doing string comparisons).

I know it's not performance-critical, but don't see any major drawbacks to it. `std::string` has never been claimed to be a zero-cost abstraction alternative to `const char*`.
💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2362266224)
(Thanks for changing to `self.wait_until(lambda: self.nodes[3].getblockcount() == 1)`).