refactor: deprecate non-deterministic IS support by ogabrielides · Pull Request #5553 · dashpay/dash

ogabrielides · 2023-08-30T15:34:56Z

Issue being fixed or feature implemented

Non-deterministic IS locks aren't used anymore since v18 dip24.
We should drop that support to make code simpler.

What was done?

Dropped non-deterministic IS code, evo_instantsend_tests and feature_llmq_is_migration.py (don't need it anymore), adjusted func tests.

How Has This Been Tested?

all tests, synced Testnet

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)

github-actions · 2023-10-23T15:49:08Z

This pull request has conflicts, please rebase.

UdjinM6

pls see d9b67c2

note: it's rebased on top of #5654 and #5655. I reindexed on testnet and mainnet with no issues. It's still incomplete though, check FIXME.

ogabrielides · 2023-10-31T12:19:05Z

@UdjinM6 Since #4917 connections are ensured among IS quorums: adjusted feature_llmq_connections.py to this logic since with this PR, only llmq_test_dip0024 provide IS functionality in regtest.
Also, borrowed fix from #5657.

UdjinM6

LGTM and seems to be working correctly (client-side) on mainnet/testnet

light ACK

github-actions · 2023-11-10T14:34:18Z

This pull request has conflicts, please rebase.

UdjinM6

rebase looks clean, utACK

UdjinM6

re-utACK

github-actions · 2023-11-11T10:18:44Z

This pull request has conflicts, please rebase.

github-actions · 2023-11-17T16:09:05Z

This pull request has conflicts, please rebase.

Co-Authored-By: UdjinM6 <UdjinM6@users.noreply.github.com>

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

Co-Authored-By: Konstantin Akimov <545784+knst@users.noreply.github.com>

UdjinM6

re-utACK

knst

re-utACK

PastaPastaPasta

utACK for squash merge

PastaPastaPasta · 2024-01-10T16:37:59Z

This PR resulted in a breaking change via the removal of option llmqtestinstantsend

Related to dashpay/dash#5553

* docs(p2p): move islock to deprecated message type table Related to dashpay/dash#5553 * docs: minor table formatting

…p in feature_llmq_is_retroactive.py c54c43a test: refactor 24 to a new constant llmq_cycle_len; the cycle_length renamed to llmq_cycle_len too (Konstantin Akimov) 9a6be1b test: rename check_no_is and sleep_and_check_no_is to more descriptive assert_no_instantlock (Konstantin Akimov) 1661495 refactor: default sleep for sleep_and_check_no_is (UdjinM6) 7336573 test: simplify helper check_if_no_islock (Konstantin Akimov) e3c7866 test: fix missing assert in feature_llmq_is_retroactive.py (Konstantin Akimov) 32d35b1 test: replace argument is_first to global flag to see if cycle quorum is ready (Konstantin Akimov) bfa3574 test: remove double delay 2second when it is sufficient to wait once in feature_llmq_is_retroactive.py (Konstantin Akimov) 2a77568 test: mine extra instant send cycle quorum, not just some quorums (Konstantin Akimov) 9c64d6c test: add helper create_fund_sign_tx for feature_llmq_is_retroactive.py (Konstantin Akimov) b91007b test: combine 2 cases of single node & all nodes in feature_llmq_is_retroactive.py (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented `feature_llmq_is_retroactive.py` is one of the slowest functional test and it has bunch of duplicated logic for "single node" and "all nodes" logic. ## What was done? - Refactored case of "single node" and "all nodes" to test simultaneously instead of doing 1-by-1 It reduced amount of code and extra delays and improvement performance. - This PR fixes feature_llmq_is_retroactive.py test by generating real extra quorum for instant send (previously wrong type of quorum has been generated). Issue has been introduced by #5553 when only 1 helper mine_quorum has been replaced to proper rotation quorum. - This PR introduces a minor refactoring `mine_cycle_quorum` to prevent a perf bug when 24*3 blocks generated again and again for sub-sequent quorums (they are needed only once) when this helper is misused. ## How Has This Been Tested? Run 20 parallel jobs. Median time for `feature_llmq_is_retroactive.py` is decreased from 134s to just 103s on my localhost. Median time for `interface_zmq_dash.py` got 3seconds boost (54s -> 51s) as expected, see related `mine_cycle_quorum` helper. ## Breaking Changes N/A ## Checklist: - [x] 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 ACKs for top commit: PastaPastaPasta: utACK c54c43a UdjinM6: utACK c54c43a Tree-SHA512: 3a886612a112b439546565e64a72530328ada3d7ae11c66a4a2db761f006c72c8c58b89c0a49b12c0bc0a0a764b54fa240724240b718fdd7ac285238f297ca4e

ogabrielides added this to the 20 milestone Aug 30, 2023

ogabrielides requested review from PastaPastaPasta and UdjinM6 August 30, 2023 15:35

ogabrielides marked this pull request as draft August 31, 2023 10:49

ogabrielides changed the title ~~chore: drop feature_is_migration~~ refactor: deprecate non-deterministic IS support Oct 10, 2023

github-actions Bot added the needs rebase label Oct 23, 2023

github-actions Bot removed the needs rebase label Oct 29, 2023

UdjinM6 requested changes Oct 30, 2023

View reviewed changes

ogabrielides marked this pull request as ready for review October 31, 2023 12:23

ogabrielides requested a review from UdjinM6 October 31, 2023 12:23

UdjinM6 reviewed Nov 1, 2023

View reviewed changes

Comment thread test/functional/feature_llmq_is_cl_conflicts.py Outdated

ogabrielides modified the milestones: 20, 20.1 Nov 1, 2023

ogabrielides requested a review from UdjinM6 November 1, 2023 11:21

UdjinM6 previously approved these changes Nov 1, 2023

View reviewed changes

knst reviewed Nov 9, 2023

View reviewed changes

Comment thread src/llmq/instantsend.cpp Outdated

github-actions Bot added the needs rebase label Nov 10, 2023

ogabrielides dismissed UdjinM6’s stale review via 864b679 November 10, 2023 15:59

github-actions Bot removed the needs rebase label Nov 10, 2023

ogabrielides requested review from UdjinM6 and knst November 10, 2023 16:04

UdjinM6 previously approved these changes Nov 10, 2023

View reviewed changes

Comment thread src/llmq/instantsend.cpp Outdated

ogabrielides dismissed UdjinM6’s stale review via 6a1480c November 10, 2023 16:20

ogabrielides requested a review from UdjinM6 November 10, 2023 16:20

UdjinM6 previously approved these changes Nov 10, 2023

View reviewed changes

github-actions Bot added the needs rebase label Nov 11, 2023

github-actions Bot added the needs rebase label Nov 17, 2023

ogabrielides and others added 9 commits November 17, 2023 18:49

deprecate non-deterministic islocks

38222f8

suggestions

cc392a3

adapations for feature_llmq_connections.py

82d3403

linter fix

d4e1c54

moved rotation and added comment

cccb224

suggestions

24b34e7

Co-Authored-By: UdjinM6 <UdjinM6@users.noreply.github.com>

Update src/llmq/instantsend.cpp

9efbfa8

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

rebase fix

dc2a121

suggestions

e638575

Co-Authored-By: Konstantin Akimov <545784+knst@users.noreply.github.com>

ogabrielides dismissed stale reviews from UdjinM6 and knst via e638575 November 17, 2023 16:57

github-actions Bot removed the needs rebase label Nov 17, 2023

ogabrielides requested review from UdjinM6 and knst November 17, 2023 17:08

UdjinM6 approved these changes Nov 17, 2023

View reviewed changes

knst approved these changes Nov 17, 2023

View reviewed changes

thephez added the P2P Some notable changes on p2p level label Nov 20, 2023

PastaPastaPasta approved these changes Nov 20, 2023

View reviewed changes

PastaPastaPasta merged commit 1125649 into dashpay:develop Nov 20, 2023

ogabrielides deleted the drop_is_migration_test branch November 20, 2023 16:21

thephez added a commit to thephez/docs-core that referenced this pull request Feb 19, 2024

docs(p2p): move islock to deprecated message type table

6c55116

Related to dashpay/dash#5553

thephez added a commit to dashpay/docs-core that referenced this pull request Feb 19, 2024

docs(p2p): islock deprecation (#100)

c205841

* docs(p2p): move islock to deprecated message type table Related to dashpay/dash#5553 * docs: minor table formatting

UdjinM6 mentioned this pull request Feb 29, 2024

docs: add release notes for 20.1.0 #5910

Merged

5 tasks

thephez mentioned this pull request Mar 4, 2024

docs(p2p): islock deprecation, 2 dashpay/docs-core#102

Merged

thephez added a commit to thephez/docs-core that referenced this pull request Mar 5, 2024

docs(p2p): islock deprecation (dashpay#100)

4d47c5f

* docs(p2p): move islock to deprecated message type table Related to dashpay/dash#5553 * docs: minor table formatting

knst mentioned this pull request Aug 19, 2025

test: reduce waste quorum generation and duplicated sleep in feature_llmq_is_retroactive.py #6824

Merged

5 tasks

Uh oh!

Conversation

ogabrielides commented Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Uh oh!

github-actions Bot commented Oct 23, 2023

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

ogabrielides commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Nov 10, 2023

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 11, 2023

Uh oh!

github-actions Bot commented Nov 17, 2023

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

knst left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta commented Jan 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ogabrielides commented Aug 30, 2023 •

edited

Loading

ogabrielides commented Oct 31, 2023 •

edited

Loading