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

Can not join the dao with a 0x0 address #285

Merged
merged 2 commits into from
May 17, 2021
Merged

Can not join the dao with a 0x0 address #285

merged 2 commits into from
May 17, 2021

Conversation

fforbeck
Copy link
Contributor

@fforbeck fforbeck commented May 17, 2021

It should not be possible to onboard a member using a 0x0 address.

Updated the deployment scripts to include the owner address in the from argument of the call, so it can be verified if the call was actually made by a member or not.

In addition to that, it updates the deployment script 2_deploy_contracts.js to include the owner address in the dao creation -which is loaded from an environment variable DAO_OWNER_ADDR if the target network is Rinkeby. Othewise, for test and ganache deployments it uses the truffle accounts[0] as the DAO_OWNER_ADDR.

Fixes #282

@fforbeck fforbeck requested a review from adridadou May 17, 2021 14:42
@fforbeck fforbeck self-assigned this May 17, 2021
@fforbeck fforbeck force-pushed the gh-282 branch 2 times, most recently from 66b3e3c to 51a92d1 Compare May 17, 2021 16:37
@fforbeck fforbeck requested a review from jdville03 May 17, 2021 16:37
@fforbeck fforbeck marked this pull request as ready for review May 17, 2021 16:37
@fforbeck fforbeck added the fix important fix label May 17, 2021
jdville03
jdville03 previously approved these changes May 17, 2021
Copy link
Member

@jdville03 jdville03 left a comment

Choose a reason for hiding this comment

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

This looks good to me. Just one small suggestion to clarify a test description.

test/core/registry.test.js Outdated Show resolved Hide resolved
Co-authored-by: Jarrel deLottinville <jarrel.delottinville@gmail.com>
@fforbeck fforbeck merged commit 4dd41b7 into master May 17, 2021
@fforbeck fforbeck deleted the gh-282 branch May 17, 2021 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix important fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

isMember is returning possible false positive results
2 participants