Bitcoin Core Github
42 subscribers
126K links
Download Telegram
👍 stickies-v approved a pull request: "[27.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/30467#pullrequestreview-2194213064)
ACK 885c4b7bc65c294b4b2fac91392d23a37a1f6485 but https://github.com/bitcoin/bitcoin/pull/29855 is missing from `release-notes.md`

I verified that all backport commits are clean, except:
- ab422066527992d53c92bb482c6b993f089b2999 backported from 16bd283b3ad05daa41259a062aee0fc05b463fa6: the relevant part of the test has been minimally carved out and added to 27.x which I think is a good approach
📝 ryanofsky opened a pull request: "multiprocess: Add IPC wrapper for Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30510)
Add Cap'n Proto wrapper for the Mining interface introduced in #30200, and its associated types.

This PR combined with #30509 will allow a separate mining process, like the one being implemented in https://github.com/Sjors/bitcoin/pull/48, to connect to the node over IPC, and create, manage, and submit block templates.

---

This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).
💬 fanquake commented on pull request "[27.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/30467#issuecomment-2245464245)
> but https://github.com/bitcoin/bitcoin/pull/29855 is missing from release-notes.md

Thanks, added.
💬 brunoerg commented on issue "fuzz: crypter timeout":
(https://github.com/bitcoin/bitcoin/issues/30503#issuecomment-2245465298)
> My recommendation would be to limit the number of iterations to something reasonable, but still enough to cover all cases.

Agreed, I just reproduced it and it executes 9176 ones. No need for this number.
💬 stickies-v commented on pull request "[27.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/30467#issuecomment-2245478885)
ACK 4f23c8636498f7e5adbe0264a0dc66a566c4b1b9
👍 hebasto approved a pull request: "depends: build expat with CMake"
(https://github.com/bitcoin/bitcoin/pull/29878#pullrequestreview-2194240049)
re-ACK a517029646ac86f9d72fcea204ff45db41702e37.
💬 fanquake commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#discussion_r1688215889)
> or do you need https://packages.debian.org/trixie/mingw-w64-x86-64-dev ?

Yea at some point (#30210), we'll need the newer version, I'll switch to Noble here, and see if things are still broken.
💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1688210420)
is this exactly the same as before?
💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1688214916)
not sure I get the SetHexFragile/FromHex naming duality here :/
💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1688212501)
hmmm, what's the point of the Assert inside?
💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1688216506)
after we get rid of the deprecation, these checks should ideally be done while iterating over the values to avoid multiple iterations (e.g. how Base58 parsing is done)
💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2245485761)
re: https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2242645460

> We should then aim to get everything merged in the PR, maybe with the exception of `multiprocess: Add bitcoin-mine test program`.

To separate essential and nonessential parts of this PR, I created #30509 and #30510 which I think contain all the needed functionality from the PR, and I also broke the changes down and wrote tests for them. I will rebase this PR on top of those two.
💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2245502110)
One thing that may be useful to clarify, IIUC, although you can connect, nothing can be done via these connections yet. That is what #30510 introduces specifically for the Mining interface.
👍 willcl-ark approved a pull request: "[27.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/30467#pullrequestreview-2194315990)
ACK 4f23c8636498f7e5adbe0264a0dc66a566c4b1b9

Checked all commits were backported cleanly and had a release note for their original PR.

I agree with stickies that carving out the relevant portion of the test for https://github.com/bitcoin/bitcoin/commit/ab422066527992d53c92bb482c6b993f089b2999 makes best sense for a backport.
🤔 instagibbs reviewed a pull request: "[WIP] Cluster mempool implementation"
(https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-2192522762)
(after fixing the reported iterations done) Seeing a lot of linerarizations being done that are kind of odd looking repeats with same number of iterations?
```
2024-07-22T21:01:29.226080Z [bench] InvokeSort linearize cluster: 15 txs, 0.0184ms, 120 iter, 153.1ns/iter
2024-07-22T21:01:29.226348Z [bench] InvokeSort linearize cluster: 15 txs, 0.0187ms, 120 iter, 155.7ns/iter
2024-07-22T21:01:29.226633Z [bench] InvokeSort linearize cluster: 15 txs, 0.0180ms, 120 iter, 149.8ns/iter
2024-07-22T21:
...
💬 instagibbs commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1687121805)
`iterations` is not modified here, so you're just reporting 0 iters used everywhere, and always PostLinearizing

Do we want to change the `Linearize` signature to report iters left, which can infer optimal or not as well?
💬 ryanofsky commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2245563392)
> One thing that may be useful to clarify, IIUC, although you can connect, nothing can be done via these connections yet. That is what #30510 introduces specifically for the Mining interface.

Thank you, added this to the description
💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#issuecomment-2245579974)
re-ACK 09ce3501fa2ea2885a857e380eddb74605f7038c 🕓

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 09ce3501fa2ea2885a85
...
💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2245586329)
Is the idea to incorporate the interface changes proposed in #30440 and #30409 after they're merged? And in the mean time keep them in #30437?
💬 Sjors commented on pull request "indexes: Stop using node internal types and locking cs_main, improve sync logic":
(https://github.com/bitcoin/bitcoin/pull/24230#issuecomment-2245622810)
There's discussion in #29770 about whether to use some or all the things in this PR. Can you give it a rebase?