search

filecoin-project / specs

The Filecoin protocol specification
https://spec.filecoin.io
Other
367 stars 170 forks source link

Clarify hashing questions #862

Open mishmosh opened 4 years ago

mishmosh commented 4 years ago

Two questions (and presumed answers) from implementers. @porcuquine @dignifiedquire can you please confirm, or weigh in?

  1. Are we using SHA256 or BLAKE2? @jzimmerman believes we have transitioned completely to SHA256.

  2. Is hashing a runtime facility? @sternhenri believes this should be a runtime/syscall and plans to PR an update to the spec.

cc @anorth

riptl commented 4 years ago

A lot of those hashing questions came up during Ledger+TrustWallet integrations, in particular how Message CIDs are built. Lotus source is using blake2b-256 with CBOR encoding prefixes but a clarification in the docs would be very helpful. Since SignedMessage signatures are build from CIDs, is it reasonable to force the use of a specific multihash prefix with Messages?

anorth commented 4 years ago

@whyrusleeping in conversation indicated to me that "everything is blake2b". I believe that the proofs world and the blockchain world here are different and neither seems to care what hashing the other is using. That makes the boundary a bit unclear.

The blockchain code is currently inconsistent. E.g. the randomness code uses SHA (@sternhenri) but addresses use blake2b.

whyrusleeping commented 4 years ago

Everything in the blockchain side of things is blake2b. Proofs is a separate thing that has a very different set of constraints, the concerns are separate.

whyrusleeping commented 4 years ago

Given that, there are a few instances in the code that use sha256 that should get switched over to blake2b for consistency. An implementer of filecoin should be able to use only a single hash function (assuming they are using an existing proofs library)

sternhenri commented 4 years ago

Im ok with that. Per notes from interop sync last week I’d understood you wanted to switch all to SHA. Are we all on same page for blake2? Either way is fine. Let’s just all agree.

On Tue, Feb 4 2020 at 20:05, Nicolas Gailly < notifications@github.com > wrote:

Given that, there are a few instances in the code that use sha256 that should get switched over to blake2b for consistency. An implementer of filecoin should be able to use only a single hash function (assuming they are using an existing proofs library)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub ( https://github.com/filecoin-project/specs/issues/862?email_source=notifications&email_token=AAZUC26DFHURY4T4ATQPCY3RBIGFTA5CNFSM4KMMZAZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKZY3SY#issuecomment-582192587 ) , or unsubscribe ( https://github.com/notifications/unsubscribe-auth/AAZUC2Y3PSXOR5AVCMFVRUTRBIGFTANCNFSM4KMMZAZA ).

porcuquine commented 4 years ago

Internally, the proofs use sha256 and pedersen (currently, will switch to poseidon). These choices should be encapsulated in the proofs library, and there is no reason they need to be considered elsewhere, except for the following points of contact (as far as I can think of now):

  1. CommD and CommP are constructed using sha256 (truncated — this should be handled by proofs).
  2. Election PoSt partial tickets are finalized with sha256. Proofs does not actually need to know about how this is done; it is a consensus concern. However, I believe (cc: @dignifiedquire) proofs currently returns finalized ticket values as a convenience. If the finalization function were to change (to, say, blake2b), proofs should probably not absorb that change but rather stop returning anything but the partial ticket.
dignifiedquire commented 4 years ago

Ticket finalization should stay with SHA256, unless there is an important reason to switch, for now this is part of proofs, even though it relates to concensus and changing it would just create more work at this point.

I agree with @whyrusleeping that anything outside of proofs should be unified to use Blake2b, and anything inside of proofs should be "ignored" by the rest of the world, as there are much more complex reasons for choosing specific hashing functions there.

One caveat is the hashing function used when creating and verifying BLS signatures, which currently is Blake, but might switch depending on our alignment with RFC drafts and Ethereum 2.0. This again should not concern most node implementers though, as this is a specific library that handles signing and verification.

sternhenri commented 4 years ago

Ok! I'll do that then. Also @porcuquine @dignifiedquire I didn't know the proofs lib returned finalized tickets, nor for instance what RegisteredProofs are. It might be time to sync about changes you've been making close to the interface...