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

Contract authz - redesign #1077

Merged
merged 16 commits into from
Nov 11, 2022
Merged

Contract authz - redesign #1077

merged 16 commits into from
Nov 11, 2022

Conversation

alpe
Copy link
Member

@alpe alpe commented Nov 4, 2022

Part of #803 - server side

Based on the great work started by @giansalex in #966

This PR includes a new and more extendable model than #978 . A contract execution or migration grant has:

# Supported "limits" - that are stateful and restrict the number of executions

  • max calls
  • max token transferred
  • max calls + max token transferred

# Supported "filters" - that check if the submitted message is allowed

  • json object key name
  • any payload wild card
  • byte equal payload (not messing with json in this iteration)

Not implemented, to discuss

@ethanfrey
Copy link
Member

Great stuff. I think these limits and filters are a very good start.
There have been requests for more power, but those shouldn't be needed in v0.30.

Important for me is that the protobuf schema is built to be extensible to possibly add options features later in a backwards compatible way. Not sure if we need some version field or such for this? But then we can keep from feature creep but allow for more power later

@kakucodes
Copy link

Looking through this I'm unclear if it's possible to make a grant without either a call or coin limit which i think is important functionality to have.
https://github.com/CosmWasm/wasmd/pull/1077/files#diff-bc6731d23885842c30d0e91892b85312fcc8cc2bb8830b2a4488346929c8ccc9R39

@alpe
Copy link
Member Author

alpe commented Nov 4, 2022

Thanks for the feedback

@ethanfrey With the interfaces new functionality can be added easily. Can you elaborate on some of the advanced use cases, please?

@kakucodes

... if it's possible to make a grant without either a call or coin limit which i think is important functionality to have.

A call limit of 2^64 is almost infinite. I have removed the infinite declarations to reduce complexity. Can you elaborate on your use case, please?

@alpe alpe mentioned this pull request Nov 4, 2022
@kakucodes
Copy link

kakucodes commented Nov 4, 2022

Thanks for the feedback

@ethanfrey With the interfaces new functionality can be added easily. Can you elaborate on some of the advanced use cases, please?

@kakucodes

... if it's possible to make a grant without either a call or coin limit which i think is important functionality to have.

A call limit of 2^64 is almost infinite. I have removed the infinite declarations to reduce complexity. Can you elaborate on your use case, please?

This is a totally fair rationale, thank you for the explanation.
Typically for Yieldmos we request a grant with unlimited access to funds and work mainly off the expiration date. I see no reason why this would be any different now with the ContractExecutionAuthorization.

@ethanfrey
Copy link
Member

First off, I think this currently solves the issues as requested in #803 (comment) perfectly. You can provide some account access to call "foo" and "bar" on address X as well as "baz" on address Y, right?

@ethanfrey
Copy link
Member

@ethanfrey With the interfaces new functionality can be added easily. Can you elaborate on some of the advanced use cases, please?

My first two thoughts of an advanced use-cases would be:

  • funds limits, but using cw20 rather than native (this can actually be handled via allowance already in cw20-base)
  • define partial payloads. like {"vote": {"proposal": 47}} but not specifying yes or no (to sell one vote). or withdrawing to a fixed recipient but arbitrary amount. This falls between "specifying the method name" and "specifying exact bytes".

I am not sure these have many real world use cases, and the second one definitely brings up a huge design issue to do safely and clearly and I would not consider this for v0.30 or even v0.31

Anyway, just some food for thought on where this might grow next year and glad there is an extension point for this

@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Merging #1077 (cc8d9bd) into main (bfb4bc0) will increase coverage by 0.88%.
The diff coverage is 76.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1077      +/-   ##
==========================================
+ Coverage   59.22%   60.10%   +0.88%     
==========================================
  Files          53       54       +1     
  Lines        6732     7061     +329     
==========================================
+ Hits         3987     4244     +257     
- Misses       2451     2516      +65     
- Partials      294      301       +7     
Impacted Files Coverage Δ
app/test_helpers.go 0.80% <ø> (+0.01%) ⬆️
x/wasm/client/cli/tx.go 28.15% <0.00%> (-4.48%) ⬇️
x/wasm/types/errors.go 0.00% <ø> (ø)
x/wasm/types/authz.go 83.39% <83.39%> (ø)
x/wasm/types/codec.go 100.00% <100.00%> (ø)
x/wasm/types/json_matching.go 92.85% <100.00%> (ø)
x/wasm/types/tx.go 48.76% <100.00%> (+4.85%) ⬆️

@alpe alpe marked this pull request as ready for review November 8, 2022 14:52
@alpe alpe requested a review from pinosu November 11, 2022 08:10
Copy link
Contributor

@pinosu pinosu left a comment

Choose a reason for hiding this comment

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

Excellent work! 🥇

@alpe alpe merged commit 5c39dae into main Nov 11, 2022
@alpe alpe deleted the 966_grants_redesign branch November 11, 2022 12:39
pinosu added a commit that referenced this pull request Nov 11, 2022
* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0

Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Remove exposed functions

* Add tests

* Add Developer's guide and best practices (#1075)

* Add Developer's guide and best practices

* Fix comments

* Change genesis preserving contract history (#1076)

* preserve contract created date on genesis import and add query contract created date

* add validate created

* fix sims test app import export

* add preserve contract history

* Make proto-all only

* Remove ResetFromGenesis

* Add validation

Co-authored-by: Alex Peters <alpe@users.noreply.github.com>

* Return contract history updates

* Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (#1082)

Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/spf13/viper/releases)
- [Commits](spf13/viper@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/viper
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (#1082)"

This reverts commit cf81eb8.

* Add cli and refactor code

* Contract authz - redesign (#1077)

* Add contract authz proto

* Implement contract autorization

* Register contract authz

* Add contract-authz tests

* Consume gas for contract authz

* Add contract authz cli

* Update cli usage

* Model spike

* Add max funds limit

* Redesign authz model

* Start e2e test

* Full e2e test

* Test filter and limits

* Test accept

* Fix description

* No linter warning

Co-authored-by: Giancarlos Salas <me@giansalex.dev>

* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Remove exposed functions

* Add tests

* Add cli and refactor code

* Fix comments

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com>
Co-authored-by: Alex Peters <alpe@users.noreply.github.com>
Co-authored-by: Giancarlos Salas <me@giansalex.dev>
NoahSaso pushed a commit to NoahSaso/wasmd that referenced this pull request Dec 2, 2022
* Add contract authz proto

* Implement contract autorization

* Register contract authz

* Add contract-authz tests

* Consume gas for contract authz

* Add contract authz cli

* Update cli usage

* Model spike

* Add max funds limit

* Redesign authz model

* Start e2e test

* Full e2e test

* Test filter and limits

* Test accept

* Fix description

* No linter warning

Co-authored-by: Giancarlos Salas <me@giansalex.dev>
NoahSaso pushed a commit to NoahSaso/wasmd that referenced this pull request Dec 2, 2022
* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0

Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Remove exposed functions

* Add tests

* Add Developer's guide and best practices (CosmWasm#1075)

* Add Developer's guide and best practices

* Fix comments

* Change genesis preserving contract history (CosmWasm#1076)

* preserve contract created date on genesis import and add query contract created date

* add validate created

* fix sims test app import export

* add preserve contract history

* Make proto-all only

* Remove ResetFromGenesis

* Add validation

Co-authored-by: Alex Peters <alpe@users.noreply.github.com>

* Return contract history updates

* Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)

Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/spf13/viper/releases)
- [Commits](spf13/viper@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/viper
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)"

This reverts commit cf81eb8.

* Add cli and refactor code

* Contract authz - redesign (CosmWasm#1077)

* Add contract authz proto

* Implement contract autorization

* Register contract authz

* Add contract-authz tests

* Consume gas for contract authz

* Add contract authz cli

* Update cli usage

* Model spike

* Add max funds limit

* Redesign authz model

* Start e2e test

* Full e2e test

* Test filter and limits

* Test accept

* Fix description

* No linter warning

Co-authored-by: Giancarlos Salas <me@giansalex.dev>

* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Remove exposed functions

* Add tests

* Add cli and refactor code

* Fix comments

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com>
Co-authored-by: Alex Peters <alpe@users.noreply.github.com>
Co-authored-by: Giancarlos Salas <me@giansalex.dev>
Magicloud pushed a commit to fpco/wasmd that referenced this pull request Jan 13, 2023
* Add contract authz proto

* Implement contract autorization

* Register contract authz

* Add contract-authz tests

* Consume gas for contract authz

* Add contract authz cli

* Update cli usage

* Model spike

* Add max funds limit

* Redesign authz model

* Start e2e test

* Full e2e test

* Test filter and limits

* Test accept

* Fix description

* No linter warning

Co-authored-by: Giancarlos Salas <me@giansalex.dev>
Magicloud pushed a commit to fpco/wasmd that referenced this pull request Jan 13, 2023
* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0

Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Remove exposed functions

* Add tests

* Add Developer's guide and best practices (CosmWasm#1075)

* Add Developer's guide and best practices

* Fix comments

* Change genesis preserving contract history (CosmWasm#1076)

* preserve contract created date on genesis import and add query contract created date

* add validate created

* fix sims test app import export

* add preserve contract history

* Make proto-all only

* Remove ResetFromGenesis

* Add validation

Co-authored-by: Alex Peters <alpe@users.noreply.github.com>

* Return contract history updates

* Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)

Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/spf13/viper/releases)
- [Commits](spf13/viper@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/viper
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)"

This reverts commit cf81eb8.

* Add cli and refactor code

* Contract authz - redesign (CosmWasm#1077)

* Add contract authz proto

* Implement contract autorization

* Register contract authz

* Add contract-authz tests

* Consume gas for contract authz

* Add contract authz cli

* Update cli usage

* Model spike

* Add max funds limit

* Redesign authz model

* Start e2e test

* Full e2e test

* Test filter and limits

* Test accept

* Fix description

* No linter warning

Co-authored-by: Giancarlos Salas <me@giansalex.dev>

* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Remove exposed functions

* Add tests

* Add cli and refactor code

* Fix comments

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com>
Co-authored-by: Alex Peters <alpe@users.noreply.github.com>
Co-authored-by: Giancarlos Salas <me@giansalex.dev>
conorpp pushed a commit to wormhole-foundation/wasmd that referenced this pull request Feb 1, 2023
* Add contract authz proto

* Implement contract autorization

* Register contract authz

* Add contract-authz tests

* Consume gas for contract authz

* Add contract authz cli

* Update cli usage

* Model spike

* Add max funds limit

* Redesign authz model

* Start e2e test

* Full e2e test

* Test filter and limits

* Test accept

* Fix description

* No linter warning

Co-authored-by: Giancarlos Salas <me@giansalex.dev>
conorpp pushed a commit to wormhole-foundation/wasmd that referenced this pull request Feb 1, 2023
* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0

Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Remove exposed functions

* Add tests

* Add Developer's guide and best practices (CosmWasm#1075)

* Add Developer's guide and best practices

* Fix comments

* Change genesis preserving contract history (CosmWasm#1076)

* preserve contract created date on genesis import and add query contract created date

* add validate created

* fix sims test app import export

* add preserve contract history

* Make proto-all only

* Remove ResetFromGenesis

* Add validation

Co-authored-by: Alex Peters <alpe@users.noreply.github.com>

* Return contract history updates

* Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)

Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/spf13/viper/releases)
- [Commits](spf13/viper@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/viper
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)"

This reverts commit cf81eb8.

* Add cli and refactor code

* Contract authz - redesign (CosmWasm#1077)

* Add contract authz proto

* Implement contract autorization

* Register contract authz

* Add contract-authz tests

* Consume gas for contract authz

* Add contract authz cli

* Update cli usage

* Model spike

* Add max funds limit

* Redesign authz model

* Start e2e test

* Full e2e test

* Test filter and limits

* Test accept

* Fix description

* No linter warning

Co-authored-by: Giancarlos Salas <me@giansalex.dev>

* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Remove exposed functions

* Add tests

* Add cli and refactor code

* Fix comments

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com>
Co-authored-by: Alex Peters <alpe@users.noreply.github.com>
Co-authored-by: Giancarlos Salas <me@giansalex.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants