Change store/publish flow to allow for timestamps to be used in review records

[fazo96]

The problem is that the code that writes the timestamps is ran twice, once when storing and again when publishing. Since the storing code cannot mutate the review passed for a number of reasons, there is no way to fix this unless we make the store function write to IPFS the review and then only return the multihash, which will be passed to the publish function.

[kulpreet]

Can you point me to the two places the timestamps are being generated? I am trying to understand why the publishing code regenerates the timestamps

[fazo96]

The flow is:

1. call storeReviewRecord with publish: false in the options so that it returns your review record multihash and runs full validations, but does not put it on IPFS or OrbitDB and does not advertise it to others in any way. It returns the multihash so that you can put it in your BTC Transaction
2. create BTC transaction with multihash from previous call
3. call storeReviewRecord with the same review record as before which produces the same multihash (because as of now the process is deterministic) but use publish: true and also pass the bitcoin transaction hash and publish it to the world

We have a created field with a timestamp in all signatures. When you store, you sign as customer (if rr is verifiable) and issuer (all the time) so you create these two timestamps. This makes the process not deterministic anymore because it depends on time, so we have to change the flow otherwise on step 3 the multihash you get will be different which makes it a different review, so the validation will fail when checking the transaction (multihash mismatch) and step 3 will error out.

Also now if I try to import the same identical unverified review twice the second time should do nothing making it idempotent. However with timestamps this breaks

We should change the flow so that at step 3 we call a publishReviewRecord function and we only pass the multihash and the bitcoin transaction hash.

[kulpreet]

Why don't we set the timestamps in 1. Wouldn't that work?

[fazo96]

@kulpreet yes, but 3 has to take the multihash from 1 as a parameter and not the review record contents, otherwise the timestamps created in 1 are lost

But to accomplish this, 1 has to not only compute the multihash but store in IPFS. This is fine since nobody is going to request that review record since the multihash isn't available anywhere. The point is that step 3 will be able to fetch the previously created review records with all the sigs and timestamps using the multihash, from IPFS, without doing any network requests since the data is already local.

Since this is a change in exposed API it's a breaking change (a small one though).
I think this is the best solution

[kulpreet]

But to accomplish this, 1 has to not only compute the multihash but store in IPFS. This is fine since nobody is going to request that review record since the multihash isn't available anywhere. The point is that step 3 will be able to fetch the previously created review records with all the sigs and timestamps using the multihash, from IPFS, without doing any network requests since the data is already local.

When you say "that step 3 will be able to fetch", do you mean it is currently able to do so or will be able to do so if we make the change being discussed?

[fazo96]

Will be able to do so if we make the change. Step 1 does not store on IPFS ATM, but if we make this change it will

[kulpreet]

But that will break the txid/cid solve, i.e what comes first, the review record or the txid

[fazo96]

In step 1 the transaction id is not passed, that's the whole point there are two steps.

Step 3 can reuse the review record from step 1 because the transaction id never goes in the review record. For now it's just written to orbit-db oplog and then orbit-db index keeps it associated to the review record multihash. Maybe we should figure out how to include it in the review record, maybe using review record updates, but this is a different issue