💬 MarcoFalke commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1612498668)
> One workaround is to use `getrawmempool false` followed by repeated calls to `getmempoolentry`. This of course is even more inefficient, leading any reasonable client implementation to maintain a replica of mempool entries to avoid having to load them all, one by one, every time a fee rate histogram is required. This trades better performance for significantly increased memory utilisation.

I think the `getmempoolentry` can be batched for a slight improvement. Also, you don't have to store t
...

💬 MarcoFalke commented on pull request "test: Fix intermittent issue in mining_getblocktemplate_longpoll.py":
(https://github.com/bitcoin/bitcoin/pull/27941#issuecomment-1612502537)
> The longpoll request is supposed to return immediately if the conditions have been met. The token includes a reference to the transaction update state as a counter.

Thanks, I'll take another look later.

✅ MarcoFalke closed a pull request: "test: Fix intermittent issue in mining_getblocktemplate_longpoll.py"
(https://github.com/bitcoin/bitcoin/pull/27941)

💬 MarcoFalke commented on pull request "test: add python implementation of Elligator swift":
(https://github.com/bitcoin/bitcoin/pull/24005#issuecomment-1612507797)
Are you sure you rebased on *current* master?

💬 MarcoFalke commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-1612510057)
CI fuzz:

```
terminate called after throwing an instance of 'NonFatalCheckError'
what(): Internal bug detected: "parser.m_keys.size() != 0"
script/descriptor.cpp:1808 (ParseScript)
Bitcoin Core v25.99.0-501b80414422
Please report this issue here: https://github.com/bitcoin/bitcoin/issues
==16697== ERROR: libFuzzer: deadly signal

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

💬 MarcoFalke commented on pull request "test: Use TestNode datadir_path or chain_path where possible":
(https://github.com/bitcoin/bitcoin/pull/27884#issuecomment-1612585592)
Is this rfm or does it need more review?

💬 fanquake commented on issue "ci: failure in `wallet_basic.py --descriptors`":
(https://github.com/bitcoin/bitcoin/issues/27974#issuecomment-1612599115)
Ah yep. Not sure how I missed that.

✅ fanquake closed an issue: "ci: failure in `wallet_basic.py --descriptors`"
(https://github.com/bitcoin/bitcoin/issues/27974)

💬 fanquake commented on issue "failure in wallet_basic.py --descriptors":
(https://github.com/bitcoin/bitcoin/issues/27249#issuecomment-1612599372)
https://cirrus-ci.com/task/5029024158711808?logs=ci#L2900
```bash
node0 2023-06-23T16:51:00.034382Z [scheduler] [wallet/sqlite.cpp:47] [TraceSqlCallback] [/tmp/cirrus-ci-build/ci/scratch/test_runner/test_runner_₿_🏃_20230623_164625/wallet_basic_242/node0/regtest/wallets/default_wallet/wallet.dat] SQLite Statement: INSERT or REPLACE into main values(?, ?)
test 2023-06-23T16:51:00.043000Z TestFramework (ERROR): Assertion failed
Traceback (most recent cal
...

💬 virtu commented on pull request "test: various USDT functional test cleanups (27831 follow-ups)":
(https://github.com/bitcoin/bitcoin/pull/27944#issuecomment-1612602693)
Concept ACK. Will take a closer look and test next week.

💬 darosior commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-1612604508)
The descriptor on which it crashed is `wsh(0)`. This is a bug in the Miniscript logic, exposed by the new check but not introduced in this PR. A `0` script should not be considered sane.

💬 MarcoFalke commented on pull request "util: Safer MakeByteSpan with ByteSpanCast":
(https://github.com/bitcoin/bitcoin/pull/27973#issuecomment-1612623203)
Closing for now to allow for more brainstorming

✅ MarcoFalke closed a pull request: "util: Safer MakeByteSpan with ByteSpanCast"
(https://github.com/bitcoin/bitcoin/pull/27973)

💬 TheCharlatan commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1246307365)
There are many cases where these fatal error codes are eventually ignored as you go up the call stack. I would like to keep this PR focused on the question whether these blockstorage flush functions should at least indicate that they ran into an error. If you want to get a sense of just how many such "ignored" cases there are, I have my own set of patches that achieve something similar to Cory's "bubble up", but with your `Result<T, E>` type and introduced granularly for each fatal error call si
...

💬 willcl-ark commented on pull request "test: Use TestNode datadir_path or chain_path where possible":
(https://github.com/bitcoin/bitcoin/pull/27884#issuecomment-1612636549)
re-ACK aaaa3aefbdfca1c9243057eeefdc19940e60bf18

LGTM with the wallets_path added.

💬 fanquake commented on pull request "test: Use same timeout for all index sync":
(https://github.com/bitcoin/bitcoin/pull/27988#issuecomment-1612637988)
> Yeah, tsan+bdb+aarch64 is impossible to run. You'll have to disable bdb.

Yea, this branch rebased on master + NO_BDB=1 works as expected.

🚀 fanquake merged a pull request: "test: Use TestNode datadir_path or chain_path where possible"
(https://github.com/bitcoin/bitcoin/pull/27884)

💬 glozow commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1612707077)
> Sparrow Wallet displays the fee rate histogram chart in the UI as another "tool in the toolbox". This allows a user to observe trends and adjust the fee rate to place a transaction in a particular fee rate band to suit their time preference.

> a fee rate histogram is a useful tool in the toolbox when estimating fees

I think it's worth noting that a feerate histogram built from individual transaction feerates can be very misleading for fee estimation. A CPFP could look like n low feerate
...

👍 fanquake approved a pull request: "Remove now-unnecessary poll, fcntl includes from net(base).cpp"
(https://github.com/bitcoin/bitcoin/pull/27530#pullrequestreview-1504921462)
ACK 8d9b90a61e3a2a451abfce25328d13aa1e8f749b