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

build: fix build with Boost 1.85 and remove instances of viewkey logging [RELEASE] #9462

Open
wants to merge 1 commit into
base: release-v0.18
Choose a base branch
from

Conversation

jeffro256
Copy link
Contributor

  1. Use std::is_standard_layout and std::is_trivially_copyable instead of std::is_pod for KV byte-wise serialization, which fixes compile issue for Boost UUIDs
  2. Removed reimplementation of std::hash for boost::uuids::uuid
  3. Removed << operator overload for crypto::secret_key
  4. Removed instances in code where private view key was dumped to the log in plaintext

Release version of #9450

Copy link
Contributor

@sneurlax sneurlax left a comment

Choose a reason for hiding this comment

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

LGTM

@woodser
Copy link
Contributor

woodser commented Sep 9, 2024

I can build v0.18.3.4 on macOS by applying this PR, but I'm still getting this error building monero-cpp, when importing cryptonote_basic/account.h:

In file included from /Users/woodser/git/monero-ts/external/monero-cpp/src/wallet/monero_wallet_keys.h:56:
In file included from /Users/woodser/git/monero-ts/external/monero-cpp/external/monero-project/src/cryptonote_basic/account.h:33:
In file included from /Users/woodser/git/monero-ts/external/monero-cpp/external/monero-project/src/cryptonote_basic/cryptonote_basic.h:41:
In file included from /Users/woodser/git/monero-ts/external/monero-cpp/external/monero-project/src/serialization/binary_archive.h:43:
/Users/woodser/git/monero-ts/external/monero-cpp/external/monero-project/contrib/epee/include/span.h:165:5: error: static assertion failed due to requirement 'std::is_trivially_copyable<epee::mlocked<tools::scrubbedcrypto::ec_scalar>>()': type must be trivially copyable
static_assert(std::is_trivially_copyable<T>(), "type must be trivially copyable");

Any ideas?

@woodser
Copy link
Contributor

woodser commented Sep 9, 2024

Any ideas?

Found the fix, to add rct::sk2rct to keys within monero-cpp.

@jeffro256
Copy link
Contributor Author

@woodser, as @vtnerd pointed out, it would be a good idea to use unwrap(unwrap(s)) instead of rct::sk2rct(s) because it is more explicit, and rct::sk2rt might copy in the future.

…ing [RELEASE]

1. Use std::is_standard_layout and std::is_trivially_copyable instead of std::is_pod for KV byte-wise serialization, which fixes compile issue for Boost UUIDs
2. Removed reimplementation of std::hash for boost::uuids::uuid
3. Removed << operator overload for crypto::secret_key
4. Removed instances in code where private view key was dumped to the log in plaintext

Release version of monero-project#9450, containing C++14 modified assertions
@woodser
Copy link
Contributor

woodser commented Sep 21, 2024

Any chance we could merge this PR to the release branch, to fix the build for folks on macOS?

Otherwise our build is broken, which is making development of PRs across people difficult.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants