#23760 closed enhancement (implemented)

Use node_get_curve25519_onion_key() in extend_info_from_node()

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: refactor, easy, review-group-26
Cc: neel@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by teor)

In #23577, we are going to implement a node_get_curve25519_onion_key() function. We can use this in extend_info_from_node() if we want to.

Child Tickets

Attachments (3)

001-node_get_extend_info.patch (1.5 KB) - added by neel 12 months ago.
Patch to introduce node_get_curve25519_onion_key() in extend_info_from_node()
002-node_get_extend_info-r1.patch (1.6 KB) - added by neel 12 months ago.
Patch to introduce node_get_curve25519_onion_key() in extend_info_from_node() (Revision 1)
003-node_get_extend_info-r2.patch (2.1 KB) - added by neel 12 months ago.
Patch to introduce node_get_curve25519_onion_key() in extend_info_from_node() (Revision 2)

Download all attachments as: .zip

Change History (17)

comment:1 Changed 14 months ago by nickm

Maybe call it node_get_ntor_key() instead?

comment:2 Changed 14 months ago by teor

If we do this, we should rename the existing node_has_curve25519_onion_key() to node_has_ntor_key().

comment:3 Changed 14 months ago by nickm

ntor_key, ntor_onion_key, curve25519_ntor_key, and curve25519_onion_key are also good names for this key -- we just shouldn't call it curve25519_key (with no indication of its purpose).

comment:4 Changed 13 months ago by teor

Description: modified (diff)
Keywords: easy added
Summary: Use node_get_curve25519_key() in extend_info_from_node()Use node_get_curve25519_onion_key() in extend_info_from_node()

Modify the name for consistency with node_has_curve25519_onion_key().

comment:5 Changed 13 months ago by teor

Keywords: prop224 tor-hs single-onion ipv6 removed
Parent ID: #23577

Removing the parent and spurious keywords.

Changed 12 months ago by neel

Patch to introduce node_get_curve25519_onion_key() in extend_info_from_node()

comment:6 Changed 12 months ago by neel

I have a patch to resolve this. The filename is 001-node_get_extend_info.patch.

comment:7 Changed 12 months ago by teor

Status: newneeds_revision

Hi neel, thanks for the patch!

node_get_curve25519_onion_key() returns a const pointer, so the variable also has to be const. Otherwise, some compilers will complain.

The rest of the patch looks good!

Changed 12 months ago by neel

Patch to introduce node_get_curve25519_onion_key() in extend_info_from_node() (Revision 1)

comment:8 Changed 12 months ago by neel

I have an updated patch under the filename 002-node_get_extend_info-r1.patch which makes curve_pubkey a const.

comment:9 Changed 12 months ago by teor

Ok, looks good to me.

Do you want to write a changes file?
Any code changes get a changes file, even simple refactoring.

Also, we should check this compiles and passes the tests before we merge.

Changed 12 months ago by neel

Patch to introduce node_get_curve25519_onion_key() in extend_info_from_node() (Revision 2)

comment:10 Changed 12 months ago by neel

I have a new patch which includes a changes file. The filename is 003-node_get_extend_info-r2.patch.

comment:11 Changed 12 months ago by teor

Hi,

When I try to compile this, I get the following errors:

src/or/circuitbuild.c:2699:5: error: implicit declaration of function
      'node_get_curve25519_onion_key' is invalid in C99
      [-Werror,-Wimplicit-function-declaration]
    node_get_curve25519_onion_key(node);
    ^
src/or/circuitbuild.c:2699:5: note: did you mean
      'node_has_curve25519_onion_key'?
./src/or/nodelist.h:78:5: note: 'node_has_curve25519_onion_key' declared here
int node_has_curve25519_onion_key(const node_t *node);
    ^
src/or/circuitbuild.c:2698:34: error: incompatible integer to pointer conversion
      initializing 'const curve25519_public_key_t *' (aka 'const struct
      curve25519_public_key_t *') with an expression of type 'int'
      [-Werror,-Wint-conversion]
  const curve25519_public_key_t *curve_pubkey =
                                 ^

Please import the correct header for node_get_curve25519_onion_key(), or put a declaration in the correct header.

comment:12 Changed 12 months ago by teor

Status: needs_revisionmerge_ready

Oops, my master branch was about 1500 commits behind.
Not sure how that happened. Sorry for any confusion.

This is fine, it passes make check and make test-network.
Let's get it merged.

It's branch bug23760_nc in my github if you prefer branches.

comment:13 Changed 12 months ago by nickm

Keywords: review-group-26 added

Creating review-group-26.

comment:14 Changed 12 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

Merging to master; thanks!

Note: See TracTickets for help on using tickets.