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

feat(cardano): add support for CIP-36 governance registration format #2561

Closed

Conversation

davidmisiak
Copy link
Contributor

@davidmisiak davidmisiak commented Oct 14, 2022

This PR adds support for CIP-36 governance registration format. Changes:

  • New field: delegations array (each delegation consists of a voting key and its voting power proportion) as an alternative to delegating the entire voting power to a single voting key. The array is embedded in the protobuf message due to simplicity (max. 32 delegations allowed).
  • New field: voting_purpose integer (currently 0 = Catalyst, 1 = other). In CIP-36 registrations, this field is always serialized, (value 0 is used if not provided by the client).
  • All Catalyst occurences are replaced by governance, since the format should be usable for other governance purposes, not only Catalyst.
  • The ed25519_pk bech32 prefix is replaced by gov_vk for the purpose of displaying voting public keys.

The new 1694' derivation path and the vote cast call implementation is not included in this PR.


EDIT - Added:

@davidmisiak davidmisiak requested a review from matejcik as a code owner October 14, 2022 12:57
Copy link
Contributor

@matejcik matejcik left a comment

Choose a reason for hiding this comment

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

it seems the new tests are not included in fixtures.json? please regenerate that

common/tests/fixtures/cardano/sign_tx.json Show resolved Hide resolved
@davidmisiak davidmisiak force-pushed the cardano-governance-cip36 branch from 6994d3f to fe78d0c Compare October 21, 2022 19:03
@davidmisiak davidmisiak force-pushed the cardano-governance-cip36 branch from fe78d0c to e57e42e Compare October 21, 2022 21:57
@davidmisiak
Copy link
Contributor Author

it seems the new tests are not included in fixtures.json? please regenerate that

Done, UI tests should be up to date. I also added changelog entries and rebased.

@davidmisiak
Copy link
Contributor Author

Also @matejcik, here is a commit that adds new testnets. Should I add it here or open a separate PR?

@matejcik
Copy link
Contributor

feel free to add here. it has no effect on tests, right?

@davidmisiak
Copy link
Contributor Author

it has no effect on tests, right?

I believe so.

Thanks, added.

@davidmisiak
Copy link
Contributor Author

Hi @matejcik, is there a chance we could merge this before the FW freeze please?

@matejcik
Copy link
Contributor

that is the plan, yes

@matejcik
Copy link
Contributor

will be merged in #2591

@matejcik matejcik closed this Oct 31, 2022
@davidmisiak
Copy link
Contributor Author

Awesome, thanks!

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.

2 participants