💬 fanquake commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2672502510)
@furszy you might want to review here?

💬 maflcko commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#issuecomment-2672504845)
lgtm ACK ca6aa0b9bee3fdf355b7154a9a686a80977f2a02

🚀 achow101 merged a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302)

💬 darosior commented on pull request "qa: fix an off-by-one in utxo snapshot fuzz target and sanity check its snapshot data":
(https://github.com/bitcoin/bitcoin/pull/31910#discussion_r1964234048)
You are right, i missed how the genesis block's `nBits` does not correspond to the `powLimit` of `7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff` but instead encodes a rounded down target of `7fffff0000000000000000000000000000000000000000000000000000000000`. This means that all values from `7fffff0000000000000000000000000000000000000000000000000000000001` to `7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff` would have the highest bit unset but also fail in the
...

💬 fanquake commented on pull request "test: fix TestShell initialization and reset()":
(https://github.com/bitcoin/bitcoin/pull/31415#issuecomment-2672524118)
@willcl-ark want to take a look here?

✅ fanquake closed a pull request: "refactor: convert ContainsNoNUL to ContainsNUL"
(https://github.com/bitcoin/bitcoin/pull/31301)

💬 fanquake commented on pull request "refactor: convert ContainsNoNUL to ContainsNUL":
(https://github.com/bitcoin/bitcoin/pull/31301#issuecomment-2672526840)
Closing for now.

💬 achow101 commented on pull request "contrib: fix BUILDDIR in gen-bitcoin-conf script and gen-manpages.py":
(https://github.com/bitcoin/bitcoin/pull/31742#issuecomment-2672527071)
ACK 63a8791e15c3ffb44b84ab3e85db62d7997d25fd

🚀 achow101 merged a pull request: "contrib: fix BUILDDIR in gen-bitcoin-conf script and gen-manpages.py"
(https://github.com/bitcoin/bitcoin/pull/31742)

💬 ajtowns commented on pull request "consensus: Consistently encode and decode `OP_1NEGATE` similar to other small ints in Script":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2672536510)
> Concept NACK. This introduces dead code and i don't think the justification for this meets the bar for touching consensus code.

I think it would make more sense to carry this patch as part of the 64 bit arithmetic patchset. If there's consensus to merge/activate 64 bit arithmetic, it could make sense at that point to split this into its own PR to make review easier, but without that motivation, I don't think this change makes much sense on its own. So Concept NACK from me too for now, unles
...

✅ fanquake closed a pull request: "consensus: Consistently encode and decode `OP_1NEGATE` similar to other small ints in Script"
(https://github.com/bitcoin/bitcoin/pull/29589)

💬 fanquake commented on pull request "consensus: Consistently encode and decode `OP_1NEGATE` similar to other small ints in Script":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2672549367)
Closing for now.

💬 1440000bytes commented on pull request "Implement OP_CHECKTEMPLATEVERIFY":
(https://github.com/bitcoin/bitcoin/pull/29280#issuecomment-2672554342)
> Closing for now, given #29198 was closed.

https://github.com/bitcoin/bitcoin/pull/29198 was closed because some reviewers preferred separate pull request for each opcode as it would make it easier to review, test etc.

Related question in a meeting: https://bitcoincore.reviews/bitcoin-inquisition-45#l-112

Related comments: https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1889817551

https://delvingbitcoin.org/t/lnhance-bips-and-implementation/376/12

🚀 fanquake merged a pull request: "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally"
(https://github.com/bitcoin/bitcoin/pull/31662)

💬 fanquake commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#issuecomment-2672564901)
Opened https://github.com/google/oss-fuzz/pull/13081.

💬 fanquake commented on pull request "wallet, rpc: deprecate settxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#issuecomment-2672598280)
@achow101 can you weigh in here? The changes seem to indicate that deprecation was targetted for `29.x`, for removal in `30.x`, but it doesn't seem like this is going into `29.x`.

💬 maflcko commented on pull request "wallet, rpc: deprecate settxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#issuecomment-2672606808)
If people think that deprecation of `settxfee` is appropriate, then I fail to see why `-paytxfee` isn't deprecated at the same time. If anything, any reason for deprecation should be even stronger for that setting, given that it isn't dynamic and thus can't be used with dynamic fee estimation at all, and even less without the rpc.

🤔 glozow reviewed a pull request: "wallet, rpc: deprecate settxfee"
(https://github.com/bitcoin/bitcoin/pull/31278#pullrequestreview-2631078036)
concept ACK, I wish I saw this earlier

💬 glozow commented on pull request "wallet, rpc: deprecate settxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1964293216)
RPC examples for "specify a feerate per transaction" would be helpful

💬 achow101 commented on pull request "wallet, rpc: deprecate settxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#issuecomment-2672613100)
Concept ACK

The startup option should also be deprecated.