Bitcoin Core Github
44 subscribers
119K links
Download Telegram
πŸ’¬ fanquake commented on pull request "build: Only use `@` prefix for `echo` command in Makefiles":
(https://github.com/bitcoin/bitcoin/pull/19480#issuecomment-3107193070)
Removed "Up for grabs" given the switch to CMake.
πŸ’¬ yuvicc commented on pull request "doc: update headers and TOC in `developer-notes.md`":
(https://github.com/bitcoin/bitcoin/pull/33040#issuecomment-3107360337)
ACK f314f01fb833a873c8c75c0d8a21023cf733de9a

Thanks!
πŸ€” janb84 reviewed a pull request: "doc: update headers and TOC in `developer-notes.md`"
(https://github.com/bitcoin/bitcoin/pull/33040#pullrequestreview-3046968519)
lgtm ACK f314f01fb833a873c8c75c0d8a21023cf733de9a
πŸ’¬ Crypt-iQ commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#issuecomment-3107517948)
cc @stickies-v @l0rinc
πŸ’¬ petertodd commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3107710821)
@murchandamus

> Reducing `DEFAULT_MIN_RELAY_TX_FEE` only makes sense to me in conjunction with reducing `DEFAULT_BLOCK_MIN_TX_FEE`.

Reducing the default minimum relay feerate is safe if we're confident that plenty of hash power is in fact mining those transactions. The block minimum tx feerate default does not need to be tied to that change if you think of it in terms of a recommendation for profitability: while miners may choose to mine transactions with feerates so low that they may be
...
πŸ’¬ petertodd commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3107762428)
@ajtowns

> A full block of min fee txs would currently set fees at 0.32% of the total block reward; dividing that by 10 will delay hitting 1% from being ~7 years away to ~20 years away. If you're looking towards keeping hashrate reltively stable in an environment where the BTC price stabilises instead of continuing to more than double every four years, this is probably a step backwards, and setting an expectation that it fees will either decrease in BTC terms or stay stable in fiat/real term
...
πŸ’¬ pablomartin4btc commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-3107951679)
Another instance here -> https://cirrus-ci.com/task/5261079850254336?logs=ci#L4536
πŸ“ fanquake opened a pull request: "contrib: drop use of `PermissionsStartOnly` & `Group=` "
(https://github.com/bitcoin/bitcoin/pull/33044)
> This removes the redundant 'Group=' directive and replaces the deprecated 'PermissionsStartOnly' directive.

Picks up #16994 / #19513. The concern in both of these PRs was changing this too early, while systemd v240 was still prelevant on supported systems. That was ~5 years ago, and from what I can see, no modern/supported OS is still using an older systemd.

Separately , I am wondering if we should move these files out to https://github.com/bitcoin-core/packaging/.
πŸ’¬ fanquake commented on pull request "contrib: replace deprecated PermissionsStartOnly in systemd init":
(https://github.com/bitcoin/bitcoin/pull/16994#issuecomment-3108501129)
PIcked up in #33044.
πŸ‘‹ fanquake's pull request is ready for review: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32969)
πŸ’¬ fanquake commented on pull request "Improve systemd service file":
(https://github.com/bitcoin/bitcoin/pull/19513#issuecomment-3108502755)
PIcked up in #33044.
πŸ€” BrandonOdiwuor reviewed a pull request: "Add bitcoin-{node,gui} to release binaries for IPC"
(https://github.com/bitcoin/bitcoin/pull/31802#pullrequestreview-3047596547)
Tested ACK 1d9ba616c91c992e64cf7bef89f80a8bf3b6e737

Verified PR's changes: (1) `bitcoin-node/bitcoin-gui` in releases, `ENABLE_IPC=ON` default in `depends` build; (2) ENABLE_IPC=ON default in `non-depends` build;

Builds succeeded without explicit -DENABLE_IPC=ON. Checked binaries in build/bin/, ran bitcoin -m node/gui, unit/functional tests (all passed)

**Non-depends build (macOS 15.5)**
```bash
$ cmake -B build -DBUILD_GUI=ON
$ cmake --build build
```
<img width="607" height="22
...
πŸ’¬ janb84 commented on pull request "refactor: inline constant return values from `dbwrapper` write methods":
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3108822061)
Is this refactor also part of a series to align the `CDBWrapper::Read` with `CDBWrapper::write` e.g. also converting that to return type void and communicating errors through exceptions ?
πŸ’¬ marcofleon commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2225759355)
I opted for passing in a GenTxid to `FindTxForGetData` and doing the visit there over the templated version for a couple of reasons:

1. It feels like slightly cleaner to me.
2. We need a GenTxid anyway for `m_most_recent_block_txs`.
3. It avoids repeating the branching logic that is already present in `ToGenTxid`.

I’m keeping the functions in `txmempool` Txid/Wtxid, as after this PR there won’t be any GenTxids at all in the current mempool code, which is nice.

> below you are removing
...
πŸ’¬ marcofleon commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2225760575)
Added.
πŸ’¬ marcofleon commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2225761745)
Fixed.
πŸ’¬ apoelstra commented on pull request "[POC] wallet: Enable non-electronic (paper-based) wallet backup with codex32":
(https://github.com/bitcoin/bitcoin/pull/33043#issuecomment-3108925454)
> For encrypted wallets, recovery requires both the codex32 seed and the original passphrase, enhancing the security.

If I understand you correctly, the intent is that an attacker needs the *wallet passphrase* in addition to the seed. But the seed is the seed -- once an attacker has it, it's game over. He doesn't need the passphrase or any other part of the wallet to sweep coins.

codex32 itself does not have any notion of passphrases or encryption.
πŸ‘ pinheadmz approved a pull request: "net: Fix Discover() not running when using -bind=0.0.0.0:port"
(https://github.com/bitcoin/bitcoin/pull/32757#pullrequestreview-3047773382)
ACK 1dc6b1e8b04363107b4b44cbf42f8a25f2efabfe

Built and tested on macos/arm64. Reviewed code, minimal changes since last ACK, addressing clean-ups in the test. Tested on mainnet and compared against master to check that local address was discovered when `-bind=0.0.0.0`

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 1dc6b1e8b04363107b4b44cbf42f8a25f2efabfe
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyTo
...
πŸ’¬ glozow commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3108978032)
> Perhaps there should be a (continuously updated) BIP of transaction standardness rules? It can track which clients and versions support them. And list which ones are configurable in clients. And finally which clients have different defaults.

I think there are a lot of cases where a document would be helpful but BIP editors have been highly resistant to it, hence doc/policy.
πŸ€” marcofleon reviewed a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32969#pullrequestreview-3047900115)
ACK a9a71b840d729332d940ebdf2cba6b53b82aa792

Built and ran tests on 28.x branch locally. Skipped `--legacy-wallet` tests so lgtm 😌