Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ’¬ ismaelsadeeq commented on pull request "doc: warn that CheckBlock() underestimates sigops":
(https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2079816983)
nit: maybe be specific.
```suggestion
// This underestimates the number of sigops, because unlike ConnectBlock it does not count the witness sigops and p2sh sigops:

```
πŸ’¬ ismaelsadeeq commented on pull request "doc: warn that CheckBlock() underestimates sigops":
(https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2079904329)
I think it's okay to have this check here to catch those violations early.
πŸš€ ryanofsky merged a pull request: "cmake: Respect user-provided configuration-specific flags"
(https://github.com/bitcoin/bitcoin/pull/32356)
πŸ’¬ ismaelsadeeq commented on pull request "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity":
(https://github.com/bitcoin/bitcoin/pull/32395#issuecomment-2863404137)
Friendly ping @glozow @willcl-ark @instagibbs
πŸ’¬ laanwj commented on pull request "build: let CMake determine the year":
(https://github.com/bitcoin/bitcoin/pull/32445#issuecomment-2863424559)
> When someone builds from a tarball that is more than a year old, this will use the current year instead of the year of the release. Is that a valid use case?

Not only with tarballs, it's true for any non-guix build, also from an old git commit.
πŸ’¬ instagibbs commented on pull request "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity":
(https://github.com/bitcoin/bitcoin/pull/32395#issuecomment-2863430236)
Is this something we need to support? In a sense the current mechanism is honestly saying it doesn't have enough data. If you don't actually know what level of fees are required to get in a block, you may miss the fact that, f.e., miners are not mining anything 2x higher than your floating minfee.

I think this circles back to what the end goal of this RPC is, A one stop shop for best guess of feerates needed?

I haven't thought about this much in years, so I'd be curious to hear what you th
...
πŸ€” mzumsande reviewed a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2825526715)
Concept / Approach ACK, wrote the reason in [the other PR](https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2855558032)
πŸ’¬ ryanofsky commented on pull request "build: let CMake determine the year":
(https://github.com/bitcoin/bitcoin/pull/32445#discussion_r2079946510)
In commit "build: let CMake determine the year" (dfeac76b951fc2d28e0832b176f3c8ad595697f1)

Doesn't this make the build nondeterministic?
πŸ’¬ laanwj commented on pull request "build: let CMake determine the year":
(https://github.com/bitcoin/bitcoin/pull/32445#discussion_r2079949509)
No; SOURCE_DATE_EPOCH, which is set by the guix build, is respected: https://github.com/bitcoin/bitcoin/pull/32445#pullrequestreview-2824986604
πŸ’¬ hebasto commented on issue "build: cmake --install fails after --target deploy":
(https://github.com/bitcoin/bitcoin/issues/32007#issuecomment-2863466359)
From CMake [docs](https://cmake.org/cmake/help/latest/variable/CMAKE_SKIP_INSTALL_ALL_DEPENDENCY.html):
> By default, the `install` target depends on the `all` target. This has the effect, that when `make install` is invoked or `INSTALL` is built, first the `all` target is built, then the installation starts.

[On the other hand](https://cmake.org/cmake/help/latest/manual/cmake.1.html#install-a-project), the `cmake --install <dir>` command:
> ... may be used _after_ building a project to run ins
...
πŸ’¬ rstmsn commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2863484560)
The OP_RETURN `-datacarriersize` limit is 100% effective in cases where miners use it to filter their local mempools & inform template construction, prior to successfully mining a block.

This viable use case would be harmed as a result of this PR. On that basis, firm NACK.
πŸ’¬ fanquake commented on pull request "cmake: Respect user-provided configuration-specific flags":
(https://github.com/bitcoin/bitcoin/pull/32356#issuecomment-2863491522)
Backported to 29.x in #32292.
πŸ’¬ fanquake commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#issuecomment-2863507309)
@laanwj you might be interested in circling back here to review?
πŸ’¬ hebasto commented on pull request "build: simplify *ifaddr handling":
(https://github.com/bitcoin/bitcoin/pull/32446#discussion_r2079994126)
nit: Is this really needed considering:https://github.com/bitcoin/bitcoin/blob/d8caa10c29321c9732dde3d4ccdcf0413041b33f/src/randomenv.cpp#L27 ?
πŸ‘ hebasto approved a pull request: "build: simplify *ifaddr handling"
(https://github.com/bitcoin/bitcoin/pull/32446#pullrequestreview-2825606189)
Approach ACK d8caa10c29321c9732dde3d4ccdcf0413041b33f. I've reviewed the code. Still waiting for my Guix build.
πŸ€” hebasto reviewed a pull request: "build: simplify *ifaddr handling"
(https://github.com/bitcoin/bitcoin/pull/32446#pullrequestreview-2825611533)
My Guix build:
```
aarch64
5bde67b2713e02dbc6fcf5aa5b90e21df89f7fd04debb4ba13302a8e6bcabc53 guix-build-d8caa10c2932/output/aarch64-linux-gnu/SHA256SUMS.part
09eeaa166590208ce1cdd5bd9298f4819a8b6014fdaf68f421f4797f5bc2d034 guix-build-d8caa10c2932/output/aarch64-linux-gnu/bitcoin-d8caa10c2932-aarch64-linux-gnu-debug.tar.gz
2ecfed14ba3fd7ee565969023e78b810a432042f719c30dd7962b6e070d712ef guix-build-d8caa10c2932/output/aarch64-linux-gnu/bitcoin-d8caa10c2932-aarch64-linux-gnu.tar.gz
1924ef5f
...
πŸ’¬ fanquake commented on pull request "build: simplify *ifaddr handling":
(https://github.com/bitcoin/bitcoin/pull/32446#discussion_r2079998731)
I've dropped the addition.
πŸ‘ hebasto approved a pull request: "build: simplify *ifaddr handling"
(https://github.com/bitcoin/bitcoin/pull/32446#pullrequestreview-2825625856)
ACK ab878a7e741073574336c9c4b1d41c6cf80b0d6a. Only addressed my [comment](https://github.com/bitcoin/bitcoin/pull/32446#discussion_r2079994126) and rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/32446#pullrequestreview-2825606189).
πŸ’¬ ismaelsadeeq commented on pull request "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity":
(https://github.com/bitcoin/bitcoin/pull/32395#issuecomment-2863565098)
> Is this something we need to support? In a sense the current mechanism is honestly saying it doesn't have enough data.

I think it is. Even based on recent mainnet network activity, it’s possible for us to see frequent empty blocks. In cases where the estimator have seen enough blocks to be able to have the data but there is just no activity, paying the floating minfee is sufficient to get transactions to miners. I think this is the sane thing to do.

In test networks, this has been the r
...
πŸ“ hebasto reopened a pull request: "cmake: Introduce `WITH_PYTHON` build option"
(https://github.com/bitcoin/bitcoin/pull/31669)
This was suggested in https://github.com/bitcoin/bitcoin/issues/31476#issuecomment-2541288435:
> An alternative would be to require python to be disabled explicitly. Otherwise, it seems odd that every setting in cmake has a static default that can only be overridden explicitly, except for some, which are silently downgraded?

This PR updates the build system to fail by default on systems where the minimum required Python version is unavailable. It introduces an option that allows users to exp
...