search

mesg-foundation / marketplace

MESG Marketplace Smart Contract on Ethereum
1 stars 1 forks source link

Simplify createServiceVersion api #15

Closed antho1404 closed 5 years ago

antho1404 commented 5 years ago

I don't really understand why we have both the versionHash and the manifest. On the client side we set versionHash == sha256(manifest).

Is there a good reason to have this ? Why not replacing with a keccak256 and simplify the api function like that.

function createServiceVersion(
    bytes memory sid,
    bytes memory manifest,
    bytes memory manifestProtocol
  )
   ...
    whenServiceHashNotExist(keccak256(manifest))
    ...
  {
    ...
    versionHash = keccak256(manifest);
    ...
  }
NicolasMahe commented 5 years ago

Because in the current implementation of the publish command hash = sha256(manifest) but this should not be the case. This issue is not related to the smart contract. Closing.

antho1404 commented 5 years ago

Don't understand why this is not related. The publish command is one thing but not related. I'm talking here about the the API and how to use this API from any client, it can be the MESG CLI or metamask, etherscan whatever. The API here is related to the smart contract.

If the publish command is doing hash = sha256(manifest) but this should not be the case then how should we use this API ?

What is versionHash and what is manifest in that case ?

NicolasMahe commented 5 years ago

version hash = service hash manifest = url to the manifest json file

The versionHash = sha256(manifest) in the cli will disappear for versionHash = service.hash. It is just a temporary implementation.

antho1404 commented 5 years ago

It doesn't need to be like that. We don't need to have the same hash for the deployed service and the markeplace.

We could have the manifest that contains with the link to the tarball the actual hash of the service as a verification (like when you download an archive you have its shasum).

We could keep versionHash = sha256(manifest) and break the dependency that the marketplace needs the deployed service version as index that would make things way simpler.