cabal: Add a flag to enable/disable scrypt #2767

rvl · 2021-07-19T18:21:56Z

Issue Number

Overview

Adds cabal flag so that cardano-wallet can be compiled without the scrypt package.
Replaces scrypt with cryptonite for the integration tests only

If scrypt does not compile for your target system, then disable it with:

cabal configure --disable-tests --enable-benchmarks -f-scrypt

Comments

I'm not too keen on changing the scrypt package in the main code, for the time being. But we can replace this package in the integration tests, without too much risk.

Need to use conditional modules instead of CPP because that breaks stylish-haskell.

HeinrichApfelmus · 2021-07-20T11:53:02Z

lib/core-integration/src/Test/Integration/Framework/DSL.hs

        . CBOR.toStrictByteString
        . CBOR.encodeBytes
        . BA.convert

+encryptPasswordWithScrypt :: (BA.ByteArrayAccess a, BA.ByteArray b) => a -> IO b


For posterity: The documentation of the scrypt package notes that the ByteString returned by Scrypt.getEncryptedPass contains not just the encrypted password, but also the parameters and salt used to encrypt it. In contrast, the generate function from the cryptonite package only seems to return that encrypted password.

The difference does not matter for the integration test, but it would matter for legacy wallets (hence the choice to keep the scrypt dependency in the core library.)

Yes good point - thanks - this will need fixing.

HeinrichApfelmus

Looks good to me!

ghost · 2021-09-16T11:17:52Z

I can confirm that without scrypt you can build v2021-09-09 of the cardano wallet on ARM64 without any further issue. I wanted to use it on the paper wallet terminal with cardano-python for testing. Which uses a raspberry pi. Just so you know it is smooth sailing after this 👍

hamishmack · 2021-10-18T03:46:01Z

I have merged in the latest master branch so that we can try this out for testing daedalus on aarch64-darwin. There were quite a few conflicts most of which related to adjacent changes to import statements.

bakon11 · 2021-11-15T18:50:26Z

I can confirm that without scrypt you can build v2021-09-09 of the cardano wallet on ARM64 without any further issue. I wanted to use it on the paper wallet terminal with cardano-python for testing. Which uses a raspberry pi. Just so you know it is smooth sailing after this +1

Hi,

How were you able to build it without scrypt? I am trying to build v2021-11-11.

I'm running:
cabal configure --disable-tests --enable-benchmarks -f-scrypt

And it still shows me that scrypt-0.5.0 needs to build and attempts to build it on cabal build all command.

Thanks.

ghost · 2021-11-16T10:57:29Z

@bakon11 Was just to check if there were major hurdles beyond this. I removed all references to scrypt in the source code. Not recommended to really use it this way. IMHO, I am not an expert. The nix build of this repository worked like this 'nix-build -A cardano-wallet'. But it requires a lot of storage space.

bakon11 · 2021-11-16T13:24:17Z

@bakon11 Was just to check if there were major hurdles beyond this. I removed all references to scrypt in the source code. Not recommended to really use it this way. IMHO, I am not an expert. The nix build of this repository worked like this 'nix-build -A cardano-wallet'. But it requires a lot of storage space.

gotcha, so the scrypt flag that should signal cabal not to build it, won't work if there is actual references to scrypt in the source correct?

Yeah, I agree removing it is not the best solution, but it's a solution to help keep on building a proof of concept project until a real fix is available.

ghost · 2021-11-16T14:28:01Z

gotcha, so the scrypt flag that should signal cabal not to build it, won't work if there is actual references to scrypt in the source correct?

Yeah, I agree removing it is not the best solution, but it's a solution to help keep on building a proof of concept project until a real fix is available.

Yes indeed. What are you building? If you want to discuss you can find me on my discord here M2tec discord

angerman · 2022-01-04T15:02:50Z

@HeinrichApfelmus, @rvl can we get this over the line? I've run into this today.

rvl · 2022-01-25T11:54:13Z

Will be updating this PR soon, please hold tight.

chisNaN · 2022-03-08T08:13:30Z

Hello thanks for the work
any news about this update?
Waiting the arm version to install daedalus on mac m1 pro

Anviking · 2022-03-18T08:59:06Z

bors try

iohk-bors · 2022-03-18T09:38:11Z

try

Build succeeded:

Anviking · 2022-03-24T15:46:07Z

bors try

Anviking · 2022-03-24T15:50:31Z

@rvl if you're ok with this we can merge

iohk-bors · 2022-03-24T17:33:13Z

try

Build succeeded:

I'm not too keen on changing the scrypt package in the main code, for the time being. But we can replace this package in the integration tests, without too much risk.

I simply moved them to LegacySpec, and ensured LegacySpec only ran when scrypt was set. However, I'm seeing the following failures with +scrypt set: test/unit/Cardano/Wallet/Primitive/Passphrase/LegacySpec.hs:53:9: 1) Cardano.Wallet.Primitive.Passphrase.Legacy, Scrypt tests-only cryptonite version, Verify passphrase expected: True but got: False To rerun use: --match "/Cardano.Wallet.Primitive.Passphrase.Legacy/Scrypt tests-only cryptonite version/Verify passphrase/" test/unit/Cardano/Wallet/Primitive/Passphrase/LegacySpec.hs:68:9: 2) Cardano.Wallet.Primitive.Passphrase.Legacy, Scrypt tests-only cryptonite version, getSalt expected: Just "abc" but got: Just "+jottFIZjVmXoC7LlTBZWlYzBX4QQwfduFuxCkMHUis=" To rerun use: --match "/Cardano.Wallet.Primitive.Passphrase.Legacy/Scrypt tests-only cryptonite version/getSalt/" test/unit/Cardano/Wallet/Primitive/Passphrase/LegacySpec.hs:84:13: 3) Cardano.Wallet.Primitive.Passphrase.Legacy, Scrypt tests-only cryptonite version, golden test legacy passphrase encryption, compare new implementation with cardano-sl - short password expected: Right () but got: Left ErrWrongPassphrase To rerun use: --match "/Cardano.Wallet.Primitive.Passphrase.Legacy/Scrypt tests-only cryptonite version/golden test legacy passphrase encryption/compare new implementation with cardano-sl - short password/" test/unit/Cardano/Wallet/Primitive/Passphrase/LegacySpec.hs:94:13: 4) Cardano.Wallet.Primitive.Passphrase.Legacy, Scrypt tests-only cryptonite version, golden test legacy passphrase encryption, compare new implementation with cardano-sl - normal password expected: Right () but got: Left ErrWrongPassphrase To rerun use: --match "/Cardano.Wallet.Primitive.Passphrase.Legacy/Scrypt tests-only cryptonite version/golden test legacy passphrase encryption/compare new implementation with cardano-sl - normal password/" test/unit/Cardano/Wallet/Primitive/Passphrase/LegacySpec.hs:103:13: 5) Cardano.Wallet.Primitive.Passphrase.Legacy, Scrypt tests-only cryptonite version, golden test legacy passphrase encryption, compare new implementation with cardano-sl - empty password expected: Right () but got: Left ErrWrongPassphrase To rerun use: --match "/Cardano.Wallet.Primitive.Passphrase.Legacy/Scrypt tests-only cryptonite version/golden test legacy passphrase encryption/compare new implementation with cardano-sl - empty password/" test/unit/Cardano/Wallet/Primitive/Passphrase/LegacySpec.hs:113:13: 6) Cardano.Wallet.Primitive.Passphrase.Legacy, Scrypt tests-only cryptonite version, golden test legacy passphrase encryption, compare new implementation with cardano-sl - cardano-wallet password expected: Right () but got: Left ErrWrongPassphrase To rerun use: --match "/Cardano.Wallet.Primitive.Passphrase.Legacy/Scrypt tests-only cryptonite version/golden test legacy passphrase encryption/compare new implementation with cardano-sl - cardano-wallet password/" Randomized with seed 866879004 Finished in 28.5483 seconds, used 40.2343 seconds of CPU time 85 examples, 6 failures

1. Swap order of cborify and hashMaybe This makes the legacy golden tests pass, and matches the old implementation. 2. Fix getSalt implementation The salt should be converted from base64. This fixes the test "Verify passphrase" calling `checkPassphraseTestingOnly`. The getSalt golden value was updated. With the "Verify passphrase" test now succeeding, it seems like the previous expected salt value — "abc" — was just a dummy value, and not the right expectation.

This way it matches the implemetnation on `master`.

I manually tested these on current master — before the commits of this PR — and they passed.

rvl · 2022-04-01T15:32:35Z

I checked it and rebased again. It looks good to merge.

rvl · 2022-04-01T15:34:51Z

bors r+

iohk-bors · 2022-04-01T16:29:17Z

Build succeeded:

chisNaN · 2022-04-06T10:05:06Z

Hello I have been following this topic and this one also input-output-hk/daedalus#2677
My question is the release of the apple silicon version wallet will happen soon?

Thanks

gaytomycode · 2022-05-21T01:16:54Z

any update on the scrypt issue? I'm getting this on an apple m1. I tried using the flag mentioned in the thread above and same error
cabal: Failed to build scrypt-0.5.0 (which is required by test:unit from cardano-wallet-core-2022.4.27).

rvl self-assigned this Jul 19, 2021

rvl mentioned this pull request Jul 19, 2021

cabal build all uses the -msse2 FLAG = Compiling on arm64 - how to reconfigure #2578

Closed

rvl force-pushed the rvl/2578/scrypt-flag branch 2 times, most recently from 5b8fd57 to 33c6bb5 Compare July 20, 2021 06:04

rvl requested a review from HeinrichApfelmus July 20, 2021 06:05

HeinrichApfelmus reviewed Jul 20, 2021

View reviewed changes

HeinrichApfelmus approved these changes Jul 20, 2021

View reviewed changes

rvl force-pushed the rvl/2578/scrypt-flag branch from bfa2462 to 497954e Compare July 22, 2021 11:13

rvl marked this pull request as draft July 22, 2021 11:13

rvl mentioned this pull request Jul 27, 2021

Rp4 patches #2777

Closed

1 task

rvl force-pushed the rvl/2578/scrypt-flag branch from 497954e to 24da1b0 Compare August 1, 2021 18:24

hamishmack force-pushed the rvl/2578/scrypt-flag branch from 09d5f87 to 2e21852 Compare October 20, 2021 02:28

hamishmack mentioned this pull request Oct 21, 2021

niv update cardano-wallet -b rvl/2578/scrypt-flag input-output-hk/daedalus#2717

Merged

rvl force-pushed the rvl/2578/scrypt-flag branch from d956343 to c77a34d Compare January 21, 2022 14:22

danielmain mentioned this pull request Feb 8, 2022

Native Apple ARM64 Support input-output-hk/daedalus#2677

Open

Anviking force-pushed the rvl/2578/scrypt-flag branch from cf505a1 to 4242c54 Compare March 9, 2022 15:34

Anviking force-pushed the rvl/2578/scrypt-flag branch 2 times, most recently from 75f4b0c to 6b48f57 Compare March 18, 2022 08:19

iohk-bors bot added a commit that referenced this pull request Mar 18, 2022

Try #2767:

6210382

iohk-bors bot added a commit that referenced this pull request Mar 24, 2022

Try #2767:

d7180f7

Anviking marked this pull request as ready for review March 24, 2022 15:47

rvl and others added 15 commits April 2, 2022 01:11

cabal: Add a flag to enable/disable scrypt

d02dade

Replace scrypt with cryptonite for tests only

3c0f89c

I'm not too keen on changing the scrypt package in the main code, for the time being. But we can replace this package in the integration tests, without too much risk.

Fixes

e56beef

Fix conversion error

d423fe0

Turn off scrypt for aarch64

af09329

Fix missing install_name_tool and work around code signing issue

330e509

Fixes for cardano-wallet-core-integration

75fb0c8

Rebase and warning fixups

7f271f8

Tweaks to PassphraseSpec

0e4d2a8

Move cborify outside preparePassphrase

a064897

This way it matches the implemetnation on `master`.

Add goldens for pbkdf2 passwords

3fcd21f

I manually tested these on current master — before the commits of this PR — and they passed.

Use more hex in passphrase tests

33d535a

Regenerate nix

72d44e9

rvl force-pushed the rvl/2578/scrypt-flag branch from cdf9a54 to 72d44e9 Compare April 1, 2022 15:32

iohk-bors bot merged commit 6035ab5 into master Apr 1, 2022

iohk-bors bot deleted the rvl/2578/scrypt-flag branch April 1, 2022 16:29

WilliamKingNoel-Bot pushed a commit that referenced this pull request Apr 1, 2022

docs: Merge #2767

c1d3946

HeinrichApfelmus mentioned this pull request Feb 5, 2024

[ADP-3173] Reinstate integration tests for macOS #4429

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cabal: Add a flag to enable/disable scrypt #2767

cabal: Add a flag to enable/disable scrypt #2767

rvl commented Jul 19, 2021 •

edited

Loading

HeinrichApfelmus Jul 20, 2021

rvl Jul 22, 2021

HeinrichApfelmus left a comment

ghost commented Sep 16, 2021

hamishmack commented Oct 18, 2021

bakon11 commented Nov 15, 2021

ghost commented Nov 16, 2021

bakon11 commented Nov 16, 2021

ghost commented Nov 16, 2021 •

edited by ghost

Loading

angerman commented Jan 4, 2022

rvl commented Jan 25, 2022

chisNaN commented Mar 8, 2022

Anviking commented Mar 18, 2022

iohk-bors bot commented Mar 18, 2022

Anviking commented Mar 24, 2022

Anviking commented Mar 24, 2022

iohk-bors bot commented Mar 24, 2022

rvl commented Apr 1, 2022

rvl commented Apr 1, 2022

iohk-bors bot commented Apr 1, 2022

chisNaN commented Apr 6, 2022

gaytomycode commented May 21, 2022 •

edited

Loading

cabal: Add a flag to enable/disable scrypt #2767

cabal: Add a flag to enable/disable scrypt #2767

Conversation

rvl commented Jul 19, 2021 • edited Loading

Issue Number

Overview

Comments

HeinrichApfelmus Jul 20, 2021

Choose a reason for hiding this comment

rvl Jul 22, 2021

Choose a reason for hiding this comment

HeinrichApfelmus left a comment

Choose a reason for hiding this comment

ghost commented Sep 16, 2021

hamishmack commented Oct 18, 2021

bakon11 commented Nov 15, 2021

ghost commented Nov 16, 2021

bakon11 commented Nov 16, 2021

ghost commented Nov 16, 2021 • edited by ghost Loading

angerman commented Jan 4, 2022

rvl commented Jan 25, 2022

chisNaN commented Mar 8, 2022

Anviking commented Mar 18, 2022

iohk-bors bot commented Mar 18, 2022

try

Anviking commented Mar 24, 2022

Anviking commented Mar 24, 2022

iohk-bors bot commented Mar 24, 2022

try

rvl commented Apr 1, 2022

rvl commented Apr 1, 2022

iohk-bors bot commented Apr 1, 2022

chisNaN commented Apr 6, 2022

gaytomycode commented May 21, 2022 • edited Loading

rvl commented Jul 19, 2021 •

edited

Loading

ghost commented Nov 16, 2021 •

edited by ghost

Loading

gaytomycode commented May 21, 2022 •

edited

Loading