Validate proof types on decode

[Stebalien]

Currently, we validate proof types manually inside our actors to match go's behavior. It would be safer to simply reject invalid proof types when we attempt to decode state/messages.

Doing this for NV16 (M1) is pretty easy: we'd just remove the Invalid cases from our enums. E.g.:

https://github.com/filecoin-project/ref-fvm/blob/0692d5bbb4d7ca588278a3544a48504a7f5ed0ee/shared/src/sector/registered_proof.rs#L30

Doing this after Nv16 is possible, but slightly tricker as these types tend to be "shared" between versions. Although it shouldn't be too bad as we won't re-compile nv16 actors in nv17.

[raulk]

@Stebalien mind pasting a link to the code that validates manually, to illustrate the case here? 🙏

[Stebalien]

Hm. That's actually interesting. Usually, when we "check" the proof types, we check if the proof type is within some set of "allowed" proof types, not if the proof type is "valid".

For example:

builtin-actors/actors/miner/src/lib.rs

Lines 4069 to 4079 in 1a4287f

	fn check_valid_post_proof_type(proof_type: RegisteredPoStProof) -> Result<(), ActorError> {
	match proof_type {
	RegisteredPoStProof::StackedDRGWindow32GiBV1
	| RegisteredPoStProof::StackedDRGWindow64GiBV1 => Ok(()),
	_ => Err(actor_error!(
	ErrIllegalArgument,
	"proof type {:?} not allowed for new miner actors",
	proof_type
	)),
	}
	}

The invalid checks are things like:

• builtin-actors/actors/runtime/src/builtin/sector.rs

  Line 29 in 1a4287f

  Invalid(i) => Err(anyhow::anyhow!("unsupported proof type: {}", i)),

• https://github.com/filecoin-project/ref-fvm/blob/0692d5bbb4d7ca588278a3544a48504a7f5ed0ee/shared/src/sector/registered_proof.rs#L85
• https://github.com/filecoin-project/ref-fvm/blob/0692d5bbb4d7ca588278a3544a48504a7f5ed0ee/shared/src/sector/registered_proof.rs#L158

I guess the primary benefit of checking earlier is taht we'd be able to get rid of all the "results" like https://github.com/filecoin-project/ref-fvm/blob/0692d5bbb4d7ca588278a3544a48504a7f5ed0ee/shared/src/sector/registered_proof.rs#L145, because those methods would be infallible.

[anorth]

@Stebalien is this still relevant?

[Stebalien]

Yes. Basically, it involves getting rid of statements things like https://github.com/filecoin-project/ref-fvm/blob/b30538700be419b42719ecf75350ef6836be770e/shared/src/sector/registered_proof.rs#L30.

That would mean we simply wouldn't decode messages that reference invalid proof types.