Skip to content

Associated type Hasher for QueryPreimage, StorePreimage and Bounded#1720

Merged
liamaharon merged 9 commits intoparitytech:masterfrom
muraca:issue#1701
Sep 27, 2023
Merged

Associated type Hasher for QueryPreimage, StorePreimage and Bounded#1720
liamaharon merged 9 commits intoparitytech:masterfrom
muraca:issue#1701

Conversation

@muraca
Copy link
Copy Markdown
Contributor

@muraca muraca commented Sep 26, 2023 •
edited
Loading

I hope it's enough to fix #1701
the only solution I found to make it happen is to put an associated type to the Bounded enum as well.
@liamaharon @kianenigma @bkchr

Polkadot address: 12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp

@muraca
added Hasher associated type to QueryPreimage and Hasher generic to B…
832dec0 
…ounded<T>

Signed-off-by: muraca <mmuraca247@gmail.com>
@muraca
associated types in pallet preimages
4940b38 
Signed-off-by: muraca <mmuraca247@gmail.com>
@muraca
associated types in other pallets
a268604 
Signed-off-by: muraca <mmuraca247@gmail.com>
@muraca muraca requested review from a team September 26, 2023 18:16
@muraca
fixed builing error in pallet-scheduler
75d1f29 
Signed-off-by: muraca <mmuraca247@gmail.com>
@muraca
use sp_runtime::traits::Hash instead of sp_core::traits::Hasher
05a6890 
Signed-off-by: muraca <mmuraca247@gmail.com>
liamaharon
liamaharon approved these changes Sep 27, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@liamaharon liamaharon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like a much more flexible, but also cleaner solution to me! Thanks!

@liamaharon
Copy link
Copy Markdown
Contributor

liamaharon commented Sep 27, 2023

/tip medium

@liamaharon liamaharon added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Sep 27, 2023
@liamaharon liamaharon requested a review from a team September 27, 2023 10:22
@muraca
cleanup and review suggestions
7bbfe1e 
Signed-off-by: muraca <mmuraca247@gmail.com>
@paritytech-cicd-pr
Copy link
Copy Markdown

paritytech-cicd-pr commented Sep 27, 2023

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable-int
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3811919

muraca and others added 2 commits September 27, 2023 12:48
kianenigma
kianenigma approved these changes Sep 27, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I wonder if there is any good documentation on your choice of type Hash and type Hasher in frame_system? Most runtimes use BlakeTwo256 and H256 so this is a noop. Is it even clear where/how those hashers are used? are they meant to be used for everything in the runtime?

@kianenigma
Copy link
Copy Markdown
Contributor

kianenigma commented Sep 27, 2023

/tip small

@substrate-tip-bot
Copy link
Copy Markdown

substrate-tip-bot bot commented Sep 27, 2023

@kianenigma A small (20 DOT) tip was successfully submitted for @muraca (12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp on polkadot).

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Frpc.polkadot.io#/referenda tip

@muraca
Copy link
Copy Markdown
Contributor Author

muraca commented Sep 27, 2023

I think the only documentation in frame_system is:

/// The hashing system (algorithm) being used in the runtime (e.g. Blake2).
type Hashing: Hash<Output = Self::Hash> + TypeInfo;

And I always assumed that it is the hasher you're supposed to use as default in the runtime

@liamaharon liamaharon merged commit 8b061a5 into paritytech:master Sep 27, 2023
@mrcnski mrcnski mentioned this pull request Sep 27, 2023
Merged
ordian added a commit that referenced this pull request Sep 27, 2023
@ordian
merge master and resolve conflicts (v8)
6fb4b7f 
* master: (61 commits)
  OpenGov in Westend and Rococo (#1177)
  Associated type Hasher for `QueryPreimage`, `StorePreimage` and `Bounded` (#1720)
  Migrate polkadot-primitives to v6 (#1543)
  genesis-builder: implemented for all runtimes (#1492)
  `BlockId` removal: `tx-pool` refactor (#1678)
  Bump directories from 4.0.1 to 5.0.1 (#1656)
  Allow debug_assertions in short-benchmarks CI job (#1711)
  chainHead/storage: Fix storage iteration using the query key (#1665)
  Implement more useful traits in `Slot` type (#1595)
  Make downloads in parallel and give more time to complete (#1699)
  Bump actions/checkout from 4.0.0 to 4.1.0 (#1688)
  contracts: Fix incorrect storage alias in mirgration (#1687)
  Fix documentation about justification and `finalized == true` requirement (#1607)
  tweak pallet macro (genesis_config etc) to cater for RA users as well. (#1689)
  Uncoupling pallet-xcm from frame-system's RuntimeCall (#1684)
  Bump aes-gcm from 0.10.2 to 0.10.3 (#1681)
  docs / Update PR template to reflect monorepo (#1674)
  update contributing guide and ui-tests scripts (#1668)
  pallet epm: add `TrimmingStatus` to the mined solution (#1659)
  Update HRMP pallet benchmarking to use benchmarks v2 (#1676)
  ...
@mrcnski mrcnski mentioned this pull request Oct 9, 2023
Merged
ordian added a commit that referenced this pull request Oct 10, 2023
@ordian
Merge branch 'tsv-disabling-node-side' into tsv-disabling-backing
26866fa 
* tsv-disabling-node-side: (69 commits)
  runtime-api: cleanup after v7 stabilization (#1729)
  Move requests-responses and polling from `ChainSync` to `SyncingEngine` (#1650)
  Add custom error message for `StorageNoopGuard` (#1727)
  Clarify docs
  cargo fmt
  add a CAVEAT comment
  implement disabled_validators correctly
  remove unnecessary hash string (#1722)
  OpenGov in Westend and Rococo (#1177)
  Associated type Hasher for `QueryPreimage`, `StorePreimage` and `Bounded` (#1720)
  Migrate polkadot-primitives to v6 (#1543)
  genesis-builder: implemented for all runtimes (#1492)
  `BlockId` removal: `tx-pool` refactor (#1678)
  Bump directories from 4.0.1 to 5.0.1 (#1656)
  Allow debug_assertions in short-benchmarks CI job (#1711)
  chainHead/storage: Fix storage iteration using the query key (#1665)
  Implement more useful traits in `Slot` type (#1595)
  Make downloads in parallel and give more time to complete (#1699)
  Bump actions/checkout from 4.0.0 to 4.1.0 (#1688)
  contracts: Fix incorrect storage alias in mirgration (#1687)
  ...
@kiltbot kiltbot mentioned this pull request Oct 19, 2023
Closed
@ahmadkaouk ahmadkaouk mentioned this pull request Nov 16, 2023
Closed
@dnjscksdn98 dnjscksdn98 mentioned this pull request Jan 7, 2024
Closed
2 tasks
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
@muraca @liamaharon @kianenigma
Associated type Hasher for QueryPreimage, StorePreimage and `Boun…
f777d3f 
…ded` (paritytech#1720)

I hope it's enough to fix paritytech#1701 
the only solution I found to make it happen is to put an associated type
to the `Bounded` enum as well.
@liamaharon @kianenigma @bkchr 

Polkadot address: 12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp

---------

Signed-off-by: muraca <mmuraca247@gmail.com>
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
bkchr pushed a commit that referenced this pull request Apr 10, 2024
@svyatonik @bkchr
tests (#1720)
dc989fa
This was referenced Jun 5, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kianenigma kianenigma kianenigma approved these changes

+1 more reviewer

@liamaharon liamaharon liamaharon approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

T1-FRAME This PR/Issue is related to core FRAME, the framework.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Hardcoded type Hash = H256 in Preimages traits

5 participants

@muraca @liamaharon @paritytech-cicd-pr @kianenigma @patriciobcs