💬 fanquake commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2077233577)
> IPC build issue should be fixed in https://github.com/zeromq/libzmq/pull/4672

This was resolved using a different change. Have pulled in that patch, rebased and updated the PR description.

💬 laanwj commented on issue "Add IPv6 pinhole support using UPnP / NAT-PMP":
(https://github.com/bitcoin/bitcoin/issues/17012#issuecomment-2077233941)
FWIW the version of UPnP in depends (2.2.2) does have the Pinhole commands:
```
upnpcommands.h: int * inboundPinholeAllowed);
upnpcommands.h:UPNP_GetOutboundPinholeTimeout(const char * controlURL, const char * servicetype,
upnpcommands.h:UPNP_AddPinhole(const char * controlURL, const char * servicetype,
upnpcommands.h:UPNP_UpdatePinhole(const char * controlURL, const char * servicetype,
upnpcommands.h:UPNP_DeletePinhole(const char * controlURL, const char * servicet
...

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2077236613)
Unfortunately that didn't help, so the mystery remains.

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2077253080)
Just curious. Does the index use [this](https://github.com/bitcoin/bitcoin/blob/0eb1459efa91c6a3bc2964007057cef2f7280a57/src/primitives/transaction.h#L46) to compare outpoints? Does it do (txid, vout) or (txid||vout)?

💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1579527839)
this is from `test_orphan_consensus_failure` causing the exact same parent to be added to `m_recent_rejects_reconsiderable`

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1579531159)
@josibake can I get a helper function along the lines of:

```cpp
bool MaybeSilentPayment(CTransactionRef tx);
```

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2077270747)
@setavenger paging @josibake.

✅ achow101 closed an issue: "Calling `walletprocesspsbt` to sign multisig containing `OP_GREATERTHANOREQUAL` op_code"
(https://github.com/bitcoin/bitcoin/issues/29949)

💬 achow101 commented on issue "Calling `walletprocesspsbt` to sign multisig containing `OP_GREATERTHANOREQUAL` op_code":
(https://github.com/bitcoin/bitcoin/issues/29949#issuecomment-2077274021)
> Is there some other way to sign transaction, through bitcoind, which tries to spend from script which is not compatible with miniscript ?

No

> Is there some problem with `OP_GREATERTHANOREQUAL` which makes it non compatible with Miniscript ?

Yes, it is third party malleable, which makes it much harder to do analysis. For any additional signatures provided after the threshold has been reached, a third party can trivially remove those signatures and the transaction is still valid. Minis
...

💬 sr-gi commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1579549901)
I've done some more digging on this and looks like this is an interference between `test_orphan_consensus_failure` and `test_parent_consensus_failure`. The `low_fee_parent` created in the former is the exact same as the one created in the latter.

💬 maflcko commented on pull request "contrib: rpcauth.py - Add new option (-json) to output text in json format":
(https://github.com/bitcoin/bitcoin/pull/29433#issuecomment-2077307791)
ACK 9adf949d2aa6d199b85295b18c08967395b5570a

💬 sr-gi commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1579554883)
> this is from `test_orphan_consensus_failure` causing the exact same parent to be added to `m_recent_rejects_reconsiderable`
>
> Can we somehow cause chaintip updates during cleanup, without dropping minfee too much? What if cleanup detects that it went down too much, then calls `fill_mempool` again?

lol, I missed this 🤣

💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1579558091)
Adding this block to `cleanup` resets the cache, though I haven't looked at if this messes with minfee too much:

```
# Cause small re-org and tickle AlreadyHaveTx to flush reject filters between tests
chaintip = self.nodes[0].getbestblockhash()
self.nodes[0].invalidateblock(chaintip)
peer_sender = self.nodes[0].add_p2p_connection(P2PInterface())
peer_sender.send_message(msg_tx(CTransaction()))
peer_sender.wait_for_d
...

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2077317678)
I don't think extracting the public keys is the issue as they are pretty standard. So I think it has to be the input_hash. I don't know C++ but I suspect that the linked part does (txid, vout_as_int) and compares the txid and then the vouts but as integers. The transaction in question has 4 outpoints for the lowest txid. And the vouts are 136, 202, 204 and 383. The LEs are 0x88000000, 0xca000000, 0xcc000000 and 0x7f010000. Lexicographically 0x7f010000<0x88000000 but sorted as vout one gets 136 <
...

👍 stickies-v approved a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2022657131)
ACK 992c714451676cee33d3dff49f36329423270c1c

nit: definitely not worth more back-and-forth, but I think (untested) maflcko's [suggestion](https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1578145369) should work with MSVC when explicitly converting iterators to pointers:

<details>
<summary>git diff on 992c714451</summary>

```diff
diff --git a/src/common/url.cpp b/src/common/url.cpp
index ecf88d07ea..c901ecb5dc 100644
--- a/src/common/url.cpp
+++ b/src/common/url.cpp
@@ -14
...

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1579563791)
thanks, I will add them when I have to retouch, if not I will make a small follow-up

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1579565183)
> Can we somehow cause chaintip updates during cleanup, without dropping minfee too much? What if cleanup detects that it went down too much, then calls fill_mempool again?

Hm, `fill_mempool` isn't really built for this (though we can change that). I'll try to see if we can make sure we don't reuse utxos.

💬 alfonsoromanz commented on pull request "depends: pass verbose through to cmake based makefiles":
(https://github.com/bitcoin/bitcoin/pull/29960#issuecomment-2077329585)
Concept ACK

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2077331441)
> I think (untested) maflcko's [suggestion](https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1578145369) should work with MSVC when explicitly converting iterators to pointers

Yes, it's certainly possible and I briefly explored it myself, but it didn't seem like much of an improvement over the current version anymore. And the current version had already received some review at that point, so I decided it would be better to just revert since it was just a style-nit anyway.

🤔 furszy reviewed a pull request: "Disable util::Result copying and assignment"
(https://github.com/bitcoin/bitcoin/pull/29906#pullrequestreview-2022455655)
ACK 6602231564