Document `SysError::LengthNotEnough` intended usage
Opened this issue ยท 9 comments
Following @XuJiandong advice:
The load_cell_data function can load all cell data at once. However, the cell data might be very large, potentially consuming all available memory in certain scenarios. It is recommended to use the following low-level syscall to load only a portion of the cell data
I switched iCKB load_cell_data
calls from the highlevel
to syscall
. At that point all the contracts stopped validating, completely.
After too much digging I understood theSysError::LengthNotEnough
lifecycle in correlation to syscalls from load_data implementation
. I did not found any other documentation except that private function, not even in the relative RFC.
Please, could you update the syscall documentation with this crucial information about SysError::LengthNotEnough
?
Keep up the great work ๐ช
Phroi
The document is here:
Line 8 in d4526dc
@XuJiandong that's way too little and way too hidden to call it documentation.
I would understand if Capsule 0.10.5 04fd58c
integrated those nice comments on errors, but it still generates the following error.rs
:`
use ckb_std::error::SysError;
/// Error
#[repr(i8)]
pub enum Error {
IndexOutOfBound = 1,
ItemMissing,
LengthNotEnough,
Encoding,
// Add customized errors here...
MyError,
}
impl From<SysError> for Error {
fn from(err: SysError) -> Self {
use SysError::*;
match err {
IndexOutOfBound => Self::IndexOutOfBound,
ItemMissing => Self::ItemMissing,
LengthNotEnough(_) => Self::LengthNotEnough,
Encoding => Self::Encoding,
Unknown(err_code) => panic!("unexpected sys error {}", err_code),
}
}
}
Also I would recommend for every low level syscall to document this particular behavior
The capsule
is deprecated and now we're migrating projects to: https://github.com/cryptape/ckb-script-templates
Capsule deprecated, really?!
As you may know, iCKB just got an internal code review and I'm sure that the reviewers would have told me right away if the Nervos standard library on which iCKB is built on top is deprecated.
Also until now no deprecation notice appears on both capsule and ckb-std Readme.
While I understand that it's in the process of being switched out (not sure why), it currently doesn't seem deprecated.
Additionally, these docs also helps understand better code that is alredy deployed.
Love & Peace, Phroi
I'm sure that the reviewers would have told me right away if the Nervos standard library on which iCKB is built on top is deprecated.
This is not correct. The capsule is just a tool and doesn't affect the final on-chain script at all. Capsule doesn't include ckb-std
or any other on-chain script libraries.
Wow, what a relief!! Then my issue still remains with ckb-std
, which is not gonna be deprecated, right? ๐
Please, could you document on the low level syscalls how the returned error should be handled?
This is definitely not enough documentation for a new developer who is just trying out ckb-std
. In my eyes the solution should be pretty similar to how #99 was resolved.
Love & Peace, Phroi
Developers can view SysError directly. Feel free to submit a PR if you believe anything should be added.
Not clear there and, to be honest, it's not even clear on the syscalls RFC... Adding to the things to do: creating two PR.
I'd kindly ask you to leave this issue open until my PR.