Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: fix test of withdrawal for more than 1000 dash #6141

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

knst
Copy link
Collaborator

@knst knst commented Jul 22, 2024

Issue being fixed or feature implemented

DIP for Credit Pool says:

The withdrawal should not be mined if:
* It requests more DASH than the credit pool contains
* It requests more than 1000 DASH
* The credit pool contains more than 1000 DASH, and the withdrawal would result in more than a 10% reduction in the credit pool over the 576-block window
* The credit pool contains less than 1000 DASH, and the withdrawal would result in more than 100 DASH being removed from the pool over the 576-block window

Though, current functional test for asset locks improperly test this case, because threshold for big withdrawal happens by 10%, not 1000 dash.

What was done?

Improvements for functional asset lock test to actually test a limit 1000 dash, not just 10%

How Has This Been Tested?

See changes

Breaking Changes

N/A, changes only for tests

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

@knst knst added this to the 21.1 milestone Jul 22, 2024
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

pls check 2bbf41f

test/functional/feature_asset_locks.py Outdated Show resolved Hide resolved
@knst knst requested a review from UdjinM6 July 23, 2024 10:17
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

ACK 4f0f22d

Copy link
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 4f0f22d; only changes tests

@UdjinM6
Copy link

UdjinM6 commented Aug 2, 2024

rebased via GH GUI to fix "Check Merge Fast-Forward Only"

@PastaPastaPasta
Copy link
Member

It's now not signed so I can't merge it ;)

@UdjinM6
Copy link

UdjinM6 commented Aug 2, 2024

well, that's better than merging a broken one anyway :D

knst and others added 3 commits August 5, 2024 10:31
Now function test doesn't distint difference between 10% or 1000.
Adjust amounts to make it less than 10% but more than 1000
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.

None yet

3 participants