💬 fjahr commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2019138847)
It seems to have worked with older macos and/or clang versions in the past and I have heard from someone that they avoid upgrading because they don't want to deal with these issues. So there are un-upgraded users that may not run into this and hopefully in the future this will be fixed again and the instructions are not necessary anymore.
Would be great to remove the section as a whole in the future. The problem is that if we make everyone run the special instructions we will not know if they
...
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2019138847)
It seems to have worked with older macos and/or clang versions in the past and I have heard from someone that they avoid upgrading because they don't want to deal with these issues. So there are un-upgraded users that may not run into this and hopefully in the future this will be fixed again and the instructions are not necessary anymore.
Would be great to remove the section as a whole in the future. The problem is that if we make everyone run the special instructions we will not know if they
...
💬 fjahr commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2019141108)
I am here commenting in this issue because I ran into the problem. Macos users that don't have this problem will probably not even look at this issue. So could be that the people that have the problem are in the minority.
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2019141108)
I am here commenting in this issue because I ran into the problem. Macos users that don't have this problem will probably not even look at this issue. So could be that the people that have the problem are in the minority.
🤔 instagibbs reviewed a pull request: "cluster mempool: add txgraph diagrams/mining/eviction"
(https://github.com/bitcoin/bitcoin/pull/31444#pullrequestreview-2725655591)
Looking good 0630995fee22990402771547be1480b8706c76ce
No real show-stoppers except for the diagram check fuzz test conceptual issue I'm still having.
(https://github.com/bitcoin/bitcoin/pull/31444#pullrequestreview-2725655591)
Looking good 0630995fee22990402771547be1480b8706c76ce
No real show-stoppers except for the diagram check fuzz test conceptual issue I'm still having.
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018742079)
micro-nit
```Suggestion
std::vector<FeeFrac> chunk_feerates;
```
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018742079)
micro-nit
```Suggestion
std::vector<FeeFrac> chunk_feerates;
```
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018744853)
We don't care or it will be respected within clusters due to CFR?
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018744853)
We don't care or it will be respected within clusters due to CFR?
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018933019)
nit: const chunk_count
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018933019)
nit: const chunk_count
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018913351)
maybe overkill but `m_main_chunkindex` since there's a lot of level==0 checks everywhere gating access/modification
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018913351)
maybe overkill but `m_main_chunkindex` since there's a lot of level==0 checks everywhere gating access/modification
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018886603)
1d500a50b698a638c840ca79504c2a9a421e8756 stretch goal, have SanityCheck check m_chunk_count directly?
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018886603)
1d500a50b698a638c840ca79504c2a9a421e8756 stretch goal, have SanityCheck check m_chunk_count directly?
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018829062)
micro-nit: I know they're identical, but having the list being built via `stage_real_diagram` seems easier to digest what's being computed
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018829062)
micro-nit: I know they're identical, but having the list being built via `stage_real_diagram` seems easier to digest what's being computed
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018906562)
nit: Could we note that this is initialized first in `AddTransaction`?
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018906562)
nit: Could we note that this is initialized first in `AddTransaction`?
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019037611)
```Suggestion
// The same order should be obtained through a BlockBuilder as implied by CompareMainOrder, if nothing is skipped.
```
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019037611)
```Suggestion
// The same order should be obtained through a BlockBuilder as implied by CompareMainOrder, if nothing is skipped.
```
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018984013)
b130e185f54c7f762da6ef435882335e4f0b98ff
Could the documentation get beefed up a bit for this? What the shape that range should look like, etc. Was only clear later when seeing usages.
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018984013)
b130e185f54c7f762da6ef435882335e4f0b98ff
Could the documentation get beefed up a bit for this? What the shape that range should look like, etc. Was only clear later when seeing usages.
💬 glozow commented on pull request "fuzz: extract unsequenced operations with side-effects":
(https://github.com/bitcoin/bitcoin/pull/32141#issuecomment-2762140874)
Backported in #32136
(https://github.com/bitcoin/bitcoin/pull/32141#issuecomment-2762140874)
Backported in #32136
💬 glozow commented on pull request "cmake: Add `NO_CACHE_IF_FAILED` option for checking linker flags":
(https://github.com/bitcoin/bitcoin/pull/32027#issuecomment-2762141394)
Backported in #32136
(https://github.com/bitcoin/bitcoin/pull/32027#issuecomment-2762141394)
Backported in #32136
💬 glozow commented on pull request "depends: set `CMAKE_*_COMPILER_TARGET` in toolchain":
(https://github.com/bitcoin/bitcoin/pull/31849#issuecomment-2762141871)
Backported in #32136
(https://github.com/bitcoin/bitcoin/pull/31849#issuecomment-2762141871)
Backported in #32136
💬 yancyribbens commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#discussion_r2019167894)
> The traceback in this case does print the source code line, so a comment on the same line could be sufficient. It depends on how the error is presented. Example failure:
This is an annoying draw back that the stack trace won't contain the line in the test that's in error, but instead the line in the `assert_not_equal` helper. In Rust, one can decorate the helper function with the `track_caller` macro. I don't see anything that is similar for C++ sadly.
(https://github.com/bitcoin/bitcoin/pull/29500#discussion_r2019167894)
> The traceback in this case does print the source code line, so a comment on the same line could be sufficient. It depends on how the error is presented. Example failure:
This is an annoying draw back that the stack trace won't contain the line in the test that's in error, but instead the line in the `assert_not_equal` helper. In Rust, one can decorate the helper function with the `track_caller` macro. I don't see anything that is similar for C++ sadly.
💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019170353)
As far as diagram is concerned (incl. for `FeeRateCompare`), the order of equal-feerate element is irrelevant, as they just represent different sections of an overall line segment with a fixed slope. As far as `FeeRateCompare` is concerned, they could even be merged into a single line segment with the combined fee and size.
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019170353)
As far as diagram is concerned (incl. for `FeeRateCompare`), the order of equal-feerate element is irrelevant, as they just represent different sections of an overall line segment with a fixed slope. As far as `FeeRateCompare` is concerned, they could even be merged into a single line segment with the combined fee and size.
💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019171810)
Because fees can be negative in the fuzz test. If they were all non-negative, then I think this suggested assertion should hold.
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019171810)
Because fees can be negative in the fuzz test. If they were all non-negative, then I think this suggested assertion should hold.
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019173335)
ah! please leave a note as to that, I always forget that's possible
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2019173335)
ah! please leave a note as to that, I always forget that's possible
💬 Christewart commented on pull request "RFC: Accept non-std transactions in Testnet4 by default again":
(https://github.com/bitcoin/bitcoin/pull/32133#issuecomment-2762182397)
> > the question is why we are not also moving back on this setting, too, so that people can use non-std transactions on Testnet4.
>
> I don't think this is enough. There are many non-std transactions that will still be rejected, even if this is turned on. One example is #29843.
Another example is 64-byte transactions which was surprising behavior to me when working the BIP to disallow them:
https://github.com/bitcoin/bitcoin/blob/e486597f9a57903600656fb5106858941885852f/src/validation.
...
(https://github.com/bitcoin/bitcoin/pull/32133#issuecomment-2762182397)
> > the question is why we are not also moving back on this setting, too, so that people can use non-std transactions on Testnet4.
>
> I don't think this is enough. There are many non-std transactions that will still be rejected, even if this is turned on. One example is #29843.
Another example is 64-byte transactions which was surprising behavior to me when working the BIP to disallow them:
https://github.com/bitcoin/bitcoin/blob/e486597f9a57903600656fb5106858941885852f/src/validation.
...