gnosisguild / zodiac-module-reality Goto Github PK

We've had a couple of people showing up in the reality.eth discord saying their templates didn't work, it turned out they'd put the name of a file containing the template, when in fact they need the contents of the template.

I feel like the principle of least astonishment here would say to do what those people have expected, use the name of a file and automatically read the file in.

Failing that it might be good to at least put in some validation that the string we're using as a template looks like JSON. This is complicated by the fact that the templates only need to make valid json once the parameters are substituted in so attempting to parse it and erroring out if it fails might error our when it shouldn't. It might be best just to do a crude check for that it begins with "{" and ends with "}", that should catch most of the mistakes.

• Use hardhat-deploy
  • an example: https://github.com/gnosis/safe-contracts/blob/main/src/deploy/deploy_safe_l2.ts
• Deploy master copy using singleton deployment
• Create release on github tracking the address

Here's the pull you requested @rmeissner, let me know what else is needed. In the meantime I'll try another proposal that does a single tx vs a multi.

Failing to execute proposal:

$ cat bridge_proposal_3.json
{
    "id": "irFNzGV0BPfAdAA4lH68mLuZXHgJ4_wJADsMPTw2PtM",
    "txs": [
        {
            "to": "0x9AD2Bc9435D41Cc6f55a681552071C2EabDB2b31",
            "value": "1",
            "data": "0x",
            "operation": 0
        },
        {
            "to": "0x140E42Ff76591Bf20675952Cc1B15774F75707c0",
            "value": "2",
            "data": "0x",
            "operation": 0
        }
    ]
}

Etherescan says the tx succeeded but my account balances are unchanged and the CLI says it failed.

$ /Users/wjm/repos/dao-module/node_modules/.bin/hardhat --network rinkeby executeProposal --module 0xD0A9A4cd312E046642341b70221358095C9c0cCE --proposal-file bridge_proposal_3.json
Transaction: 0xed4742f4e3074ba0243af1b240f2e352bc294cb54cc69a0aa4eb388db37232e3
An unexpected error occurred:

Error: cannot estimate gas; transaction may fail or may require manual gas limit (error={"name":"ProviderError","code":3,"data":"0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002550726576696f7573207472616e73616374696f6e206e6f7420657865637574656420796574000000000000000000000000000000000000000000000000000000"}, method="estimateGas", transaction={"from":"0x140E42Ff76591Bf20675952Cc1B15774F75707c0","to":"0xD0A9A4cd312E046642341b70221358095C9c0cCE","data":"0x14604b8c00000000000000000000000000000000000000000000000000000000000000e00000000000000000000000000000000000000000000000000000000000000140000000000000000000000000140e42ff76591bf20675952cc1b15774f75707c0000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000001a000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002b6972464e7a47563042506641644141346c4836386d4c755a5848674a345f774a4144734d5054773250744d00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000026e1ce05a50d11be25515bc4b7e62d4ad5022af39bda9f325bcae16c15200c857f0a7086b4ffaf456a6af8e948c704399b2e291609c3cd4830038d00d829cfd380000000000000000000000000000000000000000000000000000000000000000"}, code=UNPREDICTABLE_GAS_LIMIT, version=providers/5.0.23)

If SAFE/DAO/etc addresses are needed:

$ cat details.rinkeby
SAFE_ADDRESS='0x842f93D83095f1cF09e8C8dd87a69A8DB3d47cdF'
DAO_ADDRESS='0x842f93D83095f1cF09e8C8dd87a69A8DB3d47cdF'
TEMPLATE_ID='0x000000000000000000000000000000000000000000000000000000000000003a'
ORACLE_ADDRESS='0x3D00D77ee771405628a4bA4913175EcC095538da'
MODULE_ADDRESS='0xD0A9A4cd312E046642341b70221358095C9c0cCE'

Add support for new target parameter, also related to #37

TBD

• Setup CLA action
• Enable action for external PRS

Currently the DAO module uses the connected DAO (normally a Safe) as the arbitrator. The Realitio interface expects the arbitrator to expose some information via on-chain function calls. A fallback handler should be provided that can be set on the DAO safe to return this required information.

Required methods:

• getDisputeFee(bytes32 question_id)
  https://github.com/realitio/realitio-contracts/blob/a032359a1c7f39737825ab8e8d344766e94775a8/truffle/contracts/Arbitrator.sol#L87
  If you want to block calling it make it return a really high number

• metadata() - can return an empty string for now
  https://github.com/realitio/realitio-contracts/blob/a032359a1c7f39737825ab8e8d344766e94775a8/truffle/contracts/Arbitrator.sol#L17

• realitio() - should return the address of the contract you're accessing
  https://github.com/realitio/realitio-contracts/blob/master/truffle/contracts/Arbitrator.sol#L10

• Test that master copy cannot be initialized twice or taken over
• Test that creation flow with module factory

• Asking proposal question
• Executing proposal

• DeterministicDeploymentHelper.sol

The latest version of Reality.eth allows the question asker to require a minimum bond.
Making use of this would reduce our monitoring requirements.

https://github.com/RealityETH/monorepo/tree/feature-v3-audit/packages/contracts

Currently it is not trivial to verify a proposal that has been published on Realitio via the Snapshot plugin. It would be good to provide a guide how to do so, in order to make it easier to verify the Realitio question

In the currently deployed version, it's hard for off-chain systems (analytics, UI, etc.) to keep track of changes to a Reality Module's parameters.

This could be solved by emitting events on setter functions:

• https://github.com/gnosis/zodiac-module-reality/blob/d05a496789079b60125a24f386d1a152e1479f4f/contracts/RealityModule.sol#L138
• https://github.com/gnosis/zodiac-module-reality/blob/d05a496789079b60125a24f386d1a152e1479f4f/contracts/RealityModule.sol#L147
• https://github.com/gnosis/zodiac-module-reality/blob/d05a496789079b60125a24f386d1a152e1479f4f/contracts/RealityModule.sol#L161
• https://github.com/gnosis/zodiac-module-reality/blob/d05a496789079b60125a24f386d1a152e1479f4f/contracts/RealityModule.sol#L172
• https://github.com/gnosis/zodiac-module-reality/blob/d05a496789079b60125a24f386d1a152e1479f4f/contracts/RealityModule.sol#L179
• https://github.com/gnosis/zodiac-module-reality/blob/d05a496789079b60125a24f386d1a152e1479f4f/contracts/RealityModule.sol#L187
• [ ]

Describe the bug
Reality.eth Template Builder UI link points to a page that 404s, and looks like it was from github pages.

To Reproduce
Steps to reproduce the behavior:

1. Go to The zodiac-module-reality github page
2. Scroll down to the line in the readme.md doc that reads:

You can also create your template using the Reality.eth Template Builder UI. For more info on using it, use this guide.

3. Click on the link that reads "use this guide"
4. See a 404 page at https://gnosis.github.io/zodiac/docs/tutorial-module-reality/add-template#template-builder

Expected behavior
Should lead to a page with information on how to use the template builder

Screenshots / logs

Reality.eth's UI uses sprintf and expects some characters to be escaped, or else the UI breaks.
For example, a percent symbol (%) in the question template will break the Reality.eth UI.

We should update the hardhat task that creates the template ID so that it escapes special characters.

TBD

🍯DAO requirements example document in setup guide fails to load.

Is your feature request related to a problem? Please describe.
It's hard to know when a proposal has completed its lifecycle.

Describe the solution you'd like
Emit an event when a proposal is marked as invalid.
Emit an event when a proposal is executed.

Additional context
Both from a UI and monitoring perspective it's good to know when a proposal is no longer relevant.

• update DaoModule.sol
• update scripts to use avatar arguments instead of executor
• update tests
• update documentation

Are L117 and L139 require(...) really needed? If yes and if they are intended to prevent askQuestionWithMinBond() from reverting if the setup was incorrect, then require(timeout < 365 days, "Timeout has to be smaller than 365 days") should probably be checked too, right? See Realitio v3. Anyway, I don't think this is an issue, but it caught my attention.

In order to make testing and integrating with other contracts easier, addProposal() and addProposalWithNonce() should return the questionId of the question Reality.eth question.

Motivation

Certain proposals require all function calls are executed within one transaction to achieve atomicity. This is currently achievable by using a wrapper contract, but it is not currently enforced on a proposal at the module level.

Another desired enforcement is that of call vs. delegateCall. The effect of a proposal can differ with different operations. Once a proposal passes, there is nothing dictating which of the two can be used to execute a proposal.

Solution

• Add a way of tracking whether atomic execution is enforced on a given proposal
• Add a function to call all transactions within a proposal (will require using ABIEncoderV2)
• Add a variable to set whether call or delegateCall should be used to execute a proposal.

I have been playing with this on my fork and am happy to do the changes properly, but I was informed that the main repo is not yet configured for external contributions, so I won't push forward with PR-ready changes just yet.

• Adjust workflow to run on PRs

Apparently the way to configure the SafeSnap plugin in SnapShot has changed, so that the in setup guide is outdated. As a consequence this exemplary configuration file is not up-to-date.

In this section "daoModule" needs to be replaced with "safeSnap":

  "plugins": {
    "daoModule": {
      "address": "0x6c57cEa2dE7D386d349Ff6c4E8631bD567DFd0fC"
    }
  }

This should be:

  "plugins": {
    "safeSnap": {
      "address": "0x6c57cEa2dE7D386d349Ff6c4E8631bD567DFd0fC"
    }
  }

Whoops... created this in the wrong repo.
Recreated here: gnosisguild/zodiac#21

Currently the task expects a json string for the template parameter. This causes quite some confusion and should be changed to accept a file.