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

Add multiaddr to ENR #142

Closed
wants to merge 2 commits into from
Closed

Add multiaddr to ENR #142

wants to merge 2 commits into from

Conversation

D4nte
Copy link

@D4nte D4nte commented Oct 15, 2021

No description provided.

@D4nte
Copy link
Author

D4nte commented Oct 15, 2021

This was meant for my own fork

@D4nte D4nte closed this Oct 15, 2021
@dapplion
Copy link
Contributor

Feel free to do a PR upstream if you feel a feature is missing!

@D4nte
Copy link
Author

D4nte commented Oct 27, 2021

hi There,

Few comments and I am not sure at this stage what is the most straightforward way.

For simplicity sake, I decided to import the code in js-waku: waku-org/js-waku#324

To not depend on several crypto library, I also swapped bcrypto for secp256k1.
Note that my target is mainly browser.

Now, I intend at some point to extract the ENR code and the EIP-1459 code as their own package so other projects can use it.

I will also need to implement discv5 as well, but it needs to use the ENR with multiaddrs so I won't be able to use the present library out of the box.

Having said that, I am keen to collaborate if you think the core change I implemented (multiaddrs key on ENR) is something you need. Let me know what you think.

@dapplion
Copy link
Contributor

@wemeetagain I'm sure we can extend enr + discv5 to support this changes!

@wemeetagain
Copy link
Member

Hey @D4nte
I think the multiaddrs key is great, happy to push upstream.

Also we'd love to split out the ENR logic into a separate package.

@D4nte
Copy link
Author

D4nte commented Oct 28, 2021

Thank you both for your comments.

What are your thoughts in regards of the cryptography backend? Bcrypto vs secpk1? Was bcrypto chosen because it supports various curves, even though only secpk1 is used for ENRs?

I believe both our project target browser environment so limiting dependencies is always preferred.

Ps: I am in leave at the moment so apologies if I take time to reply/action.

@wemeetagain
Copy link
Member

What are your thoughts in regards of the cryptography backend? Bcrypto vs secpk1? Was bcrypto chosen because it supports various curves, even though only secpk1 is used for ENRs?

I think bcrypto was used because it was already being used for discv5 crypto.
My hope was ideally we could use libp2p-crypto.

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.

None yet

3 participants