Merge bitcoin/bitcoin#28354: test: default acceptnonstdtxn=0 on all chains#911
Closed
DashCoreAutoGuix wants to merge 266 commits into
Closed
Merge bitcoin/bitcoin#28354: test: default acceptnonstdtxn=0 on all chains#911DashCoreAutoGuix wants to merge 266 commits into
DashCoreAutoGuix wants to merge 266 commits into
Conversation
… member `IsMiningPhase` be anonymous function
Firstly, production code (even in debug mode) should not operate as unit test by calling heavy GetMN
just to be sure that implementation is not broken.
Secondly, since mn_rr hard-fork is activated it doesn't affect any new blocks; but it is used for
validation of old blocks only.
Thirdly, it removes useless log record (especially now when no more 4x payments for Evo Nodes).
2025-05-31T19:40:51.578629Z (mocktime: 2014-12-04T17:29:28Z) [httpworker] [evo/deterministicmns.cpp:1023] [BuildNewListFromBlock] [mnpayments] CDeterministicMNManager::BuildNewListFromBlock -- MN ae67835776b01b4ad9b153c829bf30fe70e8c3699a09ddcebe431712c38cee63, nConsecutivePayments=0
Firstly, it's easier to spot changes in mocktime and calculate how long if time is logged in seconds. Secondly, the start time `2014-12-04T17:15:37Z` (genesis block) is not round number which makes calculation of "how much exactly time pass" even more difficult.
Going through the vin and then adding the vin index as a vout position shouldn't work, especially if there are an unequal number of inputs and outputs but the test transactions tend to have spend patterns that allow the code to inadvertently work, evading detection. A new test has been added that uses a spend pattern that would've netted unexpected behavior in the previous logic. Also, we can just use `AddChildrenToWorkSet` for this because it effectively implements the same logic (going through vout, finding and inserting).
Both functions that manipulate it, `AddTx()` and `LimitOrphans()` hold the mutex.
This commit will not compile as-is as block based orphan reprocessing, a Dash-ism, has not been adapted to work with this refactor. That has been segmented out into a separate commit to aid in review.
…ate datadir fb0dbe9 docs: GetDataDirNet and GetDataDirBase don't create datadir (stickies-v) Pull request description: Since bitcoin#27073, the behaviour of `GetDataDir()` [changed](https://github.com/bitcoin/bitcoin/pull/27073/files#diff-19427b0dd1a791adc728c82e88f267751ba4f1c751e19262cac03cccd2822216L435-L443) to only return the datadir path, but not create it if non-existent. This also changed the behaviour of `GetDataDirNet()` and `GetDataDirBase()` but the docs do not yet reflect that. ACKs for top commit: TheCharlatan: ACK fb0dbe9 theStack: ACK fb0dbe9 willcl-ark: ACK fb0dbe9 Tree-SHA512: 3f10f4871df59882f3649c6d3b2362cae2f8a01ad0bd0c636c5608b0d177d279a2e8712930b819d6d3912e91fa6447b9e54507c33d8afe427f7f39002b013bfb
…wit related code
Co-authored-by: fanquake <fanquake@gmail.com>
Co-authored-by: fanquake <fanquake@gmail.com> Co-authored-by: pasta <pasta@pastas-Mac-Studio.local>
…and logging Co-authored-by: fanquake <fanquake@gmail.com> Co-authored-by: pasta <pasta@pastas-Mac-Studio.local>
… net(base).cpp Co-authored-by: fanquake <fanquake@gmail.com>
…back Co-authored-by: fanquake <fanquake@gmail.com>
…ntrol / sendcoins dialog Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
…our headers Co-authored-by: fanquake <fanquake@gmail.com>
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Co-authored-by: fanquake <fanquake@gmail.com>
Co-authored-by: glozow <gloriajzhao@gmail.com>
…achChain fails Co-authored-by: fanquake <fanquake@gmail.com>
Co-authored-by: fanquake <fanquake@gmail.com>
Co-authored-by: fanquake <fanquake@gmail.com> Co-authored-by: Claude Code <claude@anthropic.com>
Co-authored-by: fanquake <fanquake@gmail.com>
Co-authored-by: fanquake <fanquake@gmail.com>
Co-authored-by: fanquake <fanquake@gmail.com>
1c8c746 test: remove unused EncodeDecimal from test framework util (pasta) 4ffee8c Merge bitcoin#28014: ci: re-enable gui tests for s390x (PastaBot) 479c23b Merge bitcoin#29481: doc: Update OpenBSD build docs for 7.4 (PastaBot) 265eebd Merge bitcoin#29470: test: Add option to skip python unit tests (PastaBot) 33a294a Merge bitcoin#29469: doc: document preference for list-initialization (PastaBot) 7bb828b Merge bitcoin#29456: docs: ci multi-arch requires qemu (PastaBot) 6640a79 Merge bitcoin#29243: wallet: Reset chain notifications handler if AttachChain fails (PastaBot) e26ae22 Merge bitcoin#29235: doc: refer to "Node relay options" in policy/README (PastaBot) cbd2802 Merge bitcoin#29237: depends: Allow PATH with spaces in directory names. (PastaBot) 92742d8 Merge bitcoin#27928: test: Add more tests for the BIP21 implementation (PastaBot) d6a4ff5 Merge bitcoin#28040: wallet: sqlite: don't include sqlite files from our headers (PastaBot) 0a4441e Merge bitcoin-core/gui#719: Remove confusing "Dust" label from coincontrol / sendcoins dialog (PastaBot) 23c0ea2 Merge bitcoin#28011: test: Rename EncodeDecimal to serialization_fallback (PastaBot) 8b77a80 Merge bitcoin#27530: Remove now-unnecessary poll, fcntl includes from net(base).cpp (PastaBot) 5030a4d Merge bitcoin#27947: MaybePunishNodeForTx: Remove unused message arg and logging (PastaBot) 56686df Merge bitcoin#27949: http: update libevent workaround to correct version (PastaBot) c060a2a Merge bitcoin#27689: doc: remove mention of glibc 2.10+ (PastaBot) Pull request description: ## Issue being fixed or feature implemented Batch of automatically done backports by claude. ## What was done? See commits ## How Has This Been Tested? CI ## Breaking Changes None ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 1c8c746 Tree-SHA512: 8b9487375093de53dca15b15eadfb7a34b6319dd367bd204d71e0407316db891f71ba33f97327ed021423ee060cd7f095313018d80b3e526527c43ea0f8cb167
…block filters (BIP-157/158) 4b90441 docs: indicate that blockfilter will be re-created (PastaPastaPasta) b2522a2 chore: make clang format happy (UdjinM6) 09490b7 chore: drop useless feature_blockfilter_version.py (UdjinM6) 4750246 doc: shrink release notes (UdjinM6) f28bf6d feat: start blockfilter sync from scratch on db version upgrades (UdjinM6) d3f1b9e test: remove wallet dependency check from blockfilter tests (pasta) 492c109 feat: add versioning to blockfilter index to detect incompatible format (pasta) bf352e7 evo: use Span<const unsigned char> in specialtx filter extraction callback to avoid temporary allocations; adjust helpers and call site (pasta) c734b64 tests: p2p_blockfilters: use assert_greater_than for clearer failures (pasta) a5bab55 test: remove wallet dependency from p2p_blockfilters.py and refactor duplicated code (Konstantin Akimov) a1ecf5d tests: p2p_blockfilters: skip when wallet module is not available (pasta) 9266b9a refactor: break circular dependency over evo/assetlock and llmq/signing (Konstantin Akimov) 1c9809f filters: add cross-reference comments to keep bloom and compact filter extraction in sync (pasta) 7a33c14 tests: compact filters: fix F841, verify tx inclusion, and cleanups (pasta) 3fb9183 test: improve p2p_blockfilters special transaction tests (pasta) 8d08a54 Apply suggestions from code review (PastaPastaPasta) 0ea85f7 fix: remove unused variable in blockfilter_tests (pasta) 773fe9b lint: add specialtx_filter circular dependencies to allowed list (pasta) 8ae2280 feat: add special transaction support to compact block filters (BIP-157/158) (pasta) Pull request description: ## Summary - Implements extraction of special transaction fields for compact block filters - Achieves feature parity between bloom filters and compact block filters for special transactions - Enables SPV clients to detect special transactions using either filtering mechanism ## What this PR does This PR adds support for Dash special transactions to the compact block filter implementation (BIP-157/158). Previously, only bloom filters could detect special transaction fields, leaving compact block filter users unable to track masternode-related transactions and platform operations. ### Implementation Details The implementation uses a delegation pattern to extract special transaction fields without creating circular dependencies: - `ExtractSpecialTxFilterElements()` function extracts relevant fields from each special transaction type - Fields extracted match exactly those in the bloom filter implementation (`CheckSpecialTransactionMatchesAndUpdate`) - All 5 special transaction types are supported: - **ProRegTx**: collateral outpoint, owner/voting keys, payout script - **ProUpServTx**: ProTx hash, operator payout script - **ProUpRegTx**: ProTx hash, voting key, payout script - **ProUpRevTx**: ProTx hash - **AssetLockTx**: credit output scripts (excluding OP_RETURN) ### Testing Comprehensive test coverage includes: - **Unit tests**: Test all 5 special transaction types with realistic payloads - **Functional tests**: Create actual AssetLockTx transactions with proper signatures - **Edge cases**: Empty scripts, OP_RETURN exclusion, multiple outputs ## Test plan - [x] Unit tests pass (`make check`) - [x] Functional test passes (`test/functional/p2p_blockfilters.py`) - [x] All special transaction types covered in tests - [x] Verified special fields are added to filters (via debug logging) 🤖 Generated with [Claude Code](https://claude.ai/code) ACKs for top commit: knst: utACK 4b90441 UdjinM6: utACK 4b90441 Tree-SHA512: 673b47ef8626761cdffc659c6ca0ffd214b6d3e7ea6086976528ec79967f5a6a6d864b63cc1e5bac38f079fa092c49637f334c058231b4f6c687f9dae585db27
cfc7b8f to
57884f0
Compare
Owner
Author
|
🔧 Validation FIXED: Applied 1 fix Fixes applied:
This issue was causing build failures across all CI platforms. Dash Core uses bitcoin-config.h, not dash-config.h for configuration headers. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backports bitcoin#28354
Original commit: 5175ae4
Backported from Bitcoin Core v0.26
Notes
This backport is smaller than the Bitcoin commit because Dash doesn't have the kernel architecture files that Bitcoin modified (src/kernel/chainparams.*, src/kernel/mempool_options.h, src/node/mempool_args.cpp). The core change is updating the acceptnonstdtxn argument to use DEFAULT_ACCEPT_NON_STD_TXN instead of the dynamic check.
Summary by CodeRabbit
Documentation
Style
Bug Fixes
Chores