Refactor: Leverage new Hash macros + Hash type instead of previous KeyHash / vec implementation #304

lowhung · 2025-10-27T18:45:48Z

Implements #301 (and #257)

What this PR does:

Combines the byte_array.rs hash macros with the hash.rs file as these had some overlap
Create PoolId, KeyHash, VrfKeyHash and other keys to use this fixed bytes type

…ash` utility across modules. Includes updates for serialization, type conversions, and tests.

…ross pool-related operations and update associated tests

…nual conversions across modules, and adjust types for stronger consistency. Update tests and documentation accordingly.

…r cleanup. Refactor `POOL_KEY_LENGTH` to `POOL_ID_LENGTH`

…stency, update associated type aliases, imports, and serialization logic.

…s, update imports, and adjust serialization logic across modules for consistency.

golddydev · 2025-10-29T15:17:42Z

common/src/crypto.rs

-pub fn keyhash_256(key: &[u8]) -> KeyHash {
+///
+/// Returns a 32-byte hash.
+pub fn keyhash_256(key: &[u8]) -> Hash<32> {


Does it make more sense to name this function with its hash function?

Oh I see, so define this as a method on the Hash type itself?

@golddydev and I spoke, and we'll address in this in a follow-up PR. Basically, create specific types for a blake2b_256 + blake2b_224

… redundant `declare_hash_newtype_with_bech32!`, and update associated imports and Bech32 serialization logic.

…owhung/unify-hash-types

# Conflicts: # modules/governance_state/src/state.rs

…ting_mainnet_up_573`, and update `conway_verification.csv` with new governance actions.

…y-hash-types # Conflicts: # codec/src/map_parameters.rs # common/src/hash.rs # common/src/snapshot/pool_params.rs # common/src/snapshot/streaming_snapshot.rs # common/src/types.rs # modules/chain_store/src/chain_store.rs # modules/chain_store/src/stores/fjall.rs # modules/chain_store/src/stores/mod.rs # modules/epochs_state/src/state.rs # modules/genesis_bootstrapper/src/genesis_bootstrapper.rs # modules/mithril_snapshot_fetcher/src/mithril_snapshot_fetcher.rs # modules/tx_unpacker/src/tx_unpacker.rs # modules/upstream_chain_fetcher/src/body_fetcher.rs

… hash conversions across modules

…nt hash conversion across modules

# Conflicts: # modules/historical_accounts_state/src/immutable_historical_account_store.rs

sandtreader

Great, feels so much better to have things properly typed.

Only comments are whether the various calls to bech_32/HRP functions could use the Hash types' own ones now, or if they don't exist yet, could they be extended so that each type knows its HRP prefix rather than having lots of different code that repeats them.

sandtreader · 2025-10-30T15:02:15Z

common/src/address.rs

 impl Default for ShelleyAddressPaymentPart {
    fn default() -> Self {
-        Self::PaymentKeyHash(Vec::new())
+        Self::PaymentKeyHash([0u8; 28].into())


Is this not (or could be) covered by a default()?

sandtreader · 2025-10-30T15:03:01Z

common/src/address.rs

    /// Payment to a script
    #[n(1)]
-    ScriptHash(#[n(0)] ScriptHash),
+    ScriptHash(#[n(0)] KeyHash),


Have we lost ScriptHash as a separate type? This seems like a regressive step (although I may have misunderstood)

I added it back here, but it's just an aliased type on KeyHash which is a Hash<28>.

sandtreader · 2025-10-30T15:08:12Z

common/src/address.rs

        StakeAddress {
            network: NetworkId::Mainnet,
-            credential: StakeCredential::AddrKeyHash(vec![0u8; 28]),
+            credential: StakeCredential::AddrKeyHash([0u8; 28].into()),


KeyHash::default() again?

sandtreader · 2025-10-30T15:15:45Z

common/src/types.rs

    }

-    pub fn from_drep_bech32(bech32_str: &str) -> Result<Self, anyhow::Error> {
+    pub fn from_drep_bech32(bech32_str: &str) -> Result<Self, Error> {


Do we not have bech32 encode/decode in the macro-defined Hash types now?

We do! However these are implemented on the Credential struct, so the result is different vs Hash<N> types.

sandtreader · 2025-10-30T15:16:49Z

common/src/types.rs

-            Voter::DRepKey(k) => write!(f, "{}", self.to_bech32("drep", k)),
-            Voter::DRepScript(s) => write!(f, "{}", self.to_bech32("drep_script", s)),
-            Voter::StakePoolKey(k) => write!(f, "{}", self.to_bech32("pool", k)),
+            Voter::DRepKey(k) => write!(f, "{}", self.to_bech32("drep", k.as_ref())),


If these were typed, wouldn't they know how do bech32 anyway?

That's a good point, we could define hash types for those as well so their to_bech32() is cleaner.

I've been looking at this quite a bit recently https://cips.cardano.org/cip/CIP-0005

What do you think about this?

IMO, Voter should have a to_bech32(&self) -> Result<String, _> and the Display impl should call that. Would be unfortunate to let an error string like <invalid: blah> propagate around where a bech32-encoded address is expected

Yeh...that's a great point. I'll do that 👍

Updated this here d181f1c

sandtreader · 2025-10-30T15:20:16Z

modules/governance_state/data/conway_verification.csv

@@ -1,23 +1,23 @@
-gov_action1zhuz5djmmmjg8f9s8pe6grfc98xg3szglums8cgm6qwancp4eytqqmpu0pr,15f82a365bdee483a4b03873a40d3829cc88c048ff3703e11bd01dd9e035c916,0,,100000000000,45dee6ee5d7f631b6226d45f29da411c42fa7e816dc0948d31e0dba7,507,,Information,"{""deposit"":100000000000,""reward_account"":{""network"":""Mainnet"",""payload"":{""StakeKeyHash"":""45dee6ee5d7f631b6226d45f29da411c42fa7e816dc0948d31e0dba7""}},""gov_action_id"":{""transaction_id"":""15f82a365bdee483a4b03873a40d3829cc88c048ff3703e11bd01dd9e035c916"",""action_index"":0},""gov_action"":""Information"",""anchor"":{""url"":""ipfs://QmWjcHsrq9kKHZZ7aPPFjqN6wLuxH9d8bcqssmrE7H4cvb"",""data_hash"":""2f98f57c4149fdfed2b73cbd821226fe417ef5ed49d8f836a37b31edf14dea47""}}",,,,,Some(514),cy2/n0/a1:dy8971134676473/n220807337570898/a28653074063371:sy156770300966308/n1482461784403994/a27102517904169,c0:d1:s1


Not sure this shoiuld be in this PR, even if it makes the CI pass

I agree, I'll unstage this 👍

Unstaged af62ad8

…oss modules

whankinsiv

Looks good! There are a few places we can further clean the types up.

codec/src/block.rs

codec/src/map_parameters.rs

whankinsiv · 2025-10-30T17:48:31Z

common/src/address.rs

-            ShelleyAddressDelegationPart::None => (&Vec::new(), 3),
-            ShelleyAddressDelegationPart::StakeKeyHash(hash) => (hash, 0),
-            ShelleyAddressDelegationPart::ScriptHash(hash) => (hash, 1),
+        let (delegation_hash, delegation_bits): (Vec<u8>, u8) = match &self.delegation {


delegation_hash here can be Hash<28> to avoid the to_vec() conversion below.

In the situation where we have no delegation part, do you know what the hash would resolve to?

From my understanding if the delegation part is missing it is not included in the data to be Bech32 encoded.

I'll resolve this in a follow-up PR 👍

whankinsiv · 2025-10-30T17:51:31Z

common/src/queries/accounts.rs

    ActiveStakes(u64),
    /// Vec<(PoolId, StakeKey, ActiveStakeAmount)>
-    SPDDByEpoch(Vec<(KeyHash, KeyHash, u64)>),
+    SPDDByEpoch(Vec<(PoolId, KeyHash, u64)>),


Should this be StakeAddress instead of KeyHash?

modules/rest_blockfrost/src/types.rs

modules/stake_delta_filter/src/utils.rs

whankinsiv · 2025-10-30T18:08:21Z

modules/spo_state/src/state.rs


    /// Get Pool Delegators
-    pub fn get_pool_delegators(&self, pool_operator: &KeyHash) -> Option<Vec<(KeyHash, u64)>> {
+    pub fn get_pool_delegators(&self, pool_operator: &PoolId) -> Option<Vec<(KeyHash, u64)>> {


This should return Option<Vec<(StakeAddress, Lovelace)>>

…prove error handling in `tx_unpacker`

…ation support

lowhung · 2025-10-31T01:00:14Z

I will address the rest of @whankinsiv comments re: StakeAddress in a follow-up PR 👍

whankinsiv

Thanks for implementing these changes. Looking forward to the follow up PR with the KeyHash -> StakeAddress refactor.

# Conflicts: # modules/accounts_state/src/verifier.rs # modules/historical_accounts_state/src/immutable_historical_account_store.rs

Refactor: Replace Vec<u8> with KeyHash and introduce type-safe `H…

fb6432b

…ash` utility across modules. Includes updates for serialization, type conversions, and tests.

lowhung marked this pull request as draft October 27, 2025 18:45

lowhung added 7 commits October 27, 2025 11:46

Merge branch 'main' into lowhung/unify-hash-types

a4887c4

Continue coercing types and defining clearer names for the keys in use

5f65e4f

Refactor: Replace KeyHash with PoolId for stronger type safety ac…

df95809

…ross pool-related operations and update associated tests

Fix epochs state / update rest handlers

fe2b0d6

Refactor: Introduce type-safe Hash conversion utilities, replace ma…

41681ea

…nual conversions across modules, and adjust types for stronger consistency. Update tests and documentation accordingly.

Delete unused byte_array module and remove associated references fo…

a7cfb00

…r cleanup. Refactor `POOL_KEY_LENGTH` to `POOL_ID_LENGTH`

Refactor: Replace VRFKey with VrfKeyHash across modules for consi…

727ffee

…stency, update associated type aliases, imports, and serialization logic.

lowhung marked this pull request as ready for review October 28, 2025 23:11

lowhung added 2 commits October 28, 2025 17:13

Merge in main

f77d2dc

Refactor: Replace manual key hash conversions with type-safe utilitie…

fb77f49

…s, update imports, and adjust serialization logic across modules for consistency.

lowhung force-pushed the lowhung/unify-hash-types branch from 2551bd4 to fb77f49 Compare October 29, 2025 00:32

Refactor: Replace manual key hash conversions with type-safe utilitie…

5a49952

…s, update imports, and adjust serialization logic across modules for consistency.

golddydev reviewed Oct 29, 2025

View reviewed changes

lowhung added 4 commits October 29, 2025 09:34

Refactor: Consolidate declare_hash_type_with_bech32! macros, remove…

6d0ad83

… redundant `declare_hash_newtype_with_bech32!`, and update associated imports and Bech32 serialization logic.

Merge remote-tracking branch 'origin/lowhung/unify-hash-types' into l…

8dcddf4

…owhung/unify-hash-types

Merge branch 'main' into lowhung/unify-hash-types

4e9c167

# Conflicts: # modules/governance_state/src/state.rs

Refactor: Format imports for readability, add #[ignore] to `test_vo…

28cbe9d

…ting_mainnet_up_573`, and update `conway_verification.csv` with new governance actions.

lowhung force-pushed the lowhung/unify-hash-types branch from 905a28e to 28cbe9d Compare October 29, 2025 17:28

Merge branch 'main' into lowhung/unify-hash-types

0432374

lowhung changed the base branch from main to lowhung/introduce-combined-hash-file October 29, 2025 18:31

lowhung added 2 commits October 29, 2025 15:13

Refactor: Replace PoolId::new and KeyHash::new with .into() for…

87b2ea6

… hash conversions across modules

lowhung force-pushed the lowhung/unify-hash-types branch from 7c730cc to 410f572 Compare October 30, 2025 00:14

Refactor: Replace KeyHash::try_from with .try_into() for consiste…

119426f

…nt hash conversion across modules

lowhung force-pushed the lowhung/unify-hash-types branch from 410f572 to 119426f Compare October 30, 2025 00:35

lowhung requested review from SupernaviX, alexwoods and sandtreader October 30, 2025 00:38

lowhung requested review from golddydev and whankinsiv October 30, 2025 00:38

Base automatically changed from lowhung/introduce-combined-hash-file to main October 30, 2025 15:16

Merge branch 'main' into lowhung/unify-hash-types

2f61e83

# Conflicts: # modules/historical_accounts_state/src/immutable_historical_account_store.rs

sandtreader reviewed Oct 30, 2025

View reviewed changes

lowhung added 2 commits October 30, 2025 08:30

Use as_ref in immutable_historical_account_store.rs vs into_inner()

a3dd388

Refactor: Consolidate hash type definitions and unify conversions acr…

af62ad8

…oss modules

whankinsiv requested changes Oct 30, 2025

View reviewed changes

lowhung added 2 commits October 30, 2025 15:05

Refactor: Update hash type definitions and align associated conversions

d181f1c

Refactor: Update GenesisKeyhash to use consistent hash types and im…

0dc9c03

…prove error handling in `tx_unpacker`

lowhung force-pushed the lowhung/unify-hash-types branch from be9c38d to 0dc9c03 Compare October 31, 2025 00:14

Refactor: Replace gen_delegs Vec<u8> with PoolId

0f551f9

lowhung force-pushed the lowhung/unify-hash-types branch from bae3fe5 to 13c515a Compare October 31, 2025 00:49

Refactor: Replace Vec<u8> with PoolId and add Bech32 (de)serializ…

064e38b

…ation support

lowhung force-pushed the lowhung/unify-hash-types branch from 13c515a to 064e38b Compare October 31, 2025 00:49

lowhung requested review from sandtreader and whankinsiv October 31, 2025 01:02

whankinsiv approved these changes Oct 31, 2025

View reviewed changes

lowhung added 2 commits October 31, 2025 07:57

Merge branch 'main' into lowhung/unify-hash-types

68a5d59

# Conflicts: # modules/accounts_state/src/verifier.rs # modules/historical_accounts_state/src/immutable_historical_account_store.rs

Refactor: Replace KeyHash with PoolId for verifier.rs

dc38a71

lowhung merged commit 0b69ae2 into main Oct 31, 2025
2 checks passed

lowhung deleted the lowhung/unify-hash-types branch October 31, 2025 15:55

github-actions bot mentioned this pull request Nov 3, 2025

📊 Weekly Status - Week of 2025-11-03 #334

Open

		@@ -1,23 +1,23 @@
		gov_action1zhuz5djmmmjg8f9s8pe6grfc98xg3szglums8cgm6qwancp4eytqqmpu0pr,15f82a365bdee483a4b03873a40d3829cc88c048ff3703e11bd01dd9e035c916,0,,100000000000,45dee6ee5d7f631b6226d45f29da411c42fa7e816dc0948d31e0dba7,507,,Information,"{""deposit"":100000000000,""reward_account"":{""network"":""Mainnet"",""payload"":{""StakeKeyHash"":""45dee6ee5d7f631b6226d45f29da411c42fa7e816dc0948d31e0dba7""}},""gov_action_id"":{""transaction_id"":""15f82a365bdee483a4b03873a40d3829cc88c048ff3703e11bd01dd9e035c916"",""action_index"":0},""gov_action"":""Information"",""anchor"":{""url"":""ipfs://QmWjcHsrq9kKHZZ7aPPFjqN6wLuxH9d8bcqssmrE7H4cvb"",""data_hash"":""2f98f57c4149fdfed2b73cbd821226fe417ef5ed49d8f836a37b31edf14dea47""}}",,,,,Some(514),cy2/n0/a1:dy8971134676473/n220807337570898/a28653074063371:sy156770300966308/n1482461784403994/a27102517904169,c0:d1:s1

Refactor: Leverage new Hash macros + Hash type instead of previous KeyHash / vec implementation #304

Refactor: Leverage new Hash macros + Hash type instead of previous KeyHash / vec implementation #304

Uh oh!

Conversation

lowhung commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandtreader left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lowhung Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

whankinsiv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

whankinsiv Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lowhung commented Oct 31, 2025

Uh oh!

whankinsiv left a comment

lowhung commented Oct 27, 2025 •

edited

Loading

lowhung Oct 30, 2025 •

edited

Loading

whankinsiv Oct 30, 2025 •

edited

Loading