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

ERC-165 support for PermissionConditions #402

Merged
merged 19 commits into from
Jun 15, 2023

Conversation

heueristik
Copy link
Contributor

@heueristik heueristik commented Jun 5, 2023

Description

Adds ERC-165 (supportsInterface()) to IPermissionCondition implementations by providing an abstract base class for implementers to inherit from.
This allows detection if a contract provided in grantWithCondition is really an IPermissionCondition contract. This is an important feature to prevent accidental bricking of the DAO by providing a wrong condition address.

Task: OS-521

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.
  • I ran all tests with success and extended them if necessary.
  • I have updated the CHANGELOG.md file in the root folder.
  • I have updated the DEPLOYMENT_CHECKLIST file in the root folder.
  • I have updated the UPDATE_CHECKLIST file in the root folder.

@heueristik heueristik marked this pull request as draft June 5, 2023 14:08
@heueristik heueristik force-pushed the feature/permission-condition-erc-165-support branch from 0a137d4 to 3c147b6 Compare June 12, 2023 09:17
@heueristik heueristik force-pushed the feature/permission-condition-erc-165-support branch from bce9f32 to 8d8531e Compare June 12, 2023 09:29
@heueristik heueristik force-pushed the feature/permission-condition-erc-165-support branch from 8d8531e to ef528a2 Compare June 12, 2023 10:00
@heueristik heueristik force-pushed the feature/permission-condition-erc-165-support branch from ef528a2 to fb47075 Compare June 12, 2023 10:42
@heueristik heueristik force-pushed the feature/permission-condition-erc-165-support branch from fb47075 to 95b2cb4 Compare June 12, 2023 10:44
@heueristik heueristik marked this pull request as ready for review June 13, 2023 07:42
pythonpete32
pythonpete32 previously approved these changes Jun 14, 2023
Rekard0
Rekard0 previously approved these changes Jun 14, 2023
carlosgj94
carlosgj94 previously approved these changes Jun 14, 2023
Rekard0
Rekard0 previously approved these changes Jun 15, 2023
Copy link
Contributor

@Rekard0 Rekard0 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@mathewmeconry mathewmeconry left a comment

Choose a reason for hiding this comment

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

LGTM!

@heueristik heueristik merged commit 7e30ea5 into develop Jun 15, 2023
@heueristik heueristik deleted the feature/permission-condition-erc-165-support branch June 15, 2023 10:52
mathewmeconry added a commit that referenced this pull request Jun 22, 2023
* OS-459 : Subgraph update to v1.3.0 (#393)

* rename 120 to 130

* fix dates

* update change log

* revert contracts test (renaming mistake)

* remove unnecessary  log

* feat: improved update checklist (#396)

* feat: improved update checklist

* feat: update PR checklist

* feat: improved the update checklist

* fix: remove checklist item

* feat: added publishing flow by Jordi

* chore: Updates update_checklist to reflect multiple version
The checkslist allows version specific tasks in a append only format

* feat: formatting

* fix: typo

* feat: restructure README section about publication

* revert: re-formatting

* fix: wrong link

---------

Co-authored-by: Mathias Scherer <[email protected]>

* Feat/update scrips (#394)

* feat: updates networks to use v1.3.0 update scripts
fix: changes mumbai rpc url to use the same infura endpoint as the others

* feat(OS-429): replaces old ipfs endpoint with new one
switches ipfs-0.aragon.network with the load balances prod.ipfs.aragon.network endpoint

* fix(OS-429): replaces old api keys with valid ones

* fix: reverts active_contracts.json to old state
the values in active_contracts.json for mumbai were already the non applied update addresses

* feat: adds node version requirement to checklists

* feat: adds new functions to deploy helpers
fix: uses correct addresses during DAOFactory v1.3.0 update

* fix: add initializeFrom call to the managing DAO proposal (#395)

* fix: add initializeFrom call to the managing DAO proposal

* fix: empty data

---------

Co-authored-by: Michael Heuer <[email protected]>

* Clarifying NatSpec comment in the `DAORegistry.register` function (#403)

* feat: added clarifying NatSpec comment

* feat: improved docs

* Tiny typo fix

---------

Co-authored-by: Jør∂¡ <[email protected]>

* feat: improved NatSpec comment (#405)

* feat: added revert case to applySingleTargetPermissions (#400)

* Dops 531 update check user perm gh actions osx (#407)

* fix(ci): new gh actions in comment-triggers to check user permission

* Added revert case to grantWithCondition (#401)

* feat: added revert case to grantWithCondition

* fix: removed empty else branch

* fix: NatSpec corrections

* fix: error usage and tests

* fix: test description

* feat: optimization

* fix: corrected build metadata (#411)

* fix: corrected build metadata

* fix: added missing item to the change list

* feat: updated deployment checklist

* feat: improved checklist item

* bump package version (#413)

* bump package version

* add satsuma ipfs gateway

* fix polygon manifest (#414)

* Added ProtocolVersion to PluginRepoFactory and PluginRepo (#412)

* feat: added ProtocolVersion to PluginRepoFactory and PluginRepo

* fix: remove wrong ERC-165 usage

* chore: maintained changelog

* add an extra check (#410)

* ERC-165 support for PermissionConditions (#402)

* feat: added revert case to grantWithCondition

* fix: removed empty else branch

* fix: NatSpec corrections

* fix: error usage and tests

* fix: test description

* feat: optimization

* feat: added PermissionConditionBase and ERC-165 support

* feat: improved error handling and additional test

* feat: improved changelog

* feat: use address instead of explicit type

* fix: tests

* feat: improved error handling

* feat: added PermissionConditionUpgradeable base contract

* fix: forgotten rename

* fix: forgotten rename

* fix: contract name and NatSpec

* fix: typo

* Update protocol upgrade deploy scripts (#409)

* feat: added deploy scripts and correction in existing scripts

* feat: maintained and improved update checklist

* fix: remove accidentally duplicated files

* revert: renaming

* feat: improved managing DAO txns descriptions

* feat: improve logs

* feat: move import

* fix: wrong checklist items

* fix: no plural

* feat: refactored tests and added contracts

* feat: improve update test

* Fix: Add decimals to ERC20WrapperContract (#416)

* add decimals to erc20wrappercontract

* update package version

* fix tests

* Documentation update: How To Guides overview (#367)

* updating documentation to be more tutorial like and use case based

* documentation update

* added fixes to docs based on code reviews

* formatting

* fixed review from michael and jordim

* Update packages/contracts/docs/osx/02-how-to-guides/index.md

* removed blank linke;

* fixing titles and revising comments on PR

* fixed michaels review

* add documentation on non-upgradeable plugin to include a get started guide

* adding the format check

* fix jordis comments on technally incorrect info and typos

* fixing plugin setup corrections

* fixed comments based on jordis input - particularly regarding the setup processing and the relationship between the plugin and the dao

* format text

* fixed jordi comments

* removing storage gap

* fixed title

* OS-516 : add missing artifacts (#408)

* add missing artifacts

* Apply suggestions from code review

Co-authored-by: Michael Heuer <[email protected]>

---------

Co-authored-by: Michael Heuer <[email protected]>

* Fix CI/CD by supporting tags (#417)

* fix: use explicit working directory paths

* Revert "fix: use explicit working directory paths"

This reverts commit 09ef365.

* feat: support tags

* fix: rename variable

---------

Co-authored-by: Rekard0 <[email protected]>
Co-authored-by: Michael Heuer <[email protected]>
Co-authored-by: Jør∂¡ <[email protected]>
Co-authored-by: Roger Carhuatocto <[email protected]>
Co-authored-by: josemarinas <[email protected]>
Co-authored-by: juliettech <[email protected]>
mathewmeconry added a commit that referenced this pull request Jul 11, 2023
* dev2main (#397)

* OS-459 : Subgraph update to v1.3.0 (#393)

* rename 120 to 130

* fix dates

* update change log

* revert contracts test (renaming mistake)

* remove unnecessary  log

* feat: improved update checklist (#396)

* feat: improved update checklist

* feat: update PR checklist

* feat: improved the update checklist

* fix: remove checklist item

* feat: added publishing flow by Jordi

* chore: Updates update_checklist to reflect multiple version
The checkslist allows version specific tasks in a append only format

* feat: formatting

* fix: typo

* feat: restructure README section about publication

* revert: re-formatting

* fix: wrong link

---------

Co-authored-by: Mathias Scherer <[email protected]>

* Feat/update scrips (#394)

* feat: updates networks to use v1.3.0 update scripts
fix: changes mumbai rpc url to use the same infura endpoint as the others

* feat(OS-429): replaces old ipfs endpoint with new one
switches ipfs-0.aragon.network with the load balances prod.ipfs.aragon.network endpoint

* fix(OS-429): replaces old api keys with valid ones

* fix: reverts active_contracts.json to old state
the values in active_contracts.json for mumbai were already the non applied update addresses

* feat: adds node version requirement to checklists

* feat: adds new functions to deploy helpers
fix: uses correct addresses during DAOFactory v1.3.0 update

* fix: add initializeFrom call to the managing DAO proposal (#395)

* fix: add initializeFrom call to the managing DAO proposal

* fix: empty data

---------

Co-authored-by: Michael Heuer <[email protected]>

* Clarifying NatSpec comment in the `DAORegistry.register` function (#403)

* feat: added clarifying NatSpec comment

* feat: improved docs

* Tiny typo fix

---------

Co-authored-by: Jør∂¡ <[email protected]>

* feat: improved NatSpec comment (#405)

* feat: added revert case to applySingleTargetPermissions (#400)

* Dops 531 update check user perm gh actions osx (#407)

* fix(ci): new gh actions in comment-triggers to check user permission

* Added revert case to grantWithCondition (#401)

* feat: added revert case to grantWithCondition

* fix: removed empty else branch

* fix: NatSpec corrections

* fix: error usage and tests

* fix: test description

* feat: optimization

* fix: corrected build metadata (#411)

* fix: corrected build metadata

* fix: added missing item to the change list

* feat: updated deployment checklist

* feat: improved checklist item

* bump package version (#413)

* bump package version

* add satsuma ipfs gateway

* fix polygon manifest (#414)

* Added ProtocolVersion to PluginRepoFactory and PluginRepo (#412)

* feat: added ProtocolVersion to PluginRepoFactory and PluginRepo

* fix: remove wrong ERC-165 usage

* chore: maintained changelog

* add an extra check (#410)

* ERC-165 support for PermissionConditions (#402)

* feat: added revert case to grantWithCondition

* fix: removed empty else branch

* fix: NatSpec corrections

* fix: error usage and tests

* fix: test description

* feat: optimization

* feat: added PermissionConditionBase and ERC-165 support

* feat: improved error handling and additional test

* feat: improved changelog

* feat: use address instead of explicit type

* fix: tests

* feat: improved error handling

* feat: added PermissionConditionUpgradeable base contract

* fix: forgotten rename

* fix: forgotten rename

* fix: contract name and NatSpec

* fix: typo

* Update protocol upgrade deploy scripts (#409)

* feat: added deploy scripts and correction in existing scripts

* feat: maintained and improved update checklist

* fix: remove accidentally duplicated files

* revert: renaming

* feat: improved managing DAO txns descriptions

* feat: improve logs

* feat: move import

* fix: wrong checklist items

* fix: no plural

* feat: refactored tests and added contracts

* feat: improve update test

* Fix: Add decimals to ERC20WrapperContract (#416)

* add decimals to erc20wrappercontract

* update package version

* fix tests

* Documentation update: How To Guides overview (#367)

* updating documentation to be more tutorial like and use case based

* documentation update

* added fixes to docs based on code reviews

* formatting

* fixed review from michael and jordim

* Update packages/contracts/docs/osx/02-how-to-guides/index.md

* removed blank linke;

* fixing titles and revising comments on PR

* fixed michaels review

* add documentation on non-upgradeable plugin to include a get started guide

* adding the format check

* fix jordis comments on technally incorrect info and typos

* fixing plugin setup corrections

* fixed comments based on jordis input - particularly regarding the setup processing and the relationship between the plugin and the dao

* format text

* fixed jordi comments

* removing storage gap

* fixed title

* OS-516 : add missing artifacts (#408)

* add missing artifacts

* Apply suggestions from code review

Co-authored-by: Michael Heuer <[email protected]>

---------

Co-authored-by: Michael Heuer <[email protected]>

* Fix CI/CD by supporting tags (#417)

* fix: use explicit working directory paths

* Revert "fix: use explicit working directory paths"

This reverts commit 09ef365.

* feat: support tags

* fix: rename variable

---------

Co-authored-by: Rekard0 <[email protected]>
Co-authored-by: Michael Heuer <[email protected]>
Co-authored-by: Jør∂¡ <[email protected]>
Co-authored-by: Roger Carhuatocto <[email protected]>
Co-authored-by: josemarinas <[email protected]>
Co-authored-by: juliettech <[email protected]>

* R/1.3.0 rc0 (#429)

* chore: updates contracts changelog with all the changes

* chore: fixes typos in UPDATE_CHECKLIST.md

* feat(rc0): managingDAO deploy proposal is now json

* fix(rc0): error in update checklist copying wrong env variables

* Provide Governance Token Bases to `TokenVotingSetup` Constructor as Arguments to Avoid `initCode` size limit on Goerli (#425)

* fix: provide governance token bases as constructor arguments

* feat: verify governance token bases

* fix: adapt script for fresh deployment

* fix: typo

* fix: includes plugin bases in contract verification

* chore(1.3.0): update active_contract.json with new addresses of testnets
chore(1.3.0): update Releases.md with new releases in testnets

* chore: update npm package versions to 1.3.0-rc0

---------

Co-authored-by: Michael Heuer <[email protected]>

* chore: remove old managingDAO address of goerli and mumbai
chore: update Changelog to reflect v1.3.0-rc0 release

---------

Co-authored-by: Rekard0 <[email protected]>
Co-authored-by: Michael Heuer <[email protected]>
Co-authored-by: Jør∂¡ <[email protected]>
Co-authored-by: Roger Carhuatocto <[email protected]>
Co-authored-by: josemarinas <[email protected]>
Co-authored-by: juliettech <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants