fix: early EHF and buried EHF are indistinguish#6434
Merged
PastaPastaPasta merged 6 commits intodashpay:developfrom Dec 3, 2024
Merged
fix: early EHF and buried EHF are indistinguish#6434PastaPastaPasta merged 6 commits intodashpay:developfrom
PastaPastaPasta merged 6 commits intodashpay:developfrom
Conversation
|
Guix Automation has began to build this PR tagged as v22.1.0-devpr6434.c6bb9a56. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v22.1.0-devpr6434.c6bb9a56. The image should be on dockerhub soon. |
|
I like changes in |
UdjinM6
reviewed
Nov 27, 2024
Co-authored-by: UdjinM6 <[email protected]>
Collaborator
Author
but you don't need full re-index (with rescanning UTXO, etc). This simple update of EHF signal takes seconds (or minutes on slow disk). |
|
oh, I get it now. ok, makes sense |
UdjinM6
reviewed
Nov 27, 2024
Co-authored-by: UdjinM6 <[email protected]>
UdjinM6
requested changes
Nov 28, 2024
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
Seem to be working but it reindexes ehf every time you start a node. Also, if you do getblockchaininfo then it reindexes once again and crashes when you stop your node. UdjinM6@2615bcd fixes it.
Collaborator
Author
should not we safe this flag before change? Or it is guaranteed to be false before this method? |
|
|
UdjinM6
previously approved these changes
Nov 30, 2024
Comment on lines
+383
to
+393
| const bool last_legacy = bls::bls_legacy_scheme.load(); | ||
| bls::bls_legacy_scheme.store(false); | ||
| GetSignalsStage(m_chainman->ActiveChainstate().m_chain.Tip()); | ||
| bls::bls_legacy_scheme.store(last_legacy); | ||
|
|
||
| dbTx->Commit(); | ||
| // flush it to disk | ||
| if (!m_evoDb.CommitRootTransaction()) { | ||
| LogPrintf("CMNHFManager::%s -- failed to commit to evoDB\n", __func__); | ||
| return false; | ||
| } |
Member
There was a problem hiding this comment.
Shouldn't we lock cs_main during all this to avoid chain advancing between bls::bls_legacy_scheme.load and bls::bls_legacy_scheme.store?
Collaborator
Author
There was a problem hiding this comment.
it's already locked in init.cpp
try {
LOCK(cs_main);
...
if (!node.mnhf_manager->ForceSignalDBUpdate()) {
strLoadError = _("Error upgrading evo database for EHF");
break;
}maybe annotation should be add? but that's a very specific method that is supposed to be called from initialization code only, not for regular usage, doesn't seems as necessary
Member
There was a problem hiding this comment.
Let's please add annotations
UdjinM6
approved these changes
Dec 2, 2024
PastaPastaPasta
added a commit
to PastaPastaPasta/dash
that referenced
this pull request
Dec 3, 2024
4629bb9 fix: add missing cs_main annotation for ForceSignalDBUpdate (Konstantin Akimov) 05041a4 fix: force ehf signal db update (UdjinM6) 94d8032 fix: typo name of key (Konstantin Akimov) 9ceba88 style: clang suggestion (Konstantin Akimov) c6bb9a5 perf: re-use evo data about signals between v20 and mn_rr as non-corrupted (Konstantin Akimov) 7a7c9f1 fix: early EHF and buried EHF are indistinguish (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented It seems as EHF signal will be mined before node is updated, this signal is lost and node can't activate hard-fork anymore. ## What was done? EHF signals doesn't expire anymore. To avoid full re-index key in database is changed. Client with enabled "pruned mode" will be required to do re-index. Alternate solution - revert this commit 4b046bb and introduce time-out for expiring EHF signals. ## How Has This Been Tested? Test on my local instance with testnet and mainnet. Testing on miner-1 on testnet is done. First start of miner took 50 seconds, 29 of them the node was re-scanning blockchain and looking for EHF transaction ## Breaking Changes It requires re-index for nodes with enabled pruning of blocks. ## Checklist: - [ ] 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: PastaPastaPasta: utACK [4629bb9](dashpay@4629bb9) UdjinM6: utACK 4629bb9 Tree-SHA512: 189533da5726edbcf2d9cf0e9a3957a10ebc223c25fd88aec3aa9095ae2e7d955ea1f7a1384bc2c97a0cc06110c9e38845a8cafdbd56ff9637bb907ddc639850
Member
|
backported in #6447 |
PastaPastaPasta
added a commit
that referenced
this pull request
Dec 3, 2024
c7b0d80 Merge #6441: fix: hold wallet shared pointer in CJ Manager/Sessions to prevent concurrent unload (pasta) c074e09 Merge #6444: fix: add platform transfer to "most common" filter (pasta) cb04114 Merge #6442: fix: coin selection with `include_unsafe` option should respect `nCoinType` (pasta) db5b53a Merge #6434: fix: early EHF and buried EHF are indistinguish (pasta) Pull request description: ## Issue being fixed or feature implemented See commits; backports for rc.3 ## What was done? ## How Has This Been Tested? built locally ## Breaking Changes ## 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: kwvg: ACK c7b0d80 UdjinM6: utACK c7b0d80 Tree-SHA512: a64d6503a845ea86df8660d34cdf819c6fefcae5e82035bd8de40152f4f7d894cd1870315b791cca81e6d4db08d9929e4d1de3338a0338478072c9e6bb66952a
PastaPastaPasta
added a commit
to PastaPastaPasta/dash
that referenced
this pull request
Dec 14, 2024
1c7bfcb chore: set release true (pasta) c90339e Merge dashpay#6459: docs: add release notes for v22.0.0 (pasta) a6f1fc5 Merge dashpay#6475: chore: bumped chain assumed sizes based on latest usage (pasta) d7cd9f1 Merge dashpay#6464: chore: update man pages for v22 (pasta) 212f91c Merge dashpay#6461: docs: update supported versions in SECURITY.md (pasta) 9a8b685 Merge dashpay#6460: chore: Translations 2024-12 (pasta) 2f71f4d Merge dashpay#6458: chore: bump MIN_MASTERNODE_PROTO_VERSION to latest proto (pasta) fa29ed5 Merge dashpay#6456: fix(qt): allow refreshing wallet data without crashing (pasta) 758cd64 Merge dashpay#6452: fix: store ready queues on the mixing masternode (pasta) 395447b Merge dashpay#6451: depends: update 'src/dashbls' to dashpay/bls-signatures@7e747e8a as 62fa665 (pasta) c7b0d80 Merge dashpay#6441: fix: hold wallet shared pointer in CJ Manager/Sessions to prevent concurrent unload (pasta) c074e09 Merge dashpay#6444: fix: add platform transfer to "most common" filter (pasta) cb04114 Merge dashpay#6442: fix: coin selection with `include_unsafe` option should respect `nCoinType` (pasta) db5b53a Merge dashpay#6434: fix: early EHF and buried EHF are indistinguish (pasta) 8b88ff7 Merge dashpay#6414: chore: bump seeds for v22 (pasta) 02ad523 Merge dashpay#6411: chore: update nMinimumChainWork, defaultAssumeValid, checkpointData, chainTxData for mainnet and testnet (pasta) 3bbcd3d Merge dashpay#6393: docs: mention building for some HOSTs only in `release-process.md` (pasta) 18f636f Merge dashpay#6426: fix: respect SENDDSQUEUE message, move DSQ relay into net processing / peerman (pasta) 9fed456 Merge dashpay#6407: fix: dataraces (pasta) 86105da Merge dashpay#6408: refactor: removed pre-MN_RR logic of validation of CL (pasta) a1f7e96 Merge dashpay#6406: ci: use `actions/cache` to manage depends cache (pasta) 90a3807 Merge dashpay#6402: ci: cache built (pasta) 66f6787 Merge dashpay#6401: ci: deduplicate depends building (pasta) 7ca5663 Merge dashpay#6397: ci: add powerpc64 to GH Guix job matrix (pasta) Pull request description: ## Issue being fixed or feature implemented ## What was done? Suppressed changes from 1c7bfcb and resolved merge conflicts. ``` Auto-merging .github/workflows/build.yml Auto-merging configure.ac Auto-merging src/chainparams.cpp Auto-merging src/coinjoin/client.cpp CONFLICT (content): Merge conflict in src/coinjoin/client.cpp Auto-merging src/coinjoin/client.h CONFLICT (content): Merge conflict in src/coinjoin/client.h Auto-merging src/coinjoin/util.cpp CONFLICT (content): Merge conflict in src/coinjoin/util.cpp Auto-merging src/coinjoin/util.h CONFLICT (content): Merge conflict in src/coinjoin/util.h Auto-merging src/evo/specialtxman.cpp Auto-merging src/init.cpp Auto-merging src/net_processing.cpp CONFLICT (content): Merge conflict in src/net_processing.cpp Auto-merging src/net_processing.h Auto-merging src/qt/transactiontablemodel.cpp Auto-merging src/wallet/wallet.cpp Auto-merging src/wallet/wallet.h CONFLICT (content): Merge conflict in src/wallet/wallet.h Auto-merging test/functional/feature_llmq_chainlocks.py CONFLICT (content): Merge conflict in test/functional/feature_llmq_chainlocks.py ``` ## How Has This Been Tested? ## Breaking Changes ## Checklist: - [ ] 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 - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK d108579; no diff to develop Tree-SHA512: 3f063011224880fee35edb04ce265dff33a52273c3d45ef1dbcebcecb22c25d8ad7c91b83514f36142716a6fbd0ddd3a8a3f2a9b59ce78ce975bbce69a2a13b5
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
4 participants
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.
Issue being fixed or feature implemented
It seems as EHF signal will be mined before node is updated, this signal is lost and node can't activate hard-fork anymore.
What was done?
EHF signals doesn't expire anymore.
To avoid full re-index key in database is changed.
Client with enabled "pruned mode" will be required to do re-index.
Alternate solution - revert this commit 4b046bb and introduce time-out for expiring EHF signals.
How Has This Been Tested?
Test on my local instance with testnet and mainnet.
Testing on miner-1 on testnet is done.
First start of miner took 50 seconds, 29 of them the node was re-scanning blockchain and looking for EHF transaction
Breaking Changes
It requires re-index for nodes with enabled pruning of blocks.
Checklist: