Bitcoin Core Github
42 subscribers
126K links
Download Telegram
👍 TheCharlatan approved a pull request: "Encapsulate warnings in generalized node::Warnings and remove globals"
(https://github.com/bitcoin/bitcoin/pull/30058#pullrequestreview-2115950869)
Re-ACK 260f8da71a35232d859d7705861fc1a88bfbbe81
👍 theuni approved a pull request: "Update leveldb subtree to latest upstream"
(https://github.com/bitcoin/bitcoin/pull/30281#pullrequestreview-2115953719)
utACK 95812d912b6335caa7af2a084d84447fb4aad156
💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2165900439)
Thanks for the review, and for the suggestions. I might add that later to this PR or as a followup.

Trivial rebase after #29015.
🤔 ismaelsadeeq reviewed a pull request: "doc: use TRUC instead of v3 and add release note"
(https://github.com/bitcoin/bitcoin/pull/30272#pullrequestreview-2116107521)
Concept ACK
👍 BrandonOdiwuor approved a pull request: "test: cover more errors for `signrawtransactionwithkey` RPC"
(https://github.com/bitcoin/bitcoin/pull/30278#pullrequestreview-2116157106)
Code Review ACK e2779ce98b39e14cada08a654928e798436f5a46
💬 achow101 commented on pull request "refactor: Reduce memory copying operations in bech32 encoding":
(https://github.com/bitcoin/bitcoin/pull/29607#issuecomment-2166121357)
ACK 07f64177a49f1b6b4d486d10cf67fddfa3c995eb
💬 achow101 commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#issuecomment-2166141888)
ACK 5f549c35d9a75c7019fe8a96963b988df5498eed
🚀 achow101 merged a pull request: "refactor: Reduce memory copying operations in bech32 encoding"
(https://github.com/bitcoin/bitcoin/pull/29607)
💬 achow101 commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#issuecomment-2166154443)
Silent merge conflict

```
../../../src/test/fuzz/i2p.cpp:30:16: error: no viable overloaded '='
30 | CreateSock = [&fuzzed_data_provider](const sa_family_t&) {
| ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
31 | return std::make_unique<FuzzedSock>(fuzzed_data_provider);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
32 | };
| ~
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.1.1/../../../../include/c++
...
💬 paplorinc commented on pull request "refactor: Reduce memory copying operations in bech32 encoding":
(https://github.com/bitcoin/bitcoin/pull/29607#issuecomment-2166248133)
Thanks for the reviews @josibake, @sipa, @achow101.
There's a similar small optimization for [transaction input/output allocations](https://github.com/bitcoin/bitcoin/pull/30093) as well, I'd appreciate a review.
💬 davidgumberg commented on pull request "Lint: improve subtree exclusion":
(https://github.com/bitcoin/bitcoin/pull/29965#issuecomment-2166387546)
Rebased over master since #30219 was merged.
💬 mzumsande commented on pull request "assumeutxo: Check snapshot base block is not in invalid chain":
(https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1638706909)
Some checks are within `ActivateSnapshot()`, others, like the added one, are in the rpc code (which means that they can't be covered in unit tests for that function). Do you know if there is a reason for that?
💬 ryanofsky commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2166556815)
re: https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2158932014

> It is possible there is a bug here, though so if you want to post an issue to the libmultiprocess github project so I can look into it more, that would be helpful.

To follow up on this, I actually found two separate bugs which were causing wallet objects not to be destroyed when the node process is killed. Coincidentally a new test was added recently in #26606 which reproduces this problem, because it actually test
...
👍 ryanofsky approved a pull request: "Introduce Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30200#pullrequestreview-2116692613)
Code review ACK 5666af2c6e4924bf8e358ab5121c88d14cb085df, just rebased since last review
⚠️ ismaelsadeeq opened an issue: "Mini miner `AncestorFeerateComparator` Signed Integer Overflow"
(https://github.com/bitcoin/bitcoin/issues/30284)
While working on #30079, I noticed that using multiplication to compare fee rates results in a signed integer overflow.
https://github.com/bitcoin/bitcoin/blob/fcc3b653dc2bd5683193a836556de07bea4d1b11/src/node/mini_miner.cpp#L191-L195

The policy estimator fuzz tests generate transaction fee and sigops values using `ConsumeTxMemPoolEntry`.
https://github.com/bitcoin/bitcoin/blob/fcc3b653dc2bd5683193a836556de07bea4d1b11/src/test/fuzz/util/mempool.cpp#L17-L31

Here https://github.com/bitcoi
...
💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-2166643540)
> I didn't look too closely into the CI failure reason, but seems like a relatively simple fix, i.e. fixing an signed int overflow:

@josibake overflow is from mini miner `AncestorFeerateComparator`, added a fixup commit here and opened an issue for it.
https://github.com/bitcoin/bitcoin/issues/30284
🤔 ismaelsadeeq reviewed a pull request: "Introduce Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30200#pullrequestreview-2116792265)
Concept ACK
💬 ismaelsadeeq commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#issuecomment-2166662408)
> Would be nice to have this work for create_self_transfer_multi as well

I will pick this up and the outstanding review comments.
👋 sr-gi's pull request is ready for review: "p2p: Fill reconciliation sets (Erlay) attempt 2"
(https://github.com/bitcoin/bitcoin/pull/30116)
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#issuecomment-2166667309)
This should be ready for review now.