Origin: Storing Data in a Separate Contract

[shahthepro]

The Problem

At present, the following data are stored on-chain in the Marketplace contract.

Listing[] public listings;
mapping(uint => Offer[]) public offers; // listingID => Offers
mapping(address => bool) public allowedAffiliates;

In future, if the Marketplace contract has to be upgraded or updated, to add a new feature or to fix a critical bug, All the data stored (which are listings, offers and allowedAffiliated) in the old contract has to be moved to the new contract.

The data migration is not at all feasible and may require a lot of gas if on-chain data is huge. And no software is bug free. There might come a time when the contract has to be replaced, upgraded, updated or removed.

My Proposal

A data store contract, similar to this, can be used to just store the data while the existing Marketplace contract will interact with it.

Basically, The DataStore contract will have methods to read and write the primitive types (uint, int, boolean, address, string, bytes). Anyone can read from the DataStore, however only whitelisted addresses can write to it. (Whitelisting can be achieved through the RestrictableContract that I have included in PR #1792).

The DataStore contract will store only key-value pair where the key is an hash of some string. Say if the count of listings has to be stored, Something like this can be done:

dataStoreAddr.setUIntValue(keccak256("listings/count"),  1);

When listing count is needed:

uint count = dataStoreAddr.getUIntValue(keccak256("listings/count"));

For complex structures like the Listing struct, Each property of the struct can be stored separately.

uint listingID = dataStoreAddr.getUIntValue(keccak256("listings/count"));

dataStoreAddr.setAddressValue(keccak256(abi.encodePacked("listings/", listingID, "/address")), seller);
dataStoreAddr.setUIntValue(keccak256(abi.encodePacked("listings/", listingID, "/deposit")), deposit);
dataStoreAddr.setAddressValue(keccak256(abi.encodePacked("listings/", listingID, "/depositManager")), depositManager);

dataStoreAddr.setUIntValue(keccak256("listings/count"), listingID + 1);

The DataStore will be deployed only once. When deploying Marketplace contract, the address of DataStore will be passed to the constructor. After Marketplace has been deployed, it's address has to be whitelisted in DataStore for write operations.

When the Marketplace contract has to be replaced, we deploy it and whitelist it on the DataStore contract.

Pros

• Data and logic will be in separate contracts. Making it easier to upgrade the contract in future.

• New fields/parameters can be introduced to the contracts without breaking existing data.

  Cons

• Since the keys are hashed, Each operation will require a bit more gas than what it is using now.

[micahalcorn]

☝️ cc @nick @franckc @joshfraser

[joshfraser]

@shahthepro really appreciate your effort and contribution here!

For some historical context, this was actually the general approach @matthewliu and I described in our initial whitepaper. Over time, more and more of our logic has moved out of the smart contracts and into our JavaScript libraries. This includes the way we think about upgradability.

Our current approach is to handle all the upgrading in JavaScript and write adaptors to convert things between the different smart contract versions. We've intentionally kept out marketplace contract lightweight and don't expect to make many contract-level changes. When we do need new contracts, we don't worry about replacing the old ones, we just add a new one and write an adaptor that knows how to display the previous version in the new format. We can also apply these adaptors recursively. The benefit with this approach is that our JavaScript code is easy to upgrade and we don't have to pay the extra gas costs on every transaction.

There are arguments to be made for both approaches, so happy you brought this up and open to any discussion from the team. We haven't gone through the process of upgrading our contracts yet, so it's all pretty theoretical.

[shahthepro]

@joshfraser

Our current approach is to handle all the upgrading in JavaScript and write adaptors to convert things between the different smart contract versions. We've intentionally kept out marketplace contract lightweight and don't expect to make many contract-level changes.

That sounds good.

When we do need new contracts, we don't worry about replacing the old ones, we just add a new one and write an adaptor that knows how to display the previous version in the new format.

So the adaptor knows how to display the data in old contracts. What about writing to old contracts? What will happen to (unexpired and still open) listings on the older contracts? Will users be able to make offers on those listing? If yes, will it be done on older contracts (through adaptors) or on the new contracts?

There are arguments to be made for both approaches, so happy you brought this up and open to any discussion from the team.

I agree. Let's also have others to share their opinions on this.

[franckc]

To answer your question, adapters would read/write data for a given version of the contract.
So if a listing is created with marketplace contract V1, until the listing is withdrawn all offers on that listing made would use adapter V1, even if a more recent marketplace contract V2 gets released.
We had successfully validated this approach with our previous iteration of the DApp (aka DApp1).

The data store approach would work. Though as you mentioned, it would increase gas cost which is a negative from end user perspective and would also add some complexity to the contracts code.
Overall I don't feel revamping our current contract is a high priority right now since we are still very much experimenting and trying to find product/market fit...

Just my 2 cents :)

[nick]

Closing as we not have multi-contract support in GraphQL