💬 andrewtoth commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r1579492975)
To avoid any confusion

```suggestion
{RPCResult::Type::BOOL, "permitbaremultisig", "True if the mempool accepts transactions with bare multisig outputs"},
```

💬 laanwj commented on issue "Add IPv6 pinhole support using UPnP / NAT-PMP":
(https://github.com/bitcoin/bitcoin/issues/17012#issuecomment-2077224493)
Indeed, UPnP has an explicit command `AddPinhole` (https://upnp.org/specs/gw/UPnP-gw-WANIPv6FirewallControl-v1-Service.pdf) that's seperate from `Add(Any)PortMapping`.

The PCP (succssor to NAT-PMP) RFC (https://datatracker.ietf.org/doc/html/rfc6887) doesn't say anything about pinholes (although they do mention "Simple firewall control"). Though i would guess the MAP opcode allows that, and more, it just leaves it up to the router how the external to internal mapping should be.

It's kind of
...

💬 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