-
Notifications
You must be signed in to change notification settings - Fork 48
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
Add more address generation methods for AddressData
#724
Labels
kind/new-feature
Propose the addition of a new feature that does not yet exist
Comments
dadamu
added
the
kind/new-feature
Propose the addition of a new feature that does not yet exist
label
Jan 18, 2022
dadamu
changed the title
Add more address hash method for
Add more address generation methods for Jan 18, 2022
AddressData
AddressData
It makes sense to me, @dadamu would you mind opening an ADR for these changes? |
Straight forward to me too. 👍 |
19 tasks
mergify bot
pushed a commit
that referenced
this issue
Sep 1, 2022
## Description References: #724 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://docs.cosmos.network/v0.44/building-modules/intro.html) - [ ] included the necessary unit and integration [tests](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
Good |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Feature description
Currently, Desmos chain link supports bech32, base58 and hex address, but each of them utilize only one hash algorithm to generate address from the key, says:
ripemd160(sha256(compressed public key))[:20]
while legacy animo multisig address use the functionsha256(aminoCdc.Marshal(pk))[:20]
.compressed public
key directly, without any hash function.keccak64(uncompressed public key)[12:]
To support more chain with more, we should support more address generation algorithms on each address type.
Implementation proposal
I proposed using
strategy
design pattern on the AddressData structure.For instance on the
HexAddress
, Desmos can have aAddressStrategy
interface like:Then add the strategy field into
HexAddress
which can be called whenVerifyPubKey(key cryptotypes.PubKey)
:What do you think? @RiccardoM @bragaz
The text was updated successfully, but these errors were encountered: