-
Notifications
You must be signed in to change notification settings - Fork 355
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
Handle duplicate members in cw4-group create #702
Conversation
@codehans will you pick up that PR and add some proper test? |
fwiw - i do think this could be considered to be a non-backwards compatible change. for example, before our audit (see the first issue there) we did not dedup the members list when instantiating a dao that uses cw4-groups. we now do, but were this to land we'd probably stop doing that to be consistent with the new behavior. |
apologies @ueco-jb @ezekiiel missed this one. this isn't something we're actively using or particularly precious about. happy to leave as-is, especially as time is fairly scarce these days |
This looks like a serious flaw. If you don't mind, I'll pick this up, rebase, add a test, and merge. @codehans do you mind if I push here? |
@0xekez That's fair! I don't think we guarantee backwards-compatibility currently ( |
go ahead! |
@codehans Would you mind signing the CLA thing? I'll need that to merge later. |
Actually, I'm going to have duplicate entries throw an error since it's not clear what outcome the user expected. As things stand, this PR would cause instantiation and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm with error on duplicates
When a cw4-group contract is instantiated with a list of members where some are duplicated, the iterator overwrites any previous member weight, whilst still incrementing the total weight. This PR merges duplicate weights in the
create
function