Adding upgrade documentation #186
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eztierney
requested changes
Aug 7, 2018
doc/upgrade.md
Outdated
| 3) Ensure the test suite has run on the final version of the new contract with added tests and that the test suite passes with 100% code coverage | ||
| 4) Complete any external audits as deemed necessary | ||
| 5) Go back to step 1 if any prior step is not completed successfully | ||
| 6) Deploy the contract by following *Deployment Instructions* in *only* section *Deploying the implementation contract* [instructions](deployment.md#Deploying-the-implementation-contract). If new fields are added, substitute initV[X] for initialize in the instructions. If only new logic is added, no initialization is required. |
There was a problem hiding this comment.
Actually for deploying the implementation, we will want to use intitialize since the deployed V[X] impl will be unrelated to the previous impl contract, so we will want to make sure we init everything in the X-1, ... 1 versions too
doc/upgrade.md
Outdated
| ### Upgrading if *ONLY* New Logic Added | ||
| 1) Use the centre-token-client to prepare an upgradeTo transaction from the adminAccount parameterized with the address of the new upgraded token (see token-client instructions). | ||
| 2) Broadcast the transaction | ||
| 3) Call getImplementation on the proxy (a read call, no signed tx required) and ensure it matches the address of the upgraded token. |
There was a problem hiding this comment.
May want to include the getStorageAt trick since getImplementation is only visible to the admin account.
doc/upgrade.md
Outdated
| Note that `private` *functions* will not be inherited in subsequent contract versions and should be added with care. | ||
|
|
||
| ### New Fields Only | ||
| Adding new fields requires inheriting from the prior version of the contract as done in [New Logic](#new-logic-only). In addition, the new contract requires declaring new data fields and initialization logic for the new fields. The new token must add an *initialize* function and a *initV[X]* function, where X is the version of the contract. The initialization function allows the contract to be deployed from scratch and initialize all variables declared in the new contract and in prior contracts. The initV[X] function allows the contract to initialize only the new variables added in the new contract. New variables added *must* be declared as type `internal` or `public`, `private` can never be used. Note that `private` *functions* will also not be inherited in subsequent contract versions and should be added with care. A template is below shown for upgrading from version 1 to version 2. In the example, we add variables newBool, newAddress, and newUint, which would be replaced with the real variables added. |
There was a problem hiding this comment.
I think we should change this to say something like "if the new fields require initialization...". And recommend that we try to add new fields that are ok with the default value (false, 0, etc). It won't always be possible, but if it is we should default to no initialization.
eztierney
requested changes
Aug 7, 2018
doc/upgrade.md
Outdated
| @@ -87,21 +87,21 @@ Deployment can be done in the following steps: | |||
| 3) Ensure the test suite has run on the final version of the new contract with added tests and that the test suite passes with 100% code coverage | |||
| 4) Complete any external audits as deemed necessary | |||
| 5) Go back to step 1 if any prior step is not completed successfully | |||
| 6) Deploy the contract by following *Deployment Instructions* in *only* section *Deploying the implementation contract* [instructions](deployment.md#Deploying-the-implementation-contract). If new fields are added, substitute initV[X] for initialize in the instructions. If only new logic is added, no initialization is required. | |||
| 6) Deploy the contract by following *Deployment Instructions* in *only* section *Deploying the implementation contract* [instructions](deployment.md#Deploying-the-implementation-contract). When invoking `initialize` in following deployment instructions, the latest version of `initialize` should be called. | |||
|
|
|||
| ## Upgrading the Proxy to point to the UpgradedToken | |||
| The proxy must be pointed to the new upgraded token. This is accomplished two ways depending on whether only new logic was added or if new fields (and possibly new logic) were added. | |||
|
|
|||
| ### Upgrading if *ONLY* New Logic Added | |||
| 1) Use the centre-token-client to prepare an upgradeTo transaction from the adminAccount parameterized with the address of the new upgraded token (see token-client instructions). | |||
There was a problem hiding this comment.
I think we can just say "Prepare an upgrateTo transaction... " - probably shouldn't mention token-client, since its not clear what it is in this repo (and may not be OSed at same time, or be available).
doc/upgrade.md
Outdated
|
|
||
| ## Upgrading the Proxy to point to the UpgradedToken | ||
| The proxy must be pointed to the new upgraded token. This is accomplished two ways depending on whether only new logic was added or if new fields (and possibly new logic) were added. | ||
|
|
||
| ### Upgrading if *ONLY* New Logic Added | ||
| 1) Use the centre-token-client to prepare an upgradeTo transaction from the adminAccount parameterized with the address of the new upgraded token (see token-client instructions). | ||
| 2) Broadcast the transaction | ||
| 3) Call getImplementation on the proxy (a read call, no signed tx required) and ensure it matches the address of the upgraded token. | ||
| 3) Check that the implementation field of the proxy matches the address of the upgraded token by calling web3.eth.getStorageAt(proxy.address, implSlot), where implSlot is defined in the contract as a hardcoded field. As of CENTRE Fiat Token v1.0.0 that slot is 0x10d6a54a4754c8869d6886b5f5d7fbfa5b4522237ea5c60d11bc4e7a1ff9390b. Alternatively, getImplementation may be called on the proxy with the adminAccount. | ||
| 4) If the address in 3) does not match, it is likely a wrong address was used. Repeat the process from step 1). | ||
|
|
||
| ### Upgrading if New Fields (and possibly new Logic) Added | ||
| 1) Use the centre-token-client to prepare an upgradeToAndCall transaction from the adminAccount parameterized with the address of the new upgraded token and an internal call to invoke initV[X] with the new data fields. (see token-client instructions). |
eztierney
approved these changes
Aug 7, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.