search

filecoin-project / specs-actors

DEPRECATED Specification of builtin actors, in the form of executable code.
Other
86 stars 102 forks source link

Remove runtime abort from adt store `Get` method #1507

Open ZenGround0 opened 3 years ago

ZenGround0 commented 3 years ago

It has come up in a comparative audit of spec-actors and rust-actors that the adt store aborts on unsuccessful Get. This behavior is difficult to analyze and match in other implementations particularly in HAMT and AMT. It should also be wholly redundant with existing error handling semantics under the assumption we never drop ipld get errors.

Remove the abort and replace with an error return.

Stebalien commented 3 years ago

Hm. So, I think the right answer here is to fail the entire block. If we try to access something that doesn't exist, the blockstore is missing a block. At the very least, this should always be an "invalid state" error.

ZenGround0 commented 3 years ago

Today all these errors are handled by aborting with ErrIllegalState which I think is the right thing to do. This handling is unfortunately impossible to hit because the store will always abort first. This additional abort from within the store is redundant when the callers handler errors correcty. If we wanted to fail the entire block we would need to change the vm. Probably the way to do that is to abort from runtime StoreGet with a special exitcode that the vm catches which then stops message processing and fails the block. We could then get rid of boolean return value from StoreGet and propagate up error handling remove from most state accesses.

I think the current model of having individual actors fail with ErrIllegalState is actually a bit better because evidence of the failure is now on chain and can be raised to the programmers who caused the error to happen for fixing the state. In the model where illegal state messages fail the block then these messages would never make it on chain either halting the network or getting dropped by progressing tipsets and never explicitly denoting the error.