Skip to content
This repository was archived by the owner on Oct 28, 2021. It is now read-only.

sha3 xor metric#1781

Merged
gavofyork merged 2 commits into
ethereum:developfrom
subtly:sha3xor
May 6, 2015
Merged

sha3 xor metric#1781
gavofyork merged 2 commits into
ethereum:developfrom
subtly:sha3xor

Conversation

@subtly
Copy link
Copy Markdown
Member

@subtly subtly commented May 2, 2015

No description provided.

@chriseth
Copy link
Copy Markdown
Contributor

chriseth commented May 4, 2015

Wouldn't it be better to directly use the sha3 of the address as NodeId, or at least store it inside Nodeid?

@subtly
Copy link
Copy Markdown
Member Author

subtly commented May 4, 2015

@chriseth Perhaps as an optimization. The output of the distance is already being stored.

Comment thread libp2p/NodeTable.h
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this called often? sha3 is not an especially quick function to be calling...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not often. It's called by the constructor of NodeEntry and the result is stored in a property.

@fjl
Copy link
Copy Markdown
Contributor

fjl commented May 5, 2015

For cross-reference, the go-ethereum PR is here: ethereum/go-ethereum#791.
The Go PR implements a protocol change which makes findnode target hash sized instead of
public key sized.

Will submit a devp2p PR for that. If you don't accept this change, I'll roll it back in Go.

@fjl
Copy link
Copy Markdown
Contributor

fjl commented May 5, 2015

ethereum/devp2p#25 is the protocol change request.

gavofyork pushed a commit that referenced this pull request May 6, 2015
@gavofyork gavofyork merged commit 34915c7 into ethereum:develop May 6, 2015
@subtly subtly deleted the sha3xor branch May 7, 2015 16:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants