SIMD-0182: Consume requested CUs for sBPF failures #182
Conversation
Benhawkins18
changed the title
| If VM execution returns any error except `SyscallError`, transaction's CU meter | ||
| should be fully depleted, in another words, all requested CUs are consumed; | ||
| otherwise consumes the actual executed CUs. |
There was a problem hiding this comment.
Just out of curiosity, why make syscall errors exempt?
There was a problem hiding this comment.
My understanding is syscall not happen in VM realm. @Lichtso @ptaffet-jump please correct me if I'm wrong.
There was a problem hiding this comment.
Syscall errors are never thrown in the middle of basic blocks, so we can easily know exactly how many CUs were consumed when the error was thrown. This is why we can safely make them exempt.
|
|
||
| ## Detailed Design | ||
|
|
||
| If VM execution returns any error except `SyscallError`, transaction's CU meter |
There was a problem hiding this comment.
I see that with direct mapping enabled, we convert EbpfError::AccessViolation into the SyscallError variant here: https://github.com/anza-xyz/agave/blob/af0ed22174999cb62579a0621f6b274c85ebf267/programs/bpf_loader/src/lib.rs#L1501
I am assuming that this access violation should also deplete compute units and should not be considered as an exception right?
There was a problem hiding this comment.
Yes, same reasoning as: #182 (comment)
There was a problem hiding this comment.
Should consume all cu's or not?
There was a problem hiding this comment.
Yes, we should. This is not caused by a syscall so the exact instruction counter would have to be calculated otherwise.
There was a problem hiding this comment.
Just want to call out that this type of access violation is probably going to be fairly common when direct mapping is enabled. So it's not exactly true to say that these types of errors are "rare, exceptional situations" or "A rare case" as we say in the SIMD. We should update the SIMD to say that these types of failures are "less common" rather than rare at least.
There was a problem hiding this comment.
I see that with direct mapping enabled, we convert EbpfError::AccessViolation into the SyscallError variant here: https://github.com/anza-xyz/agave/blob/af0ed22174999cb62579a0621f6b274c85ebf267/programs/bpf_loader/src/lib.rs#L1501
I am assuming that this access violation should also deplete compute units and should not be considered as an exception right?
@tao-stones this should probably be made explicit in the SIMD. Maybe we can say that all EBPF errors from the vm are treated as irregular.
There was a problem hiding this comment.
Mmm, imo, SIMD's text If VM execution returns any error except SyscallError, .... is clearer.
Co-authored-by: Justin Starry <[email protected]>
Co-authored-by: Justin Starry <[email protected]>
tao-stones
changed the title
… VM error as "less common"
|
|
||
| ## Detailed Design | ||
|
|
||
| If VM execution returns any error except `SyscallError`, transaction's CU meter |
There was a problem hiding this comment.
I see that with direct mapping enabled, we convert EbpfError::AccessViolation into the SyscallError variant here: https://github.com/anza-xyz/agave/blob/af0ed22174999cb62579a0621f6b274c85ebf267/programs/bpf_loader/src/lib.rs#L1501
I am assuming that this access violation should also deplete compute units and should not be considered as an exception right?
@tao-stones this should probably be made explicit in the SIMD. Maybe we can say that all EBPF errors from the vm are treated as irregular.
No description provided.