💬 0xB10C commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1403481849)
Do you remember why you added a `do { ... } while (0)` to the `TRACEPOINT` macro?

💬 Sjors commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#issuecomment-1824570747)
> @Sjors I think you can set `-minrelaytxfee=0` to work around it?

That doesn't simulate a full mempool though.

I wrote a commit that lets my example go through: https://github.com/Sjors/bitcoin/commit/03baacd7643daf3d1c8efbc1a723719d8cc72dce

However, not sure if it's worth the extra complexity.

🤔 Sjors reviewed a pull request: "bugfix, Change up submitpackage results to return results for all transactions"
(https://github.com/bitcoin/bitcoin/pull/28848#pullrequestreview-1746652527)
Anyway, my two main issues are about pre-existing behavior: `tx-results` was already a dictionary and `-minrelaytxfee` already behaved that way.

d72aab3dbd8847e851f8b724b195181e30e7bf02 looks good, but I'm (obviously) not familiar enough with mempool stuff to know for sure.

💬 Sjors commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1403506481)
Do you still want to check "mempool full" like below?

📝 BrandonOdiwuor opened a pull request: "wallet: Add checkbalance RPC"
(https://github.com/bitcoin/bitcoin/pull/28930)
Fixes https://github.com/bitcoin/bitcoin/issues/28898

💬 maflcko commented on pull request "wallet: Add checkbalance RPC":
(https://github.com/bitcoin/bitcoin/pull/28930#issuecomment-1824634741)
Could add tests for:
* Tx in mempool
* Watchonly wallet
* ...

?

💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1824696971)
Another one:

[clusterfuzz-testcase-script_flags-4564162336129024.not.txt](https://github.com/bitcoin/bitcoin/files/13452132/clusterfuzz-testcase-script_flags-4564162336129024.not.txt)


💬 fjahr commented on pull request "contrib: add test for bucketing with asmap":
(https://github.com/bitcoin/bitcoin/pull/28869#issuecomment-1824718897)
Concept ACK

I think this could use some documentation on top of the file on how the script is intended to be used, see the text in `test_utxo_snapshots.sh`.

📝 maflcko opened a pull request: "fuzz: Limit fuzz buffer size in script_flags target"
(https://github.com/bitcoin/bitcoin/pull/28931)
Most fuzz targets have an upper limit on the buffer size to avoid excessive runtime. Do the same for `script_flags` to avoid timeouts such as https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1824696971

Also, fix iwyu. Also, remove legacy `CDataStream`.

🚀 fanquake merged a pull request: "fuzz: add target for `DescriptorScriptPubKeyMan`"
(https://github.com/bitcoin/bitcoin/pull/28578)

💬 fjahr commented on issue "wallet RPC to double-check the calculated balance":
(https://github.com/bitcoin/bitcoin/issues/28898#issuecomment-1824766692)
Allowing to call `importdescriptors` with the same descriptor but a different birthdate may also be a solution? Are users only interested in spendable outputs or may they also be interested in their transaction history in general?

💬 fjahr commented on pull request "wallet: Add checkbalance RPC":
(https://github.com/bitcoin/bitcoin/pull/28930#issuecomment-1824766969)
I think I would prefer the other option @maflcko suggested in the issue, i.e. adding a `check` flag to `getbalances`.

🚀 fanquake merged a pull request: "test: Add gettransaction test for "coin-join" tx"
(https://github.com/bitcoin/bitcoin/pull/18919)

📝 fanquake opened a pull request: "ci: remove `python3-setuptools` from macOS build deps"
(https://github.com/bitcoin/bitcoin/pull/28932)
Remove no-longer used python-setuptools.
Followup to #28432.
Related to #28845.

💬 fanquake commented on pull request "Fix SSE4.1-related issues":
(https://github.com/bitcoin/bitcoin/pull/28893#issuecomment-1824780061)
Concept ACK

💬 lrs955 commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1824797551)
> > why not save the xor key inside the leveldb block database?
>
> Just seems more complicated with no benefit?

I understand.
Maybe im wrong but isn't also the xor pattern of the chainstate saved in the database itself? Different leveldb database but still in a key a database.

👍 hebasto approved a pull request: "ci: remove `python3-setuptools` from macOS build deps"
(https://github.com/bitcoin/bitcoin/pull/28932#pullrequestreview-1746941879)
ACK 0ffcc5b680b92cf921fc11afb05e4a3607572d41, I have reviewed the code and it looks OK.

💬 furszy commented on issue "wallet RPC to double-check the calculated balance":
(https://github.com/bitcoin/bitcoin/issues/28898#issuecomment-1824916723)
An alternative/parallel course of action could be making `rescanblockchain` much faster. Removing the need for users to make an additional RPC command to identify any missing tx.
<start_shill>
The user can build the block filter index in 5-10 minutes using #26966 (per Sjors's benchmark). And, with it, `rescanblockchain` performs at a completely different speed level.
<end_shill>

👍 TheCharlatan approved a pull request: "wallet: propagete `checkChainLimits` error message to wallet"
(https://github.com/bitcoin/bitcoin/pull/28863#pullrequestreview-1747036936)
ACK bd5417e47cd2452817d7de859839b63210233a5c

💬 sipa commented on pull request "Fix SSE4.1-related issues":
(https://github.com/bitcoin/bitcoin/pull/28893#issuecomment-1824956649)
Concept ACK (if happy CI)

💬 furszy commented on pull request "wallet: propagete `checkChainLimits` error message to wallet":
(https://github.com/bitcoin/bitcoin/pull/28863#discussion_r1403766480)
Not blocking.
Could return `util::Result<void>` and inline all the `err_string` return messages within the `CheckPackageLimits` function.