💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1247821121)
> Noticed this while I was rebasing https://github.com/bitcoin/bitcoin/pull/24230, but one side effect of this commit is now CustomInit will be called unnecessarily when the index can't start up because there is a pruning violation. This doesn't seem ideal, but it probably not worth worrying about. It would be good to mention the change in the commit description, though.

Hmm, I hold the opposite view on this matter. I believe it is beneficial to call `CustomInit` within the base class's `Init
...

📝 MarcoFalke opened a pull request: "fuzz: Generate rpc fuzz targets individually"
(https://github.com/bitcoin/bitcoin/pull/28015)
The `rpc` fuzz target was added more than two years ago in e45863166f5e44cc2c380f4667812fcd3cddc73b. However, the bug https://github.com/bitcoin/bitcoin/issues/27913 was only found recently. Thus, it is pretty clear that fuzz engines can't deal with a search space that is too broad and can be extended in too many directions.

Fix that by limiting the search space to each RPC method name and then iterate over all names, instead of letting the fuzz engine do the iteration.

With this, the bug
...

💬 MarcoFalke commented on pull request "fuzz: Generate rpc fuzz targets individually":
(https://github.com/bitcoin/bitcoin/pull/28015#issuecomment-1614624535)
Would be nice if someone also implemented this for OSS-Fuzz: It would require building a fuzz exe for each RPC method, and then probably also adjust https://github.com/bitcoin-core/qa-assets/blob/main/download_oss_fuzz_inputs.py to somehow concatenate all RPC methods into one `rpc.zip` file.

💬 lontivero commented on issue "Improving fee estimation accuracy":
(https://github.com/bitcoin/bitcoin/issues/27995#issuecomment-1614639756)
Fee rates are a everyday discussion in many teams I guess. This is from today's internal discussion about a problem in our UI regarding fees:

💬 TheCharlatan commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1247847963)
In commit "Implement handling of other endianness in BerkeleyRODatabase" 0b8eac6a78351a68c1b5a6126564493ab50031dc

I might be reading something wrong, or confusing something, but I don't quite understand why the record data itself does not have to get its endianness adjusted.

💬 TheCharlatan commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1247852182)
In commit "wallet: implement independent BDB deserializer in BerkeleyRODatabase" d5fe130106bdf367c0bca157b02d12254590b585

Can this `data` field be removed? Seems to be unused.

🤔 brunoerg reviewed a pull request: "fuzz: Generate rpc fuzz targets individually"
(https://github.com/bitcoin/bitcoin/pull/28015#pullrequestreview-1507224273)
Concept ACK

👍 TheCharlatan approved a pull request: "contrib: add macOS test for fixup_chains usage"
(https://github.com/bitcoin/bitcoin/pull/27999#pullrequestreview-1507226540)
ACK 7f96638723a08edf4341a2f4c06b2aa41378fcf7

💬 brunoerg commented on pull request "fuzz: call `LookupSubNet` before calling `Ban` with a subnet":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1614675792)
Force-pushed to remove "--exclude banman" in Mac

💬 fanquake commented on pull request "ci: label docker images and prune dangling images selectively":
(https://github.com/bitcoin/bitcoin/pull/27793#issuecomment-1614676019)
Concept ACK

👍 hebasto approved a pull request: "ci: re-enable gui tests for s390x"
(https://github.com/bitcoin/bitcoin/pull/28014#pullrequestreview-1507232955)
ACK 9be4565c2db6d7a420d032d5c41843d473ed32d1, tested on Ubuntu 23.04.

💬 hebasto commented on issue "ci: s390x qemu fails after qt5.15":
(https://github.com/bitcoin/bitcoin/issues/23730#issuecomment-1614682004)
It is not an issue anymore. See: https://github.com/bitcoin/bitcoin/pull/28014.

🚀 fanquake merged a pull request: "ci: re-enable gui tests for s390x"
(https://github.com/bitcoin/bitcoin/pull/28014)

📝 sr-gi opened a pull request: "p2p: gives seednode priority over dnsseed if both are provided"
(https://github.com/bitcoin/bitcoin/pull/28016)
This is a follow-up of #27577

If both `seednode` and `dnsseed` are provided, the node will start a race between them in order to fetch data to feed the `addrman`.

This PR gives priority to `seednode` over `dnsseed` so if some nodes are provided as seeds, they can be tried before defaulting to the `dnsseeds`

💬 sr-gi commented on pull request "p2p: gives seednode priority over dnsseed if both are provided":
(https://github.com/bitcoin/bitcoin/pull/28016#issuecomment-1614740856)
I wanted to have an exit early strategy that will hand the logic back to `dnsseed` if all `seednodes` have been tried and no data has been added to the `addrman`. However, looks like checking if both `m_nodes` and `m_addr_fetch` are empty may not be sufficient (i.e. a node can be removed from `m_addr_fetch`, tried but not yet put at `m_nodes`. This seems to be specifically sensitive for privacy networks where the latency may be higher)

💬 MarcoFalke commented on pull request "p2p: gives seednode priority over dnsseed if both are provided":
(https://github.com/bitcoin/bitcoin/pull/28016#discussion_r1247939565)
nit: Could use `NodeClock::now()` for new code? Also `30s` should work for the constant.

💬 furszy commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1247950494)
Yeah, it gets cleaned at the end of the tx creation process (check https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1247203254).

But new tests never hurt, so just added it inside e46c1276. Thanks for pushing me to do it.

💬 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