💬 Empact commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#issuecomment-1664679951)
Concept ACK

💬 ariard commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1664750667)
> I'm not aware of any second layer protocols that are improved by having full-rbf more broadly available.

@TheBlueMatt See https://lists.linuxfoundation.org/pipermail/lightning-dev/2021-May/003033.html and https://lists.linuxfoundation.org/pipermail/lightning-dev/2023-May/003920.html

💬 brunoerg commented on pull request "rpc, p2p: allow `disconnectnode` with subnet":
(https://github.com/bitcoin/bitcoin/pull/26576#discussion_r1283810102)
I don't think it's really a bug. It seems like this was the expected behavior (due to examples and docs - e.g. having to specify port). However, I also think it should be able to disconnect all nodes from an IP. I'm gonna address it.

💬 1ma commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664757776)
> My goal is not "non-bitcoin stuff". My specific challenge is that Lightning and Nostr use public keys in a persistent way that are not rotatable, which is a bad cryptographic practice. By committing to a rotated key and some other cryptographic in advance in L1, they could be. I believe this could be as small as a signed hash and a little metadata.

@ChristopherA can you clarify why it is imperative for this use case to store that data on the Bitcoin blockchain?

To give you some insight i
...

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283872989)
oh interesting. do I understand this correctly: the `LOCK_RETURNED` annotation tells thread safety analysis that this function will return `m_nodes_mutex`, even though it's just an empty function, which lets the lambda use `EXCLUSIVE_LOCKS_REQUIRED`. this works because it doesn't need to actually return the mutex there, it just needs to promise the compiler it will be there, which is handled elsewhere.

💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#issuecomment-1664868727)
I haven't been able to cause a crash with the connection-based code. Given that the connection-based code is different than the crashing code here, do you want to open a new PR with the connection-based code @stickies-v ?

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283935267)
hm, thinking about this more... I believe this workaround diminishes some of the value of the compile-time notation. for example, if `ForEachNode` no longer acquired `m_nodes_mutex`, then the compiler wouldn't be able to flag the issue, but the `AssertLockHeld` would catch it at runtime.

the advantage is that this one weak check allows to maintain the compiler checks at the other levels (of functions and members that are protected), and we remove the recursive locking of the mutex.

this
...

📝 fanquake reopened a pull request: "p2p: diversity network outbounds follow-ups"
(https://github.com/bitcoin/bitcoin/pull/28189)
release notes for #27213, will rebase after it gets merge

💬 BenWestgate 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#issuecomment-1664944819)
I have a project that syncs bitcoin core on an external drive and this would really help a lot of people testing it with smaller flash drives and higher RAM. So much so that I've made scripts to do manual pruning (back to 550mib) only when disk space is getting low to emulate this.

💬 RobinLinus commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664950834)
> I can now put it into an inscription-style transaction and even get a 4x discount

This is not true. Having to use two transactions requires at least 150 bytes of overhead. Removing the spam protection for op_return outputs effectively results in a discount for small inscriptions.

Furthermore, you're saying yourself that you want this change because it makes it easier for you to use the chain for contentious non-bitcoin use cases such as inscribing Nostr keys. However, the reason why you
...

💬 MarcoFalke commented on pull request "scripted-diff: Specify Python major version explicitly on Windows":
(https://github.com/bitcoin/bitcoin/pull/28213#issuecomment-1665034791)
lgtm ACK 6a7686b44618eabd2f8ee9f1d357cfeb1bce6d91

💬 eriknylund commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1283998840)
I much appreciate the input @paplorinc, this was the syntax I was looking for 🙏 I saw the first version of your message and immediately thought about doing the other part ([0, 1, 999, 1000]). You read my mind, this makes very much sense! Also, doing [1, 999] would be quite time consuming because each run lasts about a second so in total it would need 1000 seconds to complete. I'll update the PR to support this version.

💬 MarcoFalke commented on pull request "refactor: use string_view for passing string literals to Parse{Hash,Hex}":
(https://github.com/bitcoin/bitcoin/pull/28172#issuecomment-1665040232)
lgtm ACK b94581a8acecafe5ffff15692485a5572d1db57c

💬 ajtowns commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1284001844)
Because `ForEachNode` takes a `std::function`, the thread safety annotations on the lambda are dropped, so the compiler can't verify that `ForEachNode` has actually taken all the locks that the lambda expects to have held. Problem is that doing it any other way (eg templates) requires `ForEachNode` to be annotated to require any extra locks that the lambda requires that were already held by the caller.

💬 MarcoFalke commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1665042245)
Just for context: For testing I used `-datacarriersize=9999999` and then put the following nulldata into the `mempool.dat`: `raw(6a4458354f2150254041505b345c505a58353428505e2937434329377d2445494341522d5354414e444152442d414e544956495255532d544553542d46494c452124482b482a)`. On current master I got several hits by different virus scanners. On this pull, all virus scanners were green and didn't put the `mempool.dat` in the quarantine.

💬 eriknylund commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1284003587)
@Sjors Given my basic understanding of output descriptors, I'd much appreciate your thoughts on this block of code, to make sure it meets the issue criterias and if this is a good way to test it.

💬 BrandonOdiwuor commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1284001548)
To improve maintainability and support forward compatibility, consider abstracting the handling of different versions into a separate helper function like ‘[evalCheckSig](https://github.com/bitcoin/bitcoin/blob/a4ca4975880c4f870c6047065c70610af2529e74/src/script/interpreter.cpp#L391)’ does. This approach will allow for easier modification and extension for new versions of Silent addresses, and would also be more clear how version handling is done

💬 paplorinc commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1284067105)
isn't it weird that the error message states "Cannot have 1000 keys" when k is not 1000?

💬 MarcoFalke commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#issuecomment-1665127576)
The linter is failing. Also, please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

💬 MarcoFalke commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1284072810)
Thanks, fixed CI