Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 jonatack commented on pull request "test: move remaining rand code from util/setup_common to util/random":
(https://github.com/bitcoin/bitcoin/pull/27425#discussion_r1256094854)
Good points. I'm a little hesitant to invalidate the reviews (thank you both!) but tentatively updated the name of the new helper and removed the `static` annotations as they are trivial to re-ACK (deferring discussion of the last point for a possible follow-up, if that's ok).
🚀 fanquake merged a pull request: "wallet: address book migration bug fixes"
(https://github.com/bitcoin/bitcoin/pull/28038)
📝 fanquake opened a pull request: "[25.x] Further backports for 25.1"
(https://github.com/bitcoin/bitcoin/pull/28047)
Currently backports:
* #28038
💬 fanquake commented on pull request "wallet: address book migration bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28038#issuecomment-1625667338)
Added to #28047 for backporting.
📝 ryanofsky opened a pull request: "kernel: Remove StartShutdown calls from validation code"
(https://github.com/bitcoin/bitcoin/pull/28048)
This change drops the last kernel dependency on shutdown.cpp. It also adds new hooks for libbitcoinkernel applications to be able to stop kernel operations after blocks are imported and each time the chain tip changes.
💬 hebasto commented on pull request "build: Add CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/25797#issuecomment-1625710276)
Closing this PR as its usefulness seems marginal.

Although, I'll keep maintaining this branch.

Reminding that the active reviewing process still happening in https://github.com/hebasto/bitcoin/tree/cmake-staging and related PRs.
💬 ryanofsky commented on pull request "kernel: Remove shutdown globals from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1256159653)
re: https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214568318

> I think it would be great to get rid of the 3 StartShutdown calls in kernel code, but probably better to do in a separate PR that was independent of this one, since there's lots of approaches that could be used and none of them seem to depend on the other changes here.

I implemented this idea in #28048, getting rid of the `stop_after_block_import` and `stop_at_height` kernel options and replacing them with more flex
...
hebasto closed a pull request: "build: Add CMake-based build system"
(https://github.com/bitcoin/bitcoin/pull/25797)
💬 achow101 commented on pull request "wallet: don't include bdb files from our headers":
(https://github.com/bitcoin/bitcoin/pull/28039#issuecomment-1625712726)
reACK 8b5397c00e821d7eaab22f512e9d71a1a0392ebf
💬 TheCharlatan commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#issuecomment-1625724128)
Rebased 1fe06a811bf8be941bef5336af97688f48885828 -> d1c92b57a72b62ffa202f1d3d357b59befdc9c12 ([kernelReturnOnAbort_0](https://github.com/TheCharlatan/bitcoin/tree/kernelReturnOnAbort_0) -> [kernelReturnOnAbort_1](https://github.com/TheCharlatan/bitcoin/tree/kernelReturnOnAbort_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelReturnOnAbort_0..kernelReturnOnAbort_1))

* Fixed conflict with #27861
* Fixed formatting if formatting
💬 luke-jr commented on pull request "build: Add CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/25797#issuecomment-1625724201)
>we should be able to enumerate some more benefits to doing so.

One more for the list:

* Real out-of-tree builds. Autotools requires autogen to be run in the tree, which can be complicated when you don't want the build to have write access to the source dir.
🚀 achow101 merged a pull request: "wallet: don't include bdb files from our headers"
(https://github.com/bitcoin/bitcoin/pull/28039)
💬 santochibtc commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1625747312)
Thanks @sipa, yes, it is "a histogram of the fee rates paid by transactions in the memory pool, weighted by transaction size", but my point is still valid. If you want to include your tx in the next block you must estimate your fee based on the pending txs that are paying more per byte. I think that the histogram is not always a good picture of the mempool to estimate fees.
💬 ryanofsky commented on pull request "kernel: Remove StartShutdown calls from validation code":
(https://github.com/bitcoin/bitcoin/pull/28048#issuecomment-1625782988)
Updated 828af9ad19a702e711963f9b498f7462bde8aabb -> c2bcc339d7b8def58289d3ff01b2a905dc291ed6 ([`pr/stopafter.1`](https://github.com/ryanofsky/bitcoin/commits/pr/stopafter.1) -> [`pr/stopafter.2`](https://github.com/ryanofsky/bitcoin/commits/pr/stopafter.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/stopafter.1..pr/stopafter.2)) just updating comments and adding release note about `-stopatheight` behavior
🤔 ishaanam reviewed a pull request: "wallet: Deniability API (Unilateral Transaction Meta-Privacy)"
(https://github.com/bitcoin/bitcoin/pull/27792#pullrequestreview-1519449241)
Hey @denavila, welcome to Bitcoin Core and thanks for working on this! While I think that this idea could be useful to some extent if implemented correctly, I have a few concerns about the current implementation:
- Currently there is no precaution against a user spending two outputs of a deniabilization transaction together. IMO I think that this is quite problematic and could lead to these deniabilization transactions being rendered ineffective. Not only that, but then it also becomes obvious
...
💬 theuni commented on pull request "kernel: Remove StartShutdown calls from validation code":
(https://github.com/bitcoin/bitcoin/pull/28048#issuecomment-1625907483)
Very nice. Concept ACK and quick (very shallow) codereview ACK.

I think the `-stopatheight` change is reasonable. If it turns out anyone/anything was relying on the way this worked before, we could always add functionality for that use-case (maybe `-stopatblock`?)

However, the help string should be updated from:

> Stop running after reaching the given height in the main chain

It sounds like it's not completely accurate now, and less so after this change :)
💬 achow101 commented on pull request "Add support for RFC8439 variant of ChaCha20":
(https://github.com/bitcoin/bitcoin/pull/27985#issuecomment-1625915605)
Assuming it doesn't break a whole lot of tests, I would prefer to change it always use the new version as it removes the need to check that everything is using the right variant.
💬 jonatack commented on pull request "test: move remaining rand code from util/setup_common to util/random":
(https://github.com/bitcoin/bitcoin/pull/27425#issuecomment-1625936888)
Thank you for the reviews @TheCharlatan and @MarcoFalke. Updated the name of the new helper and removed the static annotations per https://github.com/bitcoin/bitcoin/pull/27425#discussion_r1250420188, should be trivial to re-ACK.
💬 theuni commented on pull request "kernel: Remove StartShutdown calls from validation code":
(https://github.com/bitcoin/bitcoin/pull/28048#discussion_r1256350258)
Is it possible for this to be called twice?
```c++
if (m_stop_at_height) {
if (index.nHeight > m_stop_at_height) {
// assert(false) or return kernel::Interrupted{} ?
} else if (index.nHeight == m_stop_at_height) {
..
}
}
```

Not that I imagine calling `StartShutdown()` twice would be particularly harmful anyway.
⚠️ dsldsl opened an issue: "Malware reported in scan of bitcoin-qt versions 22 and 24.0.1 apple darwin"
(https://github.com/bitcoin/bitcoin/issues/28049)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

<img width="1174" alt="image" src="https://github.com/bitcoin/bitcoin/assets/50015/625e57b0-6a86-486d-b18a-58c93ce60f26">

My virus scanner reported the malware Koiot found in the dmg file and in the installed app of bitcoin core. Is this a known false positive or an actual potential issue with a bad binary I downloaded? thank you.

### Expected behaviour

Not have a virus

### Steps t
...