💬 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


🤔 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

⚠️ etfmoon opened an issue: "Mark Transactions with OFAC Addresses as Non-Standard"
(https://github.com/bitcoin/bitcoin/issues/29148)
### Please describe the feature you'd like to see added.

> Bitcoin core arbitrarily censors a lot of transactions, not just op_return data carrier size. There's an entire suite of functions in policy.cpp that define Bitcoin's censorship algorithm - isDust(), isValid(), isStandard(), isStandardTx(), AreInputsStandard(), IsWitnessStandard(). The original and sole purpose of those functions is to promote censorship.

https://x.com/AsherHopp/status/1740191968834310654

Use these functions to ma
...

🤔 furszy reviewed a pull request: "init: handle empty settings file gracefully"
(https://github.com/bitcoin/bitcoin/pull/29144#pullrequestreview-1798109497)
> I think we should still handle the case of empty file separately. But instead of passing the file as valid, we should rather print a very clear error message, pointing the user to the reason for failure and possible remedy. This will definitely better the User experience for the noderunners.

For GUI interactive users, this isn't an issue. We trigger a dialog that allows continuing execution when a parsing error is found (still, we might want to be clearer about the fact that they could be d
...

💬 pablomartin4btc commented on 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#discussion_r1437684690)
Thanks for reviewing! I'll change it if I have to re-touch it.