Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

Use libscrypt from Hunter #4873

Merged
merged 3 commits into from
Mar 6, 2018
Merged

Use libscrypt from Hunter #4873

merged 3 commits into from
Mar 6, 2018

Conversation

chfast
Copy link
Member

@chfast chfast commented Mar 6, 2018

I fixed 2 tests, disabled 1.

The disabled test tests keyfile v2. I'm not sure what the expected result should be. I've found this commit adding the compatibility with v2: 8399161 but it does not include the test. I cannot find where the test was added.

Resolves #4815.

The decrypted private key does not match the expected one. I'm not able to track down the history of this test case.
@chfast chfast requested review from gumb0 and pirapira March 6, 2018 15:18
Copy link
Member

@pirapira pirapira left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@axic
Copy link
Member

axic commented Mar 6, 2018


The following tests FAILED:
	  2 - RandomCodeTests/rndStateTest (SEGFAULT)

@@ -155,6 +158,3 @@ if (TESTS)
enable_testing()
add_subdirectory(test)
endif()

# TODO: Split out json_spirit, libscrypt and sec256k1.
add_subdirectory(utils)
Copy link
Member

Choose a reason for hiding this comment

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

So its not even needed for json_spirit?

Copy link
Member Author

Choose a reason for hiding this comment

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

We use json_spirit by loading headers only. This is an old CMake script that was not used.

@chfast
Copy link
Member Author

chfast commented Mar 6, 2018

This test is failing from time to time only on CircleCI / macOS. To be disabled, because we cannot find the reason.

@chfast chfast merged commit edea476 into develop Mar 6, 2018
@chfast chfast deleted the hunter-libscrypt branch March 6, 2018 23:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants