cosme12 / SimpleCoin

Just a really simple, insecure and incomplete implementation of a blockchain for a cryptocurrency made in Python as educational material. In other words, a simple Bitcoin clone.
http://copitosystem.com
MIT License
1.78k stars 397 forks source link

update wallet.py #25

Closed ghost closed 6 years ago

ghost commented 6 years ago

I'm very very new of python but i like this project. I have added the code to create wallet file.

adanielpincab commented 6 years ago

cosme12 said that this type of changes has to been pulled to branch v1.1.0, not master You can quickly change this in "edit pull request"

cosme12 commented 6 years ago

Dont worry. I changed it for you. But someone already did a change similar to yours. Check branch v1.1.0

ghost commented 6 years ago

sorry, I did not know he had to go on v1.1.0. I see only wallet.py in master. Sorry

cosme12 commented 6 years ago

@KKK00 dont worry. Check if this is better: branch v1.1.0

ghost commented 6 years ago

yes is the same think :)

Aareon commented 6 years ago

@KKK00 I would suggest using the with statement for opening files. So this;

new_file=open("wallet", "a")
new_file.write(f"Private key: {private_key}" + f"\nWallet address / Public key: {public_key.decode()}")
print(f"Your new address and private key are now in the file {new_file}")
new_file.close()

Becomes this (with some additional changes I’ve made);

filename = “wallet”
with open(filename, “a”) as f:
    f.write(“Private key: {}\nWallet address / Public key: {}”.format(private_key, public_key.decode())
print(“Your new address and private key have now been saved to {}”.format(filename))

I’ve removed f-strings to retain compatibility with Python versions up to 3.5. F-strings are a Python 3.6 feature. In addition, there is really no need to concatenate those two strings together. Simply make it a single string and format it all at once 👍

Is there no related file extension this file should be using? I feel like we should at least not store private keys in a plaintext file.

Aareon commented 6 years ago

I’ve created PR https://github.com/cosme12/SimpleCoin/pull/28 which should address some of these issues, plus a few other things.