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

Generator of three words identifiers #65

Closed
wants to merge 3 commits into from

Conversation

davidyuk
Copy link
Member

@davidyuk davidyuk commented Jul 2, 2018


export default address => {
const bytes = decodeBase58Check(address.split('$')[1])
let entropy = bytes.readUInt32BE(bytes.length - 4)
Copy link
Contributor

Choose a reason for hiding this comment

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

What if this function receives two different addresses with the same entropy? Looks like it would return the same words. Maybe each word should be based on separate entropy (i.e. different bytes of the buffer)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this function will return the same words if addresses have the same 4 last bytes, but it should be quite difficult for the attacker to generate a key pair with the same last bytes.

Also, this implementation does not protect against typos. Maybe aeternity accounts addresses are already somehow protected against typos?

Entropy can be generated based on the hash of an address or we can use account address as randseed like Status does, but as I understand the generation of such identifier is much cheaper than a generation of key pair so even if we will use the whole address as entropy still will be possible to generate different address with the same identifier, but without corresponding private key. Of course, we can increase the complexity of identifier generation algorithm, but it can decrease the performance of our apps in some cases.

@outergod
Copy link
Contributor

@jsnewby could you please elaborate whether this shall be part of the SDK?

@jsnewby
Copy link
Contributor

jsnewby commented Jul 27, 2018

My opinion is that this ought to be separate from the SDK, as the SDK should be application-agnostic, and this seems to belong to the aepps. Does anyone strongly disagree?

@davidyuk
Copy link
Member Author

Three words identifiers are not only for Base aepp. Usefulness of this feature depends on the number of applications that supports it. I think it should be defined as a standard on the level of aepp SDK.

@davidyuk davidyuk force-pushed the feature/three-words-identifier branch from aa38c9e to 17ade19 Compare October 25, 2018 17:48
@ricricucit
Copy link
Contributor

closed due to inactivity and age. Opened issue #168 in favour of this idea.

@ricricucit ricricucit closed this Dec 7, 2018
@davidyuk
Copy link
Member Author

davidyuk commented Dec 9, 2018

@ricricucit why we can't merge this PR? It is ready to be used in the Base app and other aepps.

@davidyuk davidyuk reopened this Dec 9, 2018

export default address => {
const bytes = decodeBase58Check(assertedType(address, 'ak'))
let entropy = bytes.readUInt32BE(bytes.length - 4)
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe it is more accurate to use dividedToIntegerBy and modulo from bignumber.js with the whole address instead of working with the last 4 bytes.

@nduchak
Copy link
Contributor

nduchak commented Jul 15, 2019

Close due to inactivity

@nduchak nduchak closed this Jul 15, 2019
@nduchak nduchak deleted the feature/three-words-identifier branch September 18, 2019 11:49
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.

6 participants