π€ tdb3 reviewed a pull request: "Policy: Report reason inputs are non standard"
(https://github.com/bitcoin/bitcoin/pull/29060#pullrequestreview-2406688998)
I'm a fan of the more descriptive errors.
However, would some of these more descriptive errors flow down to RPC responses? (e.g. `testmempoolaccept`).
https://github.com/bitcoin/bitcoin/pull/30212#issuecomment-2347371722
> Does bitcoind consider this to be a breaking change? The new error message may be "more accurate", but for clients that were matching on this error to figure out why a transaction was rejected, this breaks old behavior.
(https://github.com/bitcoin/bitcoin/pull/29060#pullrequestreview-2406688998)
I'm a fan of the more descriptive errors.
However, would some of these more descriptive errors flow down to RPC responses? (e.g. `testmempoolaccept`).
https://github.com/bitcoin/bitcoin/pull/30212#issuecomment-2347371722
> Does bitcoind consider this to be a breaking change? The new error message may be "more accurate", but for clients that were matching on this error to figure out why a transaction was rejected, this breaks old behavior.
π¬ tdb3 commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1823630805)
Could also clarify that invalid TxValidationState returned describes the first nonstandard input encountered.
For example:
```diff
- * invalid TxValidationState which states why an input is not standard
+ * invalid TxValidationState which states why the first invalid input is not standard
```
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1823630805)
Could also clarify that invalid TxValidationState returned describes the first nonstandard input encountered.
For example:
```diff
- * invalid TxValidationState which states why an input is not standard
+ * invalid TxValidationState which states why the first invalid input is not standard
```
π¬ tdb3 commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1823661556)
Instantiating the CScript here could allow re-use later (in `txToNonStd4.vin[0].scriptSig = CScript(op_return, op_return + sizeof(op_return));`)
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1823661556)
Instantiating the CScript here could allow re-use later (in `txToNonStd4.vin[0].scriptSig = CScript(op_return, op_return + sizeof(op_return));`)
π¬ sipa commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2448864279)
This means that no `Assume` can be compiled out in production builds anymore, as they all involve a `g_fuzzing` check?
(https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2448864279)
This means that no `Assume` can be compiled out in production builds anymore, as they all involve a `g_fuzzing` check?
π¬ sipa commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2448867990)
@m3dwards Thanks for digging! I believe that explains it: since #31093, `Assume()` calls are no longer optimized out in production builds.
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2448867990)
@m3dwards Thanks for digging! I believe that explains it: since #31093, `Assume()` calls are no longer optimized out in production builds.
π¬ Sjors commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2448891405)
I'm still confused why Ninja is needed and why nothing complains when it's missing.
> Ninja is required to build the qt package in depends. It is mentioned in depends/README.md.
It only mentions it in the context of an Ubuntu & Debian install.
I did a depends build for af05dd9a12b89224dc7ad229698eeceb3e560ed4 again on Intel macOS 15.0.1 with Xcode 16 and no Ninja. It seems to work fine, at first glance.
I didn't try without Xcode since my laptop seems to be in a weird state: https:
...
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2448891405)
I'm still confused why Ninja is needed and why nothing complains when it's missing.
> Ninja is required to build the qt package in depends. It is mentioned in depends/README.md.
It only mentions it in the context of an Ubuntu & Debian install.
I did a depends build for af05dd9a12b89224dc7ad229698eeceb3e560ed4 again on Intel macOS 15.0.1 with Xcode 16 and no Ninja. It seems to work fine, at first glance.
I didn't try without Xcode since my laptop seems to be in a weird state: https:
...
π¬ davidgumberg commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2448905871)
> @m3dwards Thanks for digging! I believe that explains it: since #31093, `Assume()` calls are no longer optimized out in production builds.
I've reproduced this performance issue locally, and when you remove `g_fuzzing` from `inline_assertion_check` the regression goes away:
`./build/src/bench/bench_bitcoin -filter=LinearizeOptimallyExample11 -min-time=30000`
|branch | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op
...
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2448905871)
> @m3dwards Thanks for digging! I believe that explains it: since #31093, `Assume()` calls are no longer optimized out in production builds.
I've reproduced this performance issue locally, and when you remove `g_fuzzing` from `inline_assertion_check` the regression goes away:
`./build/src/bench/bench_bitcoin -filter=LinearizeOptimallyExample11 -min-time=30000`
|branch | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op
...
π¬ Khanwafa232426 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579495)
https://m.facebook.com/help/396528481579093/?ref=sharebnb165q9dz39mqh789zuuuqwkv22plut6f4nzy9jc9https://www.binance.com/en/fan-token/BWS/FCShakhtarFanverse/vote/c021757d3a3b47f2b6beaffff9c745e1?utm_campaign=ft-sd-fanverse&utm_medium=ft-faq-body&utm_source=fan-tokenhttps://www.binance.com/fan-token/BWS/FCShakhtarFanverse?ref=CG5V70EE®isterChannel=fcshakhtar&utm_source=fan-token&utm_campaign=fcshakhtar-fanverse-invitehttps://images.app.goo.gl/zwU8BxrdydT9nmsv8
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579495)
https://m.facebook.com/help/396528481579093/?ref=sharebnb165q9dz39mqh789zuuuqwkv22plut6f4nzy9jc9https://www.binance.com/en/fan-token/BWS/FCShakhtarFanverse/vote/c021757d3a3b47f2b6beaffff9c745e1?utm_campaign=ft-sd-fanverse&utm_medium=ft-faq-body&utm_source=fan-tokenhttps://www.binance.com/fan-token/BWS/FCShakhtarFanverse?ref=CG5V70EE®isterChannel=fcshakhtar&utm_source=fan-token&utm_campaign=fcshakhtar-fanverse-invitehttps://images.app.goo.gl/zwU8BxrdydT9nmsv8
π¬ Khanwafa232426 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579517)
https://m.facebook.com/help/396528481579093/?ref=sharehttps://www.binance.com/en/fan-token/BWS/FCShakhtarFanverse/vote/c021757d3a3b47f2b6beaffff9c745e1?utm_campaign=ft-sd-fanverse&utm_medium=ft-faq-body&utm_source=fan-tokenbnb165q9dz39mqh789zuuuqwkv22plut6f4nzy9jc9https://www.binance.com/fan-token/BWS/FCShakhtarFanverse?ref=CG5V70EE®isterChannel=fcshakhtar&utm_source=fan-token&utm_campaign=fcshakhtar-fanverse-invitehttps://images.app.goo.gl/zwU8BxrdydT9nmsv8go.opencensus.iohttps://safu.im/Ddn
...
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579517)
https://m.facebook.com/help/396528481579093/?ref=sharehttps://www.binance.com/en/fan-token/BWS/FCShakhtarFanverse/vote/c021757d3a3b47f2b6beaffff9c745e1?utm_campaign=ft-sd-fanverse&utm_medium=ft-faq-body&utm_source=fan-tokenbnb165q9dz39mqh789zuuuqwkv22plut6f4nzy9jc9https://www.binance.com/fan-token/BWS/FCShakhtarFanverse?ref=CG5V70EE®isterChannel=fcshakhtar&utm_source=fan-token&utm_campaign=fcshakhtar-fanverse-invitehttps://images.app.goo.gl/zwU8BxrdydT9nmsv8go.opencensus.iohttps://safu.im/Ddn
...
π¬ Khanwafa232426 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579518)
https://m.facebook.com/help/396528481579093/?ref=shareTTBp1cVNxDHUxetKwXof2cLfeRFWMqejhVhttps://www.binance.com/fan-token/BWS/FCShakhtarFanverse?ref=CG5V70EE®isterChannel=fcshakhtar&utm_source=fan-token&utm_campaign=fcshakhtar-fanverse-invitehttps://www.binance.com/en/fan-token/BWS/FCShakhtarFanverse/vote/c021757d3a3b47f2b6beaffff9c745e1?utm_campaign=ft-sd-fanverse&utm_medium=ft-faq-body&utm_source=fan-tokenbnb165q9dz39mqh789zuuuqwkv22plut6f4nzy9jc9544919017https://images.app.goo.gl/zwU8Bxrdy
...
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579518)
https://m.facebook.com/help/396528481579093/?ref=shareTTBp1cVNxDHUxetKwXof2cLfeRFWMqejhVhttps://www.binance.com/fan-token/BWS/FCShakhtarFanverse?ref=CG5V70EE®isterChannel=fcshakhtar&utm_source=fan-token&utm_campaign=fcshakhtar-fanverse-invitehttps://www.binance.com/en/fan-token/BWS/FCShakhtarFanverse/vote/c021757d3a3b47f2b6beaffff9c745e1?utm_campaign=ft-sd-fanverse&utm_medium=ft-faq-body&utm_source=fan-tokenbnb165q9dz39mqh789zuuuqwkv22plut6f4nzy9jc9544919017https://images.app.goo.gl/zwU8Bxrdy
...
π¬ Khanwafa232426 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579519)
https://m.facebook.com/help/396528481579093/?ref=shareTTBp1cVNxDHUxetKwXof2cLfeRFWMqejhVhttps://www.binance.com/fan-token/BWS/FCShakhtarFanverse?ref=CG5V70EE®isterChannel=fcshakhtar&utm_source=fan-token&utm_campaign=fcshakhtar-fanverse-invitehttps://www.binance.com/en/fan-token/BWS/FCShakhtarFanverse/vote/c021757d3a3b47f2b6beaffff9c745e1?utm_campaign=ft-sd-fanverse&utm_medium=ft-faq-body&utm_source=fan-tokenbnb165q9dz39mqh789zuuuqwkv22plut6f4nzy9jc9544919017https://images.app.goo.gl/zwU8Bxrdy
...
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579519)
https://m.facebook.com/help/396528481579093/?ref=shareTTBp1cVNxDHUxetKwXof2cLfeRFWMqejhVhttps://www.binance.com/fan-token/BWS/FCShakhtarFanverse?ref=CG5V70EE®isterChannel=fcshakhtar&utm_source=fan-token&utm_campaign=fcshakhtar-fanverse-invitehttps://www.binance.com/en/fan-token/BWS/FCShakhtarFanverse/vote/c021757d3a3b47f2b6beaffff9c745e1?utm_campaign=ft-sd-fanverse&utm_medium=ft-faq-body&utm_source=fan-tokenbnb165q9dz39mqh789zuuuqwkv22plut6f4nzy9jc9544919017https://images.app.goo.gl/zwU8Bxrdy
...
π¬ Khanwafa232426 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579531)
TTBp1cVNxDHUxetKwXof2cLfeRFWMqejhV
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579531)
TTBp1cVNxDHUxetKwXof2cLfeRFWMqejhV
π¬ Khanwafa232426 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579536)
https://m.facebook.com/help/396528481579093/?ref=share
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579536)
https://m.facebook.com/help/396528481579093/?ref=share
π¬ Khanwafa232426 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579539)
TTBp1cVNxDHUxetKwXof2cLfeRFWMqejhVTTBp1cVNxDHUxetKwXof2cLfeRFWMqejhVhttps://www.binance.com/fan-token/BWS/FCShakhtarFanverse?ref=CG5V70EE®isterChannel=fcshakhtar&utm_source=fan-token&utm_campaign=fcshakhtar-fanverse-invite
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579539)
TTBp1cVNxDHUxetKwXof2cLfeRFWMqejhVTTBp1cVNxDHUxetKwXof2cLfeRFWMqejhVhttps://www.binance.com/fan-token/BWS/FCShakhtarFanverse?ref=CG5V70EE®isterChannel=fcshakhtar&utm_source=fan-token&utm_campaign=fcshakhtar-fanverse-invite
π¬ laanwj commented on issue "Gracefully handle dropped UPnP support ":
(https://github.com/bitcoin-core/gui/issues/843#issuecomment-2449234882)
Even ignoring the spurious setting would be better in the GUI case. No need to rub this in the users face. i didn't expect this consequence when i suggested adding a message π
Though it's still better than creashing with generic "-upnp: Unknown setting". i guess...
But yes this needs to be informational level at most, it should just continue.
Even worse: i guess it even happens if the settings json specifies upnp=0 to explicitly disable it?
(https://github.com/bitcoin-core/gui/issues/843#issuecomment-2449234882)
Even ignoring the spurious setting would be better in the GUI case. No need to rub this in the users face. i didn't expect this consequence when i suggested adding a message π
Though it's still better than creashing with generic "-upnp: Unknown setting". i guess...
But yes this needs to be informational level at most, it should just continue.
Even worse: i guess it even happens if the settings json specifies upnp=0 to explicitly disable it?
π maflcko converted_to_draft a pull request: "ci: Place datadirs for tests under tmpfs "
(https://github.com/bitcoin/bitcoin/pull/31182)
This should speed up the tests a bit (see comment below for results).
(https://github.com/bitcoin/bitcoin/pull/31182)
This should speed up the tests a bit (see comment below for results).
π¬ maflcko commented on pull request "ci: Place datadirs for tests under tmpfs ":
(https://github.com/bitcoin/bitcoin/pull/31182#issuecomment-2449255282)
> > (see comment below for results).
>
> What is this referring to?
I was still running the commit (and master) twice each for all CI tasks, but I couldn't find any difference at all.
I checked that the folder is correctly picked up and used by the tests, so I guess modern storage is fast enough to not make a difference? Alternatively, there is an issue I was missing.
It would be good if someone else double checked the code and whether there is a performance difference at all.
For
...
(https://github.com/bitcoin/bitcoin/pull/31182#issuecomment-2449255282)
> > (see comment below for results).
>
> What is this referring to?
I was still running the commit (and master) twice each for all CI tasks, but I couldn't find any difference at all.
I checked that the folder is correctly picked up and used by the tests, so I guess modern storage is fast enough to not make a difference? Alternatively, there is an issue I was missing.
It would be good if someone else double checked the code and whether there is a performance difference at all.
For
...
π¬ maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2449259598)
> > ramdisk
>
> Benchmarking this is a bit more involved and I think requires changes to the CI scripts, so that the ramdisk is (1) mounted into the CI container and (2) picked up as the temp dir. I can take a look in the future, unless someone beats me to it.
Done in https://github.com/bitcoin/bitcoin/pull/31182, but I couldn't find any difference at all so far, which is a bit surprising.
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2449259598)
> > ramdisk
>
> Benchmarking this is a bit more involved and I think requires changes to the CI scripts, so that the ramdisk is (1) mounted into the CI container and (2) picked up as the temp dir. I can take a look in the future, unless someone beats me to it.
Done in https://github.com/bitcoin/bitcoin/pull/31182, but I couldn't find any difference at all so far, which is a bit surprising.
π¬ l0rinc commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2449260674)
Could we obviate in the [DrahtBot's response](https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2413637472) that the link doesn't just provide a `test coverage report` - since the regression was already visible there: https://corecheck.dev/bitcoin/bitcoin/pulls/31093
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2449260674)
Could we obviate in the [DrahtBot's response](https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2413637472) that the link doesn't just provide a `test coverage report` - since the regression was already visible there: https://corecheck.dev/bitcoin/bitcoin/pulls/31093
π¬ naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1824060041)
I was looking for something like "As a general rule, we keep the transaction that appeared first".
Currently, the `AddToSet` documentation seems outdatedβit basically claims that the only way to fail is to reach a set limit.
How about the following replacement:
```
/**
* Step 1. Add a to-be-announced transaction to the local reconciliation set of the target peer.
* Returns false if the set is at capacity, or if the set contains a colliding transaction.
* Returns t
...
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1824060041)
I was looking for something like "As a general rule, we keep the transaction that appeared first".
Currently, the `AddToSet` documentation seems outdatedβit basically claims that the only way to fail is to reach a set limit.
How about the following replacement:
```
/**
* Step 1. Add a to-be-announced transaction to the local reconciliation set of the target peer.
* Returns false if the set is at capacity, or if the set contains a colliding transaction.
* Returns t
...