-
Notifications
You must be signed in to change notification settings - Fork 137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: cleanup event error cases #1886
Conversation
Stebalien
commented
Sep 14, 2023
- Fail with LimitExceeded where it makes sense.
- Validate that we comsume all keys/values.
Implements filecoin-project/FIPs#831 |
eecd51e
to
4fe8df4
Compare
Builds on #1885 |
Codecov Report
|
if header.key_len > MAX_KEY_LEN as u32 { | ||
let tmp = header.key_len; | ||
return Err(syscall_error!(IllegalArgument; "event key exceeded max size: {} > {MAX_KEY_LEN}", tmp).into()); | ||
return Err(syscall_error!(LimitExceeded; "event key exceeded max size: {} > {MAX_KEY_LEN}", tmp).into()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, but in filecoin-project/FIPs#831 it lists that we check each entry key individually for valid UTF8 while in our impl we do it all upfront for all keys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gah, my patch reverts my previous fixes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
1. Fail with LimitExceeded where it makes sense. 2. Validate that we comsume all keys/values.
4fe8df4
to
29aac0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
1. Fail with LimitExceeded where it makes sense. 2. Validate that we comsume all keys/values.
1. Fail with LimitExceeded where it makes sense. 2. Validate that we comsume all keys/values.
1. Fail with LimitExceeded where it makes sense. 2. Validate that we comsume all keys/values.