Track owner addresses for App/Package/Provider in network.json files

[spalladino]

We should track the owner address of the App, Package, and Provider contracts deployed to a network in the zos.network.json file. This way, we can validate whether the from address actually has permissions to run a particular action, before attempting to actually execute it, and save the user from wasting time and gas into a failing tx.

This would complement the owner tracking of individual proxies in https://github.com/zeppelinos/zos/issues/3.

[vs77bb]

@mahim23 You've been approved! A WIP PR sooner than later helps, please let us know if you have any questions along the way 🙂 cc @spalladino

[gitcoinbot]

@mahim23 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[mahim23]

Hey! I have started looking through the codebase and tried a few things. Will clarify any doubts and send a WIP PR soon.

[mahim23]

@spalladino I went through the code for the cli and the zos.network.json file is being handled by the ContollerFor network class. So, I should add the owner attribute to contract/app/package and change it in the networkFile attribute in the NetworkBaseController object, whenever it is being updated.

Is this approach right or am I missing something?

[spalladino]

@mahim23 sounds good! The ZosNetworkFile is only handled by the NetworkBaseController (via its children NetworkApp and NetworkLib), so you should be able to injet these values on the fetchOrDeploy method.

The next step, which is actually checking that the owner matches the sender address before sending every tx in the future, is more tricky. The sender may not be available in the txParams, and we may need to query the node to actually get the default sender address. We'd also need to be careful to check that these addresses match in every public method of the NetworkControllers.

Let me know if this is clear!

[gitcoinbot]

@mahim23 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[mahim23]

Hey. Sorry I got caught up in something. Will send a PR by the end of the week.

[gitcoinbot]

@mahim23 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

@mahim23 due to inactivity, we have escalated this issue to Gitcoin's moderation team. Let us know if you believe this has been done in error!

• [x] warning (3 days)
• [x] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[gitcoinbot]

@zachzundel Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[gitcoinbot]

@zachzundel Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] reminder (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[gitcoinbot]

@zachzundel Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] reminder (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[3ach]

Yes I am still working on this

[spm32]

Thanks @zachzundel! Do you have a rough estimate for a first WIP PR?

[3ach]

I think I should have one out by Thursday

On Tue, Nov 13, 2018 at 7:12 PM Scott Moore notifications@github.com wrote:

Thanks @zachzundel https://github.com/zachzundel! Do you have a rough estimate for a first WIP PR?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/zeppelinos/zos/issues/43#issuecomment-438511736, or mute the thread https://github.com/notifications/unsubscribe-auth/AC8zTe_GRRkuvHpjpnJLMHtpKTBgOSG4ks5uu3wDgaJpZM4WL8pn .

[3ach]

Hey @spalladino / @facuspagnuolo should I wait for #332 to get merged to work on this? What extra work is necessary after that point?

[spalladino]

Hey @zachzundel! I think that #332 should be covering this issue, with no further work needed. @facuspagnuolo can you confirm? If that's the case, @ceresstation how should we handle this situation, in which we already have a PR for an issue by someone who didn't lock the issue in gitcoin?

[spm32]

Hmm, @spalladino we can split the bounty between both contributors or solely pay out the contributor whose PR is merged, what would you say a reasonable split would be?

[spalladino]

I don't think that @zachzundel has sent a PR yet, but #332 has not been merged yet. @zachzundel, do you mind waiting to see if #332 gets merged before moving forward?

[3ach]

Works for me!

Zach

On Wed, Nov 21, 2018 at 05:57 Santiago Palladino notifications@github.com wrote:

I don't think that @zachzundel https://github.com/zachzundel has sent a PR yet, but #332 https://github.com/zeppelinos/zos/pull/332 has not been merged yet. @zachzundel https://github.com/zachzundel, do you mind waiting to see if #332 https://github.com/zeppelinos/zos/pull/332 gets merged before moving forward?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/zeppelinos/zos/issues/43#issuecomment-440653462, or mute the thread https://github.com/notifications/unsubscribe-auth/AC8zTYNLXDogwA9c9lioVin9Jx4IWe8Sks5uxU3RgaJpZM4WL8pn .

[Destiner]

Hey everyone. Hopefully I will finish #332 in a few days.

However, I don't think it will be enough to close this issue, as we only save owner yet, not use it to warn the user.

I hacked some code together to test whether this check works. Seems working for push command, though I'm not sure about where else we need that check. Happy to share my learnings with @zachzundel.

[hatgit]

Hi All, nice work on this bounty, looks like there were contributors that hadn't gone through Gitcoin and discussions about how to split the boutny. @ceresstation has this been resolved and was anyone paid out for work done? If so, kind update the bounty page here: https://gitcoin.co/issue/zeppelinos/zos/43/1271 and if not, and the bounty is still open, is there any work that still need to be reviewed? Thanks for the udpate! Cheers

[gitcoinbot]

Issue Status: 1. Open 2. Cancelled

---

The funding of 200.0 DAI (200.0 USD @ $1.0/DAI) attached to this issue has been cancelled by the bounty submitter

• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $89,266.60 more funded OSS Work available on the Gitcoin Issue Explorer