Skip to content

Conversation

lowhung
Copy link
Collaborator

@lowhung lowhung commented Oct 14, 2025

Description of Changes

  • Replaced RewardAccount with StakeAddress across modules for consistency.
  • Updated serialization, deserialization, and encoding/decoding logic to align with StakeAddress.
  • Adjusted tests, helpers, and corresponding comments to support the refactor.

- Replace `RewardAccount` with `StakeAddress` across modules for consistency.
- Adapt serialization, deserialization, and encoding/decoding logic for `StakeAddress`.
- Adjust related tests, helpers, and comments to align with the refactor.
- Simplify `reward_account` handling by utilizing `StakeAddress::from_binary`.
- Adjust `StakeAddress` default implementation for consistency.
…tion

- Replace `reward_account` field with `StakeAddress::default()` for consistency.
- Remove redundant `StakeAddressPayload` in test cases.
- Clean up unused imports and redundant code across modules.
- Replace `RewardAccount` with `StakeAddress` across modules for consistency.
- Adapt serialization, deserialization, and encoding/decoding logic for `StakeAddress`.
- Adjust related tests, helpers, and comments to align with the refactor.

# Conflicts:
#	modules/accounts_state/src/rewards.rs
#	modules/accounts_state/src/snapshot.rs
#	modules/accounts_state/src/state.rs
- Simplify `reward_account` handling by utilizing `StakeAddress::from_binary`.
- Adjust `StakeAddress` default implementation for consistency.
…tion

- Replace `reward_account` field with `StakeAddress::default()` for consistency.
- Remove redundant `StakeAddressPayload` in test cases.
- Clean up unused imports and redundant code across modules.
- Replace `StakeAddress::from_binary` usage with `reward_account.get_hash` for simplicity and consistency.
- Update all modules to align with the updated `StakeAddress` handling.
- Remove redundant error handling and simplify logging where applicable.
…nt-with-stake-address' into lowhung/163-replace-reward-account-with-stake-address

# Conflicts:
#	modules/accounts_state/src/rewards.rs
#	modules/accounts_state/src/snapshot.rs
#	modules/accounts_state/src/state.rs
@lowhung lowhung force-pushed the lowhung/163-replace-reward-account-with-stake-address branch from 5b75210 to 7e1fe2c Compare October 15, 2025 02:41
…ify related logic

- Update `RewardDetail` and reward handling logic to use `StakeAddress` directly.
- Replace `reward_account` field with `.get_hash()` where necessary.
@lowhung lowhung force-pushed the lowhung/163-replace-reward-account-with-stake-address branch from 7e1fe2c to c90a1c1 Compare October 15, 2025 03:08
…eded.

- Update imports and logic across modules to use `StakeAddress` exclusively.
@lowhung lowhung force-pushed the lowhung/163-replace-reward-account-with-stake-address branch from 507f8ec to f625171 Compare October 15, 2025 16:51
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the codebase to replace the RewardAccount type (which was a Vec<u8>) with the more structured StakeAddress type for improved type safety and consistency across the application.

Key Changes:

  • Replaced RewardAccount type alias with StakeAddress throughout the codebase
  • Updated serialization/deserialization logic to use StakeAddress methods
  • Modified test fixtures to use StakeAddress::default() instead of empty vectors

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
modules/tx_unpacker/src/map_parameters.rs Updated certificate and proposal mapping to deserialize reward accounts using StakeAddress::from_binary()
modules/spo_state/src/state.rs Updated test fixtures to use StakeAddress::default() instead of vec![0] and added import
modules/rest_blockfrost/src/handlers/pools.rs Changed reward account conversion from to_bech32_with_hrp() to to_string() method
modules/accounts_state/src/verifier.rs Updated verifier to use StakeAddress::get_hash() for comparisons and logging
modules/accounts_state/src/state.rs Refactored SPO retirement logic to use StakeAddress directly, updated reward processing to use get_hash(), and fixed comment grammar
modules/accounts_state/src/snapshot.rs Simplified reward account registration check by removing error handling and using StakeAddress directly
modules/accounts_state/src/rewards.rs Updated reward calculation to work with StakeAddress, replaced hash comparisons with get_hash() calls
common/src/types.rs Removed RewardAccount type alias and updated PoolRegistration and ProposalProcedure to use StakeAddress
common/src/address.rs Added CBOR encoding/decoding implementations, Default trait, reorganized methods, and added comprehensive tests for StakeAddress

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

};

let payload = match (header >> 4) & 0x0F {
let payload = match (header >> 4) & 0x0Fu8 {
Copy link

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

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

[nitpick] The explicit u8 type suffix on 0x0Fu8 is unnecessary since the expression (header >> 4) & 0x0F already produces a u8 result. The original 0x0F without suffix is clearer and follows Rust conventions for bitwise operations.

Suggested change
let payload = match (header >> 4) & 0x0Fu8 {
let payload = match (header >> 4) & 0x0F {

Copilot uses AI. Check for mistakes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, that's what I thought as well...but my editor does not seem happy about that!

No implementation for `u8 & i32` [E0369]

@lowhung lowhung force-pushed the lowhung/163-replace-reward-account-with-stake-address branch from 7cf9747 to d4e7163 Compare October 15, 2025 20:33
@lowhung lowhung requested a review from Copilot October 15, 2025 21:25
Copilot

This comment was marked as spam.

@lowhung lowhung requested a review from Copilot October 15, 2025 23:36
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant