concordium-rust-smart-contracts
concordium-rust-smart-contracts copied to clipboard
Remove the `Default` constraint for the `Err` associated type of the Write trait
The concordium_contracts_common::Write has a Default
constraint.
This is not really essential, and I think the only reason it is there is simplicity. It is used in the write_all
implementation, where it can and should just be removed and the inner error should be propagated.
It is also used in schema.rs
for some functions, such as serializing size length. There it should be replaced by custom enums that will also provide better error messages that will point to a precise source of an error.
So I briefly looked into this.
The challenge with removing Default
in write_all
is that it has the additional check for the number of bytes written is more than 0.
So we would have to change behavior to remove it
We can't change that behaviour. It's quite essential because some implementations, for example
/// This implementation overwrite the contents of the slice and updates the
/// reference to point to the unwritten part. The slice is (by necessity) never
/// extended.
/// This is in contrast to the `Vec<u8>` implementation which always extends the
/// vector with the data that is written.
impl Write for &mut [u8] {
type Err = ();
#[inline]
fn write(&mut self, buf: &[u8]) -> Result<usize, Self::Err> {
let to_write = core::cmp::min(buf.len(), self.len());
let (overwrite, rest) = core::mem::replace(self, &mut []).split_at_mut(to_write);
overwrite.copy_from_slice(&buf[..to_write]);
*self = rest;
Ok(to_write)
}
}
It would be incorrect to change the write_all behaviour here. Of course we could change the default implementation, and override it here, but having a dangerous default implementation is not very good I think.
A fancy solution could be to have write_all
in its own WriteAll
trait, define a WriteAllError
and require Err
in the trait to implement From<WriteAllError>
.
We could also do
trait Write {
type Err
....
fn write_all(&mut self, data: &[u8]) -> Result<...> where Self::Err: Default {
}
But this also requires a lot of changes since write_all is used a lot.