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

Enable larger wasm bytecode upload for gov proposals #1095

Merged
merged 2 commits into from
Nov 17, 2022

Conversation

pinosu
Copy link
Contributor

@pinosu pinosu commented Nov 17, 2022

Resolves #1038

@pinosu pinosu requested a review from alpe as a code owner November 17, 2022 10:41
@codecov
Copy link

codecov bot commented Nov 17, 2022

Codecov Report

Merging #1095 (af91fdb) into main (48ef13d) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1095      +/-   ##
==========================================
+ Coverage   59.79%   59.82%   +0.02%     
==========================================
  Files          54       54              
  Lines        7273     7273              
==========================================
+ Hits         4349     4351       +2     
+ Misses       2612     2611       -1     
+ Partials      312      311       -1     
Impacted Files Coverage Δ
x/wasm/types/genesis.go 87.87% <100.00%> (ø)
x/wasm/types/proposal.go 55.52% <100.00%> (ø)
x/wasm/types/tx.go 48.76% <100.00%> (ø)
x/wasm/types/validation.go 100.00% <100.00%> (ø)
x/wasm/keeper/keeper.go 87.71% <0.00%> (+0.31%) ⬆️

Copy link
Contributor

@alpe alpe left a comment

Choose a reason for hiding this comment

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

Very nice!
One minor issue and some feedback on the max value for gov content would be great.

)

func validateWasmCode(s []byte) error {
func validateWasmCode(s []byte, maxSize int) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

good adoption of function!

@@ -13,14 +13,17 @@ var (

// MaxWasmSize is the largest a compiled contract code can be when storing code on chain
MaxWasmSize = 800 * 1024 // extension point for chains to customize via compile flag.

// MaxProposalWasmSize is the largest a gov proposal compiled contract code can be when storing code on chain
MaxProposalWasmSize = 4 * 1024 * 1024 // extension point for chains to customize via compile flag.
Copy link
Contributor

Choose a reason for hiding this comment

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

Good to have this as a variable, too. I am not fully convinced that 4mb is a good value, given the spam case. 2mb feels a bit more conservative but it is also a random value

Copy link
Member

Choose a reason for hiding this comment

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

gov proposals charge like 3x gas for the proposal size (than just raw tx size), but yes it is written to state

It is easy to get 1.5MB contracts, maybe 2.5MB or 3MB if you want to be a bit conservative but still allow more complexity (like a js interpreter)

Also, we could just charge extra gas for every byte above the base (800kb) level up to this hard cap, but that might be too confusing/complex

Copy link
Member

Choose a reason for hiding this comment

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

If chains can override, we just need a reasonable default. I am fine with 3MB. Feel 2MB is a bit tight (I got a demo contract I could only trim to 1.8MB and could easily grow a bit if I added any features)

@@ -47,7 +47,7 @@ func (c Code) ValidateBasic() error {
if err := c.CodeInfo.ValidateBasic(); err != nil {
return sdkerrors.Wrap(err, "code info")
}
if err := validateWasmCode(c.CodeBytes); err != nil {
if err := validateWasmCode(c.CodeBytes, MaxWasmSize); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be raised or an import fails for gov contracts

Suggested change
if err := validateWasmCode(c.CodeBytes, MaxWasmSize); err != nil {
if err := validateWasmCode(c.CodeBytes, MaxProposalWasmSize); err != nil {

Copy link
Contributor

@alpe alpe left a comment

Choose a reason for hiding this comment

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

👍 thanks for the updates 🏄

@pinosu pinosu force-pushed the 1038-gov_proposal_larger_bytecode branch from 69ce1ac to af91fdb Compare November 17, 2022 12:10
@pinosu pinosu merged commit ab4ffa5 into main Nov 17, 2022
@ethanfrey
Copy link
Member

Great. Thank you for this!

orkunkl pushed a commit that referenced this pull request Nov 21, 2022
* Enable larger wasm bytecode upload for gov proposals

* Set max proposal wasm code size to 3MB
alpe added a commit that referenced this pull request Nov 22, 2022
…proposal message (#1072)

* Provide source, builder and codehash information in store code proposal message

* Make linter happy

* Update x/wasm/simulation/proposals.go

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

* Update x/wasm/client/cli/gov_tx.go

* Update x/wasm/client/cli/gov_tx.go

* Bump github.com/cosmos/gogoproto from 1.4.2 to 1.4.3

Bumps [github.com/cosmos/gogoproto](https://github.com/cosmos/gogoproto) from 1.4.2 to 1.4.3.
- [Release notes](https://github.com/cosmos/gogoproto/releases)
- [Changelog](https://github.com/cosmos/gogoproto/blob/main/CHANGELOG.md)
- [Commits](cosmos/gogoproto@v1.4.2...v1.4.3)

---
updated-dependencies:
- dependency-name: github.com/cosmos/gogoproto
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

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

* Update authz grant examples

* Enable larger wasm bytecode upload for gov proposals (#1095)

* Enable larger wasm bytecode upload for gov proposals

* Set max proposal wasm code size to 3MB

* Bump SDK to v0.45.11

* Fix license head

* Fix README header

* Bump version go to 1.19 (#1044)

* bump go 1.19

* add change log

* correct change log

* Provide source, builder and codehash information in store code proposal message

* Implement source, builder, code_info for StoreAndInstantiateProposal

* Apply review recommendations

* Make linter happy

* Fix tests

* Formatting only

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Alexander Peters <alpe@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Giancarlos Salas <giansalex@gmail.com>
Co-authored-by: pinosu <95283998+pinosu@users.noreply.github.com>
Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com>
NoahSaso pushed a commit to NoahSaso/wasmd that referenced this pull request Dec 2, 2022
* Enable larger wasm bytecode upload for gov proposals

* Set max proposal wasm code size to 3MB
NoahSaso pushed a commit to NoahSaso/wasmd that referenced this pull request Dec 2, 2022
…proposal message (CosmWasm#1072)

* Provide source, builder and codehash information in store code proposal message

* Make linter happy

* Update x/wasm/simulation/proposals.go

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

* Update x/wasm/client/cli/gov_tx.go

* Update x/wasm/client/cli/gov_tx.go

* Bump github.com/cosmos/gogoproto from 1.4.2 to 1.4.3

Bumps [github.com/cosmos/gogoproto](https://github.com/cosmos/gogoproto) from 1.4.2 to 1.4.3.
- [Release notes](https://github.com/cosmos/gogoproto/releases)
- [Changelog](https://github.com/cosmos/gogoproto/blob/main/CHANGELOG.md)
- [Commits](cosmos/gogoproto@v1.4.2...v1.4.3)

---
updated-dependencies:
- dependency-name: github.com/cosmos/gogoproto
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

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

* Update authz grant examples

* Enable larger wasm bytecode upload for gov proposals (CosmWasm#1095)

* Enable larger wasm bytecode upload for gov proposals

* Set max proposal wasm code size to 3MB

* Bump SDK to v0.45.11

* Fix license head

* Fix README header

* Bump version go to 1.19 (CosmWasm#1044)

* bump go 1.19

* add change log

* correct change log

* Provide source, builder and codehash information in store code proposal message

* Implement source, builder, code_info for StoreAndInstantiateProposal

* Apply review recommendations

* Make linter happy

* Fix tests

* Formatting only

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Alexander Peters <alpe@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Giancarlos Salas <giansalex@gmail.com>
Co-authored-by: pinosu <95283998+pinosu@users.noreply.github.com>
Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com>
Magicloud pushed a commit to fpco/wasmd that referenced this pull request Jan 13, 2023
* Enable larger wasm bytecode upload for gov proposals

* Set max proposal wasm code size to 3MB
Magicloud pushed a commit to fpco/wasmd that referenced this pull request Jan 13, 2023
…proposal message (CosmWasm#1072)

* Provide source, builder and codehash information in store code proposal message

* Make linter happy

* Update x/wasm/simulation/proposals.go

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

* Update x/wasm/client/cli/gov_tx.go

* Update x/wasm/client/cli/gov_tx.go

* Bump github.com/cosmos/gogoproto from 1.4.2 to 1.4.3

Bumps [github.com/cosmos/gogoproto](https://github.com/cosmos/gogoproto) from 1.4.2 to 1.4.3.
- [Release notes](https://github.com/cosmos/gogoproto/releases)
- [Changelog](https://github.com/cosmos/gogoproto/blob/main/CHANGELOG.md)
- [Commits](cosmos/gogoproto@v1.4.2...v1.4.3)

---
updated-dependencies:
- dependency-name: github.com/cosmos/gogoproto
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

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

* Update authz grant examples

* Enable larger wasm bytecode upload for gov proposals (CosmWasm#1095)

* Enable larger wasm bytecode upload for gov proposals

* Set max proposal wasm code size to 3MB

* Bump SDK to v0.45.11

* Fix license head

* Fix README header

* Bump version go to 1.19 (CosmWasm#1044)

* bump go 1.19

* add change log

* correct change log

* Provide source, builder and codehash information in store code proposal message

* Implement source, builder, code_info for StoreAndInstantiateProposal

* Apply review recommendations

* Make linter happy

* Fix tests

* Formatting only

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Alexander Peters <alpe@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Giancarlos Salas <giansalex@gmail.com>
Co-authored-by: pinosu <95283998+pinosu@users.noreply.github.com>
Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com>
conorpp pushed a commit to wormhole-foundation/wasmd that referenced this pull request Feb 1, 2023
* Enable larger wasm bytecode upload for gov proposals

* Set max proposal wasm code size to 3MB
conorpp pushed a commit to wormhole-foundation/wasmd that referenced this pull request Feb 1, 2023
…proposal message (CosmWasm#1072)

* Provide source, builder and codehash information in store code proposal message

* Make linter happy

* Update x/wasm/simulation/proposals.go

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

* Update x/wasm/client/cli/gov_tx.go

* Update x/wasm/client/cli/gov_tx.go

* Bump github.com/cosmos/gogoproto from 1.4.2 to 1.4.3

Bumps [github.com/cosmos/gogoproto](https://github.com/cosmos/gogoproto) from 1.4.2 to 1.4.3.
- [Release notes](https://github.com/cosmos/gogoproto/releases)
- [Changelog](https://github.com/cosmos/gogoproto/blob/main/CHANGELOG.md)
- [Commits](cosmos/gogoproto@v1.4.2...v1.4.3)

---
updated-dependencies:
- dependency-name: github.com/cosmos/gogoproto
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

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

* Update authz grant examples

* Enable larger wasm bytecode upload for gov proposals (CosmWasm#1095)

* Enable larger wasm bytecode upload for gov proposals

* Set max proposal wasm code size to 3MB

* Bump SDK to v0.45.11

* Fix license head

* Fix README header

* Bump version go to 1.19 (CosmWasm#1044)

* bump go 1.19

* add change log

* correct change log

* Provide source, builder and codehash information in store code proposal message

* Implement source, builder, code_info for StoreAndInstantiateProposal

* Apply review recommendations

* Make linter happy

* Fix tests

* Formatting only

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Alexander Peters <alpe@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Giancarlos Salas <giansalex@gmail.com>
Co-authored-by: pinosu <95283998+pinosu@users.noreply.github.com>
Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com>
@alpe alpe deleted the 1038-gov_proposal_larger_bytecode branch May 10, 2023 08:55
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.

Allow gov votes to upload larger wasm bytecode
3 participants