fix: fix syscall length calculation #82
Conversation
| machine.memory_mut().store64(size_addr, real_size as u64)?; | ||
| let full_size = data.len() - offset; | ||
| let real_size = cmp::min(size, full_size); | ||
| machine.memory_mut().store64(size_addr, full_size as u64)?; |
There was a problem hiding this comment.
Why we don't need the length prefix before?
There was a problem hiding this comment.
This bug already exists before the refactor, but it's not used: previously we have 2 modes: ALL mode and PARTIAL mode. In ALL mode, you can either read all the data or none, there's no way to read partial data. In the other PARTIAL mode, you can read partial data, then get how much data is still available for future reading(which is the length returned here via full_size).
Throughout the building of SDK, it comes to my mind that we might not need 2 modes here, we can just stick to PARTIAL mode and keep the code in CKB simple. So in the refactoring work, I removed ALL mode and only keep PARTIAL mode. But that raises another problem: our original PARTIAL mode implementation has this bug that the returned length here is not calculated correctly, but due to our current SDK either 1) read all data using ALL mode; 2) read a given length of data using PARTIAL mode hence ignoring returned length prefix, this bug is not discovered till now.
No description provided.