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): allow external reward addresses in governance registrations #2692

Merged

Conversation

davidmisiak
Copy link
Contributor

Changes in governance registrations:

  • Reward address can be provided as an address string instead of address parameters.
  • If the reward address is provided as address parameters, we display its credentials instead of address string.
  • If a non-payment reward address is provided, we display a warning that the address is not eligible for voting rewards (context).

@davidmisiak
Copy link
Contributor Author

Hi @matejcik, any updates on this please?

Also, there was a recent change in CIP-36:

  • The gov_vk bech32 prefix was replaced by cvote_vk.
  • Usage of governance in naming is discouraged (they reserve the term for other purposes to avoid confusion) and they recommend using CIP-36 vote instead.

Therefore I suggest changing gov_vk to cvote_vk and replacing all occurences of governance in the codebase with cvote - what do you think? If you agree, shall I add the change to this PR?

@matejcik
Copy link
Contributor

matejcik commented Feb 8, 2023

Therefore I suggest changing gov_vk to cvote_vk and replacing all occurences of governance in the codebase with cvote - what do you think? If you agree, shall I add the change to this PR?

I have no opinion on the change. "cvote" is shorter than "governance" so that's nice, the prefixes are completely up to Cardano and/or you.
Feel free to add it to this PR if you can.

@davidmisiak davidmisiak force-pushed the cardano-governance-reward-address branch from 18ab139 to eeeea4b Compare February 13, 2023 20:48
@davidmisiak
Copy link
Contributor Author

Done - I changed the bech32 prefix, replaced governance by cvote and renamed reward_address to payment_address in CIP36 registrations (as they did in CIP36). I also rebased onto the current master.

@davidmisiak davidmisiak force-pushed the cardano-governance-reward-address branch from eeeea4b to 133e70e Compare February 13, 2023 22:02
@davidmisiak davidmisiak force-pushed the cardano-governance-reward-address branch 2 times, most recently from eb836f2 to 2752b4c Compare February 21, 2023 18:26
@davidmisiak
Copy link
Contributor Author

I added one more minor rename (voting public key -> vote public key), 2752b4c.

Shall I also update UI tests or should I wait?

@matejcik
Copy link
Contributor

matejcik commented Mar 3, 2023

please go ahead with the UI tests

@davidmisiak davidmisiak force-pushed the cardano-governance-reward-address branch from 2752b4c to d124fa6 Compare March 4, 2023 15:40
@davidmisiak
Copy link
Contributor Author

Rebased, updated UI tests and added changelog entries.

@bosomt
Copy link

bosomt commented Mar 11, 2023

Is there anything that can/needs to be done by QA team ?

@matejcik
Copy link
Contributor

don't think so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants