Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 murchandamus commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1870633370)
ACK https://github.com/bitcoin/bitcoin/pull/27307/commits/d4259cdaff2128c25e877d6a64e6ce15ecb9beca, only documentation change
💬 mzumsande commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#discussion_r1437279158)
Ok, I clarified the comments added in this PR.
I think that a single connection / reconnection attempt doesn't waste a meaningful amount of resources of either side, so I'm not sure if there is an actual problem here. Also, often one might not want the other side to know that you evicted them because they are banned (after all you banned them for a reason) - that might just help them evade the ban.
💬 mzumsande commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1870670097)
[a30a1d9 ](https://github.com/bitcoin/bitcoin/commit/a30a1d9318d08ddd1007d7fe7c19e4175faa8b64)to [fb5bfed](https://github.com/bitcoin/bitcoin/commit/fb5bfed26a564014b83ccfc96ff00b630930fc61): small doc-only update.
💬 luke-jr commented on pull request "RPC/Wallet: Convert walletprocesspsbt to use options parameter":
(https://github.com/bitcoin/bitcoin/pull/24963#discussion_r1437296646)
Fixed
💬 luke-jr commented on pull request "RPC/Wallet: Convert walletprocesspsbt to use options parameter":
(https://github.com/bitcoin/bitcoin/pull/24963#discussion_r1437297031)
Done
💬 andrewtoth commented on pull request "During IBD, prune as much as possible until we get close to where we will eventually keep blocks":
(https://github.com/bitcoin/bitcoin/pull/20827#discussion_r1437296904)
nit: Any reason this declaration is up here and not directly above where it is used below?
💬 andrewtoth commented on pull request "During IBD, prune as much as possible until we get close to where we will eventually keep blocks":
(https://github.com/bitcoin/bitcoin/pull/20827#discussion_r1437297033)
nit: could remove the cast in the next line with
```suggestion
const uint64_t chain_tip_height = chain.m_chain.Height();
```
💬 ns-xvrn commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1870828421)
@petertodd

> As long as miners continue to mine these transactions they will end up in blocks, requiring node operators to process those transactions. Censoring those transactions simply shifts when that bandwidth usage happens. Indeed, it'll make block propagation happen somewhat slower for some users/miners due to interference with compact blocks. This is harmful to many users, as well as some smaller miners.

What kind of magnitude for the impact on block propagation here? Is it bigger
...
📝 luke-jr opened a pull request: "guix: Use DOS newlines for SHA256SUMS files"
(https://github.com/bitcoin/bitcoin/pull/29147)
OpenPGP specifies that plain text should use CR LF for newlines. By doing so, it becomes possible to include the hashes directly in the .asc file.

(Currently untested, looking for Concept ACKs)
💬 Sjors commented on pull request "guix: Use DOS newlines for SHA256SUMS files":
(https://github.com/bitcoin/bitcoin/pull/29147#discussion_r1437517112)
Maybe refer to https://www.rfc-editor.org/rfc/rfc4880.html section 5.2.4 here.
💬 maflcko commented on pull request "guix: Use DOS newlines for SHA256SUMS files":
(https://github.com/bitcoin/bitcoin/pull/29147#issuecomment-1871026463)
> By doing so, it becomes possible to include the hashes directly in the .asc file.

Not sure. Wasn't the goal the exact opposite, so that it is easier to `cat` the hashes once and as many signatures as one wants?
🤔 BrandonOdiwuor reviewed a pull request: "fuzz: set `m_fallback_fee` and `m_fee_mode` in `wallet_fees` target"
(https://github.com/bitcoin/bitcoin/pull/29076#pullrequestreview-1797930387)
Concept ACK
🤔 BrandonOdiwuor reviewed a pull request: "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog"
(https://github.com/bitcoin-core/gui/pull/762#pullrequestreview-1797941528)
Concept ACK
💬 Emzy commented on pull request "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes-maybe-malware.us":
(https://github.com/bitcoin/bitcoin/pull/29145#issuecomment-1871081589)
DNS points to the same host.

```
dnsseed.bitcoin.dashjr.org. 40000 IN NS ipv4.jun.dashjr.org.
dnsseed.bitcoin.dashjr-list-of-p2p-nodes-maybe-malware.us. 39994 IN NS ipv4.jun.dashjr.org.
```
So should be no difference is answers.
💬 fanquake commented on pull request "RPC/Wallet: Convert walletprocesspsbt to use options parameter":
(https://github.com/bitcoin/bitcoin/pull/24963#issuecomment-1871095812)
```bash
2023-12-28T00:06:54.183352Z [scheduler] [util/thread.cpp:22] [TraceThread] scheduler thread exit
unknown location:0: fatal error: in "psbt_wallet_tests/parse_hd_keypath": NonFatalCheckError: Internal bug detected: !(param_type & POSITIONAL) || inner.m_opts.also_positional
rpc/util.cpp:536 (RPCHelpMan)
```
🤔 shaavan reviewed a pull request: "init: handle empty settings file gracefully"
(https://github.com/bitcoin/bitcoin/pull/29144#pullrequestreview-1798049445)
I agree with the reasoning ryanofsky has given [here](https://github.com/bitcoin/bitcoin/pull/23096#issuecomment-927785519).

> A zero-size settings file is a corrupt settings file.

But going through the [issue](https://community.umbrel.com/t/bitcoin-docker-container-keeps-restarting/2144) mentioned in the description. I don't think `Unable to parse settings file /data/.bitcoin/settings.json` is a very helpful message when it comes to bitcoind detecting an empty settings.json file.

I thi
...
💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1871164506)
Hopefully fixed the "test each commit" CI. Added documentation, moved testing instructions from the PR descriptor to it. The doc now also explains how to test this PR with a custom signet - which so far seems the most practical approach, despite its many moving parts.
💬 pablomartin4btc commented on pull request "Fix: Ensure 'Transaction View' remains disabled if no wallet is selected":
(https://github.com/bitcoin-core/gui/pull/780#issuecomment-1871174218)
Thanks @MarnixCroes, @luke-jr and @alfonsoromanz for reviewing!
👍 BrandonOdiwuor approved a pull request: "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog"
(https://github.com/bitcoin-core/gui/pull/762#pullrequestreview-1798100255)
Tested ACK 30c235b2e6fd9b7e3634d795cedc171d4d8d0e27

![Screenshot from 2023-12-28 16-34-43](https://github.com/bitcoin-core/gui/assets/15610188/9e70728e-6ee9-4981-9c5a-9797d1a8a170)
🤔 BrandonOdiwuor reviewed a pull request: "Fix: Ensure 'Transaction View' remains disabled if no wallet is selected"
(https://github.com/bitcoin-core/gui/pull/780#pullrequestreview-1798105922)
Concept ACK