(Feature, MASTER) ValidatorMetadata upgradable and voting for change metadata addresses

[varasev]

(Mandatory) Description

The purpose of these changes is to make ValidatorMetadata contract upgradable and to let the validators to vote for changing ValidatorMetadata implementation address and storage address. Changes are related to #63.

Now ValidatorMetadata will have the storage and the implementation separately.

If we need to change the implementation and to keep the storage of this contract in the future the validators will be able to do this through the Governance dApp.

This PR also includes additional tests to cover ValidatorMetadata upgradability and voting for it's addresses changing.

Also it includes the fixes described in #39, #45, #54, #55, #58, #59, #60, #62 and #64.

(Mandatory) What is it: (Fix), (Feature) or (Refactor)

(Feature)

[vbaranov]

There is no restriction, who can call this method. Let's limit to the creator of contract.

[vbaranov]

@varasev also, please process solhint errors:

line 80:13 missing ':' at '{'
line 81:14 missing ':' at '{'

./contracts/eternal-storage/EternalStorage.sol
   1:17  warning  Compiler version must be fixed                                   compiler-fixed
   8:1   error    Definition must be surrounded with two blank line indent         two-lines-top-level-separator
   8:1   error    Contract has 16 states declarations but allowed no more than 15  max-states-count
  11:3   error    Expected indentation of 4 spaces but found 2                     indent
  14:3   error    Expected indentation of 4 spaces but found 2                     indent
  17:3   error    Expected indentation of 4 spaces but found 2                     indent
  18:3   error    Expected indentation of 4 spaces but found 2                     indent
  19:3   error    Expected indentation of 4 spaces but found 2                     indent
  20:3   error    Expected indentation of 4 spaces but found 2                     indent
  21:3   error    Expected indentation of 4 spaces but found 2                     indent
  22:3   error    Expected indentation of 4 spaces but found 2                     indent
  23:3   error    Expected indentation of 4 spaces but found 2                     indent
  25:3   error    Expected indentation of 4 spaces but found 2                     indent
  26:3   error    Expected indentation of 4 spaces but found 2                     indent
  27:3   error    Expected indentation of 4 spaces but found 2                     indent
  28:3   error    Expected indentation of 4 spaces but found 2                     indent
  29:3   error    Expected indentation of 4 spaces but found 2                     indent
  30:3   error    Expected indentation of 4 spaces but found 2                     indent
  31:3   error    Expected indentation of 4 spaces but found 2                     indent
  37:3   error    Expected indentation of 4 spaces but found 2                     indent
  38:5   error    Expected indentation of 8 spaces but found 4                     indent
  39:3   error    Expected indentation of 4 spaces but found 2                     indent
  45:3   error    Expected indentation of 4 spaces but found 2                     indent
  46:5   error    Expected indentation of 8 spaces but found 4                     indent
  47:3   error    Expected indentation of 4 spaces but found 2                     indent

./contracts/eternal-storage/EternalStorageProxy.sol
   1:17  warning  Compiler version must be fixed                                                   compiler-fixed
   3:8   error    Use double quotes for string literals                                            quotes
   4:8   error    Use double quotes for string literals                                            quotes
  12:1   error    Definition must be surrounded with two blank line indent                         two-lines-top-level-separator
  19:3   error    Expected indentation of 4 spaces but found 2                                     indent
  21:3   error    Expected indentation of 4 spaces but found 2                                     indent
  22:5   error    Expected indentation of 8 spaces but found 4                                     indent
  23:5   error    Expected indentation of 8 spaces but found 4                                     indent
  24:3   error    Expected indentation of 4 spaces but found 2                                     indent
  26:3   error    Expected indentation of 4 spaces but found 2                                     indent
  27:5   error    Expected indentation of 8 spaces but found 4                                     indent
  28:5   error    Expected indentation of 8 spaces but found 4                                     indent
  29:3   error    Expected indentation of 4 spaces but found 2                                     indent
  35:3   error    Expected indentation of 4 spaces but found 2                                     indent
  36:5   error    Expected indentation of 8 spaces but found 4                                     indent
  38:5   error    Expected indentation of 8 spaces but found 4                                     indent
  39:5   error    Expected indentation of 8 spaces but found 4                                     indent
  40:5   error    Expected indentation of 8 spaces but found 4                                     indent
  42:5   error    Expected indentation of 8 spaces but found 4                                     indent
  43:5   error    Expected indentation of 8 spaces but found 4                                     indent
  44:3   error    Expected indentation of 4 spaces but found 2                                     indent
  53:3   error    Expected indentation of 4 spaces but found 2                                     indent
  54:5   error    Visibility modifier must be first in list of modifiers                           visibility-modifier-order
  57:3   error    Indentation is incorrect                                                         indent
  58:5   error    Expected indentation of 6 spaces but found 4                                     indent
  59:13  error    Avoid to use ".call.value()()"                                                   avoid-call-value
  59:5   error    Avoid to use ".call.value()()"                                                   avoid-call-value
  59:5   error    Expected indentation of 6 spaces but found 4                                     indent
  60:3   error    Indentation is incorrect                                                         indent
  66:3   error    Expected indentation of 4 spaces but found 2                                     indent
  66:3   warning  Fallback function must be simple                                                 no-complex-fallback
  66:15  error    Visibility modifier must be first in list of modifiers                           visibility-modifier-order
  66:3   error    Function order is incorrect, fallback function can not go after public function  func-order
  67:5   error    Expected indentation of 8 spaces but found 4                                     indent
  69:5   error    Expected indentation of 8 spaces but found 4                                     indent
  70:5   error    Expected indentation of 8 spaces but found 4                                     indent
  72:5   error    Expected indentation of 8 spaces but found 4                                     indent
  72:5   warning  Avoid to use inline assembly. It is acceptable only in rare cases                no-inline-assembly
  73:7   error    Indentation is incorrect                                                         indent
  74:7   error    Indentation is incorrect                                                         indent
  76:7   error    Indentation is incorrect                                                         indent
  77:7   error    Indentation is incorrect                                                         indent
  79:7   error    Indentation is incorrect                                                         indent
  80:7   error    Indentation is incorrect                                                         indent
  81:7   error    Indentation is incorrect                                                         indent
  83:3   error    Expected indentation of 4 spaces but found 2                                     indent

./contracts/interfaces/IBallotsStorage.sol
  1:17  warning  Compiler version must be fixed  compiler-fixed

./contracts/interfaces/IEternalStorageProxy.sol
  1:17  warning  Compiler version must be fixed                            compiler-fixed
  3:1   error    Definition must be surrounded with two blank line indent  two-lines-top-level-separator
  5:47  error    Visibility modifier must be first in list of modifiers    visibility-modifier-order

./contracts/interfaces/IKeysManager.sol
  1:17  warning  Compiler version must be fixed  compiler-fixed

./contracts/interfaces/IPoaNetworkConsensus.sol
  1:17  warning  Compiler version must be fixed  compiler-fixed

./contracts/interfaces/IProxyStorage.sol
  1:17  warning  Compiler version must be fixed  compiler-fixed

[varasev]

Fixes have been made.

[varasev]

I've removed initProxyAddress because it actually duplicates proxyAddress initialization in EternalStorageProxy constructor.

[varasev]

Also I think we could keep only EternalStorageProxy (ValidatorMetadataEternalStorage) address without it's implementation address in ProxyStorage contract because we can read implementation address from EternalStorageProxy. I'll remove address validatorMetadata; from ProxyStorage.sol and fix related tests a little bit later.

[rstormsf]

Please take a look at OpenZeppelin/openzeppelin-labs#49 there was some interesting find.

[varasev]

@rstormsf Yes, this has already been implemented from the very beginning :) Please see contracts/eternal-storage/EternalStorageProxy.sol and contracts/ValidatorMetadata.sol code.

[varasev]

@rstormsf If you mean that the internal _version and _implementation variables should only be in EternalStorageProxy I think we can keep them in ValidatorMetadata too. It allows us to use version() and implementation() functions not only from EternalStorageProxy but also from ValidatorMetadata. I think it is more convenient.

[varasev]

I've removed an excess ValidatorMetadata implementation address from the ProxyStorage contract. I'll repeat all of the poa-test-setup steps a bit later.

[rstormsf]

I don't think those variables should be in the metadata contract.

[varasev]

Ok, @vbaranov please express your point of view: do we need the version() and implementation() functions to be in the ValidatorMetadata contract? Or we need them only in EternalStorageProxy contract? Right now we have these functions both in ValidatorMetadata and EternalStorageProxy (see EternalStorage.sol).

[varasev]

I've repeated all the steps of https://github.com/varasev/poa-test-setup - all of them are successful.

[vbaranov]

Wrong review

[vbaranov]

version and implementation indicate a new version of implementation. So, for me seems natural to have such properties right in implementation contract too.

[varasev]

Please don't merge this PR yet until I make another PRs for poa-dapps-voting and poa-dapps-validators related to 20fb25e commit.

[vbaranov]

Please don't merge this PR yet until I make another PRs for poa-dapps-voting and poa-dapps-validators related to 20fb25e commit.

I won't. Let's wait for @rstormsf review also.