Bitcoin Core Github
44 subscribers
120K links
Download Telegram
👋 glozow's pull request is ready for review: "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs"
(https://github.com/bitcoin/bitcoin/pull/31829)
💬 fanquake commented on pull request "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task":
(https://github.com/bitcoin/bitcoin/pull/32586#issuecomment-2901167009)
Not sure; now we wouldn't have any (non-msan, non-32 bit task) CI using `DEBUG=1`? It'd at least be good to note in the CI config, why this is being changed this way / when it could be removed.
💬 maflcko commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2901167688)
> Kind of weird that only one environment failed

There are not enough CI resources to re-run every task on every pull request every few days. The CI resources would have to be ~doubled for that. For now, only the lint and previous-releases task are re-run.
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-2901169323)
This is ready for review again. Main changes from February: it includes the rewrite as a boost::multi_index container, removes `-maxorphantxs` entirely, and drops the benches that weren't demonstrating anything. I've updated the PR description.

Rewrite and eviction changes are in the same commit. I didn't think it made sense to first reimplement the old design as a multi_index, because the old eviction strategy requires twice as many indexes. If you are familiar with the old design and just w
...
💬 maflcko commented on pull request "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task":
(https://github.com/bitcoin/bitcoin/pull/32586#issuecomment-2901183220)
> Not sure; now we wouldn't have any (non-msan, non-32 bit task) CI using `DEBUG=1`? It'd at least be good to note in the CI config, why this is being changed this way / when it could be removed.

Of course there is no right or wrong answer here, but I'd say that the two libc++ debug builds are probably enough and redundant with this one anyway, so one could even fully remove the debug setting here. (Also, there is other, possibly more important stuff, only run in nightly CIs outside this rep
...
💬 hebasto commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#discussion_r2102535478)
A new https://git.guix.gnu.org/guix.git seems up.
💬 fjahr commented on pull request "test: properly check for per-tx sigops limit":
(https://github.com/bitcoin/bitcoin/pull/32533#issuecomment-2901192518)
tACK 7bc64a8

Confirmed that the test fails if the limit is not exceeded.
💬 fjahr commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2901197602)
> > Kind of weird that only one environment failed
>
> There are not enough CI resources to re-run every task on every pull request every few days. The CI resources would have to be ~doubled for that. For now, only the lint and previous-releases task are re-run.

Good to know, thanks!
📝 i-am-yuvi opened a pull request: "[WIP] test: Fix reorg patterns in mempool tests to use proper fork-based approach"
(https://github.com/bitcoin/bitcoin/pull/32587)
Addresses #32531

This is a Work In Progress, not ready for review yet.

Currently updating functional tests to replace direct use of `invalidateblock` with proper fork-based reorg behaviour. The direct invalidation approach bypasses important validation checks and has depth limitations(10 block) that don't match real-world reorg scenarios.

Plan:
- [ ] Fix mempool_ephemeral_dust.py reorg patterns
- [ ] Audit and fix other mempool_*.py tests
- [ ] Update any additional tests using pro
...
💬 instagibbs commented on pull request "[WIP] test: Fix reorg patterns in mempool tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-2901230119)
feel free to ping me when it's ready
📝 maflcko opened a pull request: " util: Abort on failing CHECK_NONFATAL in debug builds "
(https://github.com/bitcoin/bitcoin/pull/32588)
A failing `CHECK_NONFATAL` will throw an exception. This is fine and even desired in production builds, because the program will not abort and the user has a chance to easily report the bug upstream.

However, in debug development builds, exceptions for internal bugs are problematic:

* The exception could accidentally be caught and silently ignored
* The exception does not include a full stacktrace, possibly making debugging harder

Fix all issues by turning the exception into an abort i
...
💬 i-am-yuvi commented on issue "avoid using invalidateblock to directly test reorg behavior":
(https://github.com/bitcoin/bitcoin/issues/32531#issuecomment-2901240394)
> ### Motivation
> While working on [#32516](https://github.com/bitcoin/bitcoin/pull/32516) I found that for testing mempool entry from reorged blocks, directly using invalidateblock to trigger the reorg has at least a couple behaviors that don't match real reorgs:
>
> 1. Only allows 10 deep reorg before it stops trying to re-enter things into the mempool
> 2. doesn't respect descendant chain limits(Sets of PreChecks isn't run once per reorg but once per block, letting longer chains slip in)
>
...
💬 i-am-yuvi commented on pull request "[WIP] test: Fix reorg patterns in mempool tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-2901241194)
> feel free to ping me when it's ready

Sure!
💬 instagibbs commented on issue "avoid using invalidateblock to directly test reorg behavior":
(https://github.com/bitcoin/bitcoin/issues/32531#issuecomment-2901247824)
Also wondering aloud how we can accurately test deep reorgs on real networks, like a mainnet node for performance testing. Anyone have suggestions on this?
💬 laanwj commented on pull request "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task":
(https://github.com/bitcoin/bitcoin/pull/32586#issuecomment-2901265062)
> Not sure; now we wouldn't have any (non-msan, non-32 bit task) CI using DEBUG=1? It'd at least be good to note in the CI config, why this is being changed this way / when it could be removed.

In the short term, getting the CI to pass reliably again is most important imo. Adding another DEBUG run can always be considered, but shouldn't come at the cost of CI flakiness.
💬 romanz commented on issue "Support `Accept` HTTP header in REST API":
(https://github.com/bitcoin/bitcoin/issues/32583#issuecomment-2901271892)
Thanks @maflcko - you're right, it's better to have one way for specifying the expected content format.

@yancyribbens WDYT?
💬 maflcko commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2102633359)
> programs

thx, I may address this, if I have to re-touch for other reasons
📝 fanquake opened a pull request: "[29.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/32589)
Backports
- #32551 (just 800b7cc42ca63f2a6b245a4d327c7092289da6e1)
💬 instagibbs commented on pull request "test: properly check for per-tx sigops limit":
(https://github.com/bitcoin/bitcoin/pull/32533#discussion_r2102559718)
Took me a minute to realize this is only the 2nd(?) case where it's valid in block but non-standard-even-with-acceotnonstdtxn set.

Also meta: would be nice if there was a border testing capability, where you could pair it with a tx that would be successful and obviously just-at the limit.
💬 instagibbs commented on pull request "test: properly check for per-tx sigops limit":
(https://github.com/bitcoin/bitcoin/pull/32533#discussion_r2102602017)
```Suggestion
MAX_STANDARD_TX_SIGOPS = MAX_BLOCK_SIGOPS/5
```