Clean up upgradeable contract examples #1697
Conversation
This wasn't really a good showcase of an upgradeable contract. This is because it didn't use `delegate_call`, meaning that the `Proxy` contract wouldn't be able to retain state between updates.
HCastano
requested review from
a team,
cmichi,
ascjones and
SkymanOne
as code owners
| /// In a production contract you would do some authorization here! | ||
| #[ink(message)] | ||
| pub fn set_code(&mut self, code_hash: [u8; 32]) { | ||
| ink::env::set_code_hash(&code_hash).unwrap_or_else(|err| { |
There was a problem hiding this comment.
I'm wondering whether we should add this to EnvAccess so you can do self.env().set_code_hash()
There was a problem hiding this comment.
LGTM!
P.S. ink-waterfall needs updating for the new contract structure, see https://github.com/paritytech/ink-waterfall/blob/master/.gitlab-ci.yml#L21
| let get_res = client.call_dry_run(&ink_e2e::alice(), &get, 0, None).await; | ||
|
|
||
| // Remember, we updated our incrementer contract to increment by `4`. | ||
| assert!(matches!(get_res.return_value(), 4)); |
There was a problem hiding this comment.
An extremely tiny 🔎 🤏 nitpick, we should call inc on the original contract to increment by one first, before calling inc on the upgraded one. Then test for 5 here. Because it would be possible for this test to pass by having the original contract increment by 4 and not upgrading itself. Not absolutely necessary just pointing it out.
Ah yeah, good point. I'm so used to ignoring the waterfall I was going to merge it anyways 😆 |
I got rid of this in use-ink/ink#1697.
I got rid of this in use-ink/ink#1697.
|
Hmm okay it looks like the waterfall failure is related to |
There was a problem hiding this comment.
Thanks!
Can you reflect those changes on https://github.com/paritytech/ink-examples?
Aaand is there anything you think should be put into https://use.ink/basics/upgradeable-contracts as a consequence?
Follow-up to use-ink/ink#1697.
* Move `set-code-hash` example out of `upgradeable-contracts` folder * Clean up code a bit * Rename folder to use snake_case * More snake casing * Make it more explicit that upgraded constructor won't be called * Add an E2E test showing upgrade workflow * Remove `forward-calls` example This wasn't really a good showcase of an upgradeable contract. This is because it didn't use `delegate_call`, meaning that the `Proxy` contract wouldn't be able to retain state between updates. * Remove `upgradeable-contracts` from CI * Move `edition` after `author` field * Use American spelling, I guess... * Fix some paths * More path fixes * Remove comment * Move `edition` below `authors` key * Appease Clippy * Ensure that initial `inc` is only by `1`
This PR improves the
set_code_hashexample by simplifying the code a little bit, addingmore comments, and adding an E2E test to show the workflow.
It also removes the
forward-callsexample. This example was supposed to demonstrate theusage of proxies for upgrades, but since it used regular calls instead of delegate calls
it didn't really work as intended.
This is because the state was being stored in the logic/implementation contract as opposed
to the proxy, which would get lost between upgrades.
I'm not sure if we want to replace this with an upgradeable proxy example, or if we
should stick to using
set_code_hashas the "recommended" upgradeability pattern.