Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

BEEFY: Simplify hashing for pallet-beefy-mmr #12387

Closed

Description

@serban300

Right now pallet-beefy-mmr Declares a new Hasher trait and a new Keccak256 struct which implements it. I was wondering if this is needed.

I was thinking of 2 possible simplifications:

  1. Instead of defining a new struct we could reuse the Keccak256 struct that's already present in sp_runtime, doing something like:
#[cfg(feature = "keccak")]
mod keccak256 {
	pub use sp_runtime::traits::Keccak256;

	/// Keccak256 hasher implementation.
	impl super::Hasher for Keccak256 {
		fn hash(data: &[u8]) -> super::Hash {
			Self::hash(data).into()
		}
	}
}

Or we could do even something more generic:

	impl<T> super::Hasher for T
		where T: sp_runtime::traits::Hash,
		<T as sp_runtime::traits::Hash>::Output: Into<super::Hash> {
		fn hash(data: &[u8]) -> super::Hash {
			Self::hash(data).into()
		}
	}
  1. Could we go one step further and remove the Hasher trait completely and use sp_runtime::traits::Hash instead ? And maybe in the end we could also remove this.

@acatangiu WDYT ? If this issue makes sense I would be happy to implement it.

Activity

  1. self-assigned this
    on Sep 29, 2022
  2. moved this to Some Day Maybe in BEEFYon Sep 30, 2022
  3. acatangiu commented on Sep 30, 2022

    @acatangiu
    Contributor

    No 1 sounds good to me, for no.2 it'd be better to see an RFC/PR 👍

  4. serban300 commented on Oct 4, 2022

    @serban300
    ContributorAuthor

    Implemented in #12393

  5. Repository owner moved this from Some Day Maybe to Done ✅ in BEEFYon Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

I7-refactorCode needs refactoring.U3-nice_to_haveIssue is worth doing eventually.

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions