Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🤔 fjahr reviewed a pull request: "validation: Send correct notification during snapshot completion"
(https://github.com/bitcoin/bitcoin/pull/31556#pullrequestreview-2522565392)
tACK ca0753160a8ebc113e08d62c7b6cbe8fa98455e6

Good catch, feel free to ignore my comments unless you have to retouch.
💬 fjahr commented on pull request "validation: Send correct notification during snapshot completion":
(https://github.com/bitcoin/bitcoin/pull/31556#discussion_r1897340026)
nit: I prefer to not log the prep work of these tests, only what describes the actual point of the test either at the start or at the check. (It can still be a comment though)

I see that you follow roughly the same style of the logging in the rest of the file but the actually interesting part that sets this test apart from the other tests is in the comment. The logs are just printing rather generic stuff. So I basically revert the logging ad comments in this block. I.e. L201 becomes a log and
...
💬 fjahr commented on pull request "validation: Send correct notification during snapshot completion":
(https://github.com/bitcoin/bitcoin/pull/31556#discussion_r1897367226)
nit: could have been `const`
💬 fjahr commented on pull request "validation: Send correct notification during snapshot completion":
(https://github.com/bitcoin/bitcoin/pull/31556#discussion_r1897370877)
I like to see `ensure_for` getting used but I think I would have opted for `self.wait_until(lambda: (n2.getbalance() == 34), timeout=5)` or so here. Is there a case I am missing where the balance might change after having been 34 that you are covering with this?
💬 fjahr commented on pull request "validation: Send correct notification during snapshot completion":
(https://github.com/bitcoin/bitcoin/pull/31556#discussion_r1897368195)
nit: newline not necessary imo
🤔 ismaelsadeeq reviewed a pull request: "rpc: Extend scope of validation mutex in generateblock"
(https://github.com/bitcoin/bitcoin/pull/31563#pullrequestreview-2522632931)
re-ACK fa63b8232f38e78d3c6413fa7d51809f376de75c

nice test!
💬 Candyman1878 commented on issue "b-msghand invoked oom-killer: Master (v28.99) crashing during IBD":
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-2561981985)
I'm having a lot of trouble switching over my assets due to me not having access to my coin-daddy account or being able to update my email and phone number
⚠️ Aaronminer1 opened an issue: "Single-Glyph Bitcoin Transaction System"
(https://github.com/bitcoin/bitcoin/issues/31568)
### Please describe the feature you'd like to see added.

# Single-Glyph Bitcoin Transaction System

## 1. Core Concept

A single glyph would encode an entire Bitcoin transaction, including:
- Transaction ID
- Input/Output addresses
- Amounts
- Signatures
- Timestamp
- Network data

Think of it like a visual hash that contains all transaction information in a single symbol, but unlike a hash, it's:
- Fully decomposable
- Visually unique
- Instantly verifiable
- Human-distinguisha
...
💬 laanwj commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1897481126)
Correct. Thinking of it, we could define a constant for this in the MockableClock like `INITIAL_MOCK_TIME`. This would document it at the same time as preventing that mistake being made in future tests that use the mockable steady clock.

The alternative was to make the mock time in MockableClock an optional, but this would mean it could no longer simply be an atomic value, so was kind of a hassle compared to starting at non-zero.
💬 davidgumberg commented on pull request "rpc: Extend scope of validation mutex in generateblock":
(https://github.com/bitcoin/bitcoin/pull/31563#issuecomment-2562085838)
reACK https://github.com/bitcoin/bitcoin/commit/fa63b8232f38e78d3c6413fa7d51809f376de75c

Verified that the functional test added here usually fails on master[^1] (17/20 runs failed for me)

```console
$ git clean -dfx && git switch --detach fc7b2148 && git cherry-pick fa63b823
$ CC=clang CXX=clang++ cmake -B build && cmake --build build -j $(nproc)
$ ./build/test/functional/rpc_generate.py
[...]
2024-12-26T01:19:59.394000Z TestFramework (INFO): Generate blocks in parallel
2024-12-26T0
...
💬 i-am-yuvi commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2562463835)
https://github.com/bitcoin/bitcoin/issues/31446

This should fix the issue with CI failure!!
👍 i-am-yuvi approved a pull request: "Extend signetchallenge to set target block spacing"
(https://github.com/bitcoin/bitcoin/pull/29365#pullrequestreview-2523126624)
Tested ACK 6ce1b0ed7b8e4115f5ffed4c51c1d4eb0d6d5ddf

Tested using various signet challenges(30s, 1min, 5 mins, 30mins, etc), worked as expected.
📝 vtjl10 opened a pull request: "fix: typos in documentation files"
(https://github.com/bitcoin/bitcoin/pull/31569)
This pull request contains changes to improve clarity, correctness and structure.

**Description correction:**
Corrected `block chain` to `blockchain` x2
Corrected `OLD_CMAKE_REQURED_FLAGS` to `OLD_CMAKE_REQUIRED_FLAGS` x4

Please review the changes and let me know if any additional changes are needed.
fanquake closed a pull request: "fix: typos in documentation files"
(https://github.com/bitcoin/bitcoin/pull/31569)
💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2562913180)
I'll rebase after https://github.com/bitcoin/bitcoin/pull/31468 is merged to make CI green.
💬 mzumsande commented on pull request "validation: Send correct notification during snapshot completion":
(https://github.com/bitcoin/bitcoin/pull/31556#discussion_r1898015187)
I used `ensure_for` because otherwise the added test might not always have failed on master before (if the first check had been done immediately, before the incorrect notification was processed by the wallet, the balance would have still been 34).
💬 mzumsande commented on pull request "validation: Send correct notification during snapshot completion":
(https://github.com/bitcoin/bitcoin/pull/31556#discussion_r1898025732)
done
💬 mzumsande commented on pull request "validation: Send correct notification during snapshot completion":
(https://github.com/bitcoin/bitcoin/pull/31556#discussion_r1898025835)
removed, that was not on purpose
💬 mzumsande commented on pull request "validation: Send correct notification during snapshot completion":
(https://github.com/bitcoin/bitcoin/pull/31556#discussion_r1898026354)
Makes sense to me! I think that the code is self-explanatory enough that it's not necessary to keep the lines as comments, so I just removed them.