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 delivery of upgradeability milestone 1 #126

Merged
merged 1 commit into from
Apr 19, 2021

Conversation

hugopeixoto
Copy link
Contributor

Milestone Delivery Checklist

@alxs
Copy link
Contributor

alxs commented Mar 25, 2021

Thanks for the delivery. We will look into it as soon as possible.

@mmagician mmagician self-assigned this Mar 25, 2021
@mmagician
Copy link
Contributor

@hugopeixoto I've started looking at your delivery today. First of all, the explanations behind the logic are quite clear.

I've struggled to actually deploy the contracts & upgrade them as I was getting errors. So I thought - this must be because of me missing some steps...so I decided to automate the process and write some deployment scripts.
Due to both ink! & polkadot-js being moving targets, this proved to be somewhat cumbersome, but in the end I managed to get to a state, where my contracts were deployed to the local canvas node (latest master)
I have not been successful in calling the proxy contract, neither to insert values nor to upgrade the internal logic to V2.

I've forked your repo and created a TypeScript project there for the automation.
I would be grateful if you could have a look at these and point me to the steps that I was missing.

It's not absolutely necessary to do this in an automated way in TS, although it would be preferable, as then it's much faster to reproduce. Otherwise, I would like to ask you for a step-by-step walkthrough in your README, where you describe the entire process (i.e. all the CLI steps as well as UI calls with inputs, parameters etc.) and ideally a video/screen recording. I hope the TS project I bootstrapped should make it more attractive to go for the automated method :)

Let me know!

@hugopeixoto
Copy link
Contributor Author

Thanks for reviewing it. I'll take a look at the typescript automation. I wasn't able to call the contracts directly without increasing the gas amount, maybe because the estimator doesn't properly account for cross contract calling. I also had to increase the initial endowment. I think I can make a small video reproducing it from scratch.

@hugopeixoto
Copy link
Contributor Author

hugopeixoto commented Apr 13, 2021

I've managed to get the typescript automation working. I've merged it to the main branch, but here's the PR: trustfractal/ink-upgrade-template#1

I added my changes to your initial PR in a single commit, if you're interested in what I had to fix. It was mostly waiting for transactions to finish.

I used this repository as inspiration for the wait code: https://github.com/naps62/ink-deployer

@mmagician
Copy link
Contributor

I've submitted one final PR to include the instructions on how to run the automated workflow.
Anyway, that's just cosmetic, and I'm happy to tell you the milestone has been accepted! You can find the evaluation here - though there's nothing there that we haven't already discussed.

Am I right in thinking that you would need to wait for dynamic trait based contract calling (issue and PR) in order to take your work further, as described in your readme?

In the meantime, in case you are interested in working on other projects in the ecosystem, we have a couple of RFPs open, including an implementation of candle auctions within a smart contract.

@mmagician mmagician merged commit 02cc118 into w3f:master Apr 19, 2021
@hugopeixoto
Copy link
Contributor Author

I've submitted one final PR to include the instructions on how to run the automated workflow.

Thanks, it's merged.

Anyway, that's just cosmetic, and I'm happy to tell you the milestone has been accepted! You can find the evaluation here - though there's nothing there that we haven't already discussed.

Thank you for helping. The typescript bootstrap really helped me getting started with the automation.

Am I right in thinking that you would need to wait for dynamic trait based contract calling (issue and PR) in order to take your work further, as described in your readme?

Yes, having trait support would let us improve our solution, but the linked PR is not sufficient. I asked in the PR if it would cover what we need to fix our typing, but it seems like it won't.

Every reference to V1 on the proxy code would have to be replaced with a trait representing the InternalContract. Having V1 and V2 implement the trait would help us guarantee that the contracts are compatible.

failfmi pushed a commit to LimeChain/Grant-Milestone-Delivery that referenced this pull request Sep 26, 2022
Update google_sheet_update.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants