Skip to content

test: redistribution integration #1415

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

Merged
merged 18 commits into from
May 30, 2025

Conversation

0xClandestine
Copy link
Member

@0xClandestine 0xClandestine commented May 28, 2025

Motivation:

We want integration test coverage for the redistribution relaese.

Modifications:

  • Fixed view revert bug (see comment bellow).
  • Added timing tests (cannot release escrow before delay, can after)

Result:

Integration test coverage.

@0xClandestine 0xClandestine changed the title fix: use tryGet in view methods to avoid reverts test: redistribution integration May 28, 2025
Comment on lines -467 to +469
shares[i] = burnOrRedistributableShares.get(keys[i]);
(, shares[i]) = burnOrRedistributableShares.tryGet(keys[i]);
Copy link
Member Author

Choose a reason for hiding this comment

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

There was a small bug present with the getBurnOrRedistributableShares getters. EnumerableMap.get() reverts if a key does not exist.

Copy link
Collaborator

@ypatil12 ypatil12 left a comment

Choose a reason for hiding this comment

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

Minor comments

}

function testFuzz_fullSlash_EscrowTiming_RightAfterPasses(uint24 _random) public rand(_random) {
// 6) Operator is full slashed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we do random? Also, this can be in the init step since you do it for all of these tests

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see why you're slashing full -> to not have to re-calculate the amount they would get out? I guess we can add in a separate PR to not block this

Copy link
Member Author

Choose a reason for hiding this comment

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

could maybe do a slash half

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can still do random if get_slashing_rand returns the shares out

Copy link
Member Author

Choose a reason for hiding this comment

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

will improve in next pr

Copy link
Collaborator

@ypatil12 ypatil12 left a comment

Choose a reason for hiding this comment

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

LGTM

@0xClandestine 0xClandestine merged commit cb68310 into release-dev/redistribution May 30, 2025
10 of 11 checks passed
@0xClandestine 0xClandestine deleted the test/integration branch May 30, 2025 15:29
0xClandestine added a commit that referenced this pull request May 30, 2025
**Motivation:**

We want integration test coverage for the redistribution relaese.

**Modifications:**

- Fixed view revert bug (see comment bellow).
- Added timing tests (cannot release escrow before delay, can after)

**Result:**

Integration test coverage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants