-
Notifications
You must be signed in to change notification settings - Fork 225
Add integration tests for postRandomWalletFromXPrv #3196
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
Conversation
\326f4f777a736e4e746f4e655049416e4f6b7978426549494a6b59623039574\ | ||
\b564a7159493d" | ||
, password = | ||
"0000000000000000000000000000000000000050415441544520504154415445" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think password is probably not fit for use litelarly in cardano-wallet as it's most likely old wallet's "format".
In old wallet password was:
The spendingPassword is optional but highly recommended. It a string of 32 characters, encoded in base 16, yielding to an hexadecimal sequence of 64 bytes. This passphrase is required for sensitive operations on the wallet and adds an extra security layer to it.
Having said that:
$ echo "0000000000000000000000000000000000000050415441544520504154415445" | xxd -p -r
PATATE PATATE
(classic) ;P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
THANKS! It now works!
I had to both
- fix the passphrase
- drop the
hashMaybe
I was suspicious of frompreparePassphrase
(and saw no trace of in cardano-sl)
This might mean we have found the culprit for https://input-output.atlassian.net/browse/ADP-1335 then 🤞
bors try I'm guessing some old verify-passphrase golden test might fail because the checkPassphrase function wasn't updated. Let's see. |
tryBuild failed: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤞
62d63e9
to
cde1767
Compare
, password = "" | ||
} | ||
where | ||
runByronScryptGolden :: ByronScryptGolden -> ActionWith Context |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should add a comment somewhere explaining what it does. Or add to the hspec title.
cde1767
to
db9c4ad
Compare
bors try |
tryMerge conflict. |
postRandomWalletFromXPrv
can spend funds (using goldens Piotr pointed to: export legacy wallets in a friendly way for automatic import in cardano-wallet input-output-hk/cardano-sl#4278 (comment))hashMaybe
frompreparePassphrase
(required to make the test with a non-empty passphrase pass)Comments
Integration tests are:
Some tested scenarios
master
(without cde1767) -> first test failscabal: Add a flag to enable/disable scrypt #2767 -> first test fails
cabal: Add a flag to enable/disable scrypt #2767 with e3edfa0 reverted -> both tests fail
cabal: Add a flag to enable/disable scrypt #2767 dropping
hashMaybe
-> both tests passI hope the passing tests shows the need for e3edfa0 in that PR, but haven't tested it.
Mainly wanted to bring this to your attention, but I guess it might as well be merged as well.
Issue Number
Partly related to ADP-842 and potentially (that's what we'd like to find out) to https://input-output.atlassian.net/browse/ADP-1335