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.

Sign up for GitHub

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

Jump to bottom

Implement ADR 034: Account Rekeying #7452

Closed
wants to merge 113 commits into from
Closed

Implement ADR 034: Account Rekeying #7452

wants to merge 113 commits into from

Conversation

sunnya97
Copy link
Member

@sunnya97 sunnya97 commented Oct 4, 2020 •
edited
Loading

Description

This PR implements ADR 034 for a new MsgType to allow the updating of the public key for an account, along with the corresponding implementation.

ADR: #7491

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

antstalepresh and others added 30 commits September 25, 2020 21:41
@antstalepresh
add pubkey proto
4d91c5a
@antstalepresh
change proto description
95f35b8
@antstalepresh
modify MsgChangePubKey proto
93f453a
@antstalepresh
remove moretag
2fb8ce0
@antstalepresh
change proto package name
1700bcc
@antstalepresh
add PubKeyChangeCost to auth.Params
7fc644f
@antstalepresh
add prototype codebase for pubkey
8a5b23e
@antstalepresh
add files for pubkey types
907b823
@antstalepresh
account pubKey modification logic on pubkey handler
71a6d95
@antstalepresh
add gas logic prototype
25a2e61
modify change-pubkey cli command long description
1b9772e
modify package import url and add example bech32 pubkey
9834f16
implement GetPubKeyFromBech32 and force conversion from crypto.PubKey…
c8b0f59 
… to secp256k1.PubKey
Register pubkey module and pubkeycli command on simapp && Integrate s…
dd65c64 
…takingKeeper into pub key module to get bond denominator && Fix pubkey handler build error
fix unnecessary conversion to uint64
4247745
@antstalepresh
implement TestMsgChangePubKey && add correct pubKey for TestNewMsgCha…
8a2f38d 
…ngePubKeyCmd
@antstalepresh
Add correct pubKey for TestNewMsgChangePubKeyCmd && Register codec an…
4a44f5a 
…d interfaces for pub key module && Implement TestNewMsgChangePubKeyCmd prototype cli test
@antstalepresh
Modify change pub key message type to use generalize pub key instead …
acb0862 
…of secp256k1 only support
@antstalepresh
remove charging additional fees using bankkeeper.SendCoinsFromAccount…
f28dd06 
…ToModule
@antstalepresh
modify module name from pubkey to changepubkey
eea5fd0
@antstalepresh
Modify package name from pubkey to changepubkey to eliminate confusing
cf6dcc0
@antstalepresh
Modify MsgChangePubKey ValidateBasic function for pubkey validation
ad82782
@antstalepresh
create ADR 028: Change PubKey prototype
1a7df9d
@antstalepresh
Modify ADR paper from comment
e85f705
@antstalepresh
Create params port for change pubkey and write utils / unit test && a…
ea1c9f5 
…dd query params cli
@antstalepresh
implement params inside changepubkey
dec7387
@antstalepresh
update changepubkey params proto index to 1 and do part revert of aut…
f0122ad 
…h params modification
@antstalepresh
revert auth param changes
42a4e14
@antstalepresh
revert ante_test
30b6f8c
@antstalepresh
modify pubkey to changepubkey term
916c3b8
@ethanfrey
Copy link
Contributor

  1. client behavior today

Client behavior today is very sub-par. In fact, it was so bad that Aaron spent almost 1 year to replace amino with protobuf and enable non golang client libraries. As a first stepping stone to actual intelligent off-chain agents.

With stargate, we are finally unleashing the potential of off-chain actors. Able to create and parse tx natively. I fully support this direction and I think we should do all we can to nurture a very powerful client-side ecosystem. Where ADR036 is only one baby step along the way.

@robert-zaremba
Copy link
Collaborator

Client behavior today is very sub-par.

This is what I thought. So there are not that many legacy off-chain applications with some hard assumptions, right?

@robert-zaremba
Copy link
Collaborator

BTW, we should move this discussion to ADR-34 PR, not the implementation draft ;)

@sunnya97 sunnya97 changed the title Implement ADR 034: Change Account PubKey Implement ADR 034: Account Rekeying Nov 20, 2020
antstalepresh and others added 10 commits December 31, 2020 00:39
@tac0turtle
Copy link
Member

Where are we with this PR? can we get it over the finish line?

@robert-zaremba
Copy link
Collaborator

Heads up - we are working on integrating DID standards and this could be naturally solved on a different level.
We are proposing a solution to separate 2 concepts:

  • credentials -- eg key pair, multisig, pay2hash ...
  • accounts -- they will define authorization methods and hence will implement this concept.

@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Apr 17, 2021
@aaronc
Copy link
Member

aaronc commented Apr 17, 2021

@sunnya97 does your team have any bandwidth to wrap this up? Would love to see this land.

@github-actions github-actions bot removed the stale label Apr 18, 2021
@sunnya97
Copy link
Member Author

@aaronc Yeah, this accidentally fell off our plate. We'll try to get to this soon.

@robert-zaremba
Copy link
Collaborator

@sunnya97 , @aaronc - with the idea we discussed to break down accounts into credentials and addressables, the account rekeying will be solved automatically.

@aaronc
Copy link
Member

aaronc commented Apr 21, 2021

Well not necessarily automatically. It just allows for more flexibility.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jun 6, 2021
@sunnya97
Copy link
Member Author

sunnya97 commented Aug 9, 2021 •
edited
Loading

@robert-zaremba where can I read about this credentials and addressables system? Is there an ADR for this?

@tac0turtle
Copy link
Member

tac0turtle commented Nov 9, 2021 •
edited
Loading

closing this due to staleness.

Please do not reopen unless there is a plan to complete it, within a timely manner. Im trying to clean the sdk from prs sitting around.

@tac0turtle tac0turtle closed this Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronc aaronc aaronc left review comments

@tac0turtle tac0turtle tac0turtle left review comments

@alessio alessio Awaiting requested review from alessio

@alexanderbez alexanderbez Awaiting requested review from alexanderbez

Assignees
No one assigned
Labels
C:x/auth
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sunnya97 @tac0turtle @aaronc @robert-zaremba @antstalepresh @ethanfrey