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

refactor(governance)!: Aligned package with updates to CIP-36 #592

Merged
merged 2 commits into from
Feb 28, 2023

Conversation

Ryun1
Copy link
Contributor

@Ryun1 Ryun1 commented Feb 2, 2023

Context

CIP-36 (Catalyst/Voltaire Registration Transaction Metadata Format (Updated)) has seen some a couple recent changes:

  • #373 adjusted the metadata standard, replacing the reward_address field with payment_address. Where payment_address contains a payment address rather than stake/rewards address. To accompany this new test vectors were supplied.
  • #427 focussed on adjusting semantics to reduce ambiguity for tool makers and potential future protocol governance. Adding clarity around naming of vote keys (see here).

Proposed Solution

  • Alter the governance package to reflect the changes introduced in #373:
    • Replaced rewards address with payment address.
    • Added new test vectors, to match those provided in the CIP.
  • Alter the governance package to reflect the naming conventions introduced in #427:
    • Renaming elements that use naming such as votingKey to use CIP36VoteKey.
    • Altering comments that use terms such as "voting key" to use "vote key".

Important Changes Introduced

  • cip36.ts:
    • Renamed interface VotingKeyDerivationPath to be interface CIP36VoteKeyDerivationPath.
    • renamed interface GovernanceKeyDelegation to be interface VoteKeyDelegation.
    • In interface GovernanceKeyDelegation I renamed votingKey to be cip36VoteKey.
    • In interface BuildVotingRegistrationProps I replaced rewardAccount with paymentAddress and changed the type to be a Cardano.Address rather than Cardano.RewardAccount.
    • Changed comments to reflect.
  • cip36.test.ts:
    • Matched test vectors used in CIP-36.
    • Matched field changes from cip36.ts.
  • cip36KeyAgents.test.ts:
    • Matched field changes from cip36.ts.
    • Renamed instances of votingKey to CIP36VoteKey.
    • Changed comments to reflect.

Copy link
Member

@mkazlauskas mkazlauskas left a comment

Choose a reason for hiding this comment

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

Thanks @Ryun1! Please update commit message to refactor(governance)!: aligned with updates to cip-36 to indicate that it has breaking changes

packages/governance/src/cip36.ts Outdated Show resolved Hide resolved
@mkazlauskas mkazlauskas requested a review from rhyslbw February 3, 2023 10:19
@Ryun1 Ryun1 force-pushed the refactor/governance-cip-36 branch from a71a653 to 5d071a4 Compare February 3, 2023 10:51
@Ryun1 Ryun1 changed the title refactor(governance): Aligned package with updates to CIP-36 refactor(governance)!: Aligned package with updates to CIP-36 Feb 3, 2023
@Ryun1 Ryun1 force-pushed the refactor/governance-cip-36 branch from 5d071a4 to 2ab8946 Compare February 3, 2023 14:43
@Ryun1
Copy link
Contributor Author

Ryun1 commented Feb 3, 2023

(Pushed changes to address prettier test failing)

Copy link
Member

@rhyslbw rhyslbw left a comment

Choose a reason for hiding this comment

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

Looks good @Ryun1. There's a conflict after #589 was merged earlier, so you'll need to rebase this branch.

@Ryun1 Ryun1 force-pushed the refactor/governance-cip-36 branch 3 times, most recently from 0299d19 to fc5843c Compare February 7, 2023 13:48
@Ryun1 Ryun1 force-pushed the refactor/governance-cip-36 branch from fc5843c to e7825ef Compare February 7, 2023 13:56
@rhyslbw rhyslbw merged commit 5e1de74 into input-output-hk:master Feb 28, 2023
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.

3 participants