Return encoded extrinsics without padding #1505
Conversation
clangenb
added
A0-core
| skip_ra.into(), | ||
| ) | ||
| }; | ||
|
|
||
| ensure!(result == sgx_status_t::SGX_SUCCESS, Error::Sgx(result)); | ||
| ensure!(retval == sgx_status_t::SGX_SUCCESS, Error::Sgx(retval)); | ||
|
|
||
| Ok(unchecked_extrinsic) | ||
| Ok(Vec::from(&unchecked_extrinsic[..unchecked_extrinsic_size as usize])) |
There was a problem hiding this comment.
I think this needs a check that unchecked_extrinsic_size < unchecked_extrinsic.len() to prevent panics. This true for all the following usages too.
There was a problem hiding this comment.
I think this is implicitly guaranteed by the ffi impl, isn't it
If it's out of index, it should have error'ed out within write_slice_and_whitespace_pad
There was a problem hiding this comment.
Hmm, yes and no. If we have implemented everything correctly, yes, but the compiler can't make any checks across the ffi-boundaries. So I prefer being defensive here, as it just needs an error of some programmer in the future to introduce potential panics here.
Sorry, for being nitpicky here. 🙏
There was a problem hiding this comment.
Hmm, on the other hand the panic is on the untrusted side, so returning an error or panicking here, is essentially the same from the enclave's perspective.
hmm, still I would like to push the best practices as much as possible, so please fix it. :)
There was a problem hiding this comment.
Hmm, it is actually not the same, one case writes it in the, mut sgx_staus_t result and the other one returns the sgx error from the function. :)
|
good spot @Kailai-Wang, thanks a lot for this contribution. the remaining CI error is most likely a credential access issue because your PR is from external. |
|
I agree, sorry for being picky! |
It should fix the
ExtrinsicNotFounderror during enclave registration.With the padded version (post-padded with spaces
0x20) the extrinsic submission would succeed since the decoding goes through even with postpositioned (unused) bytes. However, the substrate-api-client wouldn't find the hash with the prolonged extrinsic that we pass in.Related: scs/substrate-api-client#624