sdk: Extract hash and hasher crates #2015

kevinheavey · 2024-07-04T09:05:49Z

Problem

The code in solana_program::hash is used all over the repo so it's one of the first things that needs to be made available outside of solana-program.

Summary of Changes

Add two new crates, solana-hash and solana-hasher. The solana-hash crate has no cryptography dependencies. The motivation is that sha2 takes a meaningful amount of time to compile and we use the Hash type in a lot of places where we don't compute the hash, e.g. parsing RPC responses.
Move the sol_sha256 syscall definition to solana-hasher to avoid depending on solana-program
Re-export the public contents of solana-hash and solana-hasher (except for sol_sha256) in solana_program::hash for backwards compatibility. I have not put a deprecation notice on the re-export because it's a core part of solana-program.
Re-export sol_sha256 in definitions.rs with a deprecation notice
Make serde and bytemuck optional in solana-hash
~~Make field 0 of the Hash struct pub (had no choice here)~~ Update: I realised there was a way to keep field 0 pub(crate) when wasm_bindgen freaked out at the field being pub
Make the MAX_BASE58_LEN const pub. I only needed to import it for tests but I think it's fine for this to be pub
move the contents of program/src/wasm/hash.rs to the new solana-hash crate. This file was just an impl block so I don't think its removal requires any further re-exports
replace solana-program dep in solana-merkle-tree with solana-hash and solana-hasher
make the new crates no_std by default (the solana-hash crate gets a std feature)

kevinheavey · 2024-07-06T20:44:32Z

@ryoqun I'm quite stuck trying to understand this abi_digest test failure and was wondering if you could help:

thread 'blockhash_queue::BlockhashQueue_frozen_abi::test_abi_digest' panicked at accounts-db/src/blockhash_queue.rs:29:5:
assertion `left == right` failed: Possibly ABI changed? Confirm the diff by rerunning before and after this test failed with SOLANA_ABI_DUMP_DIR!
  left: "HzbCqb1YtCodkx6Fu57SpzkgaLp9WSrtw8texsjBhEDH"
 right: "BxykY65dC2NCcDm17rHQPjEY8wK55sKAhfhKVFGc5T1u"

This PR doesn't touch BlockhashQueue directly but it does move the Hash struct to its own crate. However I don't see how the Hash ABI could have changed? It's still just

#[repr(transparent)]
pub struct Hash(pub(crate) [u8; HASH_BYTES]);

Is there more to it than that? Other types using Hash appear to also have changed digests

kevinheavey · 2024-07-07T09:51:04Z

OK the failing ABI tests passed when I temporarily added this to AbiDigester::update after the normalize_type_name call:

.replace("solana_hash::Hash", "solana_program::hash::Hash")

So looks like the change in digest is just from moving the Hash struct and not from an actual ABI change

buffalojoec

Lgtm, just some small details, then we can ship it. Thanks for your patience on the few open PRs!

sdk/hash/Cargo.toml

sdk/program/Cargo.toml

mergify · 2024-08-09T12:15:10Z

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

kevinheavey · 2024-08-20T10:17:19Z

I think #2569 has been up for long enough to unpause this? @joncinque

joncinque

This is really great work! Mostly small things that I hope will make it even better. Can you pass solana-hash to anza-team? Once we agree on a name for the hasher crate, we'll need to do the same

merkle-tree/Cargo.toml

sdk/hash/src/lib.rs

sdk/hasher/Cargo.toml

sdk/hasher/src/lib.rs

sdk/hasher/Cargo.toml

kevinheavey · 2024-09-04T14:39:36Z

I just noticed the Debug and Display impls were using the std feature of bs58 so I changed them to avoid that

…ikely to be used in that case

Co-authored-by: Jon C <[email protected]>

joncinque

Looking really close! Sorry, I noticed a last thing on this pass

sdk/hash/src/lib.rs

Co-authored-by: Jon C <[email protected]>

joncinque

Ok, all good to go on my side! Once the crate check passes, I'll approve and we can merge

yihau · 2024-09-05T02:35:40Z

all pipelines are green now!

joncinque

Thanks for the work and addressing all the comments!

* extract hash and hasher crates and re-export their contents in solana_program::hash * make bytemuck and serde optional, and activate features in solana-program * fix frozen-abi support * fix import * update lock file * cargo sort * fix wasm_bindgen import * typo * fmt * make the inner field of Hash pub(crate) again because otherwise wasm_bindgen breaks * move program/src/wasm/hash.rs contents to solana-hash crate * update lock file * remove duplicate frozen-abi stuff * fix specialization stuff * delete tmp tests * update BlockhashQueue digest * Revert "update BlockhashQueue digest" This reverts commit 591302b. * update expected digests after confirming that the change is merely from moving the Hash struct and not a real ABI change * update another digest * update digests in sdk and program * update digests in runtime * update VoteTransaction digest * conditionally activate solana-hash/borsh in solana-program * move js-sys dep under cfg(target_arch = "wasm32") * remove thiserror from hash crate * remove solana-program dependency from merkle-tree * make solana-hash no_std by default * fmt * fmt after rebase * make std feature default * make sha2 an optional dep when target_os = "solana", because it's unlikely to be used in that case * fmt * make rustc_version optional * update lock file * fix frozen-abi lint * another lint fix * add comment about sha2 removal * avoid Vec in FromStr Co-authored-by: Jon C <[email protected]> * put Hash::new_unique behind #[cfg(feature = "dev-context-only-utils")] * move tests from solana-hasher to solana-hash * rename solana-hasher to solana-sha256-hasher * fmt * make conditional import more consistent Co-authored-by: Jon C <[email protected]> * don't use std feature of bs58 in solana-hash * undo putting new_unique behind dev-context-only-utils * missing feature * rename to write_as_base58 and reorder params Co-authored-by: Jon C <[email protected]> * update write_as_base58 usage * fix feature activation for tests * remove part of doc that no longer makes sense Co-authored-by: Jon C <[email protected]> --------- Co-authored-by: Jon C <[email protected]>

mergify bot added the need:merge-assist label Jul 4, 2024

kevinheavey force-pushed the extract-hash-and-hasher branch 2 times, most recently from 396d3f6 to 25fe3be Compare July 6, 2024 18:08

kevinheavey force-pushed the extract-hash-and-hasher branch 3 times, most recently from 1c9da29 to f2d4d05 Compare July 10, 2024 09:05

kevinheavey requested a review from buffalojoec July 10, 2024 10:02

kevinheavey force-pushed the extract-hash-and-hasher branch 2 times, most recently from 85a000b to bf6ef2a Compare July 12, 2024 08:28

buffalojoec reviewed Jul 17, 2024

View reviewed changes

sdk/hash/Cargo.toml Outdated Show resolved Hide resolved

sdk/program/Cargo.toml Outdated Show resolved Hide resolved

kevinheavey force-pushed the extract-hash-and-hasher branch 3 times, most recently from 4af6de1 to 5b0bef1 Compare July 17, 2024 17:34

kevinheavey force-pushed the extract-hash-and-hasher branch 3 times, most recently from 6673647 to b19b1ce Compare July 31, 2024 11:25

kevinheavey mentioned this pull request Aug 1, 2024

Extract pubkey crate #2394

Merged

kevinheavey force-pushed the extract-hash-and-hasher branch 2 times, most recently from 8ebe1f0 to 9e28716 Compare August 9, 2024 11:56

kevinheavey force-pushed the extract-hash-and-hasher branch 2 times, most recently from 6ddd652 to bde4101 Compare August 13, 2024 09:58

kevinheavey force-pushed the extract-hash-and-hasher branch 2 times, most recently from 2a1e005 to 8085f94 Compare August 19, 2024 10:37

kevinheavey force-pushed the extract-hash-and-hasher branch from 8085f94 to cd681ca Compare August 27, 2024 11:16

joncinque reviewed Sep 4, 2024

View reviewed changes

kevinheavey and others added 19 commits September 4, 2024 20:00

make sha2 an optional dep when target_os = "solana", because it's unl…

fdf9edc

…ikely to be used in that case

fmt

ac40c2b

make rustc_version optional

3034854

update lock file

0f5a144

fix frozen-abi lint

8240997

another lint fix

4b75d5f

add comment about sha2 removal

0afdb74

avoid Vec in FromStr

db12076

Co-authored-by: Jon C <[email protected]>

put Hash::new_unique behind #[cfg(feature = "dev-context-only-utils")]

aa8962b

move tests from solana-hasher to solana-hash

a981abe

rename solana-hasher to solana-sha256-hasher

c6cc1e8

fmt

58ca2a8

make conditional import more consistent

d23c6b3

Co-authored-by: Jon C <[email protected]>

don't use std feature of bs58 in solana-hash

8e200b8

undo putting new_unique behind dev-context-only-utils

8747184

missing feature

1dff01e

rename to write_as_base58 and reorder params

4aed6b7

Co-authored-by: Jon C <[email protected]>

update write_as_base58 usage

7d265c4

fix feature activation for tests

2523482

kevinheavey force-pushed the extract-hash-and-hasher branch from e974c01 to 2523482 Compare September 4, 2024 16:03

joncinque reviewed Sep 4, 2024

View reviewed changes

sdk/hash/src/lib.rs Outdated Show resolved Hide resolved

remove part of doc that no longer makes sense

295b66a

Co-authored-by: Jon C <[email protected]>

joncinque reviewed Sep 4, 2024

View reviewed changes

kevinheavey added the automerge automerge Merge this Pull Request automatically once CI passes label Sep 5, 2024

joncinque approved these changes Sep 5, 2024

View reviewed changes

joncinque changed the title ~~Extract hash and hasher crates~~ sdk: Extract hash and hasher crates Sep 5, 2024

joncinque merged commit 27f4b3d into anza-xyz:master Sep 5, 2024
53 checks passed

kevinheavey mentioned this pull request Oct 31, 2024

Extract keccak-hasher crate #3408

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sdk: Extract hash and hasher crates #2015

sdk: Extract hash and hasher crates #2015

kevinheavey commented Jul 4, 2024 •

edited

Loading

kevinheavey commented Jul 6, 2024 •

edited

Loading

kevinheavey commented Jul 7, 2024

buffalojoec left a comment

mergify bot commented Aug 9, 2024

kevinheavey commented Aug 20, 2024

joncinque left a comment

kevinheavey commented Sep 4, 2024

joncinque left a comment

joncinque left a comment

yihau commented Sep 5, 2024

joncinque left a comment

sdk: Extract hash and hasher crates #2015

sdk: Extract hash and hasher crates #2015

Conversation

kevinheavey commented Jul 4, 2024 • edited Loading

Problem

Summary of Changes

kevinheavey commented Jul 6, 2024 • edited Loading

kevinheavey commented Jul 7, 2024

buffalojoec left a comment

Choose a reason for hiding this comment

mergify bot commented Aug 9, 2024

kevinheavey commented Aug 20, 2024

joncinque left a comment

Choose a reason for hiding this comment

kevinheavey commented Sep 4, 2024

joncinque left a comment

Choose a reason for hiding this comment

joncinque left a comment

Choose a reason for hiding this comment

yihau commented Sep 5, 2024

joncinque left a comment

Choose a reason for hiding this comment

kevinheavey commented Jul 4, 2024 •

edited

Loading

kevinheavey commented Jul 6, 2024 •

edited

Loading