👍 tdb3 approved a pull request: "Revert "test: p2p: check that connecting to ourself leads to disconnect""
(https://github.com/bitcoin/bitcoin/pull/30374#pullrequestreview-2152339467)
ACK 9ec2c53701a391629b55aeb2804e8060d2c453a4
Glad this test found a bug!
Makes sense to revert this out until the issue is fixed.
Ran the test locally.

📝 crypto-perry opened a pull request: "Qrwit nonbc"
(https://github.com/bitcoin/bitcoin/pull/30375)
This is a draft implementation of the scheme described in this (research paper)[https://royalsocietypublishing.org/doi/10.1098/rsos.180410] which introduces a commit delay reveal scheme for safely transitioning ECDSA secured UTXOs to a quantum resistant signature scheme.

👋 crypto-perry's pull request is ready for review: "Qrwit nonbc"
(https://github.com/bitcoin/bitcoin/pull/30375)

📝 crypto-perry converted_to_draft a pull request: "QRWit Implementation Proposal"
(https://github.com/bitcoin/bitcoin/pull/30375)
This is a draft implementation of the scheme described in this [research paper](https://royalsocietypublishing.org/doi/10.1098/rsos.180410) which introduces a commit delay reveal scheme for safely transitioning ECDSA secured UTXOs to a quantum resistant signature scheme.

👍 tdb3 approved a pull request: "#27307 follow-up: update mempool conflict tests + docs"
(https://github.com/bitcoin/bitcoin/pull/30365#pullrequestreview-2152389534)
ACK 7d55796c530f891493302059c6200d4a606c1ca9
Good follow up. Ran `wallet_conflicts` locally without issue.

✅ achow101 closed a pull request: "QRWit Implementation Proposal"
(https://github.com/bitcoin/bitcoin/pull/30375)

💬 achow101 commented on pull request "QRWit Implementation Proposal":
(https://github.com/bitcoin/bitcoin/pull/30375#issuecomment-2201418712)
Thank you for your contribution, however consensus changes need to be discussed on the mailing list first, and preferably have a BIP, before implementation here should begin being discussed.

💬 tdb3 commented on pull request "test: Added coverage to Block not found error using gettxoutsetinfo":
(https://github.com/bitcoin/bitcoin/pull/30340#issuecomment-2201432813)
> Hmm, not sure if that is expected behavior or not for gettxoutsetinfo. If the block is invalidated, seems counterintuitive to consider it as a hash_or_height for calculating txout set info. Not sure what piece is missing here, but will dig more.

Following up:

- Generated a chain of length 101.
- Invalidated block 2.
- `gettxoutsetinfo` with no arguments returns info for the chain (of height 1) as expected.
- `gettxoutsetinfo` with a block height >= 2 returns the expected error mes
...

🤔 tdb3 reviewed a pull request: "net: Allow -proxy=[::1] on nodes with IPV6 lo only"
(https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2152531616)
Concept ACK
Good find. Left a couple of comments.

💬 tdb3 commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1661731868)
Instead of using 0 as the first argument, might be better to remove only `AI_ADDRCONFIG`, e.g. something like
```c++
callgetaddrinfo(ai_flags & (~AI_ADDRCONFIG), true);
```

💬 tdb3 commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1661728878)
This makes sense to me as well and it might make for a significantly smaller code change.

👍 maflcko approved a pull request: "Revert "test: p2p: check that connecting to ourself leads to disconnect""
(https://github.com/bitcoin/bitcoin/pull/30374#pullrequestreview-2152794183)
lgtm

💬 maflcko commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2202071660)
The test is run inside a docker,

https://github.com/bitcoin/bitcoin/blob/ca76d180d2a30d063154be8101bce64e308866ac/ci/test/02_run_container.sh#L50-L59

, so I think it needs to be something inside the docker container. Also, I am not aware of a process inside the CI env that would echo back what it got.

💬 maflcko commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#discussion_r1661927436)
Thanks, fixed typo.

💬 maflcko commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2202184017)
I presume, if you wanted to speed up `listunspent` in this case of a bip-32 base58 encoded descriptor string, a higher speed-up could be achieved by caching the descriptor string results, because the expectation is that there are generally only a few (or one) per wallet. But I am not sure how easy that is, or whether it is worth it.

💬 paplorinc commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2202194543)
I was following the benchmarks to decide what to work on, but since you've mentioned this is an important usecase, I measured it as well.
Is there anything else you'd like me to measure or change in this pull request?

💬 maflcko commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2202249394)
I don't think I said that "this is an important usecase". I simply said that the reason for the improvement in https://github.com/bitcoin/bitcoin/pull/7656#issue-139438690 was `listunspent`. That pull request was created before bech32(m) addresses and descriptor wallets existed. Considering that bech32 is the default address type and that descriptor wallets are the default as well, the only remaining use of base58 should be in the bip-32 descriptor string. My guess is that, if it was important t
...

💬 Sjors commented on pull request "doc: external-signer, example 'createwallet' RPC call requires eight argument":
(https://github.com/bitcoin/bitcoin/pull/30354#discussion_r1662079837)
You should be able to drop most of these named arguments, except `wallet_name` and `external_signer`. Most are already defaults, except `avoid_reuse` but that's not relevant to this tutorial.

💬 Sjors commented on pull request "doc: external-signer, example 'createwallet' RPC call requires eight argument":
(https://github.com/bitcoin/bitcoin/pull/30354#issuecomment-2202306333)
Do you happen to know which pull request added a new positional argument in the middle? Or was this instruction always broken? cc @achow101

🚀 fanquake merged a pull request: "Revert "test: p2p: check that connecting to ourself leads to disconnect""
(https://github.com/bitcoin/bitcoin/pull/30374)