Listing IPFS data should include onchain data

[franckc]

In order to save gas, the contract data related to a listing gets deleted when a buyer withdraws her listing. See (code).

If we want to be able to reconstruct the exact history for a listing, we should store in IPFS the onchain listing data. Specifically:

• seller
• deposit
• arbitrator

[franckc]

@tyleryasaka @DanielVF We discussed this a bit during our engineering weekly meeting and you had some opinions... :)

A few additional notes on my side:

• I agree it is not ideal to duplicate data onchain and offchain, but it might not be that bad if we have the simple rule that if the data exists onchain it always takes precedence over offchain.
• I don't anticipate we would ever use the offchain data for any sort of transaction - really, the only use case I can think of for that offchain data is for showing transaction history (ex: my-purchases of my-sales on the DApp, in order to show that data on Listings that have been closed).
• Offers behave the same way (data gets deleted from contract when offer finalizes or is withdrawn) so whatever decision we make for Listing, we should be consistent and also do the same for Offers.

[tyleryasaka]

I like your rule, but my only worry would be that people may forget or simply not know about the rule sometime in the future. This could include people at Origin or simply people building on top of Origin. They may see the seller listed in the ipfs data and simply grab the information from there without realizing that that data can't really be relied on. Feels dangerous to me.

Do we know specifically what the use case for having this historical data is? Just to show interesting stats, or for reputation purposes, or just in case we ever want it later?

If it's just because the data is interesting or we think we may want to do something with it later, then I think backing up the data ourselves (in a backup database, for example) makes more sense. If we're going to do something meaningful with it, then I'm worried about trusting arbitrary ipfs blobs.

[DanielVF]

It is possible to query the blockchain as it was in the past. So for example, I can run a getListing(...) call against the blockchain as it was at any point in it's history. (Look at the "BLOCK PARAMETER" docs in https://infura.io/docs/api/post/eth_call)

Because of this, even if we are deleting data, it's still there for the getting - we just have to allow a block number to be passed as an option to our get'ing methods. (This of course, requires knowing the block number you want, which would requiring querying for events first, then doing the call)

I think it would be awful to duplicate critical data into the IPFS. Echoing what Tyler said, even if we somehow manage not to mess up and improperly use this data, someone else will. It's an unnecessary security problem, especially since we already can get the old data in other ways.

[franckc]

CC @nick

Super helpful comments - thanks @tyleryasaka and @DanielVF ! In particular I did not realize the blockchain can be queried at any point in history (it does make sense BTW).

To help making a decision, let's gather the use cases we anticipate we could need that deleted data for. I can drive this.

[nick]

Although we can query old blockchain data, I think that's only possible on nodes with a 'full sync' (ie complete blockchain history). 'Light' clients (with only current state) would need to ask a full archive node for that data. I guess that's not a big deal though.

I don't think we need to worry about duplicating data on IPFS. Our primary interactions with listings and offers should be via GraphQL or the OriginJS API which can abstract away the current 'source of truth', whatever we decide that should be. Perhaps we can put the data in a field that makes it obvious it shouldn't be trusted, like __untrustworthyChainData or something (following React's lead of properties like __dangerouslySetHTML).

[tyleryasaka]

Yeah if we do put the fields in IPFS, I would feel better if they were nested under a property that denotes them as being unsafe, such as the one @nick suggested (__untrustworthyChainData).