Bitcoin Core Github
44 subscribers
121K links
Download Telegram
⚠️ 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.
βœ… fanquake closed an issue: "Mark Transactions with OFAC Addresses as Non-Standard"
(https://github.com/bitcoin/bitcoin/issues/29148)
πŸ€” pablomartin4btc reviewed a pull request: "getrawtransaction implementation"
(https://github.com/bitcoin-core/gui/pull/777#pullrequestreview-1798125825)
utACK 9659890f7c9223d85dda3d066c2e3de42cabe7cf

I agree that the new place for this PR's feature looks better under the new "Tools" menu.

Also code-wise it's better to have a new separate dialog (`toolsdialog`) rather than re-using the existing help dialog (`HelpMessageDialog`). Thinking it loud, I wonder if we need to specify a more specific name instead of the current generic one (what other use cases would open this `ToolsDialog`? Other RPC commands?). Another thoughts: how this new feat
...
πŸ’¬ pablomartin4btc commented on pull request "getrawtransaction implementation":
(https://github.com/bitcoin-core/gui/pull/777#discussion_r1437698849)
nit:
```suggestion
getRawTransactionAction = new QAction(tr("&Verify transaction"), this);
```

Some thoughts: perhaps we use a more generic naming here and in the form we indicate if it's a wallet transaction with a checkbox (and then we change the form removing the blockhash option and call `gettransaction`?). Or maybe just on a follow-up.
πŸ’¬ pablomartin4btc commented on pull request "getrawtransaction implementation":
(https://github.com/bitcoin-core/gui/pull/777#discussion_r1437690450)
nit:
```suggestion
<string>Blockhash (optional): </string>
```
πŸ’¬ pablomartin4btc commented on pull request "getrawtransaction implementation":
(https://github.com/bitcoin-core/gui/pull/777#discussion_r1437689605)
nit:
```suggestion
<string>Transaction id: </string>
```
or `ID`?
πŸ’¬ fanquake 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-1871225764)
The main outcome I can see from this change is confusion from (maybe less-technical) users, when they see this unusual domain / `maybe-malware` printed to their console/logs:
```bash
2023-12-28T11:54:27.034530Z [dnsseed] Loading addresses from DNS seed dnsseed.bitcoin.dashjr-list-of-p2p-nodes-maybe-malware.us.
```

I don't think we should ship software that will do that.
πŸ’¬ petertodd 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-1871266126)
I agree with @fanquake, so NACK the choice of domain name.

Anyone who actually sees the domain name used and uses it for some purpose will very likely be able to understand that these IP addresses may be anything, so there is no need for this disclaimer. Meanwhile, putting malware in the name just invites confusion and unnecessary support requests from curious people who don't understand what they are seeing.
πŸ’¬ Sjors commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#issuecomment-1871307676)
I believe I've processed all feedback on this PR into my PR, either as documentation, TODO or actually implemented.
πŸ’¬ kristapsk commented on pull request "getrawtransaction implementation":
(https://github.com/bitcoin-core/gui/pull/777#issuecomment-1871335533)
Concept ACK
πŸ’¬ luke-jr commented on pull request "guix: Use DOS newlines for SHA256SUMS files":
(https://github.com/bitcoin/bitcoin/pull/29147#issuecomment-1871354004)
>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?

You can still do that with the content in the file. Furthermore, this change does not _require_ us to produce an attached SHA256SUMS.asc, it only _enables_ us to do so if desired.
πŸ’¬ luke-jr commented on pull request "guix: Use DOS newlines for SHA256SUMS files":
(https://github.com/bitcoin/bitcoin/pull/29147#discussion_r1437791949)
Done
πŸ’¬ luke-jr 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-1871362078)
Seems like if debug log is the concern, we could just mask it there... Doesn't seem like a good reason to make it easier for scammers by removing the warning?
πŸ’¬ 1440000bytes 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-1871364149)
NACK with this domain and I shared it in the first line of my last comment. I agree with _fanquake_ although domain could even get suspended for such domain name and some of the IPs it gets resolved to.

This is important even if not directly related to PR but domain and author: https://github.com/bitcoin/bitcoin/pull/29145#pullrequestreview-1797445282

With these warnings on **luke.dashjr.org** I am not okay with author running a DNS seed for bitcoin core:

> WARNING: In November and Dece
...
πŸ€” murchandamus 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-1798320554)
ACK e03d6f7ed534f423f58236866f8e83beee1871e1
πŸ€” murchandamus reviewed a pull request: "wallet: Keep track of the wallet's own transaction outputs in memory"
(https://github.com/bitcoin/bitcoin/pull/27286#pullrequestreview-1798367193)
ACK 0c0022311a0cffc4c03b3763c9b60f93ede13687

While I am pretty sure that the reorg logic should handle any transactions getting added or removed from the wallet, it might be useful to add a test that the `m_txos` set would be correct after a reorg, if that’s not obvious.
πŸ’¬ murchandamus commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r1437849004)
I have the impression that `m_txos` is also used to look up information on old transactions and we therefore want to keep all transaction outputs the wallet ever had.

Would it perhaps make sense to also have a reduced set that only keeps the potentially _unspent_ TXOs? I could see a wallet that participates in a few dozen transactions every day have magnitudes more TXOs than UTXOs. So perhaps the work the wallet has to perform every time it builds a transaction could be shortcut by storing e.
...
πŸ’¬ murchandamus commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r1437851827)
Ah, nevermind, I just saw https://github.com/bitcoin/bitcoin/pull/27865