maidsafe / sn_dbc

Safe Network DBCs
BSD 3-Clause "New" or "Revised" License
15 stars 16 forks source link

store mint's root public key in Dbc rather than derived-from-denom key #100

Closed dan-da closed 3 years ago

dan-da commented 3 years ago

Previously, Dbc::mint_public_key contained a pk derived from the Mint's root public key using Dbc::denomination().to_bytes() as the derivation index.

This had the nice property that one could simply perform: self.mint_public_key.verify(self.signature)

However, given only the derived key, one cannot determine the Mint's public key, which can be useful for validating the mint's authority.

It would be possible to simply add the root pk in addition to the derived key, however that seems wasteful because it enlarges the DBC and the derived key can be calculated.

So the approach taken here is to store the root key and derive the denomination key when signing or validating. Because the pk and signature stored in the Dbc are no longer "paired", I renamed mint_public_key to mint_root_public_key to hopefully show this distinction and also added a comment about it.

Performance Impact:

This change require that the Mint node perform an extra ::derive_child() operation for each input dbc when validating. Also clients have to derive when building, but I don't think we would care about that.

If we later wish to optimize for speed instead of size, then we could consider storing both PKs in the Dbc instead.

Unresolved

One thing I don't understand yet, and would like more eyes on @davidrusu @iancoleman before merging is that when the client is combining mint SignatureShares, it does not appear to matter if we use the root PublicKeySet or derived PublicKeySet.

For example, we can comment out builder.rs line 266 and/or mint.rs line 277, and the resulting DBCs still validate, all tests pass. Does that seem correct?

davidrusu commented 3 years ago

when the client is combining mint SignatureShares, it does not appear to matter if we use the root PublicKeySet or derived PublicKeySet.

Yep, I had the same question a week or so ago and traced through blsttc/threshold_crypto code. Turns out the crypto behind the aggregation doesn't need a public-key-set, it just needs the count of keys in the set, public key set is just a convenient place to get that key count from

The key count is the same for the derived key sets as the parent key sets, so signature aggregation still works as you'd expect.

dan-da commented 3 years ago

The key count is the same for the derived key sets as the parent key sets, so signature aggregation still works as you'd expect.

ok, I've removed the ::derive_child calls, and added a comment about it.