Opened 2 years ago

Closed 21 months ago

#22173 closed enhancement (implemented)

Support looking up node by ed25519 identity (backend)

Reported by: dgoulet Owned by: nickm
Priority: High Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: review-group-23
Cc: nickm Actual Points: 2
Parent ID: Points: 1
Reviewer: catalyst Sponsor:

Description

Currently we can only lookup a node_t object using the SHA1 identity digest (node_get_by_id() and friends).

It would be good to have a map of node_t indexed by ed25519 identity so we can look them up that way.

Child Tickets

Change History (15)

comment:1 Changed 2 years ago by nickm

Cc: nickm added
Points: 1
Priority: MediumHigh

comment:2 Changed 22 months ago by nickm

I've got the backend here in ed25519_lookup, but no front end yet.

comment:3 Changed 22 months ago by nickm

Status: newaccepted

comment:4 Changed 21 months ago by nickm

I added some tests on ed25519_lookup, and started working on the front-end code, but I realized we hadn't actually settled on a UI format. Hex seems too long; base64 seems maybe better, but there are issues with squeezing base64 into browsers and getting it in a nice-to-select format. Teor suggested that I look at Y64 on https://en.wikipedia.org/wiki/Base64#Implementations_and_history , and maybe other variants as well.

comment:5 Changed 21 months ago by nickm

Summary: Support looking up node by ed25519 identitySupport looking up node by ed25519 identity (backend)

comment:6 Changed 21 months ago by nickm

Actual Points: 2
Status: acceptedneeds_review

I'm putting this in needs_review; the frontend work will have to wait for 0.3.3.

comment:7 Changed 21 months ago by nickm

If you'd rather look on oniongit, I've also put the branch in a merge request at https://oniongit.eu/nickm/tor/merge_requests/7

comment:8 Changed 21 months ago by nickm

Keywords: review-group-23 added

Put 0.3.2 needs_review and merge_ready tickets into review-group-23.

comment:9 Changed 21 months ago by catalyst

Reviewer: catalyst

comment:10 Changed 21 months ago by catalyst

Front end support in #15059.

comment:11 Changed 21 months ago by catalyst

This looks good, but please check my comments in oniongit about the final commit. Also this looks like it might have some overlap with #22215, especially d7a3e336ee505bcbeb30117d91067810ad096130. Has anyone checked for merge conflicts?

comment:12 Changed 21 months ago by catalyst

Status: needs_reviewneeds_revision

comment:13 Changed 21 months ago by nickm

Status: needs_revisionneeds_review

Thanks, catalyst! I just asked a question about your question on the oniongit ticket.

I also tried to apply the patch from #22215 on top of this, and it worked okay with "git am -3".

comment:14 Changed 21 months ago by catalyst

Status: needs_reviewmerge_ready

OK, thanks! I responded on oniongit -- I was mistaken. Looks good now!

comment:15 Changed 21 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

thanks! Merging this to master.

Note: See TracTickets for help on using tickets.