💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1612198266)
Found an issue with prefix cursor handling, updated the tests to check for that. Also pulled in @TheCharlatan's fuzz test and changed several asserts to instead throw exceptions which resolves the fuzz crash.
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1612198266)
Found an issue with prefix cursor handling, updated the tests to check for that. Also pulled in @TheCharlatan's fuzz test and changed several asserts to instead throw exceptions which resolves the fuzz crash.
💬 MarcusMWilliams commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#issuecomment-1612199588)
Marcus Maurice Williams
On Mon, May 8, 2023, 10:12 AM jamesob ***@***.***> wrote:
>
> - Background and FAQ:
> https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
> - Prior progress/project:
> https://github.com/bitcoin/bitcoin/projects/11
> - Replaces #15606 <https://github.com/bitcoin/bitcoin/pull/15606>,
> which was closed due to Github slowness. Original description and
> commentary can be found there.
>
> -----------------------------
...
(https://github.com/bitcoin/bitcoin/pull/27596#issuecomment-1612199588)
Marcus Maurice Williams
On Mon, May 8, 2023, 10:12 AM jamesob ***@***.***> wrote:
>
> - Background and FAQ:
> https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
> - Prior progress/project:
> https://github.com/bitcoin/bitcoin/projects/11
> - Replaces #15606 <https://github.com/bitcoin/bitcoin/pull/15606>,
> which was closed due to Github slowness. Original description and
> commentary can be found there.
>
> -----------------------------
...
💬 LarryRuane commented on pull request "logging: use bitset for categories":
(https://github.com/bitcoin/bitcoin/pull/26697#issuecomment-1612227295)
Rebased to fix merge conflict
(https://github.com/bitcoin/bitcoin/pull/26697#issuecomment-1612227295)
Rebased to fix merge conflict
💬 Trrvrr commented on pull request "[24.1] Final Changes":
(https://github.com/bitcoin/bitcoin/pull/27660#issuecomment-1612298568)
> Final changes for `v24.1`.
>
> PR for bitcoincore.org is here: https://github.com/bitcoin-core/bitcoincore.org/pull/968.
(https://github.com/bitcoin/bitcoin/pull/27660#issuecomment-1612298568)
> Final changes for `v24.1`.
>
> PR for bitcoincore.org is here: https://github.com/bitcoin-core/bitcoincore.org/pull/968.
💬 craigraw commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1612482503)
To be clear, my desire for an efficient way to retrieve fee rate histogram data has nothing to do with Bitcoin Core's fee estimation algorithm. Because fee estimation is inherently difficult and no algorithm can know the user's time preference for every transaction, 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 pre
...
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1612482503)
To be clear, my desire for an efficient way to retrieve fee rate histogram data has nothing to do with Bitcoin Core's fee estimation algorithm. Because fee estimation is inherently difficult and no algorithm can know the user's time preference for every transaction, 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 pre
...
💬 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
...
(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.
(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)
(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?
(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
(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?
(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?
(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.
(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)
(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
...
(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.
(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.
(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
(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)
(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
...
(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
...