💬 ekzyis commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1520485069)
utACK d8434da3c14ed6723d86ef2cd266008d366e1413

> **If we want to allow for more configurable signet consensus rules**, then I think we need to do it in a more robust way rather than ad-hoc adding options that all users must remember to set if they want to use a particular signet. As these are consensus parameters, forgetting to set the option correctly will result in eventual consensus failure which is generally only resolved by nuking the data directory. This may garner additional and unnece
...

⚠️ darosior opened an issue: "wallet coin selection: don't mixup coins with absolute timelocks of different types"
(https://github.com/bitcoin/bitcoin/issues/27526)
We now support spending from coins containing `CHECKLOCKTIMEVERIFY` instructions. `CLTV` is a check on the value of the `nLockTime` field of the spending transaction. There are two types of timelocks possible: by timestamp and by block. Both are incompatible, the `nLockTime` field cannot satisfy both a timestamp and a block timelock.

The coin selection is at the moment completely unaware of timelocks. If we have two coins for two different descriptors, one with a timestamp absolute timelock a
...

⚠️ darosior opened an issue: "wallet coin selection: be aware of timelocks and allow commands to set an optional block height when selecting coins"
(https://github.com/bitcoin/bitcoin/issues/27527)
The wallet should not select a yet-unavailable coin to fund a transaction. We could use the latest block height by default and allow the user to set one when calling for instance `walletcreatefundedpsbt`, to effectively tell the wallet "select me coins such as they'll be available after block height `N`).

Also maybe the wallet should set the right values for `nSequence` / `nLockTime` if it spends timelocked coins? Curious what people think.

cc @ishaanam since we discussed it together.

💬 benthecarman commented on issue "-fallbackfee should apply to estimatesmartfee":
(https://github.com/bitcoin/bitcoin/issues/27415#issuecomment-1520674421)
+1 this would be nice, applications like CLN use this rpc and makes it hard to test in regtest / signet environments

📝 theStack opened a pull request: "test: fix `feature_addrman.py` on big-endian systems"
(https://github.com/bitcoin/bitcoin/pull/27529)
The test `feature_addrman.py` currently serializes the addrdb without specifying endianness for `int`s, so the machine's native byte order is used (see https://docs.python.org/3/library/struct.html#byte-order-size-and-alignment) and the generated `peers.dat` would be invalid on big-endian systems (our internal (de)serializers always use little-endian, see `ser_{read,write}data32`). Fix this by explicitly specifying little-endian serialization via the `<` character in `struct.pack(...)`.

This
...

💬 MarcoFalke commented on pull request "test: fix `feature_addrman.py` on big-endian systems":
(https://github.com/bitcoin/bitcoin/pull/27529#issuecomment-1521387810)
There is `ci/test/00_setup_env_s390x.sh`, but I guess it runs qemu-user, not qemu-system, so the python part will run on the host endianness. I wonder how hard it would be to spin up qemu-system in the CI env?

💬 achow101 commented on pull request "p2p: skip netgroup diversity follow-up":
(https://github.com/bitcoin/bitcoin/pull/27467#issuecomment-1521500955)
While renaming this variable is technically correct, I don't think it's useful to have these kind of PRs. Although it is small and trivial, it does take away time from review, has an effect on git blaming, and possibly conflicts with other PRs in this area that do introduce meaningful changes.

💬 jonatack commented on pull request "p2p: skip netgroup diversity follow-up":
(https://github.com/bitcoin/bitcoin/pull/27467#issuecomment-1521526214)
@achow101 The code comment needs to be fixed as it is now incorrect, and the naming is now misleading. I think the change is clearly justified here.

💬 MarcoFalke commented on pull request "Fix unsigned integer overflows in interpreter":
(https://github.com/bitcoin/bitcoin/pull/24214#issuecomment-1521549361)
Closing for now due to inactivity. I still think this is useful (see OP) and we should re-evaluate it the next time the interpreter.cpp is changed to catch newly introduced unintended unsigned integer overflow.

✅ MarcoFalke closed a pull request: "Fix unsigned integer overflows in interpreter"
(https://github.com/bitcoin/bitcoin/pull/24214)

🚀 fanquake merged a pull request: "test: simplify uint256 (de)serialization routines"
(https://github.com/bitcoin/bitcoin/pull/27516)

💬 jonatack commented on pull request "refactor: move index class members from protected to private":
(https://github.com/bitcoin/bitcoin/pull/24150#issuecomment-1521605660)
Closing temporarily so that I can re-open it -- am still interested to work on this.

✅ jonatack closed a pull request: "refactor: move index class members from protected to private"
(https://github.com/bitcoin/bitcoin/pull/24150)

💬 jonatack commented on pull request "Add cs_main annotation to WriteBatchSync(), drop lock in CDiskBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/24199#issuecomment-1521606641)
Closing temporarily so that I can re-open it -- am still interested to work on this.

✅ jonatack closed a pull request: "Add cs_main annotation to WriteBatchSync(), drop lock in CDiskBlockIndex"
(https://github.com/bitcoin/bitcoin/pull/24199)

💬 jonatack commented on pull request "Severity-based logging -- parent PR":
(https://github.com/bitcoin/bitcoin/pull/25203#issuecomment-1521607155)
Closing temporarily so that I can re-open it -- am still interested to finish this.

✅ jonatack closed a pull request: "Severity-based logging -- parent PR"
(https://github.com/bitcoin/bitcoin/pull/25203)

💬 jonatack commented on pull request "p2p: always provide CNodeStateStats and getpeerinfo/netinfo/gui updates":
(https://github.com/bitcoin/bitcoin/pull/25923#issuecomment-1521607657)
Closing temporarily so that I can re-open it -- am still interested to finish this.

✅ jonatack closed a pull request: "p2p: always provide CNodeStateStats and getpeerinfo/netinfo/gui updates"
(https://github.com/bitcoin/bitcoin/pull/25923)

💬 sipa commented on pull request "Optimizations & simplifications following #25717":
(https://github.com/bitcoin/bitcoin/pull/25968#issuecomment-1521954195)
Closing as up for grabs.