Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 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
```
👍 instagibbs approved a pull request: "test: properly check for per-tx sigops limit"
(https://github.com/bitcoin/bitcoin/pull/32533#pullrequestreview-2861227732)
crACK 7bc64a8859c3644fdf2eeff59f72a778ae60ea3f

just nits you can ignore
💬 fanquake commented on pull request "cmake: Remove `ENABLE_{SSE41,AVX2,X86_SHANI,ARM_SHANI}` from `bitcoin-build-config.h`":
(https://github.com/bitcoin/bitcoin/pull/32551#issuecomment-2901333121)
Backported to 29.x in #32589.
🚀 glozow merged a pull request: "test: properly check for per-tx sigops limit"
(https://github.com/bitcoin/bitcoin/pull/32533)
💬 pinheadmz commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2901371140)
Approach ACK fa53098472 (still reviewing code changes around blocktip notifications)

With fresh brain this morning I think this is the best possible approach if we make any change at all. The regtest stuff I observed is expected behavior - if there's no block for 2 days then yes you are 288 blocks behind and progress should estimate that. The new chainparams define regtest as having an expected 0.001 tx/s which is approximately equal to one (coinbase) tx every ten minutes.


Example catchi
...
💬 maflcko commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2102669940)
Just to avoid to take the lock again. You suggestion is also fine. Happy to push it, just let me know.
💬 maflcko commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2102672146)
the lock is recursive and all callers already held it, except for one, which is handled in the next commit: https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2100887156
💬 pinheadmz commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2102678855)
Ah thanks, good justification, thats good 👍
💬 hebasto commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2901391171)
> > I've restored the `shell` option in the `subprocess` API, and employed it on non-Windows systems.
>
> Hmm the shell support implemented in [eb16060](https://github.com/bitcoin/bitcoin/commit/eb160602a50bebcca3f53cdae741e1732738d51a) actually seems somewhat broken as it is still splitting and joining instead of preserving the original string passed to Popen() instead of just passing it unaltered to execvp as I was suggesting.

Thanks! Implemented as you suggested.

(no further clean up
...