No return code from accelerator function

[marcinbugaj]

zkvm_status cannot be used in any meaningful by the called other than just terminating execution with failure. Let's simplify interface and say that if the operation cannot be performed for some reason the accelerator function shall terminate with failure itself.

If a function is called with a NULL pointer, the function SHOULD panic.

That requirement was removed. The contract is that pointers must be valid. It does not make sense for each accelerator function to internally check if the pointer is null.

Instead such specification was added:

The functions may panic if any pointer pointer argument is not valid of if an operation cannot be performed.

[kevaundray]

Can you explain the rationale here?

Previously we were returning an error if the operation could not be performed; maybe some input fails validation checks. Now it says that the function MAY panic?

[marcinbugaj]

Previously we were returning an error if the operation could not be performed; maybe some input fails validation checks

Yes, if the input validation fails then the caller won't fix the input and won't try to re-execute the precompile. It will simply panic - that's the only reasonable way to handler error from precompile. That's why I think that it's cleaner if precompiles panic themselves.

[kevaundray]

It will simply panic - that's the only reasonable way to handler error from precompile

why is panicking the only reasonable way to handle an error from a library call?

[marcinbugaj]

Because error returned by the precompile is necessarily a result of a (non-recoverable) bug in the application. The only way to handle it is to panic.

[kevaundray]

The only way to handle it is to panic.

I'm not sure -- to be concrete, so when the EVM calls a precompile, and the input is perhaps malformed (lets say the public key is not correct), then calling into this library would panic vs returning an error.

If the EVM doesn't want to panic, but instead bubble up an error to say block validation failed, then it would need to do these checks upfront and always pass valid input?

[marcinbugaj]

Well, looking at this interface some functions return error, some don't. sha256, secp256r1_verify_signature don't. For those that do the error is detailed, with the possible reason being: OutOfGas and others being precompile specific. We don't provide detailed error mapping. Breaking the contract with the function being called is IMO a valid reason to panic. I think that panicking precompiles don't change the semantics of the STF guest.

[kevaundray]

Breaking the contract with the function being called is IMO a valid reason to panic.

Most cryptography libraries do not assume that the points/inputs being passed in are valid for example. The EVM cannot check by itself that the curve point is valid, so it will pass the point to the cryptography library and it would panic instead of returning an error

I think that panicking precompiles don't change the semantics of the STF guest.

Can you elaborate?

[marcinbugaj]

Can you elaborate?

My assumption is that if a precompile returns error then EVM will directly translate it to panic or failed transaction because it can't recover. That's why panicking early in a precompiles would be no different than returning error from it.

Also, the error returned in the current API is not very informative - it doesn't differentiate error codes so it's of limited use - in particular one cannot tell OutOfGas from other error types.

[kevaundray]

My assumption is that if a precompile returns error then EVM will directly translate it to panic or failed transaction because it can't recover

panic and failed tx are different things though -- even if this was the case, I'm saying that the cryptographic library should not panic on untrusted input. The logic that a library should panic because its caller will panic doesn't really make sense.

one cannot tell OutOfGas from other error types.

I don't think it's possible to return out of gas with a cryptographic operation, since its an evm specific thing

[marcinbugaj]

Ok. You're right. Closing the PR.