👍 theStack approved a pull request: "test: add assumeutxo wallet test"
(https://github.com/bitcoin/bitcoin/pull/28838#pullrequestreview-1813788224)
Code-review ACK 997b9a73e5166b4244f7c5b4fe144d524f3005f4

Also ran the test locally.

💬 theStack commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#discussion_r1447730295)
nit: looks like an unintended newline here?
```suggestion
assert_equal(n.getblockchaininfo()["headers"], SNAPSHOT_BASE_HEIGHT)
```

⚠️ glozow opened an issue: "`-maxtxfee` is used as a fee and a feerate"
(https://github.com/bitcoin/bitcoin/issues/29220)
### Please describe the feature you'd like to see added.

The `-maxtxfee` (which is put into `m_default_max_tx_fee` is documented as "the maximum total fees to use in a single wallet transaction"
https://github.com/bitcoin/bitcoin/blob/632a2bb731804dffe52bd4cbd90bfee352d25ede/src/wallet/init.cpp#L63

This seems true in many instances when we look at the code:
https://github.com/bitcoin/bitcoin/blob/632a2bb731804dffe52bd4cbd90bfee352d25ede/src/wallet/spend.cpp#L1293-L1295
https://github.com/
...

💬 theuni commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1885372563)
> Would still like to see them PR'd separately, but I cherry-picked the two relevant commits, dropped `native_cmake`, and have dropped anything libtool related here.

It's been quite a while since I worked on those, I don't remember if they were completely merge-ready (I suspect windows cross might be broken for ex.). Playing with them locally now.

💬 achow101 commented on pull request "Add Gutter Guard Selector":
(https://github.com/bitcoin/bitcoin/pull/28977#issuecomment-1885378443)
Is there a benefit to using SRD here over something oldest first? We are already doing SRD so it might be better to use a different underlying strategy. Having a selection based on oldest first would also be useful for consuming negative EV UTXOs at very low feerates, e.g. less than 3 s/vb.

It also seems like having the limit be inversely scaled by feerate would be better than the fixed limit. That way, as the feerates increase, we get less consolidatory instead of current behavior where it s
...

📝 xonx4l opened a pull request: "FIX:When opening or autoloading wallets there should be clear messages about rescanning in progress and wallets' names."
(https://github.com/bitcoin-core/gui/pull/786)
issue-: #259

The "Opening Wallet" popup would now show "Opening Wallet: " instead of just "Opening Wallet". This identifies the specific wallet.

The "Rescanning" popup would show "Rescanning: " or "Rescanning - 0%" if a progress bar is added. Again, this identifies the wallet.

The main window title would include the wallet name after being opened/loaded thanks to the change in WalletView::showNormalIfMinimized().

Other dialogs like Send/Receive coins would also show the wallet name i
...

💬 m3dwards commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1885402883)
I've reviewed the changes and they appear correct to me. libbitcoinconsensus will continue to not get SSE4 ASM (nor any other optimisations) and libbitcoinkernel (external?) will now get all of them (if available and enabled by ./configure).

I think `DISABLE_OPTIMIZED_SHA256` is much clearer macro name. I find the `USE_ASM` symbol / macro and `asm` feature in autoconf confusing.

If I've read it correctly, the `ASM` feature in autoconf enables not only the assembly optimisations but also st
...

🤔 pablomartin4btc reviewed a pull request: "test: detect OS in functional tests consistently using `platform.system()`"
(https://github.com/bitcoin/bitcoin/pull/29034#pullrequestreview-1813903714)
tACK 878d914777a03a04ecb84217152e8b7fd73a5062

Tested on both Linux (Ubuntu 22.04 & WSL) and Windows.

As @kevkevinpal [pointed it out](https://github.com/bitcoin/bitcoin/pull/29034#issuecomment-1847703106), there's still usage of `os.name` in `test_framework.py`, not sure if you want to cover that as well.

💬 achow101 commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#issuecomment-1885446023)
ACK e60fc7d5d34f23cccbff6e4f5f3d716fa8dad50c

I also prefer the use of macros/functions for the different categories as it is much less verbose than `LogPrintLevel(CAT, LEVEL, ...)`. This pattern is also similar to what many other logging frameworks do, e.g. python's `logging` module.

💬 achow101 commented on pull request "test: wallet migration, add coverage for tx extra data":
(https://github.com/bitcoin/bitcoin/pull/29204#issuecomment-1885450376)
ACK 016cc807f77a9128d430a0df1edd133521628a33

💬 theuni commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1885470222)
> > Would still like to see them PR'd separately, but I cherry-picked the two relevant commits, dropped `native_cmake`, and have dropped anything libtool related here.
>
> It's been quite a while since I worked on those, I don't remember if they were completely merge-ready (I suspect windows cross might be broken for ex.). Playing with them locally now.

Here's a cleaned up version: https://github.com/theuni/bitcoin/commits/depends-no-libtool/
They needed an update after 63c0c4ff10b2f6ed85
...

🚀 achow101 merged a pull request: "logging: Simplify API for level based logging"
(https://github.com/bitcoin/bitcoin/pull/28318)

💬 achow101 commented on pull request "fuzz: fix `connman` initialization":
(https://github.com/bitcoin/bitcoin/pull/29211#issuecomment-1885493837)
ACK e84dc36733687fffe08ae4621b571cc66afc047d

🚀 achow101 merged a pull request: "fuzz: fix `connman` initialization"
(https://github.com/bitcoin/bitcoin/pull/29211)

💬 theuni commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1885576324)
Blah, it looks like the win32 filename is borked with cmake: `liblibminiupnpc.a`

Want to punt the libtool removal to a follow-up PR to avoid slowing this one down?

🚀 achow101 merged a pull request: "test: wallet migration, add coverage for tx extra data"
(https://github.com/bitcoin/bitcoin/pull/29204)

✅ achow101 closed a pull request: "Allow configuring target block time for a signet"
(https://github.com/bitcoin/bitcoin/pull/27446)

💬 achow101 commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1885622170)
Closing as there is continued disagreement over the approach and no further changes have been made to this PR. This seems to be at a stalemate and unlikely to go anywhere.

💬 glozow commented on issue "`-maxtxfee` is used as a fee and a feerate":
(https://github.com/bitcoin/bitcoin/issues/29220#issuecomment-1885647767)
This was partially motivated by seeing #29217, as I thought "don't we already have a max feerate?" and realized it was a max absolute fee.

📝 Christewart opened a pull request: "Implement OP_ADD64, OP_SUB64, OP_MUL64, OP_DIV64, OP_LESSTHAN64, OP_L…"
(https://github.com/bitcoin/bitcoin/pull/29221)
This PR adds 64 bit arithmetic op codes to the Script interpreter.


This PR corresponds with this BIP proposal: https://github.com/bitcoin/bips/pull/1538

This work is heavily borrowed from the elements implementation of 64 bit op codes: https://github.com/ElementsProject/elements/pull/1020