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

ERC930 - Eternal Storage Standard #930

Open
AugustoL opened this Issue Mar 14, 2018 · 20 comments

Comments

Projects
None yet
8 participants
@AugustoL
Copy link

AugustoL commented Mar 14, 2018

EIP: 930
Title: ERC930 Eternal Storage
Author: Augusto Lemble <me@augustolemble.com>
Type: Contract Standard
Category: ERC
Status: Draft
Created: 2018-03-15

Simple Summary

This contract provides the necessary logic to store any type of data in a smart contract using it as a storage.

Abstract

The ES (Eternal Storage) contract is owned by an address that have write permissions. The storage is public, which means everyone has read permissions.
It store the data on mappings, using one mapping per type of variable.
The use of this contract allows the developer to migrate the storage easily to another contract if needed.

// Using contract storage
string myName = "Vitalik"; 

// Using Eternal Storage
s.getString(keccak256("myName")) = "Vitalik";

Motivation

There is some implementations of Eternal Storage contracts already done and being used but there is not some consensus over it. Storage is one of the most important parts of smart contracts development and using this contract will allow developers to use a standardized version of ES and therefore have safer contracts and also use this standard with another that define only certain logic and behavior over a contract, for example: An ERC20 token can use the contract or eternal storage it wont affect the standard since the only thing that changes is the write/read operations used inside the functions.

Specification

EternalStorage

Note The nomenclature used for the functions and variables was tried to be as short as possible, since we don't want to have to use +30 more characters per line to assign an uint.

  • s == storage
  • h == hash
  • v == value

Storage

The storage of the contract is kept in a internal variable, it is also an struct with a set of mappings, one per each variable type.

  struct Storage {
    mapping(bytes32 => bool) _bool;
    mapping(bytes32 => int) _int;
    mapping(bytes32 => uint) _uint;
    mapping(bytes32 => string) _string;
    mapping(bytes32 => address) _address;
    mapping(bytes32 => bytes) _bytes;
  }

  Storage internal s;

Methods

owner

Returns the owner address of the ES.

function owner() constant returns (address owner)

SET methods

Execute a write operation over the storage, it can only be called by the ES
owner. It will write the value v over the boolean value identified with the
hash h.

The function SHOULD revert if the msg.sender is not the owner.

  function setBoolean(bytes32 h, bool v) public onlyOwner {
    s._bool[h] = v;
  }
  function setInt(bytes32 h, int v) public onlyOwner {
    s._int[h] = v;
  }
  function setUint(bytes32 h, uint256 v) public onlyOwner {
    s._uint[h] = v;
  }
  function setAddress(bytes32 h, address v) public onlyOwner {
    s._address[h] = v;
  }
  function setString(bytes32 h, string v) public onlyOwner {
    s._string[h] = v;
  }
  function setBytes(bytes32 h, bytes v) public onlyOwner {
    s._bytes[h] = v;
  }

GET methods

Execute a read operation over the storage. It receives the hash identifier h of the variable stored and it returns the value of it.

function getBoolean(bytes32 h) public view returns (bool){
  return s._bool[h];
}
function getInt(bytes32 h) public view returns (int){
  return s._int[h];
}
function getUint(bytes32 h) public view returns (uint256){
  return s._uint[h];
}
function getAddress(bytes32 h) public view returns (address){
  return s._address[h];
}
function getString(bytes32 h) public view returns (string){
  return s._string[h];
}
function getBytes(bytes32 h) public view returns (bytes){
  return s._bytes[h];
}

Events

OwnershipTransfered

Triggered when the ownership of the contract change.

event OwnershipTransfered(address indexed previousOwner, address indexed newOwner);

Implementations

Revisions

  • 2018/03/15: Initial Draft

Copyright

Copyright and related rights waived via CC0

@VoR0220

This comment has been minimized.

Copy link

VoR0220 commented Mar 14, 2018

It's an interesting way of solving a problem many are trying to solve.

@AugustoL

This comment has been minimized.

Copy link
Author

AugustoL commented Mar 14, 2018

I have to add a section about gas costs, hopefully I will be able to add it soon, anyone that wants to collaborate on this send me an email to me@augustolemble.com :)

@AugustoL

This comment has been minimized.

Copy link
Author

AugustoL commented Mar 14, 2018

@VoR0220 yes right? I think it is going to be used a lot whit the use of proxys for upgradeability and it will be great to have consensus over the use of it :)

@VoR0220

This comment has been minimized.

Copy link

VoR0220 commented Mar 14, 2018

Well here's what would be the way to better define that would be through a template or a generic type which I believe is already in the works, but fleshing that out would enable this in a very simplistic way. All you would need to do is take an index of templateKey and resolve it to field of templateStore which would be interpreted at runtime. This will definitely be going in the "dangerous" category as it requires low level assembly but that's what I'm thinking from a compiler definition. @chriseth @axic what do you think?

@Arachnid

This comment has been minimized.

Copy link
Collaborator

Arachnid commented Mar 16, 2018

Is it necessary for this to be an EIP? It seems like it could just be a software library.

@AugustoL

This comment has been minimized.

Copy link
Author

AugustoL commented Mar 16, 2018

@Arachnid It is not an EIP, it is a Ethereum Request for Comments . We would use this in our smart contracts and I think more projects will use it too. So this is just to kickstart the discussion about a contract standard that we can use.
Since the ERCs are posted here thats why I submitted it here 🤓

@Arachnid

This comment has been minimized.

Copy link
Collaborator

Arachnid commented Mar 16, 2018

ERCs are a subset of EIPs. Either way, my question stands - since you're looking at one particular implementation, rather than an interface multiple contracts will implement, what's the need for an EIP/ERC?

@AugustoL

This comment has been minimized.

Copy link
Author

AugustoL commented Mar 16, 2018

@Arachnid Im not looking for a particular implementation, Im looking to discuss how the interface of a smart contract with only storage purposes will be, we have ERCs with standards for tokens and proxies but there is none for a storage contract.
II think it is needed because some smart contracts will need it and is better if we all use the same standard for it.

@fulldecent

This comment has been minimized.

Copy link
Contributor

fulldecent commented Mar 20, 2018

This smells like an antipatern

@AugustoL

This comment has been minimized.

Copy link
Author

AugustoL commented Mar 20, 2018

@fulldecent

This comment has been minimized.

Copy link
Contributor

fulldecent commented Mar 20, 2018

We already have bytes4 function selectors to query any arbitrary type.

Also the set of types is arbitrarily small. The built in types are much richer and composible.

If dynamic naming is required at runtime then applications will want to explain why by using a name.

I can't think of any application where storing random numbers, identifying them by name, and those names having no context, being useful.

@AugustoL

This comment has been minimized.

Copy link
Author

AugustoL commented Mar 23, 2018

@fulldecent thx for the reply, you are right about dynamic naming and that the set of types are very limited. I imagine this being used in contracts with a simple storage like ERC20-721-827 tokens where you would be able to reuse the same storage in another contracts.
I can't think of any application where storing random numbers, identifying them by name, and those names having no context, being useful. I wouldnt describe it that way, but it is useful if you need to export the storage to another contract and have control over it. How would you do it?

@trigun0x2

This comment has been minimized.

Copy link

trigun0x2 commented Apr 12, 2018

@AugustoL I've put some thought into this topic before and I've never been sure about what layer to authenticate the user.

My thought was to have an array of whitelisted contracts that are able to interact with the ES then do the authentication (onlyOwner, onlyCOO) in the smart contract using the ES.

Would love to get your input on this topic.

@shrugs

This comment has been minimized.

Copy link

shrugs commented Apr 14, 2018

re: owner; If you need more comprehensive access control, you could compose this function with an RBAC-backed proxy that lets you have multiple different access layers and logics.

This sort of approach is basically necessary for contract upgradability. You can see various approaches attempted in https://github.com/zeppelinos/labs as well as some articles about the different approaches for eternal and unstructured storages (with a comprehensive overview of the various pros/cons and implementations coming from Elena in the near future).

I have no opinion on whether or not it fits within the ERC/EIP scope, but I very much support a standard interface for these getters and setters.

@fulldecent

This comment has been minimized.

Copy link
Contributor

fulldecent commented Apr 15, 2018

@AugustoL

but it is useful if you need to export the storage to another contract and have control over it. How would you do it?

I would specifically identify the storage requirements for that contract and handle the use case as it arrived.

I imagine this being used in contracts with a simple storage like ERC20-721-827 tokens where you would be able to reuse the same storage in another contracts.

ERC-721 is perhaps the most thoroughly reviewed ERC in modern history. A specific goal of 721 was to identify ALL potential uses cases up front. In all of this discussion I have never heard a use case that required off-the-shelf backend storage as ERC-930 is proposing. And to be sure, 721 was reviewed at four different Ethereum conferences and we had a 4-day discussion just for latin etymology and naming.

@AugustoL

This comment has been minimized.

Copy link
Author

AugustoL commented Apr 17, 2018

@trigun0x2 Ive been thinking about having something like that and also about using RBAC, I think in this case using RBAC is the best option for the same reason @shrugs mentioned, this storage contract is useful for upgradeability purposes and I saw the necessity on having a storage like this while I was working on token proxy and upgradeability.

@AugustoL

This comment has been minimized.

Copy link
Author

AugustoL commented Apr 17, 2018

@fulldecent I understand what you mean, I was looking for something more standarized to not work over different storage requirements for every smart contract, I think that using an storage like these fulfill more of the storage requirements of smart contracts. There is some cases maybe most of them where you dont even need something like this, but in my case where Im working on a set of different contracts using token proxy upgrade ability this is the best thing I came up with to use as storage for our contracts.
One of the contracts that we will use is ERC-721, but we will need the token to be upgradebale and I thought on how can I use the standard with ERC930 storage on it instead of the contact storage, only that.

@ondratra

This comment has been minimized.

Copy link

ondratra commented Apr 24, 2018

I would specifically identify the storage requirements for that contract and handle the use case as it arrived.

I must agree with @fulldecent on this. I can't imagine situation where I need generic data storage because Ethereum's smart contract itself is generic data storage, thus when I want to upgrade for example ERC20 contract it usually requires upgrade of logic(adding new implementation to proxy, etc.) but not the change of storage.

@AugustoL Can you extend the Motivation section with more detail description of situation(s) where you need such kind of contract proposed in EIP930 and what would be the tradeoffs against proxy pattern?

@jaycenhorton

This comment has been minimized.

Copy link

jaycenhorton commented May 28, 2018

Is there any reason you are taking a bytes32 for every get/set function? Why not just have the contract perform the keccak function?

IE

function getString(string h) public view returns (string){
    return _storage._string[keccak256(h)];
  }
@jaycenhorton

This comment has been minimized.

Copy link

jaycenhorton commented Jun 3, 2018

To give some more context on the above, it seems like it would be error prone to force the client to use a keccak256 compliant function to get the correct hash when retrieving or setting storage with 32 bytes. It assumes all clients interacting with the contract are using something like web3.sha3 to hash the var to retrieve, but I don't think its safe to assume that all clients will always use the same keccak256 type function, or even format it in the same 32 byte 0x prefixed hash. It seems like you should instead just have the contract perform keccak256 on the getter and setter as it would ensure consistency. It also makes client interaction simpler since the client doesn't need to rely on a sha3 provider such as web3, as they could instead just pass the string/int/etc directly into the function.

Is there some other oddity I am missing, maybe something to do with the variant type sizes (uint8/16/...256)?

In addition, Im also running into some collision issues in this type of implementation (see: https://medium.com/nomic-labs-blog/malicious-backdoors-in-ethereum-proxies-62629adf3357) . Basically, if I use this pattern, inside of something like this , and then declare a contract.at(proxy.address) then when I invoke .set or .get on the proxy, it doesn't actually work as intended due to the way DELEGATECALL works. I actually have to make a differently named function, such as actuallyGetUint which in turn calls getUint for the behavior to work as expected. This is due to an issue described somewhat in the linked article.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.