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 page on smart contract upgradeability #6472

Merged
merged 6 commits into from
Jul 14, 2022

Conversation

emmanuel-awosika
Copy link
Contributor

@emmanuel-awosika emmanuel-awosika commented May 26, 2022

This page provides a high-level introduction to smart contract upgrades for developers.

Description

This is a review of various patterns for upgrading Ethereum smart contracts. It also contains information on the pros and cons of smart contract upgrades and recommendations for using upgrades safely.

Related Issue

Closes #1511

This page provides a high-level introduction to smart contract upgrades for developers.
@samajammin
Copy link
Member

Awesome, thanks! Looking forward to reviewing this 😄

Curious, are you familiar with the diamond pattern at all?

I feel like that could be another good approach to list. I don't think adding that would block this PR in any way, just wanted to call that out.

@emmanuel-awosika
Copy link
Contributor Author

Awesome, thanks! Looking forward to reviewing this 😄

Curious, are you familiar with the diamond pattern at all?

I feel like that could be another good approach to list. I don't think adding that would block this PR in any way, just wanted to call that out.

Yeah, I came across the concept while researching, but I thought I'd just focus on upgrade patterns with considerable usage (I didn't see many examples of projects using the diamond pattern). I did go through Nick Mudgen's Substack blog, and it looks like some audited projects have implemented it.

My GitHub is having an issue at the moment (I can't edit files or propose changes), so I can't add it. But I'll go through those resources and prepare to add a section for diamond upgrades once I can.

@emmanuel-awosika
Copy link
Contributor Author

Awesome, thanks! Looking forward to reviewing this 😄

Curious, are you familiar with the diamond pattern at all?

I feel like that could be another good approach to list. I don't think adding that would block this PR in any way, just wanted to call that out.

Hi @samajammin, I just updated the page to add the diamond pattern (also contrasted with proxy patterns since they are similar). The update also includes a reference to the smart contract size limit and how the diamond pattern solves that, so maybe it closes this issue?

@emmanuel-awosika
Copy link
Contributor Author

Hi @samajammin. So, most of the articles I read on upgrading contracts had some code examples to demonstrate the concepts. I don't code, so I couldn't create example code snippets. But the OpenZeppelin article has some useful code snippets, and I wanted to suggest if we could use them?

Perhaps someone on the team can look at the code and see if they can create a version for the article. Here are two examples for proxy patterns and diamond patterns:

Proxy patterns

// Sample code, do not use in production!
contract Proxy {
address implementation;
fallback() external payable {
return implementation.delegatecall.value(msg.value)(msg.data);
}
}


Diamond pattern

// Sample code, do not use in production!
contract Proxy {
mapping(bytes4 => address) implementations;
fallback() external payable {
address implementation = implementations[msg.sig];
return implementation.delegatecall.value(msg.value)(msg.data);
}
}


This article has some code examples for the strategy/interface pattern and the data separation patterns, too. They were too long to copy here, but maybe they can help with creating examples for the page.

@samajammin
Copy link
Member

Hey @emmanuel-awosika! Apologies for going radio silent here... 😳

I've reached out to a few folks who could potentially give this a technical review. Hopefully they can take a look. If not, we'll try to get this merged by the end of this week.


The diamond pattern can be considered an improvement on the proxy pattern. Diamond patterns differ from proxy patterns because the proxy contract delegates function calls to more than one logic contract.

The logic contracts in the diamond pattern are known as *facets*. To make the diamond pattern work, you need to create an array in the proxy contract that maps [function selectors](https://docs.soliditylang.org/en/v0.8.12/abi-spec.html#function-selector) to different facet addresses.
Copy link
Contributor

Choose a reason for hiding this comment

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

that is usually a mapping, not an array. might be confusing to some.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Fixed this in the page now.

4. Be aware of the costs involved in upgrading contracts. For instance, copying state (e.g., user balances) from an old contract to a new contract during contract migration may require more than one transaction, meaning more gas fees.

## Resources {#resources}
**OpenZeppelin Upgrades OS - _A suite of tools for deploying and securing upgradeable smart contracts._**
Copy link
Contributor

Choose a reason for hiding this comment

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

It's called OpenZeppelin Upgrades plugins now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this now.

emmanuel-awosika and others added 2 commits July 12, 2022 14:48
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Edits to incorporate feedback.
Copy link
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

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

This looks great @emmanuel-awosika! Sorry for the delays on this.. Topic is certainly not one I'm an expert in, so as always it would nice for others to weigh in, but I don't think it needs to be blocked before then.

Just left a couple small adjustments/comments, but after that I think we can pull this in. Nice job!

| A smart contract upgrade can make it easier to fix vulnerabilities discovered in the post-deployment phase. | Upgrading smart contracts negates the idea of code immutability, which has implications for decentralization and security. |
| Developers can use upgrades to add new features to smart contract | Users must trust developers not to modify smart contracts arbitrarily. |
| Smart contract upgrades can improve safety for end-users since bugs can be fixed quickly. | Programming upgrade functionality into smart contracts adds another layer of complexity and increases the possibility of critical flaws. |
| The opportunity to upgrade smart contracts may encourage developers to launch projects faster without doing due dilligence during the development phase. | Insecure access control or centralization in smart contracts can make it easier for malicious actors to perform unauthorized upgrades.
Copy link
Member

Choose a reason for hiding this comment

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

The opportunity to upgrade smart contracts may encourage developers to launch projects faster without doing due dilligence during the development phase.

Is this supposed to be in the "pro" column worded this way?

(Also, "diligence")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's definitely a mistake. I'll move it to the cons section.

emmanuel-awosika and others added 2 commits July 13, 2022 17:31
Co-authored-by: Paul Wackerow <54227730+wackerow@users.noreply.github.com>
Edits to incorporate feedback.
Copy link
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

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

Great work @emmanuel-awosika! Thank you for another awesome PR 🙏 Pulling it in now

@wackerow wackerow merged commit 537244c into ethereum:dev Jul 14, 2022
@wackerow wackerow mentioned this pull request Jul 14, 2022
@emmanuel-awosika
Copy link
Contributor Author

Great work @emmanuel-awosika! Thank you for another awesome PR 🙏 Pulling it in now

Thanks for reviewing, @wackerow. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content 🖋️ This involves copy additions or edits
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Help needed: smart contract upgradeability page
4 participants