Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 tdb3 commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1606079769)
Thanks. Fixed
💬 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_r1606087141)
done!
💬 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_r1606087218)
done!
💬 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#issuecomment-2119337707)
Updates:
<ul><li><details>
<summary>Addressed @hebasto's <a href="https://github.com/bitcoin-core/gui/pull/762#pullrequestreview-2058694015">feedback</a>.</summary>

```
git diff -U0 HEAD~1.. | ./contrib/devtools/clang-format-diff.py -p1 -i -v
Formatting src/qt/bitcoingui.cpp
Formatting src/qt/utilitydialog.cpp
Formatting src/qt/utilitydialog.h

```
</details></li></ul>

- As part of the above, [functions](https://github.com/bitcoin-core/gui/pull/762#discussion_r1602056777) were re-
...
💬 brunoerg commented on pull request "fuzz: add more coverage for `ScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/30134#issuecomment-2119342047)
> I generated a coverage report for the current ScriptPubKeyMan harness and for the updated harness in this PR.

@marcofleon It's good to mention how you run it. How many hours? From seed corpus (note that changes can invalidate it)?
💬 mjdietzx commented on pull request "tests: improve wallet multisig descriptor test and docs":
(https://github.com/bitcoin/bitcoin/pull/29154#issuecomment-2119495691)
I addressed @achow101 's comments:
- re [this comment](https://github.com/bitcoin/bitcoin/pull/29154#discussion_r1567805211), I removed `random.sample`. it's unnecessary
- re [this comment](https://github.com/bitcoin/bitcoin/pull/29154#discussion_r1567795960), I removed the unnecessary explanation of where descriptor came from

Thank you all for your reviews and patience
💬 mjdietzx commented on pull request "tests: improve wallet multisig descriptor test and docs":
(https://github.com/bitcoin/bitcoin/pull/29154#discussion_r1606146880)
I just pushed the changes now 🤦‍♂️
💬 willcl-ark commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1606432601)
I also don't agree? This tool is designed to work with Bitcoin Core's RPC server.
💬 willcl-ark commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1536908349)
Correct, I thought this had been interferring with the manpage section title, but it's not. Reverted.
💬 willcl-ark commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1536908415)
I suppose you mean https://delvingbitcoin.org/t/revisiting-bip21/630 ? Whilst I am in favour of many parts of that discussion, istm that we should probably update this to include the new format, after a new format is specified?

I'm going to take the suggestion to just use `[URI]`, but will still give BIP21 as an example, so folks reading the manpage can find the format of a (the only) valid URI format accepted (for now).
💬 willcl-ark commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1606433867)
Done
💬 willcl-ark commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#issuecomment-2119939115)
> I would keep the old comment for each command even in face of the additional information provided (see review comments). I tend to rely a lot on those to quickly remember what the commands do (but will not oppose taking them out if this is the consensus). I understand that they may break the formatting of the manual pages, so feel free to discard them.

Yes, these extra, "inline" descriptions do get in the way of automatic manpage generation unfortunately.
🚀 glozow merged a pull request: "test: add conflicting topology test case"
(https://github.com/bitcoin/bitcoin/pull/30066)
👍 hebasto approved a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/30120#pullrequestreview-2065686592)
ACK a057869aa3c42457570765966cb66accb2375b13, I've got a zero diff with my local branch, which reproduces the subtree update, and `ecmult gen table size = 86 KiB` in the configure summary.
🤔 glozow reviewed a pull request: "test: remove unneeded `-maxorphantx=1000` settings"
(https://github.com/bitcoin/bitcoin/pull/30133#pullrequestreview-2065702888)
ACK 8950053636cb38ed85fe2d58b53e5d0acb35c390 From skimming the tests, it appears that none of these need a larger `-maxorphantx`.

It looks like the extra `-maxorphantx=1000`s have existed since each of the tests were introduced. My best guess is this was common when package limits were higher and/or tx relay and orphanage worked a bit differently, and then not cleaned up. For example, descendant packages of 1000 were allowed when mempool_packages.py was first added (along with package trackin
...
🚀 glozow merged a pull request: "test: remove unneeded `-maxorphantx=1000` settings"
(https://github.com/bitcoin/bitcoin/pull/30133)
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1606441254)
Removed from this location
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1606440905)
done
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1606441158)
Thanks, taken here and elsewhere it was being used.
👍 jonasnick approved a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/30120#pullrequestreview-2065791232)
utACK