💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1578354870)
I agree that could be good! Though I think it would add more complexity to this pull, so I would prefer not to do it here - could be a good follow-up though.

Just noting that we do add all fixed seeds to addrman though, so with the connection logic from #27213 we'd eventually still make a connection to the network.

📝 MarnixCroes opened a pull request: "guiconstants: update ORG_DOMAIN to bitcoincore.org"
(https://github.com/bitcoin-core/gui/pull/818)
if desired, can also update `ORG_NAME` to _Bitcoin Core_
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
signific
...

💬 achow101 commented on pull request "wallet, rpc: document and update `sendall` behavior around unconfirmed inputs":
(https://github.com/bitcoin/bitcoin/pull/28979#issuecomment-2075656821)
ACK modulo existing comments.

🤔 ryanofsky reviewed a pull request: "Disable util::Result copying and assignment"
(https://github.com/bitcoin/bitcoin/pull/29906#pullrequestreview-2020422224)
Updated c45cb13b9e4f6eae50ab6dc42acf471921980591 -> 66022315641934bda301d61f009dae9cb3b45da0 ([`pr/saferesult.2`](https://github.com/ryanofsky/bitcoin/commits/pr/saferesult.2) -> [`pr/saferesult.3`](https://github.com/ryanofsky/bitcoin/commits/pr/saferesult.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/saferesult.2..pr/saferesult.3)) adding change to actually disable the copy constructor

💬 ryanofsky commented on pull request "Disable util::Result copying and assignment":
(https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1578217883)
re: [#29906 (comment)](https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1572632242)

> nit: Any reason not to just wrap the args in `std::move`? Seems like the more intuitive approach, and probably slightly more performant?

Actually this turns out not to work because `std::initializer_list` list [does not work](https://old.reddit.com/r/cpp/comments/7a1o7f/why_does_stdinitializer_list_prevents_using/) with move-only types. So unfortunately vector initialization needs to be a little
...

💬 iotamega commented on issue "Manually Banning Peers Results in Crash":
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2075676471)
> I just tried in out under ubuntu, and banning both single and multiple peers worked fine. What do you mean with "extensive testing"? If it happens always, just starting the program and banning a peer should be enough? Or does it happen only sometimes for you?

I'd say it happens perhaps 10% of the time, I can ban 100 peers without an issue before it crops up.

I operate more than fifty nodes at this point and pruning them daily gives me some insight to this issue. It isn't a frequent occur
...

💬 Sjors commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1578401700)
After a full day my virtual box also managed to complete `brew install llvm` without needing a version :-) Will update the PR tomorrow.

💬 laanwj commented on pull request "guiconstants: update ORG_DOMAIN to bitcoincore.org":
(https://github.com/bitcoin-core/gui/pull/818#issuecomment-2075690269)
> if desired, can also update ORG_NAME to Bitcoin Core

The reason that this was never done is because it changes where the Qt settings files are stored. For example this is `~/.config/<ORG_NAME>/<APP_NAME>`.

i'm not sure what actually is the effect of changing the domain, but this shouldn't be done without checking.

💬 hebasto commented on pull request "guiconstants: update ORG_DOMAIN to bitcoincore.org":
(https://github.com/bitcoin-core/gui/pull/818#issuecomment-2075723918)
> if desired, can also update `ORG_NAME` to _Bitcoin Core_

Are there any issues with the current code? If not, then this PR lacks motivation.

📝 kristapsk opened a pull request: "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`"
(https://github.com/bitcoin/bitcoin/pull/29954)
Other node relay settings like `fullrbf` and `minrelaytxfee` are already returned, makes sense to add these two too.

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1578496251)
The snake case is already applied in the scripted diff but I applied the clang format suggestions

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1578496354)
neat, I applied this change

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1578496508)
Changed and added a test for this case as well

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2075804791)
> `unsigned` may be better

No idea what this means ;) but I addressed the feedback, thanks a lot!

👍 kristapsk approved a pull request: "doc: Bash is needed in gen_id and is not installed on FreeBSD by default"
(https://github.com/bitcoin/bitcoin/pull/29953#pullrequestreview-2020936557)
ACK 9381052194a78024b3994cc6ad906858c477b88f

💬 TheCharlatan commented on pull request "kernel: De-globalize fReindex":
(https://github.com/bitcoin/bitcoin/pull/29817#discussion_r1578521404)
Good catch! To be clear from my comments before, it is not that big of an issue if the indexes are wiped again. AFAICT the operations executed while the `reindexing` atomic is true do not issue any validation signals, so they don't have an effect on the indexes. However recreating them is redundant and I don't think it is particularly useful. I think it would also be good to make the behavior between the coins and the other indexes consistent, so I'll take your suggestion here.

💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#discussion_r1578530492)
Done.

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1578533717)
Ugh, looks like MSVC has a problem with this, I am reverting it for now (see https://github.com/bitcoin/bitcoin/actions/runs/8823024109/job/24222527982?pr=29904)

👍 alfonsoromanz approved a pull request: "lint: scripted-diff verification also requires GNU grep"
(https://github.com/bitcoin/bitcoin/pull/29689#pullrequestreview-2021071819)
Tested ACK 3bf4f8db669e1e274ce2633cf84add2938b9914b

I was using incompatible versions for both sed and grep, so I was able to verify both error messages. After installing the GNU versions I don't get any of the error messages.

⚠️ mouhand-a opened an issue: "Use custom elements on TestPanelContainerItemElement"
(https://github.com/bitcoin/bitcoin/issues/29955)
Use custom elements on TestPanelContainerItemElement

_Originally posted by @sadick254 in https://github.com/atom/atom/pull/22769_

💬 mouhand-a commented on issue "Use custom elements on TestPanelContainerItemElement":
(https://github.com/bitcoin/bitcoin/issues/29955#issuecomment-2076010481)
[‎⁨تصدير سِجِل الطلب-2024-04-20 09_25_30⁩ 2.xlsx](https://github.com/bitcoin/bitcoin/files/15101794/-2024-04-20.09_25_30.2.xlsx)
@
```c++
```