Skip to content

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
@knst knst requested review from PastaPastaPasta and UdjinM6 July 22, 2024 20:05
@knst knst force-pushed the tests-1000-limit branch from 45949cc to 4653789 Compare July 23, 2024 05:26
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

@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 4f0f22ddec5738aac61c89ea11b67c06fe7c4ded

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 4f0f22ddec5738aac61c89ea11b67c06fe7c4ded; only changes tests

@knst knst force-pushed the tests-1000-limit branch from 4f0f22d to bcdb696 Compare August 2, 2024 15:42
@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
@knst knst force-pushed the tests-1000-limit branch from 2d2c05a to f22ade3 Compare August 5, 2024 03:31
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 f22ade3

@PastaPastaPasta PastaPastaPasta merged commit 9b03903 into dashpay:develop Aug 5, 2024
@knst knst deleted the tests-1000-limit branch August 5, 2024 10:51
@UdjinM6 UdjinM6 modified the milestones: 21.1, 21.2 Aug 8, 2024
@UdjinM6 UdjinM6 modified the milestones: 21.2, 22 Oct 29, 2024
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