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

Add cosmwasm #1103

Merged
merged 11 commits into from
Mar 1, 2024
Merged

Add cosmwasm #1103

merged 11 commits into from
Mar 1, 2024

Conversation

sampocs
Copy link
Collaborator

@sampocs sampocs commented Feb 14, 2024

Context

Added permissioned cosmwasm to Stride for hyperlane

Brief Changelog

  • Added wasm module
  • Added upgrade handler and restricted permissions (see next section)
  • Updated dockernet image to include wasm bindings
  • Removed the cosmwasm protos as they are no longer needed as the types can now be imported
  • Switched msg types for icaoracle (used wasm types)

Context on Permissions

  • You can restrict (a) who can upload and (b) the default instantiation permissions. Instantiation permissions can also be specified when uploading
  • Regardless of how the permissions are configured, governance can always upload or instantiate
  • Setting "Nobody" as the access type means it's governance only
  • For uploading, I think we should restrict it to governance only
  • For instantiating, I think we should explicitly set an admin address during uploading to make the instantiation process easier. Since we're doing this, we can set the default permissions to governance only.

Open Questions

  • Are we in agreement on the permissions?
  • Do we need IBC hooks? assuming no
  • Do we need stargate? depends on hyperlane but assuming no
  • Do we need custom wasm options?
  • Are default wasm config settings fine? (i.e. gas limit, mem cache size, etc.)

Testing

This was tested with the ICA oracle contract

Testing w/o Upgrade

First this was tested without the upgrade and with open permissions

  • Started dockernet
make start-docker 
  • Uploaded contract
strided tx wasm store ./artifacts/ica_oracle.wasm --gas-prices 0.1ustrd --gas auto --gas-adjustment 1.3 --from val1 -y
  • Instantiated contract
init_msg='{"admin_address": "stride1uk4ze0x4nvh4fk0xm4jdud58eqn4yxhrt52vv7", "transfer_channel_id": "channel-0"}'
strided tx wasm instantiate 1 "$init_msg" --from val1 --label "ica-oracle" --no-admin --gas auto --gas-adjustment 1.3 -y
  • Confirmed query worked (indicating that the contract was instantiated)
>>> stridedl q wasm contract-state smart stride14hj2tavq8fpesdwxxcu44rty3hh90vhujrvcmstl4zr3txmfvw9szh70ur '{"all_latest_metrics": {}}'
data:
  metrics: []

Testing Permissioned Upload after Upgrade

This was then tested via running an upgrade and confirming the permissions were set correctly

  • Started dockernet with the old binary and ran through the upgrade
make UPGRADE_OLD_VERSION=v18.0.0 upgrade-build-old-binary
make UPGRADE_OLD_VERSION=v18.0.0 UPGRADE_NAME=v19 start-docker 
make UPGRADE_NAME=v17 submit-upgrade-immediately
  • Attempted to upload the contract using the code above and confirmed it failed with error cannot create code: unauthorized
  • Submitted proposal to upload contract
strided tx wasm submit-proposal wasm-store ./artifacts/ica_oracle.wasm \
   --title "upload oracle contract" --summary "upload oracle contract" \
   --authority stride10d07y265gmmuvt4z0w9aw880jnsr700jefnezl \
    --from val1 -y --gas 20000000 --deposit 10000000ustrd

strided tx gov vote 2 yes --from val1 -y 
strided tx gov vote 2 yes --from val2 -y 
strided tx gov vote 2 yes --from val3 -y 
  • Confirmed contract was uploaded after the vote passed
>>> strided q wasm list-code
code_infos:
- code_id: "1"
  creator: stride10d07y265gmmuvt4z0w9aw880jnsr700jefnezl
  data_hash: 5EE10302357FF0F8531FE00029CD35CFE5A2C521C6A818CEE06CD61A3DF0CB42
  instantiate_permission:
    addresses: []
    permission: Nobody
pagination:
  next_key: null
  total: "0"

Testing Permissioned Instantiation after Upgrade

Continuing off the previous example:

  • Attempted to instantiate the contract using the code above and confirmed it failed with error cannot instantiate: unauthorized
  • Submitted a proposal to instantiate through goverance
init_msg='{"admin_address": "stride1uk4ze0x4nvh4fk0xm4jdud58eqn4yxhrt52vv7", "transfer_channel_id": "channel-0"}'
strided tx wasm submit-proposal instantiate-contract 1 "$init_msg" \
    --authority stride10d07y265gmmuvt4z0w9aw880jnsr700jefnezl \
    --from val1 -y --gas 20000000 --deposit 10000000ustrd \
    --label "ica-oracle" --title "instantiate oracle" --summary "instantiate oracle" --no-admin
  • Confirmed contract was instantiated
>>> stridedl q wasm contract-state smart stride14hj2tavq8fpesdwxxcu44rty3hh90vhujrvcmstl4zr3txmfvw9szh70ur '{"all_latest_metrics": {}}'
data:
  metrics: []

@github-actions github-actions bot added C:app-wiring C:CLI C:proto C:test dependencies Pull requests that update a dependency file T:build labels Feb 14, 2024
go.mod Show resolved Hide resolved
app/app.go Show resolved Hide resolved
@sampocs
Copy link
Collaborator Author

sampocs commented Feb 29, 2024

per open questions:

Are we in agreement on the permissions?

Discussed offline, in agreement. Need to remember to set the instantiate address when uploading.

Do we need IBC hooks? Do we need stargate? Do we need custom wasm options?

Successful hyperlane deployment confirms we do not need any of these

Are default wasm config settings fine? (i.e. gas limit, mem cache size, etc.)

I believe these are just validator config settings so easy to change later if needed. And I assume the defaults are fine since this is a light-weight deployment

Copy link
Contributor

@ethan-stride ethan-stride left a comment

Choose a reason for hiding this comment

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

LGTM
As discussed none of us are experts for many of these settings so I am just calling out anything which looks potentially unneeded at this point. I was comparing against the osmosis cosmwasm setup but they probably have additional usecases we don't want so we might be able to lock it down even more.

app/app.go Show resolved Hide resolved
app/app.go Show resolved Hide resolved
app/upgrades/v19/upgrades.go Show resolved Hide resolved
app/app.go Show resolved Hide resolved
@sampocs sampocs merged commit 6a8f0ce into main Mar 1, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants