Skip to content

Replace wallet.core with multiversx_sdk_wallet#186

Merged
popenta merged 7 commits into
feat/v4from
integrate-mx-wallet
Jan 17, 2023
Merged

Replace wallet.core with multiversx_sdk_wallet#186
popenta merged 7 commits into
feat/v4from
integrate-mx-wallet

Conversation

@popenta
Copy link
Copy Markdown
Collaborator

@popenta popenta commented Jan 16, 2023

In this PR the a part of the wallet package was replaced with the multiversx_sdk_wallet package. In this PR only wallet.core was replaced. A full replacement will come in a later PR.

@popenta popenta self-assigned this Jan 16, 2023
Comment thread setup.py
"multiversx-sdk-rust-contract-builder==4.0.2",
"multiversx-sdk-network-providers==0.6.4"
"multiversx-sdk-network-providers==0.6.4",
"multiversx-sdk-wallet==0.4.1"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's check if one of the following isn't used anymore:

"pycryptodomex",
"cryptography==36.0.2",

If not used, can be removed from setup.py and requirements.txt.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Removed "pycryptodomex"

Comment thread multiversx_sdk_cli/tests/test_wallet.py Outdated

def test_generate_pair_pem(self):
secret_key, pubkey = generate_pair()
mnemonic = Mnemonic.generate()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps this test can be removed, as well, since it does not test a functionality defined by multiversx_sdk_cli.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Removed

Comment thread multiversx_sdk_cli/tests/test_wallet.py Outdated
from multiversx_sdk_cli.wallet.core import bytes_to_binary_string, generate_mnemonic_from_entropy, split_to_fixed_size_slices
from multiversx_sdk_cli.wallet import pem
from multiversx_sdk_wallet.mnemonic import Mnemonic
from multiversx_sdk_wallet.core import mnemonic_to_bip39seed, bip39seed_to_secret_key
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If not used (should not be used), can be removed.

Copy link
Copy Markdown
Collaborator Author

@popenta popenta Jan 17, 2023

Choose a reason for hiding this comment

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

it is used in the test test_derive_secret_key, but I've deleted the test because it's no longer relevant since we are using the wallet package.

Comment thread multiversx_sdk_cli/tests/shared.sh Outdated
}

export PYTHONPATH=$(absolute_path ../../)
export PYTHONPATH=$PYTHONPATH::$(absolute_path ../../)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we can undo this line.

${CLI} wallet derive ./testdata-out/myaccount.pem || return 1
assertFileExists "./testdata-out/myaccount.pem" || return 1
echo "foo bar\n" | ${CLI} wallet derive --mnemonic ./testdata-out/myaccount-from-mnemonic.pem || return 1
echo -e "moral volcano peasant pass circle pen over picture flat shop clap goat never lyrics gather prepare woman film husband gravity behind test tiger improve\n" | ${CLI} wallet derive --mnemonic ./testdata-out/myaccount-from-mnemonic.pem || return 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍


testGenerationWithSpaces() {
echo "testGenerationWithSpaces"
echo -e "moral volcano peasant pass circle pen over picture flat shop clap goat never lyrics gather prepare woman film husband gravity behind test tiger improve " | ${CLI} wallet derive --mnemonic ./testdata-out/myaccount-from-mnemonic.pem || return 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

mnemonic = generate_mnemonic()
print(f"Mnemonic: {mnemonic}")
secret_key, pubkey = wallet.derive_keys(mnemonic)
mnemonic = Mnemonic.generate()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

json_file = prepare_file(args.output_path, ".json")
password = getpass.getpass("Enter a new password:")
save_to_key_file(json_file, secret_key, pubkey, password)
save_to_key_file(json_file, secret_key.hex(), pubkey.hex(), password)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All right. In a future PR, save_to_key_file can be replaced by the new functions from multiversx-sdk-wallet.

Comment thread multiversx_sdk_cli/cli_wallet.py Outdated
pem.write(pem_file, secret_key, pubkey, name=address.bech32())
if ask_mnemonic:
mnemonic = input("Enter mnemonic:\n").rstrip().replace("\n", "")
print(mnemonic, 'asad')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove debugging print.

Comment thread multiversx_sdk_cli/cli_wallet.py Outdated
pubkey = secret_key.generate_public_key()
else:
mnemonic = Mnemonic.generate()
secret_key = mnemonic.derive_key(index)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now I see that the following can be extracted after if-else (being the same):

secret_key = mnemonic.derive_key(index)
pubkey = secret_key.generate_public_key()

secret_key = mnemonic.derive_key(index)
pubkey = secret_key.generate_public_key()

secret_key = mnemonic.derive_key(index)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now I see that, above, mnemonic = input(...) can be mnemonic_str = input(...), since below it, the same variable is used, but it has a different type - works, of course, but can be adjusted.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Will do it in the next pr.

@popenta popenta merged commit 8f940eb into feat/v4 Jan 17, 2023
@popenta popenta deleted the integrate-mx-wallet branch January 17, 2023 14:23
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.

3 participants