💬 ismaelsadeeq commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2285961327)
The `test_sub_1s_per_vb_estimates` is not even necessary hence can be removed because we have a general test in `sanity_check_estimates_range`.

I added `test_sub_1s_per_vb_estimates` to convince reviewers and myself that it works.

However that said, I don't think its okay to bundle test together like this, but curious to know your opinion on why we would want to avoid adding the test.

💬 Sjors commented on issue "depends: `native_libmultiprocess` fails to build on OpenBSD":
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3201700143)
@vasild if I remember correctly you had a PR to add ?BSD CI coverage?

💬 polespinasa commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#issuecomment-3201704367)
ACK ba33047b6f3ee468eab250c8515e92b595e08322 reviewed the code and build and run the tests locally.
Adding a mempool-based fee estimation is a great step towards improving Core fee estimation.

🤔 enirox001 reviewed a pull request: "wallet: Remove isminetypes"
(https://github.com/bitcoin/bitcoin/pull/32523#pullrequestreview-3133328366)
ACK 620abe9 — built & tested locally; wallet tests passed. Behavior-preserving refactor; removing isminetype in favor of explicit booleans improves clarity.

Non-blocking nit: wallet_migration.py still references ISMINE_SPENDABLE / ISMINE_NO in comments. Functionally the tests use the boolean ismine; could you briefly explain why you kept the enum names in the comments, or consider changing them to ismine == True / ismine == False for consistency?

💬 hebasto commented on issue "depends: `native_libmultiprocess` fails to build on OpenBSD":
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3201706852)
> [@vasild](https://github.com/vasild) if I remember correctly you had a PR to add ?BSD CI coverage?

For Bitcoin Core, see https://github.com/hebasto/bitcoin-core-nightly.

💬 polespinasa commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2285977318)
Less code the better :)

It's not a new or modified feature to be tested, it's just a value that has been lowered.

💬 achow101 commented on pull request "wallet: Remove isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-3201717327)
> Non-blocking nit: wallet_migration.py still references ISMINE_SPENDABLE / ISMINE_NO in comments. Functionally the tests use the boolean ismine; could you briefly explain why you kept the enum names in the comments, or consider changing them to ismine == True / ismine == False for consistency?

https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-3190062828

💬 Sjors commented on issue "depends: `native_libmultiprocess` fails to build on OpenBSD":
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3201724485)
Maybe we could have a label like "moar-ci" that runs these extra systems? Similar to how we can request guix builds.

💬 TheBlueMatt commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3201748467)
> The only difference between the binary that supports the -ipcbind option and the binary that does not support it

Forgive my ignorance, but ISTM a simple solution to @sipa's concern, then, would be for the *only* release `bitcoind` binary to be the `bitcoin-node` one. Is there some reason why someone should prefer to have run a binary which is simply missing the option (given release builds optionally provide it)?

🤔 janb84 reviewed a pull request: "test: modify logging_filesize_rate_limit params"
(https://github.com/bitcoin/bitcoin/pull/33211#pullrequestreview-3133387403)
re ACK 5dda364c4b1965da586db7b81de8be90b6919414

changes since last ACK:
- more informative test failure loggin

💬 fanquake commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3201822267)
> Maybe add a note so we don't forget?

Any preference to where we add that?

📝 polespinasa opened a pull request: "doc: truc packages allow sub min feerate transactions"
(https://github.com/bitcoin/bitcoin/pull/33220)
Fixes https://github.com/bitcoin/bitcoin/issues/32067

Some policy documentation is outdated since TRUC. This PR aims to update the documentation to the actual policy state.

💬 polespinasa commented on issue "doc: Mempool Policy documentation Outdated since TRUC":
(https://github.com/bitcoin/bitcoin/issues/32067#issuecomment-3201836465)
Went ahead and opened a PR https://github.com/bitcoin/bitcoin/pull/33220 as draft to not create much noise.
@glozow feel free to review and suggest text :)

💬 ryanofsky commented on issue "depends: `native_libmultiprocess` fails to build on OpenBSD":
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3201847735)
I am waiting for openbsd to slowly install in a vm. It might not be too hard to add an openbsd job in the libmultiprocess repository too with https://github.com/vmactions/openbsd-vm

🤔 yuvicc reviewed a pull request: "kernel: improve BlockChecked ownership semantics"
(https://github.com/bitcoin/bitcoin/pull/33078#pullrequestreview-3133490635)
Code Review ACK 1d9f1cb4bd6b119e1d56cbdd7f6ce4d4521fffa3

- the benchmark looks encouraging
- avoids ref-counts and no atomic overhead.

💬 marcofleon commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3201890300)
It's my first time fully reading through this PR (along with https://github.com/bitcoin/bitcoin/issues/31756 and https://github.com/bitcoin/bitcoin/pull/33190) and looking at the code changes so my understanding is high level and likely incomplete. If there are details I'm missing or other PRs I should check out, please let me know.

Based on https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-3191623241 and https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3190601926, it see
...

💬 achow101 commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3201901283)
ACK 3aef38f44b76dfda77f47dc1a0e1fdc6ff3c7766

🚀 achow101 merged a pull request: "index: fix wrong assert of current_tip == m_best_block_index"
(https://github.com/bitcoin/bitcoin/pull/32878)

💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3201962009)
> > The only difference between the binary that supports the -ipcbind option and the binary that does not support it
>
> Forgive my ignorance, but ISTM a simple solution to @sipa's concern, then, would be for the _only_ release `bitcoind` binary to be the `bitcoin-node` one.

If the only purpose of the ENABLE_IPC option were to enable the IPC mining interface this would make sense. But that's not the only purpose or even the intended one. The point of the option is to turn on IPC features b
...

💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3201997480)
> This would give us more confidence to include the new binary in the next official release.

marcofleon, I think your understanding is all correct but note that -ipcbind feature and mining interface are available in v29 and can be easily toggled on in cmake. If you check discussion starting https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3189623427 not having the feature enabled in binary releases and depends builds has seemed to be a barrier to adoption. It do think it might not b
...