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

Add Member::expelled #454

Merged
merged 5 commits into from
Aug 7, 2023
Merged

Add Member::expelled #454

merged 5 commits into from
Aug 7, 2023

Conversation

sigridjineth
Copy link
Member

@sigridjineth sigridjineth commented Jul 21, 2023

Accomplished

  • Add Member::expelled in a boolean type
  • Verify reserved_state in apply_commit()
  • Remove unnecessary state change check for delegation
  • Elaborate consensus_leader_order check logic with additional conditions
  • Change logic for ensuring the uniqueness of member names
  • Change logic for ensuring monotonic increment in member names
  • Remove the previous test and add tests for the new validation logic
  • Parses the version field of the current and new reserved states into semver::Version objects
  • Compare two versions in that the new version is greater than the previous one if they are not the same

Shout-out to collaborators

@sejongk @yuhatown and @sigridjineth

Issues

This pull request hopes to close #164.

@sigridjineth sigridjineth self-assigned this Jul 21, 2023
@sigridjineth sigridjineth force-pushed the feat/#164 branch 3 times, most recently from a517290 to bcc5eee Compare July 22, 2023 05:02
core/src/verify.rs Outdated Show resolved Hide resolved
core/src/verify.rs Outdated Show resolved Hide resolved
core/src/verify.rs Outdated Show resolved Hide resolved
core/src/verify.rs Outdated Show resolved Hide resolved
@@ -1508,6 +1557,61 @@ mod test {
.unwrap_err();
}

#[test]
fn test_verify_reserved_state_expelled_member() {
// Given: Configuration for the test
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be better to change the format of comments below, for example, just leave a sentence with additional explanation rather than use Given: or When: .

@sigridjineth sigridjineth requested a review from JeongHunP July 22, 2023 14:09
@sigridjineth sigridjineth marked this pull request as draft July 23, 2023 13:19
@sigridjineth sigridjineth force-pushed the feat/#164 branch 4 times, most recently from be32ea9 to 29888e3 Compare July 30, 2023 14:28
@sigridjineth sigridjineth marked this pull request as ready for review July 30, 2023 14:31
@sigridjineth sigridjineth force-pushed the feat/#164 branch 2 times, most recently from 00be04d to e5a1ebe Compare August 3, 2023 12:23
sigridjineth and others added 5 commits August 7, 2023 23:29
write tests to verify reserved state with expelled members and/or non-expelled members
refactor test_push_eligibility to elaborate using Member::expelled
add expelled field to Member struct in all of the existing tests

Add validation logic for non-mutated delegation state

rename Member to member to make it lower cases
rewrite comments for test_verify_reserved_state
Remove unnecessary state change check for delegation

Change validation login in verify_reserved_state()
Write tests that collects voting power after filtering expelled members
@junha1 junha1 merged commit ee433e1 into postech-dao:main Aug 7, 2023
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.

Add Member::expelled
5 participants