db-api: trait Compress and Encode use reference &self

[nkysg]

Describe the feature

Trait Encode fn encodes use self https://github.com/paradigmxyz/reth/blob/1994959fb28c9f9a17128ca27b6f550b2d8df1d3/crates/storage/db-api/src/table.rs#L55

It's a bit weird. I see many libraries use &self, because when you have an object use fn encode, maybe we need to clone.

This is alloyed rlp Encode trait. https://github.com/alloy-rs/rlp/blob/9aef28e6da7c2769460dd6bf22eeacd53adf0ffc/crates/rlp/src/encode.rs#L15

/// A type that can be encoded via RLP.
pub trait Encodable {
    /// Encodes the type into the `out` buffer.
    fn encode(&self, out: &mut dyn BufMut);

    /// Returns the length of the encoding of this type in bytes.
    ///
    /// The default implementation computes this by encoding the type.
    /// When possible, we recommender implementers override this with a
    /// specialized implementation.
    #[inline]
    fn length(&self) -> usize {
        let mut out = Vec::new();
        self.encode(&mut out);
        out.len()
    }
}

It same as trait Compress https://github.com/paradigmxyz/reth/blob/1994959fb28c9f9a17128ca27b6f550b2d8df1d3/crates/storage/db-api/src/table.rs#L28

I think we can use &self, maybe we can remove some clone. And It seems some db fn like put, also can use reference.

Additional context

No response

[emhane]

do you mean that the Compress trait is redundant and can be removed in favour of alloy_rlp::Encodable?

[emhane]

second @DaniPopes suggestions in https://github.com/paradigmxyz/reth/pull/11186. let's go forth with this and replace the use of reth_db_api::Encode with alloy_rlp::Encodable. let's do this in several PRs though, one PR for each implementation of reth_db_api::Encode + remove the trait reth_db_api::Encode in the final PR. please link the prs here in the task list.

let's look at the Compress trait after replacing reth_db_api::Encode.

[onbjerg]

@emhane those two traits are not the same. What @nkysg meant is that the layouts of the traits are similar, as a way to explain that most enc/dec setups use &self and not self. We should not replace the Encode trait in reth with the one in alloy. The one in reth is for db, the one in alloy is for RLP. It does however make sense to just take &self as @nkysg suggested:)

[nkysg]

I wasn't clear enough on the issue @emhane . just as @onbjerg mentioned.

[github-actions[bot]]

This issue is stale because it has been open for 21 days with no activity.

[github-actions[bot]]

This issue was closed because it has been inactive for 7 days since being marked as stale.