🤔 fjahr reviewed a pull request: "Rework validation logic for assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/27746#pullrequestreview-1512960883)
Code review ACK 03dfe04075668aa3b99a5d6d68adfcecffca0593

(modulo my question about the rev file management)

I will also sync a node with this code and report if I see anything unusual in the coming days.

💬 fjahr commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1252074194)
I must either be missing something about this line or it might be unnecessary. If I understand correctly this is updating `m_undo_height_in_last_blockfile` when the next block is connected after the flush happened. However, why not update it right away when the flush happens? I tried that by removing this line it seems like all tests are still passing without it.

💬 dooglus commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1620521045)
> Do you happen to know what libevent version you were on?

It looks like version 2.1.12.

```
$ dpkg -l | grep libevent
ii libev4:amd64 1:4.33-1 amd64 high-performance event loop library modelled after libevent
ii libevent-2.1-7:amd64 2.1.12-stable-1 amd64 Asynchronous event notification library
ii libevent-core-2.1-7:amd64
...

💬 luke-jr commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#discussion_r1252222085)
Why not `UNKNOWN_DESCRIPTOR` instead?

🚀 fanquake merged a pull request: "ci: Remove deprecated container.greedy"
(https://github.com/bitcoin/bitcoin/pull/28024)

💬 luke-jr commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#issuecomment-1620528011)
Tag for backport?

💬 fanquake commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#issuecomment-1620530627)
Not if this has (as I understand it) only been broken on master?

💬 achow101 commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#discussion_r1252228384)
The descriptor isn't unknown. It's a corruption error as the id written in the database doesn't match the id that we calculate.

💬 achow101 commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#issuecomment-1620534607)
> Tag for backport?

No need, only affects master.

📝 pablomartin4btc converted_to_draft a pull request: "httpserver, rest: improving URI validation"
(https://github.com/bitcoin/bitcoin/pull/27253)
This PR contains an isolated enhancement of the segfault bugfix https://github.com/bitcoin/bitcoin/pull/27468 (already merged), improving the way we handle the URI validation, which will be performed only once instead of on each query parameter of a rest service endpoint.

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1620535396)
After a chat with @stickies-v where we were discussing different approaches on this enhancement and other details regarding fuzz testing, libevent functionality and each commit intention, I've decided to put this onto draft. Firstly we would need to define the purpose of the `httprequest` object, do we want/ need the `httprequest` obj to exist even with an invalid request?, as @stickies-v [raised his concerns before](https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1481163868), it seem
...

💬 ajtowns commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1620538136)
> I'm starting to think that something closer to your idea here is right: trying ancestor sets of every transaction in the linearization in order, if the ancestor set feerate is suffiicently high. This indeed won't deal with multiple-children-pay-for-parent cases perfectly, but including everything connected may be too much as well. I'll try to think about this more.

I don't think handling multiple-children-pay-for-parent cases perfectly should be a goal here -- we're not fixing eviction (or
...

💬 luke-jr commented on pull request "exclude ipc scheme from port check":
(https://github.com/bitcoin/bitcoin/pull/28020#issuecomment-1620538924)
nit: Rebasing onto bbbf89a9de0757c44880495244f90967f7147c0d would enable a clean merge to 25.x also

💬 MarcoFalke commented on pull request "descriptors: Add a KEY expression representing a list of individual keys":
(https://github.com/bitcoin/bitcoin/pull/26626#issuecomment-1620571802)
Needs rebase if still relevant

💬 MarcoFalke commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#issuecomment-1620572868)
From CI https://cirrus-ci.com/task/4675435070488576?logs=ci#L3365:

```
File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/wallet_fundrawtransaction.py", line 337, in test_double_change
assert_equal(wallet.gettransaction(desc_tx['vin'][0]['txid'])['amount'], Decimal(50)) # assert input value
File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i
...

💬 MarcoFalke commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1620574057)
> Looks like the fuzz target doesn't compile on windows?

Looks like this still wasn't addressed?

📝 luke-jr opened a pull request: "Fix issues in ZMQ error handling"
(https://github.com/bitcoin/bitcoin/pull/28029)
Behaves better if abnormal issues occur

💬 Sjors commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#issuecomment-1620591164)
If anyone on master is affected by this, we could perhaps add a command to wallet-tool that resets the descriptor ID and cache.

👍 brunoerg approved a pull request: "fuzz: Generate rpc fuzz targets individually"
(https://github.com/bitcoin/bitcoin/pull/28015#pullrequestreview-1513276185)
ACK fa1e27fe8ec42764d0250c82a83d774c15798c4a

Running only with `rpc`, `targets` in `generate_corpus` becomes:
```
[('rpc', {'LIMIT_TO_RPC_COMMAND': 'analyzepsbt'}), ('rpc', {'LIMIT_TO_RPC_COMMAND': 'clearbanned'}), ('rpc', {'LIMIT_TO_RPC_COMMAND': 'combinepsbt'}), ('rpc', {'LIMIT_TO_RPC_COMMAND': 'combinerawtransaction'}), ('rpc', {'LIMIT_TO_RPC_COMMAND': 'converttopsbt'}), ('rpc', {'LIMIT_TO_RPC_COMMAND': 'createmultisig'}), ('rpc', {'LIMIT_TO_RPC_COMMAND': 'createpsbt'}), ('rpc', {'LIMIT_
...

💬 ariard commented on pull request "policy: make unstructured annex standard":
(https://github.com/bitcoin/bitcoin/pull/27926#discussion_r1252291878)
> Are you proposing to limit the annex size including any tags to 257 bytes? I see the conceptual difference, but there wouldn't be a functional difference for now?

Yes, for now it doesn’t make a functional difference. The issue I have in mind is the following, assuming the unstructured annex format is deployed and we see applications building on top of it, the 256 (or 257 now) annex size limit will have to become a max-size data payload, which is going to leak in the all application toolchai
...