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

Change bech32 prefix for 1854h purpose (shared wallets) #2870

Merged
merged 5 commits into from
Sep 7, 2021

Conversation

paweljakubas
Copy link
Contributor

@paweljakubas paweljakubas commented Sep 6, 2021

  • Prefix in bech32 is now acct_shared_ when 1854H is used
  • core unit tests and swagger updated
  • added integration tests check of the fix

Comments

Issue Number

adp-1114

@paweljakubas paweljakubas self-assigned this Sep 6, 2021
@piotr-iohk
Copy link
Contributor

Ok, so now when we create acct pub key from "Shelley" wallet with purpose 1854H we get acct_shared_xvk prefix:

curl -X POST http://localhost:8090/v2/wallets/82e11bf3aea612e92158855f702180eda754cea3/keys/0H \
-d '{"passphrase":"Secure Passphrase","format":"extended","purpose":"1854H"}' \
-H "Content-Type: application/json"

acct_shared_xvk1afqnpkmp32s54cl0w6k65tuewujm0v74ll0hclq67cae42u569edztaj7g4a2w0jpxa9avzhwkp2450acwf34mn0khw2g8fpd477tdglcle2g

OK.

However, it is not quite in-line with cardano-addresses, because when we create acct pub key there on "Shelley" wallet with purpose 1854H we still get acct_xvk:

echo budget picture inside clown oxygen grab laugh together emotion doctor betray defy ski face retire include loan salmon sign poverty soccer risk install gift \
| cardano-address key from-recovery-phrase Shelley \ <--------------------
| cardano-address key child 1854H/1815H/0H \
| cardano-address key public --with-chain-code

acct_xvk1afqnpkmp32s54cl0w6k65tuewujm0v74ll0hclq67cae42u569edztaj7g4a2w0jpxa9avzhwkp2450acwf34mn0khw2g8fpd477tdgwxt5v3

We need to explicitly say "Shared" to get proper prefix:

echo budget picture inside clown oxygen grab laugh together emotion doctor betray defy ski face retire include loan salmon sign poverty soccer risk install gift \
| cardano-address key from-recovery-phrase Shared \ <--------------------
| cardano-address key child 1854H/1815H/0H \
| cardano-address key public --with-chain-code

acct_shared_xvk1afqnpkmp32s54cl0w6k65tuewujm0v74ll0hclq67cae42u569edztaj7g4a2w0jpxa9avzhwkp2450acwf34mn0khw2g8fpd477tdglcle2g

So, I guess we need a change on cardano-addresses side too? What do you think @paweljakubas?

Copy link
Contributor

@piotr-iohk piotr-iohk left a comment

Choose a reason for hiding this comment

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

LGTM.

(note the remark/question around cardano-address change)

@@ -963,6 +969,8 @@ spec = describe "SHELLEY_ADDRESSES" $ do
}|]
(_, accXPub3) <- unsafeRequest @ApiAccountKey ctx accountPath payload3
accXPub1 `shouldNotBe` accXPub3
let (Aeson.String accXPub3Txt) = toJSON accXPub3
T.isPrefixOf "acct_shared_xvk" accXPub3Txt `shouldBe` True
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@piotr-iohk
Copy link
Contributor

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 7, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit 64e2614 into master Sep 7, 2021
@iohk-bors iohk-bors bot deleted the paweljakubas/adp-1114/change-prexix-for-1854H branch September 7, 2021 09:19
WilliamKingNoel-Bot pushed a commit that referenced this pull request Sep 7, 2021
@rvl rvl changed the title Change bench prefix when 1854h purpose Change bech32 prefix for 1854h purpose (shared wallets) Sep 8, 2021
@piotr-iohk piotr-iohk removed the Bug label Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants