💬 hebasto commented on pull request "depends: qrencode 4.1.1":
(https://github.com/bitcoin/bitcoin/pull/27312#issuecomment-1481028288)
Concept ACK.

> Note that upstream, libqrencode is effectively unmaintained. No code changes for > 2 years. No responses to issues/PRs. Seems like the author has mostly dropped off of GitHub as well.

Yeah, this situation is a real concern, unfortunately.

💬 darosior commented on pull request "wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction":
(https://github.com/bitcoin/bitcoin/pull/25273#discussion_r1146013592)
Making sure i got this one right. `use_anti_fee_sniping` may only be `false` if this is called from `FundTransaction`. And `FundTransaction` diregards the value of the `nLockTime` that may be set here anyways.

💬 darosior commented on pull request "wallet: BIP 326 sequence based anti-fee-snipe for taproot inputs":
(https://github.com/bitcoin/bitcoin/pull/24128#issuecomment-1481033569)
People interested in seeing this move forward may be interested in giving https://github.com/bitcoin/bitcoin/pull/25273 a look. It was recently rebased and reviewed twice.

💬 TheCharlatan commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1146057675)
The `_POSIX_C_SOURCE` should be moved here too.

💬 jamesob commented on pull request "fix: contrib: allow multi-sig binary verification":
(https://github.com/bitcoin/bitcoin/pull/23020#issuecomment-1481055451)
I probably won't have time in the next few days to attend to this, so feel free to open a replacement PR with @achow101's branch if I'm holding anything up.

💬 theStack commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1146099889)
@josibake: yes, agree that considering on what we need in `address_to_scriptpubkey` returning `(None, None)` is better than asserting. LGTM!

👍 theStack approved a pull request: "test: Support decoding segwit address in address_to_scriptpubkey()"
(https://github.com/bitcoin/bitcoin/pull/27269)
ACK d178082996dc3000f42816f89afcf3fa4d31e159 ✔️

> I'd love to see a follow-up PR targeting areas where we are a) only testing base58 addresses but should be testing bech32 as well and b) anywhere we are making a getaddressinfo RPC call to get the spk to instead use address_to_scriptpubkey

👍 If anyone wants to work on this, feel free to tag me as reviewer.

💬 instagibbs commented on pull request "rpc: Add submit option to generateblock":
(https://github.com/bitcoin/bitcoin/pull/18933#issuecomment-1481098679)
ACK fa18504d5767a40dc9827fb081633219bf251001

💬 furszy commented on pull request "wallet: return error msg for "too-long-mempool-chain"":
(https://github.com/bitcoin/bitcoin/pull/24845#discussion_r1146122336)
Nop. Could also have zero conf change (or send-to-self) if the wallet's `m_spend_zero_conf_change` flag is disabled. In other words, unconfirmed coins accepted by the mempool limits (ancestors/descendants < 25).

💬 stickies-v commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1481163868)
> It would be better to have two separate commits - one with the non-functional optimization and one with the fix.

Agreed, that could probably be improved.

> Idea: I think it would be better to disallow the creation of HTTPRequest if the request is invalid/unparsable.

This was the first approach I considered, but I think it's better not to. A request with an invalid URI is still a request, and thus it should be representable. Moving the URI validation to the constructor feels like a sho
...

👍 vasild approved a pull request: "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg"
(https://github.com/bitcoin/bitcoin/pull/27257)
ACK 3566aa7d495bb783bbd135726238d9f2a9e9f80e

🚀 fanquake merged a pull request: "ci: Use clang-15 in "tidy" task"
(https://github.com/bitcoin/bitcoin/pull/27311)

💬 beirut-boop commented on pull request "wallet: unify “allow/block other inputs“ concept":
(https://github.com/bitcoin/bitcoin/pull/25118#issuecomment-1481195546)
Noticing some new irrational behavior in a forked coin control and this is the most significant work on coin control pulled from upstream. @furszy any change you could glance over the screenshots and assess the probability of this being an upstream issue?

https://github.com/syscoin/syscoin/issues/524

💬 furszy commented on pull request "reduce cs_main scope, guard block index 'nFile' under a local mutex":
(https://github.com/bitcoin/bitcoin/pull/27006#discussion_r1146202462)
> Shouldn't these be fetched atomically? Maybe there should be a getter that returns all three values, similar to how there's a setter (`CBlockIndex::SetFileData`) than sets all three atomically.

@LarryRuane I didn't add it merely to not introduce that many changes but could be done too. The members set is guarded by `cs_main` (for now) so there is no possible race.

> Starting to feel like a struct for those 3 members would be appropriate

Why exactly?. So far in the code, we either want
...

👍 theStack approved a pull request: "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg"
(https://github.com/bitcoin/bitcoin/pull/27257)
re-ACK 3566aa7d495bb783bbd135726238d9f2a9e9f80e

👍 brunoerg approved a pull request: "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg"
(https://github.com/bitcoin/bitcoin/pull/27257)
re-ACK 3566aa7d495bb783bbd135726238d9f2a9e9f80e

💬 fanquake commented on pull request "wallet: unify “allow/block other inputs“ concept":
(https://github.com/bitcoin/bitcoin/pull/25118#issuecomment-1481215346)
Sorry, this is not a support forum for forks of this project.

🚀 fanquake merged a pull request: "rpc: Add submit option to generateblock"
(https://github.com/bitcoin/bitcoin/pull/18933)

💬 fanquake commented on pull request "test: getblock on header throws":
(https://github.com/bitcoin/bitcoin/pull/27237#issuecomment-1481216289)
#18933 is now in.

💬 dergoegge commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1146221073)
fwiw, I would prefer either one of these two options over what is currently in this PR:
> * Use findBlock from the node interface. The overhead should only be another map lookup by the block hash? You can ask Ryan to confirm.
> * Pass down a blockman ref into the zmq stuff (massive change)

I implemented the second option here: https://github.com/dergoegge/bitcoin/commit/a17844fe3e3c97c0eb5906536b96d1ce634b790c feel free to pick that, it's not to crazy of a change imo (haven't tested that bu
...