Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Upgradable contracts using set_code function #10567

Closed
yarikbratashchuk opened this issue Dec 28, 2021 · 10 comments · Fixed by #10690
Closed

Upgradable contracts using set_code function #10567

yarikbratashchuk opened this issue Dec 28, 2021 · 10 comments · Fixed by #10690
Labels
J0-enhancement An additional feature request.

Comments

@yarikbratashchuk
Copy link
Contributor

yarikbratashchuk commented Dec 28, 2021

Together with a team at Supercolony we want to propose and discuss possible first class approach for upgradability in substrate smart contracts. This is up for discussion and comments/suggestions are very welcome.

The approach:

  • Create a separate set_code function in contracts exposed api. The signature could look like this:
fn set_code(code_hash_ptr: u32) -> Result<(), Error>

This function will update code_hash for existing contract. This way we can keep the same contract and its storage, but upgrade the executable code. it is a contract owner responsibility to make sure that new code is compatible with existing storage and that it remains upgradable (by having a method for using set_code).

  • No need to use Proxy pattern for upgradable contracts anymore and so the execution cost is reduced as there is no call to proxy contract.

The upgrade flow:

  1. New contract is being deployed.
  2. The existing contract has a method that calls set_code with a new deployed contract's code_hash as a parameter.

Status of Ethereum like !ink based upgradable contracts can be tracked here - #7

@bkchr
Copy link
Member

bkchr commented Dec 29, 2021

CC @athei

@shawntabrizi
Copy link
Member

This should already be possible with a set_storage call, so this request is just to make a more easy to use extrinsic?

@yarikbratashchuk
Copy link
Contributor Author

@shawntabrizi, please take a look at this wip pr to see what is used to make it work.
This request is to discuss and add first class solution for contract upgradability.

@xgreenx
Copy link
Contributor

xgreenx commented Dec 30, 2021

This should already be possible with a set_storage call, so this request is just to make a more easy to use extrinsic?

set_storage uses the write function that changes some value by key in key-value storage.

image

The idea of set_code is that you can change the ContractInfo itself and set the hash of the code that should be used by the Contract in the future(this information is not stored in key-value storage).

@athei
Copy link
Member

athei commented Dec 30, 2021

This should already be possible with a set_storage call, so this request is just to make a more easy to use extrinsic?

You mean the set_storage callable from a contract? Nope. This cannot change the contracts code. This is on purpose because immutability is the defining feature of contracts.

How could I ever trust a contract which uses set_code? The only scenario I can imagine where this is OK to call is whenever seal_terminate is OK to call (when no user generated data is in the contract). But doesn't this defeat the purpose of set_code?

@yarikbratashchuk
Copy link
Contributor Author

yarikbratashchuk commented Dec 30, 2021

@athei, let me bring up some background for this:
As we're planning to implement delegate_call (I assume this based on previous discussions on upgradable contracts in substrate/!ink) to allow developers to build upgradable contracts, we came up with the idea of set_code as a cheaper in terms of gas and cleaner in terms of an upgrade flow.
Imagine having a contract A that has a state and a library contract B that has a logic to update the state of contract A using delegate_call:

ContractA (proxy) -> delegate_call -> ContractB (library)
  state  <--------------modify---------  logic

The upgrade flow looks like this:

- deploy contractB' -> instantiate -> lib_contract_address
- update lib_contract_address in ContractA state

We have to trust that new lib contract is compatible with state of ContractA

With set_code approach we keep logic and state in one contract and the upgrade flow looks like this:

- make sure new contract version has set_code(code_hash) method to keep contract upgradable
- deploy new contract version -> new_contract_code_hash
- call ContractA.set_code(new_contract_code_hash) method to change the executable code

We achieve the same functionality with lower costs for user (no cross contract call) and easier upgrade flow for developer.
We still need delegate_call to be implemented though (#10566)

Feels like there is the same amount of trust as with a delegate_call approach.

@shawntabrizi
Copy link
Member

@athei if the argument is around "trust" or worrying that a contract can change itself, there can simply be a flag which enables or disables a contract from using this feature. Once it is disabled, it cannot be enabled again without ROOT.

@athei
Copy link
Member

athei commented Jan 3, 2022

Feels like there is the same amount of trust as with a delegate_call approach.

I agree. Having some storage field that can be changed to call an arbitrary code in the storage context of the current contract has the same consequences. This is merely moving this popular pattern to pallet-contracts in order to optimize it. I think this makes sense.

@athei if the argument is around "trust" or worrying that a contract can change itself, there can simply be a flag which enables or disables a contract from using this feature. Once it is disabled, it cannot be enabled again without ROOT.

I don't think this changes anything. Contracts which don't make use of this are already identifiable by not importing the set_code host function. Even if there is an additional "switch" that must be unlocked by governance the "proxy admin" could then still deploy any rogue code. Do we want code hashes be decided on by governance like with a runtime upgrade? I don't think so as this wouldn't scale well: Contracts should code their own mechanism to authorize set_code. Having a "proxy admin" is just the simplest form.

@athei athei added J0-enhancement An additional feature request. and removed J2-unconfirmed Issue might be valid, but it’s not yet known. labels Jan 3, 2022
@athei
Copy link
Member

athei commented Jan 17, 2022

Anyone want to tackle this: @yarikbratashchuk @xgreenx ?

@xgreenx
Copy link
Contributor

xgreenx commented Jan 17, 2022

Anyone want to tackle this: @yarikbratashchuk @xgreenx ?

Yea, we already implemented it. Now it is on review in our company repo, after passing it, we will create a PR=)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants