refactor: optimize read_register to read to non-zeroed buffer #804
Conversation
I think yes, this is guaranteed, by both the config and by fees. But I think the current impl with |
near-sdk/src/environment/env.rs
Outdated
| // Get register length and convert to a usize. The max register size in config is much less | ||
| // than the u32 max so the abort should never be hit, but is there for safety because there | ||
| // would be undefined behaviour if the case would be hit. | ||
| let len: usize = register_len(register_id)?.try_into().unwrap_or_else(|_| abort()); |
There was a problem hiding this comment.
Is it really undefined behavior though? From the docs, casting just truncates:
Casting from a larger integer to a smaller integer (e.g. u32 -> u8) will truncate
but it's not ideal either for us to have a truncated len either, so the change still seems valid to me
There was a problem hiding this comment.
The UB would happen not during truncation, but when the runtime writes past the truncated lenght.
There was a problem hiding this comment.
Yes, as Aleksey mentions, this is just used to set the capacity of the buffer. When the write happens with read_register, it will write past the buffer, because the length is truncated, into other parts of memory (UB).
But yeah I suppose the doc comment could be more explicit to mention that the UB would happen on a later line than this one
Edit: new wording "would be undefined behaviour during read_register if the buffer length is truncated." hopefully that is more clear!
Removed need to zero out the buffer before writing to it, seems it would be an impactful optimization.
The only opinionated point is if conversion from u64 to usize (u32 in wasm32) should be checked or not. The maximum buffer size in configs in the runtime is less than
u32::MAXbut there doesn't seem to be any blockers from this changing with a custom config -- potentially this should be unchecked for a trivial optimization in code size? The only issue is that there is UB if the register length is >u32::MAXbecause the buffer created wouldn't be large enough and memory would be written outside of the buffer. cc @matklad will there be any guarantee that the max will always be <u32::MAX?