💬 furszy commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1247950832)
done

🤔 furszy reviewed a pull request: "wallet: don't duplicate change output if already exist"
(https://github.com/bitcoin/bitcoin/pull/27601#pullrequestreview-1507352028)
Updated per feedback, thanks @Xekyo.

Added test coverage verifying that the same fee is paid when the wallet creates a change output automatically vs when the user provides the change output and the wallet just use it as recipient for the fee surplus.

👍 fanquake approved a pull request: "script, test: python typing and linter updates"
(https://github.com/bitcoin/bitcoin/pull/28009#pullrequestreview-1507399382)
ACK 6c97757a480b6e71a0750330d69ff18ac7cc6127

🚀 fanquake merged a pull request: "script, test: python typing and linter updates"
(https://github.com/bitcoin/bitcoin/pull/28009)

💬 sipa commented on pull request "Miniscript: always treat unsatisfiable scripts as insane":
(https://github.com/bitcoin/bitcoin/pull/27997#issuecomment-1614821612)
The requirement for a signature actually stems from the same question, though it's not as exact.

If a script can be satisfied without signatures, it means there may be no signatures at all in the whole transaction or input, and thus nothing that actually commits to the nLockTime or nSequence values respectively, and so an attacker could just modify those values without invalidating anything.

💬 sr-gi commented on pull request "p2p: gives seednode priority over dnsseed if both are provided":
(https://github.com/bitcoin/bitcoin/pull/28016#discussion_r1248003587)
Should be fixed in ac7ce24

🚀 fanquake merged a pull request: "contrib: add macOS test for fixup_chains usage"
(https://github.com/bitcoin/bitcoin/pull/27999)

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1248008111)
The record data should just be arbitrary byte strings so they don't have an endianness.

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1248013942)
Done

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1248014091)
Done as suggested

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1248014465)
Removed. I think originally I though I could put the overflow page data into this field, but I didn't end up doing it that way.

💬 Sjors commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1614851497)
I get the same hashes.

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1248024010)
Not anymore, there was an `Assume()` somewhere in `HTTPRequest::GetQueryParameter` checking for `m_uri_parsed` at some point. Thanks for checking, I'll removed it.

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1248057852)
IIRC I did this to avoid issues with old wallets that the wallet loading logic would do automatic upgrading of which expects writing to not fail.

👋 fanquake's pull request is ready for review: "[25.x] Parallel compact block downloads"
(https://github.com/bitcoin/bitcoin/pull/27752)

💬 fanquake commented on pull request "build: LLVM 16 & LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1614912597)
Rebased past #27999. Now that we require `fixup_chains`, we also require at least LLVM 16+ (which doesn't yet exist in Guix). Hopefully we'll have the infra to construct it ourselves shortly: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=64100 (which will mean a more recent time-machine bump). https://reviews.llvm.org/D151926 / https://reviews.llvm.org/D151944 has also landed in LLVM (17.x), so we can backport if needed.

In the meantime, I've rebased this, and updated depends to use LLVM 16.0
...

💬 fanquake commented on pull request "guix: Use LTO to build releases":
(https://github.com/bitcoin/bitcoin/pull/25391#issuecomment-1614914758)
Rebased this on top of #27897. Dropped the commit to backport a patch to GCC 10.

💬 fanquake commented on pull request "build: LLVM 16 & LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1614924588)
Also added a commit to drop explicit `-bind_at_load` usage from configure for now. Cleaned up the PR description and instructions. Note that Guix build are basically expected to fail at this point. However if anyone would like to test, cross-compilation can be done using depends.

💬 achow101 commented on pull request "addrman: select addresses by network follow-up":
(https://github.com/bitcoin/bitcoin/pull/27745#issuecomment-1614950818)
ACK cd8ef5b3e66b3f766c9c883259b5feb44540d7df

Checked that the changes here resolve the remaining review comments on #27214

💬 mzumsande commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1248094492)
I think I also like the `src/node` option most, but it's ok for me to keep in init for now and move that later.