Skip to content

Conversation

@laser
Copy link
Contributor

@laser laser commented May 31, 2019

We would like to roll out support for new sector sizes without requiring a change to the protocol.

A Filecoin node can verify seal and PoSt proofs for any sector size which they have verifying keys for. Adding support for a new sector size is as simple as distributing new verifying keys (for PoSt and seal) for that sector size.

The Filecoin node (i.e. not the protocol) should specify an enumeration of sector sizes for which Groth parameters and verifying keys are available. The protocol does not prevent an operator from creating a storage miner actor with a sector size for which no Groth parameters have been published - but they won't be able to generate proofs.

@laser laser requested review from porcuquine and whyrusleeping May 31, 2019 15:23
@laser laser changed the title network does not define an enumeration of supported sector sizes [PROPOSAL] network does not define an enumeration of supported sector sizes May 31, 2019
@laser laser requested a review from acruikshank May 31, 2019 15:24
@dignifiedquire
Copy link
Contributor

Adding support for a new sector size is as simple as distributing new verifying keys

how would "the network" verify these keys?

@whyrusleeping
Copy link
Member

@laser yeah, this doesnt work. Everyone in the network has to have all the verifying keys. Even if you post the keys on the chain so everyone has them, how do you know someone didn't generate their own parameters and keep the secret keys, thus allowing them to craft arbitrary fake proofs and game the market?

@porcuquine
Copy link
Contributor

All miners on the network will need to agree on the current set of valid verification keys, and everyone will have to have all verification keys. If and only if this is true, proofs can be trusted.

We already have a mechanism for publishing the officially trusted set of verification keys: parameters.json — the manifest published by rust-fil-proofs. If we want a mechanism for the source code of a Filecoin node to commit to a particular set of verification keys, we can use some form of vector commitment -- like a hash of the sorted hashes of all the keys, or whatever.

If we want to centralize publication of that blessed verification key set in some place which is neither rust-fil-proofs nor every Filecoin node implementation, we can do that independently. We will need to publish a verifiable outcome of the trusted setup in any case, and if we perform later amendments to that setup in order to increase the size of the trusted verification key set, we can do that using the same channel.

As long as all verifiers have access to the full set of verification keys and can validate that no rogue keys have been included, nothing further should be required. The parameters.json manifest ensures everyone can find and acquire the needed keys. A trusted channel — whether source code or otherwise — can be used to validate the content of the manifest (i.e. the membership of the verification key set).

@laser
Copy link
Contributor Author

laser commented Jun 2, 2019

Some thoughts, @whyrusleeping

Everyone in the network has to have all the verifying keys.

When new Groth parameters become available (because we create and distribute them), all nodes in the network which process commitSector messages must have the corresponding verifying key (or will not be able to verify the seal). To ensure that miners are prepared for the release of a new sector size, we could publish the corresponding verifying key several weeks before Groth parameters.

how do you know someone didn't generate their own parameters and keep the secret keys

The parameters.json file in rust-fil-proofs contains an enumeration of the Groth parameters and verifying keys which we have blessed, along with a hash of each parameter and key.

Assuming that miners trust the contents of that file, I do not understand how a malicious miner would be able to convince other miners that their (bogus) verifying key was legitimate. If the malicious miner can't convince others to accept their bogus verifying key, their commitSector messages won't be processed successfully and they won't be able to increase their power.

@laser
Copy link
Contributor Author

laser commented Jun 3, 2019

@whyrusleeping - After the above messages from myself and @porcuquine, do you still believe that my proposal is not acceptable?

@porcuquine
Copy link
Contributor

@laser

When new Groth parameters become available (because we create and distribute them), all nodes in the network which process commitSector messages must have the corresponding verifying key (or will not be able to verify the seal). To ensure that miners are prepared for the release of a new sector size, we could publish the corresponding verifying key several weeks before Groth parameters.

I don't think this is the best approach. It complicates distribution with an implied guarantee that after a certain amount of time, new keys will have been downloaded. I think it makes most sense to distribute parameters and corresponding keys together as soon as they exist. Miners should still probably wait to use new keys until they can reasonably expect verifiers to have them, but I'm not sure the distribution mechanism should take responsibility for managing how long that waiting period 'should' be. I think it's a matter of individual risk, somewhat like reliance on transaction confirmation times.

Verifiers should be motivated to have up-to-date keys, since if most other verifiers do and one doesn't, then the one without valid keys will wrongly reject valid proofs and therefore have a technically wrong view of the chain.

@whyrusleeping
Copy link
Member

@laser @porcuquine the problem I have is that adding (or removing) parameters is a 'hard fork' level network upgrade. It requires everyone to now have (effectively) new software (even if its something that could be automatically downloaded). So the question is, what is the coordination mechanism? Who gets to make the calls? Do these changes happen automatically?

Having the allowable parameters hard coded into the chain is much simpler to reason about than this proposal.

@whyrusleeping
Copy link
Member

A solution that is easier to think about would be having an actor on chain that holds all the available parameters, and we could add new parameters to that actor via message. However, that's an unworkable solution as it puts an unacceptable amount of responsibility and power in the hands of whoever is able to send that message (and this gets into governance).

@porcuquine
Copy link
Contributor

@whyrusleeping

  1. I agree that adding parameters amounts to a hard fork.
  2. We can choose whatever strategy we want for how such a hard fork would be effected if we choose to add parameters. This can be optimized to be 'lightweight' or 'heavy'.
  3. I don't think there needs to be anything on chain — that adds the complexity you note.
  4. None of this comes up if we don't perform such a hard fork, and we don't necessarily have to worry about this if we don't intend to ever have one or don't want to design the process in advance. That is, the mechanism for supporting the change could also just be provided in the code implementing the hard fork more generally.

What I'm getting at is that when the network launches, there will have been a single trusted setup. As a result, there will be a single initial set of verification keys. At network launch, everyone participating will need to know what these are and to have received them.

We don't actually need anything more. Exactly what characteristics we want when making changes in the future is a decision that can be deferred until then. I believe this approach maximizes freedom to make that decision later because it doesn't require that any particular mechanism be in place — simply releasing and (somehow) 'blessing' new parameters/keys is a minimal solution that would work, but we're free to add more mechanism as part of a hard fork if we feel it adds value.

@whyrusleeping
Copy link
Member

@porcuquine How about, we have a place on chain for the parameters to go, effectively a mapping of sector size to verification key. This is hard coded, and can be changed during a network upgrade (fork). This way, theres no ambiguity around exactly which parameters are being used when, and it also answers the question of 'where do these parameters come from?' better.

@porcuquine
Copy link
Contributor

Maybe. However, it's not a straightforward mapping of sector-size -> verification key — because (at least as of now), multiple proof sizes are also possible. So any such mapping would need to be of (sector-size, proof-size) pairs to verification keys.

I don't completely understand how it would work, but in principle I don't see a problem with allowing the chain to supply the identify of the verification key -- presumably as a Cid. Is that what you had in mind?

I think the cheapest way to allow deterministic verification is probably just to have the chain supply a hash so that the explicit set provided in the manifest can be validated. I don't have strong feelings apart from wanting to minimize the development effort beyond what's strictly necessary. So I'd be curious to hear laser's assessment of the relative implementation cost of the options.

@laser
Copy link
Contributor Author

laser commented Jun 3, 2019

I think the cheapest way to allow deterministic verification is probably just to have the chain supply a hash so that the explicit set provided in the manifest can be validated.

This sounds sane to me.

  • We will not ask the world to trust the contents of a mutable, GitHub-hosted JSON file in a Git repo which PL controls.
  • Instead, we commit to a hash of the parameters.json file at a point in time (or, as @porcuquine suggests, a hash of the sorted content identifiers for each verifying key) and place that commitment on chain.
  • The addition of new sector sizes will necessitate a fork, as @whyrusleeping pointed out.
  • The addition of new sector sizes will not require a protocol change.
  • We can delete the SupportedSectorSize function from the protocol.

How's that sound, @whyrusleeping @porcuquine?

@porcuquine
Copy link
Contributor

With the stipulation that I would prefer that there be no explicit dependency on the format of parameters.json, that seems fine. I think it balances needs in a way that minimizes both effort and complexity.

@whyrusleeping
Copy link
Member

With the stipulation that I would prefer that there be no explicit dependency on the format of parameters.json

I think I agree with this, but the computer software needs to be able to pull this and parse it and verify it automatically. It shouldnt arbitrarily change.

@porcuquine
Copy link
Contributor

With the stipulation that I would prefer that there be no explicit dependency on the format of parameters.json

I think I agree with this, but the computer software needs to be able to pull this and parse it and verify it automatically. It shouldnt arbitrarily change.

Agreed. I think parameters.json should still exist and serve as the means by which the official parameter/verification-key set is acquired. I believe knowledge of the official set should be used to derive the on-chain commitment to that set.

Since what goes on chain will be part of the protocol, I'm saying it shouldn't depend on the exact content and format of the parameters.json source file itself. I continue to repeat the example (but could be different) of a hash of the sorted Cids of the verification keys (could also include the Groth parameters for completeness, if we are doing it this way).

What we don't want is a situation in which — for example — a whitespace change to parameters.json causes breakage. Nor do we want to specify the details of the json wrapping, or even the chosen parameter/key 'file names' as part of the protocol itself. All that matters is the payload (the verification keys and perhaps Groth parameters).

@laser laser changed the title [PROPOSAL] network does not define an enumeration of supported sector sizes network does not define an enumeration of supported sector sizes Jun 4, 2019
@laser laser changed the title network does not define an enumeration of supported sector sizes protocol does not define an enumeration of supported sector sizes Jun 4, 2019
We would like to roll out support for new sector sizes without requiring a change to the protocol.

A Filecoin node can verify seal and PoSt proofs for any sector size which they have verifying keys for. Adding support for a new sector size is as simple as distributing new verifying keys (for PoSt and seal) for that sector size.

The Filecoin node (i.e. not the protocol) should specify an enumeration of sector sizes for which Groth parameters and verifying keys are available. The protocol does not prevent an operator from creating a storage miner actor with a sector size for which no Groth parameters have been published - but they won't be able to generate proofs.
@laser laser force-pushed the feat/protocol-does-not-constrain-sector-sizes branch from d128c2d to a3434e0 Compare June 4, 2019 18:33
@laser
Copy link
Contributor Author

laser commented Jun 4, 2019

Just rebased; conflicts in actors.md now resolved.

@laser
Copy link
Contributor Author

laser commented Jun 19, 2019

@whyrusleeping - Have we successfully convinced you that supported sector sizes will not be something defined by the protocol? If so, is there anything I can do to move this PR along?

@dignifiedquire
Copy link
Contributor

Given that we are not sure how exactly adding sector sizes will work I would suggest for now to keep a hard coded list of supported sector sizes in the spec.

@pooja pooja mentioned this pull request Jul 30, 2019
18 tasks
@pooja pooja mentioned this pull request Aug 27, 2019
22 tasks
@laser
Copy link
Contributor Author

laser commented Aug 28, 2019

I don't think there's any further work to be done on this PR, since the actor method already calls in to some fantasy SupportedSectorSize function whose implementation is not specified (which follows the pattern established by the GenerationAttackTime fantasy function). Long story long, the protocol will in fact define an enumeration of supported sector sizes.

@laser laser closed this Aug 28, 2019
@nicola nicola deleted the feat/protocol-does-not-constrain-sector-sizes branch November 29, 2019 02:09
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.

5 participants