Skip to content

tests: Enable InstantSend and ChainLocks by default#3853

Merged
PastaPastaPasta merged 2 commits into
dashpay:developfrom
UdjinM6:fixCLlogic_tweaktests
Dec 12, 2020
Merged

tests: Enable InstantSend and ChainLocks by default#3853
PastaPastaPasta merged 2 commits into
dashpay:developfrom
UdjinM6:fixCLlogic_tweaktests

Conversation

@UdjinM6
Copy link
Copy Markdown

@UdjinM6 UdjinM6 commented Dec 5, 2020

That's how mainnet and testnet operate for a long time already, so it makes sense to test this scenario as the default one imo. This is also a preparation for #3845 - it makes sure that everything behaves as expected before any related changes in c++ code.

Note: This PR is expected to fail in multiple tests without #3846

b0b7ab3 failed: https://gitlab.com/dashpay/dash/-/pipelines/225785896

@UdjinM6 UdjinM6 force-pushed the fixCLlogic_tweaktests branch from b0b7ab3 to aa2f62c Compare December 6, 2020 00:15
@UdjinM6 UdjinM6 marked this pull request as ready for review December 6, 2020 16:15
@UdjinM6 UdjinM6 added this to the 17 milestone Dec 6, 2020
@UdjinM6 UdjinM6 marked this pull request as draft December 8, 2020 01:01
@UdjinM6 UdjinM6 changed the title tests: Enable ChainLocks by default tests: Enable InstantSend and ChainLocks by default Dec 8, 2020
@UdjinM6 UdjinM6 force-pushed the fixCLlogic_tweaktests branch from 92caf44 to 49831cb Compare December 8, 2020 09:01
@UdjinM6 UdjinM6 marked this pull request as ready for review December 8, 2020 15:13
@PastaPastaPasta
Copy link
Copy Markdown
Member

Note: This PR is expected to fail in multiple tests without #3684

I don't understand, why? (I think you put in the wrong number?)

Copy link
Copy Markdown

@xdustinface xdustinface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

(I think you put in the wrong number?)

But i also wonder about the PR linked in the description :D Shouldn't it be #3846 (what you had before the edit) instead?

@UdjinM6
Copy link
Copy Markdown
Author

UdjinM6 commented Dec 11, 2020

🙈
fixed

Copy link
Copy Markdown
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@PastaPastaPasta PastaPastaPasta merged commit b95cf01 into dashpay:develop Dec 12, 2020
xdustinface added a commit to xdustinface/dash that referenced this pull request Dec 12, 2020
Current `develop` tests fail. This was basically introduced by dashpay#3844 but it didn't come up before dashpay#3853 because the `v17` fork wasn't activated in `feature_llmq_dkgerrors.py`.

After  dashpay#3853 `dip0008` activation takes [200 blocks](dashpay@b95cf01#diff-4a04bc0b355c780033960e8c261ee9b6d3c452897e1dcd88a15d272512266c76R539) which was normally activated after [10 blocks](dashpay@b95cf01#diff-b92fa0fafafa27172736ebc88f9f9b658b1160caca512a318eefb7d93d22bf3cL18) in `feature_llmq_dkgerrors.py`. Now with the 200 blocks `v17` gets activated during test which then leads to MN1, MN2 banning MN0 because it lies in DKG of `LLMQ_TEST` and `LLMQ_TEST_V17`.

There are other ways to solve it, like enabling `dip0008` earlier or enable `v17` later but IMO its anyway better to restrict `ShouldSimulateError` to only trigger for `LLMQ_TEST`.
UdjinM6 added a commit that referenced this pull request Dec 14, 2020
…3871)

* llmq: Restrict `ShouldSimulateError` to trigger for LLMQ_TEST only

Current `develop` tests fail. This was basically introduced by #3844 but it didn't come up before #3853 because the `v17` fork wasn't activated in `feature_llmq_dkgerrors.py`.

After  #3853 `dip0008` activation takes [200 blocks](b95cf01#diff-4a04bc0b355c780033960e8c261ee9b6d3c452897e1dcd88a15d272512266c76R539) which was normally activated after [10 blocks](b95cf01#diff-b92fa0fafafa27172736ebc88f9f9b658b1160caca512a318eefb7d93d22bf3cL18) in `feature_llmq_dkgerrors.py`. Now with the 200 blocks `v17` gets activated during test which then leads to MN1, MN2 banning MN0 because it lies in DKG of `LLMQ_TEST` and `LLMQ_TEST_V17`.

There are other ways to solve it, like enabling `dip0008` earlier or enable `v17` later but IMO its anyway better to restrict `ShouldSimulateError` to only trigger for `LLMQ_TEST`.

* Revert "llmq: Restrict `ShouldSimulateError` to trigger for LLMQ_TEST only"

This reverts commit ec42d86.

* llmq: Restrict `ShouldSimulateError` to trigger for LLMQ_TEST only (alternative)

Move ShouldSimulateError into CDKGSession

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
@UdjinM6 UdjinM6 deleted the fixCLlogic_tweaktests branch July 1, 2021 21:46
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Mar 11, 2022
* tests: Enable ChainLocks by default

* tests: Enable InstantSend (including filtering) by default
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Apr 1, 2022
…ashpay#3871)

* llmq: Restrict `ShouldSimulateError` to trigger for LLMQ_TEST only

Current `develop` tests fail. This was basically introduced by dashpay#3844 but it didn't come up before dashpay#3853 because the `v17` fork wasn't activated in `feature_llmq_dkgerrors.py`.

After  dashpay#3853 `dip0008` activation takes [200 blocks](dashpay@b95cf01#diff-4a04bc0b355c780033960e8c261ee9b6d3c452897e1dcd88a15d272512266c76R539) which was normally activated after [10 blocks](dashpay@b95cf01#diff-b92fa0fafafa27172736ebc88f9f9b658b1160caca512a318eefb7d93d22bf3cL18) in `feature_llmq_dkgerrors.py`. Now with the 200 blocks `v17` gets activated during test which then leads to MN1, MN2 banning MN0 because it lies in DKG of `LLMQ_TEST` and `LLMQ_TEST_V17`.

There are other ways to solve it, like enabling `dip0008` earlier or enable `v17` later but IMO its anyway better to restrict `ShouldSimulateError` to only trigger for `LLMQ_TEST`.

* Revert "llmq: Restrict `ShouldSimulateError` to trigger for LLMQ_TEST only"

This reverts commit ec42d86.

* llmq: Restrict `ShouldSimulateError` to trigger for LLMQ_TEST only (alternative)

Move ShouldSimulateError into CDKGSession

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants