Skip to content

Voting Manager fix to ignore duplicate collateral members (1.1.0.0)#672

Merged
quantumagi merged 5 commits intostratisproject:release/1.1.0.0from
quantumagi:vmcollfix
Aug 25, 2021
Merged

Voting Manager fix to ignore duplicate collateral members (1.1.0.0)#672
quantumagi merged 5 commits intostratisproject:release/1.1.0.0from
quantumagi:vmcollfix

Conversation

@quantumagi
Copy link
Contributor

@quantumagi quantumagi commented Aug 21, 2021

PR #272 specified that voting, which would otherwise result in adding a member, would not do so if collateral address re-use is detected.

Similarly (this PR) such members should also be omitted when the federation is later re-created from the polls.

On a side-note, a follow-up PR is required to somehow strengthen/fix the VotingRequestValidationRule to prevent such duplicate addresses from appearing in voting requests when already in a voting request, the active federation, pending or approved polls. I.e. these duplications should never reach the VM in the first place.

@quantumagi quantumagi requested a review from fassadlr August 21, 2021 13:50
@quantumagi
Copy link
Contributor Author

image

if (poll.VotingData.Key == VoteKey.AddFederationMember)
federation.Add(federationMember);
{
if (!federation.Any(m => m is CollateralFederationMember colMember && federationMember is CollateralFederationMember colMember2 && colMember.CollateralMainchainAddress == colMember2.CollateralMainchainAddress))
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldnt the federationMember is CollateralFederationMember colMember2 part here be outside of the Any ?

It probs amounts to the same but it reads better and also the type doesnt happen for every member in the federation?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or is this acting as a compare then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot

@quantumagi
Copy link
Contributor Author

quantumagi commented Aug 24, 2021

We need a PR to bump the NBitcoin version.

Some other PR somehow got merged with a change to NBitcoin without bumping the version...

@quantumagi quantumagi merged commit bed9156 into stratisproject:release/1.1.0.0 Aug 25, 2021
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.

2 participants