Skip to content

Conversation

@benbierens
Copy link
Contributor

No description provided.

originalDatasetSize: NBytes
case verifiable {.serialize.}: bool # Verifiable datasets can be used to generate storage proofs
of true:
datasetRoot: VerificationHash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
datasetRoot: VerificationHash
verificationRoot: VerificationHash

type
ManifestCoderType*[codec: static MultiCodec] = object
DagPBCoder* = ManifestCoderType[multiCodec("dag-pb")]
VerificationHash* = F
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot hardcode this, this needs to be parametrizable with the correct hash type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes perfect sense. I would like a nice generalized way we can wrap up the Poseidon2 field elements and plug them into the manifest, and drop the VerificationHash type completely.

@dryajov
Copy link
Contributor

dryajov commented Nov 29, 2023

In general, the manifest should never be hardcoded with any particular set of hashes or other types. We should use a combination of templates/generics and multicodecs to keep the manifest generic and hash type agnostic.

@benbierens benbierens force-pushed the verifiable-manifests branch 2 times, most recently from a53e4c7 to d06452d Compare December 6, 2023 08:00
@benbierens benbierens marked this pull request as ready for review December 6, 2023 08:26
Copy link
Contributor

@tbekas tbekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. I agree that it would be nice to keep manifest agnostic to the hash type. But I don't know how to do it. With multicodec we're not able to specify poseidon2 as a codec.

@benbierens benbierens force-pushed the verifiable-manifests branch from b157e3b to 9a5e0b4 Compare December 6, 2023 15:01
@dryajov
Copy link
Contributor

dryajov commented Dec 6, 2023

Overall looks good. I agree that it would be nice to keep manifest agnostic to the hash type. But I don't know how to do it. With multicodec we're not able to specify poseidon2 as a codec.

This is just a matter of adding the missing multicodecs and I already did it in vacp2p/nim-libp2p#974

@benbierens benbierens force-pushed the verifiable-manifests branch from b566f56 to 8fe5e9a Compare December 7, 2023 10:02
@benbierens benbierens enabled auto-merge (squash) December 12, 2023 07:42
@benbierens benbierens merged commit 0c3d1dd into master Dec 12, 2023
@benbierens benbierens deleted the verifiable-manifests branch December 12, 2023 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants