Bitcoin Core Github
44 subscribers
120K links
Download Telegram
🤔 luke-jr reviewed a pull request: "wallet/rpc: add scan_utxoset option to getbalance(s) to verify wallet balance accuracy"
(https://github.com/bitcoin/bitcoin/pull/33392#pullrequestreview-3242264016)
I agree getbalance* is the wrong place for this kind of check.

But it also will fail to detect incorrect birthdates if the TXOs are spent, so it can't be relied on either...
🤔 l0rinc reviewed a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3242130677)
Thanks for the comments, adjusted the test and code to differentiate between cases when the block is after the assumevalid block from when it's not even on the same chain.
Also adjusted the style of the test to have more self-contained blocks separated by the `with` blocks, unified the test string styles, updated old commit messages.
Pushed the [change](https://github.com/bitcoin/bitcoin/compare/f66036d47e221bed7fe94c517631be920da66d96..fb5405e43acf6d0e6ce74643f6372410ef8cc41d) and the rebase se
...
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2361341176)
I missed this comment somehow.
While this is a tiny change to the validation logic, I think we can make it more focused by putting it inside the original `GetAncestor(pindex->nHeight) != pindex` block.
Now that it was removed from V30 we can safely do this I think - and the messages would be a lot more useful this way. Added a new test to cover this and unified the test styles as well since it's not *that* urgent anymore.
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2361340578)
Done
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2361210534)
Done
🤔 l0rinc requested changes to a pull request: "qa: Improvements to debug_assert_log + busy_wait_for_debug_log"
(https://github.com/bitcoin/bitcoin/pull/33423#pullrequestreview-3241292554)
Thanks for reducing useless work - did you measure any speedup?
I think we could make the result simpler and more pythonic - it's a test, I find visibly simpler code better here than theoretically faster one that we can't even measure, so I suggested a few changes.
Here's the end-result containing most of my suggestions:
```patch
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index e28140c17b..ccbde4702c 100755
--- a/test/functional/t
...
💬 l0rinc commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2360549564)
this was weird before, it logged `print_log` which was created in different scope. Having `log` here is still weird, but I guess Python is weird.

nit: based on the docs, [`!r`](https://docs.python.org/3/library/functions.html#repr) might be more appropriate here:
```suggestion
self._raise_assertion_error(f'Expected message(s) {expected_msgs!r} not found in log:\n\n{print_log(log)}\n\n')
```
💬 l0rinc commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2360551867)
nit: you could mention the reason for changing the texts as well
💬 l0rinc commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2361380779)
what's the reason for regex iteration above and here if we're just escaping it anyway? We're not doing it in `busy_wait_for_debug_log`
I have added
```python
found = []
for i, expected_msg in enumerate(expected_msgs):
if re.search(re.escape(expected_msg), log, flags=re.MULTILINE):
found.append(i)
found2 = [i for i, m in enumerate(expected_msgs) if m in log]
assert found == found2, f"Mismatch: {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_r2361431692)
Wouldn't this return if the same expected message appears multiple times in the log? I think we should collect the indexes into a set instead.
💬 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
...