trezor: support c++17 and protobuf v25, libusb fix #smol #9064

ph4r05 · 2023-11-09T23:37:02Z

fixes support for C++ 17 && Protobuf v22+
fixes libusb compile check for ubuntu

@selsta c++17 support came sooner than expected 😅 I've modified the code a bit to protobufv25 and c++17 works

ph4r05 · 2023-11-10T09:33:29Z

@jeffro256 @moneromooo-monero when you have a minute for a review, this one is small 😅 thanks!

ph4r05 · 2023-11-21T12:09:28Z

Oor maybe @vtnerd @rbrunner7 ? :)

This PR is minimalistic, thanks!

selsta · 2023-11-21T12:11:49Z

I will review it but didn't get to it yet.

cmake/CheckTrezor.cmake

ph4r05 · 2023-12-18T11:40:37Z

Hello, just checking if you have a minute for the review :)

jeffro256 · 2023-12-19T01:45:24Z

I'm getting linking errors with target release-static on Ubuntu 23.

Selection from error messages:

/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `my_signal_new':
(.text+0xbb): undefined reference to `event_set'
/usr/bin/ld: (.text+0xc7): undefined reference to `event_base_set'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `my_event_new':
(.text+0x15d): undefined reference to `event_set'
/usr/bin/ld: (.text+0x16a): undefined reference to `event_base_set'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `my_timer_add':
(.text+0x1e6): undefined reference to `event_set'
/usr/bin/ld: (.text+0x1f2): undefined reference to `event_base_set'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `my_event_base_free':
(.text+0x25d): undefined reference to `event_base_free'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `ub_default_event_base':
(.text+0x2cd): undefined reference to `event_base_new'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `ub_get_event_sys':
(.text+0x3a6): undefined reference to `event_get_version'
/usr/bin/ld: (.text+0x3bc): undefined reference to `event_base_get_method'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `my_event_del':
(.text+0x1b9): undefined reference to `event_del'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `my_timer_add':
(.text+0x205): undefined reference to `event_add'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `my_event_add':
(.text+0x229): undefined reference to `event_add'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `my_event_base_loopexit':
(.text+0x239): undefined reference to `event_base_loopexit'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `my_event_base_dispatch':
(.text+0x249): undefined reference to `event_base_dispatch'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `my_signal_del':
(.text+0x279): undefined reference to `event_del'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `my_timer_del':
(.text+0x289): undefined reference to `event_del'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(ub_event_pluggable.o): in function `my_signal_add':
(.text+0x299): undefined reference to `event_add'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(random.o): in function `ub_initstate':
(.text+0x40): undefined reference to `nettle_yarrow256_init'
/usr/bin/ld: (.text+0x62): undefined reference to `nettle_yarrow256_seed'
/usr/bin/ld: (.text+0x6a): undefined reference to `nettle_yarrow256_is_seeded'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(random.o): in function `ub_random':
(.text+0x139): undefined reference to `nettle_yarrow256_random'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(random.o): in function `ub_random_max':
(.text+0x203): undefined reference to `nettle_yarrow256_random'
/usr/bin/ld: (.text+0x251): undefined reference to `nettle_yarrow256_random'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(val_secalgo.o): in function `_digest_nettle':
(.text+0x49): undefined reference to `nettle_sha256_init'
/usr/bin/ld: (.text+0x57): undefined reference to `nettle_sha256_update'
/usr/bin/ld: (.text+0x67): undefined reference to `nettle_sha256_digest'
/usr/bin/ld: (.text+0xa6): undefined reference to `nettle_sha512_init'
/usr/bin/ld: (.text+0xb4): undefined reference to `nettle_sha512_update'
/usr/bin/ld: (.text+0xc4): undefined reference to `nettle_sha512_digest'
/usr/bin/ld: (.text+0xd7): undefined reference to `nettle_sha384_init'
/usr/bin/ld: (.text+0xe5): undefined reference to `nettle_sha512_update'
/usr/bin/ld: (.text+0xf5): undefined reference to `nettle_sha384_digest'
/usr/bin/ld: (.text+0x107): undefined reference to `nettle_sha1_init'
/usr/bin/ld: (.text+0x115): undefined reference to `nettle_sha1_update'
/usr/bin/ld: (.text+0x125): undefined reference to `nettle_sha1_digest'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(val_secalgo.o): in function `_verify_nettle_rsa':
(.text+0x1cf): undefined reference to `nettle_rsa_public_key_init'
/usr/bin/ld: (.text+0x1f9): undefined reference to `nettle_mpz_set_str_256_u'
/usr/bin/ld: (.text+0x211): undefined reference to `nettle_mpz_set_str_256_u'
/usr/bin/ld: (.text+0x21f): undefined reference to `nettle_mpz_init_set_str_256_u'
/usr/bin/ld: (.text+0x240): undefined reference to `nettle_rsa_public_key_clear'
/usr/bin/ld: (.text+0x248): undefined reference to `__gmpz_clear'
/usr/bin/ld: (.text+0x2cb): undefined reference to `nettle_sha1_init'
/usr/bin/ld: (.text+0x2d9): undefined reference to `nettle_sha1_update'
/usr/bin/ld: (.text+0x2f1): undefined reference to `nettle_sha1_digest'
/usr/bin/ld: (.text+0x301): undefined reference to `nettle_rsa_sha1_verify_digest'
/usr/bin/ld: (.text+0x310): undefined reference to `nettle_rsa_public_key_clear'
/usr/bin/ld: (.text+0x318): undefined reference to `__gmpz_clear'
/usr/bin/ld: (.text+0x343): undefined reference to `nettle_sha512_init'
/usr/bin/ld: (.text+0x351): undefined reference to `nettle_sha512_update'
/usr/bin/ld: (.text+0x369): undefined reference to `nettle_sha512_digest'
/usr/bin/ld: (.text+0x379): undefined reference to `nettle_rsa_sha512_verify_digest'
/usr/bin/ld: (.text+0x39b): undefined reference to `nettle_sha256_init'
/usr/bin/ld: (.text+0x3a9): undefined reference to `nettle_sha256_update'
/usr/bin/ld: (.text+0x3c1): undefined reference to `nettle_sha256_digest'
/usr/bin/ld: (.text+0x3d1): undefined reference to `nettle_rsa_sha256_verify_digest'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(val_secalgo.o): in function `_verify_nettle_ecdsa':
(.text+0x466): undefined reference to `nettle_dsa_signature_init'
/usr/bin/ld: (.text+0x484): undefined reference to `nettle_get_secp_384r1'
/usr/bin/ld: (.text+0x499): undefined reference to `nettle_ecc_point_init'
/usr/bin/ld: (.text+0x4a9): undefined reference to `nettle_mpz_init_set_str_256_u'
/usr/bin/ld: (.text+0x4bf): undefined reference to `nettle_mpz_init_set_str_256_u'
/usr/bin/ld: (.text+0x4cf): undefined reference to `nettle_mpz_set_str_256_u'
/usr/bin/ld: (.text+0x4e2): undefined reference to `nettle_mpz_set_str_256_u'
/usr/bin/ld: (.text+0x4fc): undefined reference to `nettle_sha384_init'
/usr/bin/ld: (.text+0x514): undefined reference to `nettle_sha512_update'
/usr/bin/ld: (.text+0x524): undefined reference to `nettle_sha384_digest'
/usr/bin/ld: (.text+0x532): undefined reference to `nettle_ecc_point_set'
/usr/bin/ld: (.text+0x547): undefined reference to `nettle_ecdsa_verify'
/usr/bin/ld: (.text+0x551): undefined reference to `__gmpz_clear'
/usr/bin/ld: (.text+0x55c): undefined reference to `__gmpz_clear'
/usr/bin/ld: (.text+0x564): undefined reference to `nettle_ecc_point_clear'
/usr/bin/ld: (.text+0x56c): undefined reference to `nettle_dsa_signature_clear'
/usr/bin/ld: (.text+0x591): undefined reference to `nettle_get_secp_256r1'
/usr/bin/ld: (.text+0x5a6): undefined reference to `nettle_ecc_point_init'
/usr/bin/ld: (.text+0x5b6): undefined reference to `nettle_mpz_init_set_str_256_u'
/usr/bin/ld: (.text+0x5cc): undefined reference to `nettle_mpz_init_set_str_256_u'
/usr/bin/ld: (.text+0x5dc): undefined reference to `nettle_mpz_set_str_256_u'
/usr/bin/ld: (.text+0x5ef): undefined reference to `nettle_mpz_set_str_256_u'
/usr/bin/ld: (.text+0x609): undefined reference to `nettle_sha256_init'
/usr/bin/ld: (.text+0x621): undefined reference to `nettle_sha256_update'
/usr/bin/ld: (.text+0x631): undefined reference to `nettle_sha256_digest'
/usr/bin/ld: (.text+0x63f): undefined reference to `nettle_ecc_point_set'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(val_secalgo.o): in function `secalgo_hash_create_sha384':
(.text+0x6e7): undefined reference to `nettle_sha384_init'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(val_secalgo.o): in function `secalgo_hash_create_sha512':
(.text+0x71a): undefined reference to `nettle_sha512_init'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(val_secalgo.o): in function `secalgo_hash_update':
(.text+0x76b): undefined reference to `nettle_sha512_update'
/usr/bin/ld: (.text+0x788): undefined reference to `nettle_sha512_update'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(val_secalgo.o): in function `secalgo_hash_final':
(.text+0x7c5): undefined reference to `nettle_sha512_digest'
/usr/bin/ld: (.text+0x80a): undefined reference to `nettle_sha384_digest'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libunbound.a(val_secalgo.o): in function `verify_canonrrset':
(.text+0xa7a): undefined reference to `nettle_ed25519_sha512_verify'
collect2: error: ld returned 1 exit status

ph4r05 · 2024-02-03T22:35:16Z

@jeffro256 it does not seem related to this PR, can you pls try again?

jeffro256 · 2024-02-06T06:23:45Z

Pulled latest commit and retried compiling, got the same results. I can run make release-static just fine on master. I'm not quite sure what the reason is, I'll keep looking...

ph4r05 · 2024-02-25T10:06:32Z

looks similar to #8439

ph4r05 · 2024-02-25T10:31:39Z

@jeffro256 It looks unrelated to my changes :/

Static linking for libunbound requires also libnettle, which is not linked. The following commit fixed the linking on Ubuntu 23.04 04ccc0f

Or installing static unbound from sethforprivacy/simple-monerod-docker#61 may help:

wget https://www.nlnetlabs.nl/downloads/unbound/unbound-1.16.1.tar.gz && \
    echo "2fe4762abccd564a0738d5d502f57ead273e681e92d50d7fba32d11103174e9a  unbound-1.16.1.tar.gz" | sha256sum -c && \
    tar -xzf unbound-1.16.1.tar.gz && \
    rm unbound-1.16.1.tar.gz && \
    cd unbound-1.16.1 && \
    ./configure --disable-shared --enable-static --without-pyunbound --with-libexpat=/usr --with-ssl=/usr --with-libevent=no --without-pythonmodule --disable-flto --with-pthreads --with-libunbound-only --with-pic && \
    make -j${NPROC:-$(nproc)} && \
    make -j${NPROC:-$(nproc)} install

or

apt install nettle-dev

Now checking if it passes on the CI. If everything is green, can you pls review? Thanks!

Edit: still issues with static build

ph4r05 · 2024-02-25T20:21:27Z

@jeffro256 ok I figured out that libunbound + nettle does not statically compile on Ubuntu 23.04 by default. I tried several combinations, nothing worked. Default nettle did not have for example nettle_mpz_set_str_256_uz required by unbound.

After installing unbound-1.61.1 according to sethforprivacy/simple-monerod-docker#61 compilation started to work out of the box.

wget https://www.nlnetlabs.nl/downloads/unbound/unbound-1.16.1.tar.gz && \
    echo "2fe4762abccd564a0738d5d502f57ead273e681e92d50d7fba32d11103174e9a  unbound-1.16.1.tar.gz" | sha256sum -c && \
    tar -xzf unbound-1.16.1.tar.gz && \
    rm unbound-1.16.1.tar.gz && \
    cd unbound-1.16.1 && \
    ./configure --disable-shared --enable-static --without-pyunbound --with-libexpat=/usr --with-ssl=/usr --with-libevent=no --without-pythonmodule --disable-flto --with-pthreads --with-libunbound-only --with-pic && \
    make -j${NPROC:-$(nproc)} && \
    make -j${NPROC:-$(nproc)} install

ph4r05 · 2024-02-26T21:46:07Z

@selsta would you pls find a minute for a review? :)

ph4r05 · 2024-02-27T19:10:48Z

ooor @vtnerd 🙏 ? @moneromooo-monero 🙏 ? :)

selsta · 2024-02-28T18:47:51Z

I will try to review it next week

selsta · 2024-03-10T21:30:51Z

src/device_trezor/README.md

-Trezor uses [Protobuf](https://protobuf.dev/) library. As Monero is compiled with C++14, the newest Protobuf library version cannot be compiled because it requires C++17 (through its dependency Abseil library).
-This can result in a compilation failure.
+Trezor uses [Protobuf](https://protobuf.dev/) library. Monero is now compiled with C++ 17 by default.
+Thus, protobuf v23 and higher has to be used. 


Is v23 a necessity? We build release binaries with v21 (https://github.com/monero-project/monero/blob/master/contrib/depends/packages/native_protobuf.mk#L2) and it compiled with Trezor support enabled according to CI in this PR.

No, it is not a hard requirement, it is changed now

selsta · 2024-03-10T21:47:12Z

cmake/CheckTrezor.cmake

    if (NOT Protobuf_FOUND)
        FIND_PACKAGE(Protobuf)
    endif()

    _trezor_protobuf_fix_vars()

    # Early fail for optional Trezor support
-    if(${Protobuf_VERSION} GREATER 21)
-        trezor_fatal_msg("Trezor: Unsupported Protobuf version ${Protobuf_VERSION}. Please, use Protobuf v21.")
+    if(${CMAKE_CXX_STANDARD} LESS 17 AND ${Protobuf_VERSION} GREATER 21)


If there is no protobuf version installed, this if fails with this CMake error

CMake Error at cmake/CheckTrezor.cmake:92 (if): if given arguments: "17" "LESS" "17" "AND" "GREATER" "21" Unknown arguments specified Call Stack (most recent call first): CMakeLists.txt:704 (include) -- Configuring incomplete, errors occurred!

Which aborts CMake. This is unintended behaviour for USE_DEVICE_TREZOR=ON and USE_DEVICE_TREZOR_MANDATORY=OFF. The issue already exists in the current code (see #9040) but it can be fixed together with this PR.

aah you are right, I exchanged those two ifs so first protobuf is checked. it fixes it.

cmake/CheckTrezor.cmake

ph4r05 · 2024-03-17T09:58:14Z

@selsta thanks for the review! pls when you have a minute to check again, I addressed your comments. Thanks a lot!

selsta · 2024-03-17T13:08:29Z

src/device_trezor/README.md

-Trezor uses [Protobuf](https://protobuf.dev/) library. As Monero is compiled with C++14, the newest Protobuf library version cannot be compiled because it requires C++17 (through its dependency Abseil library).
-This can result in a compilation failure.
+Trezor uses [Protobuf](https://protobuf.dev/) library. Monero is now compiled with C++ 17 by default.
+Protobuf v21+ is required. Note that Protobuf v23+ requires C++ 17.


Why is v21 required?

v21 (and greater) is the tested version. Would you prefer different wording?

Yes, maybe reword it as something like v21+ is tested, older versions are not guaranteed to work.

ph4r05 · 2024-03-20T19:29:14Z

@selsta when you have a minute, all comments should be addressed by now. thanks! :)

ph4r05 · 2024-04-03T21:12:01Z

@selsta ping ping :)

selsta · 2024-04-10T01:51:55Z

@ph4r05 sorry, was busy with other things. please squash and then it can be merged

- fix If there is no protobuf version installed, if fails - passphrase test fix, wallet keys init was missing

ph4r05 · 2024-04-10T16:19:46Z

squashed @selsta, thank you!

ph4r05 force-pushed the pr/trezor-cpp17 branch 2 times, most recently from a81b638 to 381d848 Compare November 10, 2023 09:29

ph4r05 changed the title ~~trezor: support c++17 and protobuf v25~~ trezor: support c++17 and protobuf v25, libusb fix Nov 10, 2023

ph4r05 force-pushed the pr/trezor-cpp17 branch from 381d848 to 10b275e Compare November 10, 2023 09:30

ph4r05 changed the title ~~trezor: support c++17 and protobuf v25, libusb fix~~ trezor: support c++17 and protobuf v25, libusb fix #smol Nov 10, 2023

ph4r05 closed this Nov 10, 2023

ph4r05 reopened this Nov 10, 2023

vtnerd reviewed Nov 21, 2023

View reviewed changes

cmake/CheckTrezor.cmake Outdated Show resolved Hide resolved

ph4r05 requested a review from vtnerd November 21, 2023 19:21

0xFFFC0000 added cmake documentation allarch labels Dec 18, 2023

ph4r05 force-pushed the pr/trezor-cpp17 branch from e6a190e to c353be9 Compare February 3, 2024 22:33

ph4r05 force-pushed the pr/trezor-cpp17 branch from 8e3c89a to 18879e8 Compare February 25, 2024 08:40

ph4r05 force-pushed the pr/trezor-cpp17 branch 3 times, most recently from f70de2c to 18879e8 Compare February 25, 2024 20:18

selsta reviewed Mar 10, 2024

View reviewed changes

cmake/CheckTrezor.cmake Outdated Show resolved Hide resolved

selsta reviewed Mar 17, 2024

View reviewed changes

trezor: support c++17 and protobuf v25, libusb fix

4689ad2

- fix If there is no protobuf version installed, if fails - passphrase test fix, wallet keys init was missing

ph4r05 force-pushed the pr/trezor-cpp17 branch from b701c25 to 4689ad2 Compare April 10, 2024 16:19

ph4r05 requested a review from selsta April 10, 2024 16:19

selsta approved these changes Apr 10, 2024

View reviewed changes

jeffro256 approved these changes May 18, 2024

View reviewed changes

luigi1111 merged commit 69ffc1a into monero-project:master May 21, 2024
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

trezor: support c++17 and protobuf v25, libusb fix #smol #9064

trezor: support c++17 and protobuf v25, libusb fix #smol #9064

ph4r05 commented Nov 9, 2023 •

edited

Loading

ph4r05 commented Nov 10, 2023

ph4r05 commented Nov 21, 2023

selsta commented Nov 21, 2023

ph4r05 commented Dec 18, 2023

jeffro256 commented Dec 19, 2023

ph4r05 commented Feb 3, 2024

jeffro256 commented Feb 6, 2024

ph4r05 commented Feb 25, 2024

ph4r05 commented Feb 25, 2024 •

edited

Loading

ph4r05 commented Feb 25, 2024

ph4r05 commented Feb 26, 2024

ph4r05 commented Feb 27, 2024

selsta commented Feb 28, 2024

selsta Mar 10, 2024

ph4r05 Mar 17, 2024

selsta Mar 10, 2024

ph4r05 Mar 17, 2024

ph4r05 commented Mar 17, 2024

selsta Mar 17, 2024

ph4r05 Mar 17, 2024

selsta Mar 17, 2024 •

edited

Loading

ph4r05 Mar 17, 2024

ph4r05 commented Mar 20, 2024

ph4r05 commented Apr 3, 2024

selsta commented Apr 10, 2024

ph4r05 commented Apr 10, 2024

trezor: support c++17 and protobuf v25, libusb fix #smol #9064

trezor: support c++17 and protobuf v25, libusb fix #smol #9064

Conversation

ph4r05 commented Nov 9, 2023 • edited Loading

ph4r05 commented Nov 10, 2023

ph4r05 commented Nov 21, 2023

selsta commented Nov 21, 2023

ph4r05 commented Dec 18, 2023

jeffro256 commented Dec 19, 2023

ph4r05 commented Feb 3, 2024

jeffro256 commented Feb 6, 2024

ph4r05 commented Feb 25, 2024

ph4r05 commented Feb 25, 2024 • edited Loading

ph4r05 commented Feb 25, 2024

ph4r05 commented Feb 26, 2024

ph4r05 commented Feb 27, 2024

selsta commented Feb 28, 2024

selsta Mar 10, 2024

Choose a reason for hiding this comment

ph4r05 Mar 17, 2024

Choose a reason for hiding this comment

selsta Mar 10, 2024

Choose a reason for hiding this comment

ph4r05 Mar 17, 2024

Choose a reason for hiding this comment

ph4r05 commented Mar 17, 2024

selsta Mar 17, 2024

Choose a reason for hiding this comment

ph4r05 Mar 17, 2024

Choose a reason for hiding this comment

selsta Mar 17, 2024 • edited Loading

Choose a reason for hiding this comment

ph4r05 Mar 17, 2024

Choose a reason for hiding this comment

ph4r05 commented Mar 20, 2024

ph4r05 commented Apr 3, 2024

selsta commented Apr 10, 2024

ph4r05 commented Apr 10, 2024

ph4r05 commented Nov 9, 2023 •

edited

Loading

ph4r05 commented Feb 25, 2024 •

edited

Loading

selsta Mar 17, 2024 •

edited

Loading