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

Change Account ids/addresses to truncated hash of the public key #2940

Merged
merged 16 commits into from
Jun 11, 2020

Conversation

tjanez
Copy link
Member

@tjanez tjanez commented May 26, 2020

Closes #2928.

TODO:

  • Use Bech32-encoding for marshaling/unmarshaling addresses to/from their text form.
  • Implement address context separation in the form of H(<context> || <pubkey>), e.g. H("oasis-core/address: staking" || <pubkey>).
  • Add versioning to Account addresses.
  • Implement a notion of reserved addresses.
  • Add staking transaction checks for reserved addresses.
  • Choose Oasis staking account address Bech32 human readable part.
  • Fix E2E tests.

@@ -0,0 +1,80 @@
// Package address implements a generic cryptographic address.
Copy link
Contributor

Choose a reason for hiding this comment

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

A better description would be nice.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried, please take another look.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Yawning, ping

@tjanez tjanez force-pushed the tjanez/new-account-ids branch from 3649ad6 to 86e1bed Compare May 29, 2020 12:05
@tjanez tjanez changed the title Change Account ids to truncated hash of the public key Change Account ids/addresses to truncated hash of the public key May 29, 2020
@tjanez tjanez force-pushed the tjanez/new-account-ids branch from 86e1bed to 88954c5 Compare May 29, 2020 12:53
@tjanez tjanez force-pushed the tjanez/new-account-ids branch 8 times, most recently from 97b9716 to 20554a5 Compare June 3, 2020 15:50
@tjanez tjanez force-pushed the tjanez/new-account-ids branch 2 times, most recently from 6e46fc3 to ea9cec4 Compare June 3, 2020 16:32
@tjanez tjanez force-pushed the tjanez/new-account-ids branch 4 times, most recently from 6cb53bf to 951b70e Compare June 4, 2020 12:43
@tjanez tjanez force-pushed the tjanez/new-account-ids branch from 951b70e to 05ca1e6 Compare June 5, 2020 08:29
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

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

Don't forget to mention all the API and config parameter changes in the respective changelog fragments.

@tjanez tjanez force-pushed the tjanez/new-account-ids branch from 05ca1e6 to 0787fc7 Compare June 5, 2020 16:35
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

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

Looked at the recent Bech32 registration addition, looks good!

@tjanez tjanez force-pushed the tjanez/new-account-ids branch from 0787fc7 to f446c78 Compare June 8, 2020 12:34
@tjanez tjanez force-pushed the tjanez/new-account-ids branch 2 times, most recently from fd9df12 to 7c44974 Compare June 11, 2020 08:53
tjanez added 15 commits June 11, 2020 13:06
Rename NewBlacklistedKey() to NewBlacklistedPublicKey() for consistency.

Update the tests to cover these two functions.
Add OwnTxSignerAddress() to ApplicationState.
The test scenario that used it was removed when support for v0 node
descriptor was removed in #2963.
tests/fixture-data: Update genesis documents to use the new staking
account address format.
Replace GetSignerNonceRequest's ID field with AccountAddress field to
reflect the recent staking account id/address change.
Rename EstimateGasRequest's Caller field to Signer to better describe
the field's value which is the public key of the transaction's signer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:breaking/cfg Category: breaks configuration c:breaking/consensus Category: breaking consensus changes c:common Category: common libraries c:consensus/cometbft Category: CometBFT c:staking Category: staking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change Account ids to truncated hash of the public key
3 participants