Refactor: SPO distribution store refactored to use StakeAddress #326
Conversation
…and consistency across modules
# Conflicts: # common/src/stake_addresses.rs # modules/rest_blockfrost/src/handlers/epochs.rs # modules/rest_blockfrost/src/handlers/pools.rs # modules/rest_blockfrost/src/types.rs # modules/tx_unpacker/src/tx_unpacker.rs
…s and use to_/from_binary to encode the entire stake address (network + hash) so we can retrieve the stake address out of it.
…s and use to_/from_binary to encode the entire stake address (network + hash) so we can retrieve the stake address out of it.
…oving `Result` return type and add back documentation for SPDDStore methods and fields
…nd epochs handlers
lowhung
force-pushed
the
lowhung/refactor-key-hash-to-stake-address
branch
from
d6604ec to
aeb3a09
Compare
| fn decode_key(key: &[u8]) -> Result<(u64, PoolId, StakeAddress)> { | ||
| let epoch_bytes: [u8; EPOCH_LEN] = key[..EPOCH_LEN] | ||
| .try_into() | ||
| .map_err(|_| anyhow!("Failed to extract epoch bytes (offset 0-{})", EPOCH_LEN))?; |
There was a problem hiding this comment.
How do we feel about error handling like this? I see a few different conventions and I'd be curious to better understand
There was a problem hiding this comment.
I think let epoch_bytes: [u8; EPOCH_LEN] = key[..EPOCH_LEN].try_into()? is fine so that the try_into error bubbles up.
There was a problem hiding this comment.
Good call 👍 that appears to be how we've done things elsewhere in this file too so I'll stick with letting it bubble up.
| hex = { workspace = true } | ||
| imbl = { workspace = true } | ||
| serde = { workspace = true } | ||
| serde_json = { workspace = true } |
There was a problem hiding this comment.
These weren't being used
| rayon = "1.10.0" | ||
| csv = "1.3.1" | ||
| itertools = "0.14.0" | ||
| tempfile = "3" |
There was a problem hiding this comment.
I'm adding this so that the directories we store the test spo data into is cleaned up post test run. It wasn't being cleaned up before.
There was a problem hiding this comment.
This should go under [dev-dependencies] since it's only used for tests and shouldn't be included in production builds.
There was a problem hiding this comment.
Yes, good catch
lowhung
force-pushed
the
lowhung/refactor-key-hash-to-stake-address
branch
from
f0bfcd1 to
4037f85
Compare
|
|
||
| match serde_json::to_string_pretty(&response) { | ||
| Ok(json) => Ok(RESTResponse::with_json(200, &json)), | ||
| &format!("Failed to serialize DRep delegators: {e}"), |
There was a problem hiding this comment.
Would recommend split-view for this diff
lowhung
requested review from
SupernaviX,
alexwoods,
golddydev,
sandtreader and
whankinsiv
| ShelleyAddressPaymentPart::ScriptHash(data) => (data, 1), | ||
| }; | ||
|
|
||
| // TODO: MH - make delegation hash an Option<Hash<28>> type |
There was a problem hiding this comment.
I'll do this in another PR!
whankinsiv
left a comment
whankinsiv
left a comment
There was a problem hiding this comment.
Great job! This is looking a lot cleaner and I'm glad we were able to reduce the number of queries for the epoch endpoints.
| rayon = "1.10.0" | ||
| csv = "1.3.1" | ||
| itertools = "0.14.0" | ||
| tempfile = "3" |
There was a problem hiding this comment.
This should go under [dev-dependencies] since it's only used for tests and shouldn't be included in production builds.
| fn decode_key(key: &[u8]) -> Result<(u64, PoolId, StakeAddress)> { | ||
| let epoch_bytes: [u8; EPOCH_LEN] = key[..EPOCH_LEN] | ||
| .try_into() | ||
| .map_err(|_| anyhow!("Failed to extract epoch bytes (offset 0-{})", EPOCH_LEN))?; |
There was a problem hiding this comment.
I think let epoch_bytes: [u8; EPOCH_LEN] = key[..EPOCH_LEN].try_into()? is fine so that the try_into error bubbles up.
…decode_key` in `spo_distribution_store.rs`, and add `tempfile` as a dev dependency
What
KeyHash->StakeAddresswhere it made senseStakeAddressstruct when decoding the key