builtin-actors
builtin-actors copied to clipboard
Validate proof types on decode
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.
@Stebalien mind pasting a link to the code that validates manually, to illustrate the case here? 🙏
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: https://github.com/filecoin-project/builtin-actors/blob/1a4287f1452e0a1a226ccfcaec105b370af9cc3d/actors/miner/src/lib.rs#L4069-L4079
The invalid checks are things like:
- https://github.com/filecoin-project/builtin-actors/blob/1a4287f1452e0a1a226ccfcaec105b370af9cc3d/actors/runtime/src/builtin/sector.rs#L29
- 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.
@Stebalien is this still relevant?
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.