Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hardcoded type Hash = H256 in Preimages traits #1701

Closed
muraca opened this issue Sep 25, 2023 · 3 comments · Fixed by #1720
Closed

Hardcoded type Hash = H256 in Preimages traits #1701

muraca opened this issue Sep 25, 2023 · 3 comments · Fixed by #1720
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.

Comments

@muraca
Copy link
Contributor

muraca commented Sep 25, 2023

QueryPreimage and StorePreimage traits use a hardcoded Hash type set to H256

Pallet Preimage itself is bounding to this trait I think the opposite way as it should be

impl<T: Config<Hash = PreimageHash>> QueryPreimage for Pallet<T> {

As I was working on #1445 I incurred in this issue, and I was forced to do some weird things that include

let proposal_hash: PreimageHash = 
    Decode::decode(&mut &T::Hashing::hash_of(&proposal).encode()[..])
        .expect("Hash length is always 32; qed");

and opposite

if new_hash != PreimageHash::from_slice(hash.as_ref()) {
    // ...	
}

or having to use BlakeTwo256 explicitly instead of T::Hashing in tests/benchmarks.

I think it can be fixed by removing the hardcoded type, and putting a type Hash in the traits, to be used like

impl<T: Config> QueryPreimage for Pallet<T> {
    type Hash = T::Hash;
    // ...
}

Should I proceed? If so, should I handle it in #1445 or is it better to open a new PR on which it will depend?

@kianenigma

@github-actions github-actions bot added the I10-unconfirmed Issue might be valid, but it's not yet known. label Sep 25, 2023
@liamaharon liamaharon added T1-FRAME This PR/Issue is related to core FRAME, the framework. and removed I10-unconfirmed Issue might be valid, but it's not yet known. labels Sep 25, 2023
@bkchr
Copy link
Member

bkchr commented Sep 26, 2023

Yeah sounds like something that could be made generic. Please create a separate pr for it.

@kianenigma
Copy link
Contributor

Should I proceed? If so, should I handle it in #1445 or is it better to open a new PR on which it will depend?

Yes looks reasonable to me. I suggest proceeding as a small separate PR 👍

liamaharon added a commit that referenced this issue Sep 27, 2023
…ded` (#1720)

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

---------

Signed-off-by: muraca <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
@ggwpez
Copy link
Member

ggwpez commented Sep 27, 2023

Just FYI this was done somewhat intentional though:
Screenshot 2023-09-27 at 15 07 33

https://github.com/paritytech/substrate/pull/11649/files#r905676844
But i guess its fine either way.

bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this issue Mar 25, 2024
…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 <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Mar 26, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Mar 27, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
)

* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
bkchr pushed a commit that referenced this issue Apr 10, 2024
* store block number ++ state root in parachains pallet

* fixed parachains finality APIs

* (test commit)

* removed test code

* deduplicated code a bit

* removed commented code

* spelling

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/lib.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/parachains/src/mock.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* added comment

Co-authored-by: Adrian Catangiu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants