search

gnosisguild / zodiac-safe-app

https://gnosis-safe.io/app/share/safe-app?appUrl=https://zodiac.gnosisguild.org/&chainId=5
GNU Lesser General Public License v3.0
26 stars 28 forks source link

Feat/add arbitrator field #45

Closed fnanni-0 closed 3 years ago

fnanni-0 commented 3 years ago

In a previous PR to the Reality Zodiac Module repo, adding an arbitrator parameter on construction was proposed. Here are the UI changes that would allow users to make use of said proposal by selecting an arbitrator in the Reality Zodiac Module modal.

Note that these addresses need to be updated to the new contract versions. Otherwise, the proposed changes in the UI won't work.

Note as well that the Reality-Kleros proxies currently live are not compatible with Reality v3 yet. Once the updated arbitration proxies are deployed, their addresses have to be added to this function and the chain IDs in which Kleros is available has to be added to this list.

netlify[bot] commented 3 years ago

👷 Deploy request for gnosis-zodiac-app pending review. Visit the deploys page to approve it

🔨 Explore the source changes: 8b620992afd560fda2e280b7c7b3534dd95e1b5f

fnanni-0 commented 3 years ago
default-modal dropdown custom-address
carlosfebres commented 3 years ago

Hi @fnanni-0! Great job on this PR, I'll deploy the new version of the Reality Module on the different networks and adding the new addresses to Module Manager. I think we can get the addresses of the arbitrator per address and contract version from the @reality.eth/contracts package, using this file, what do you think?

carlosfebres commented 3 years ago

@auryn-macmillan I think we need to audit the Reality module after the new changes before replacing the master copy in the module factory

fnanni-0 commented 3 years ago

@carlosfebres Thanks! Using that file looks good to me.

auryn-macmillan commented 3 years ago

@auryn-macmillan I think we need to audit the Reality module after the new changes before replacing the master copy in the module factory

I don't think there was any critical logic changed, right? It only changes the constructor and setup function to set the arbitrator. If that's the case, then I think we're fine to go ahead without an audit on the change.

fnanni-0 commented 3 years ago

@auryn-macmillan I think we need to audit the Reality module after the new changes before replacing the master copy in the module factory

I don't think there was any critical logic changed, right? It only changes the constructor and setup function to set the arbitrator. If that's the case, then I think we're fine to go ahead without an audit on the change.

Right, only constructor and setup changed.

auryn-macmillan commented 3 years ago

@carlosfebres or @cbrzn, anything else that needs to be done before merging this? (I'm guessing we need to add the addresses of the new mastercopies? Or is that pulled from the other repo automatically?)