Skip to content
This repository was archived by the owner on Nov 15, 2021. It is now read-only.

Add support for wallet send, sendmany, sign, open, and close #726

Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
ba231b1
Update Send.py
jseagrave21 Nov 27, 2018
1529505
Update Wallet.py
jseagrave21 Nov 27, 2018
dba3ffb
Update prompt.py
jseagrave21 Nov 27, 2018
7d94d3c
Update Send.py
jseagrave21 Nov 27, 2018
191b28a
Update Wallet.py
jseagrave21 Nov 27, 2018
7d14b07
Update prompt.py
jseagrave21 Nov 27, 2018
dd8030f
Update CommandBase.py
jseagrave21 Nov 27, 2018
d9f5102
Update Wallet.py
jseagrave21 Nov 27, 2018
ad864fc
Update CommandBase.py
jseagrave21 Nov 27, 2018
309c594
Update Wallet.py
jseagrave21 Nov 27, 2018
0a12c55
Update Wallet.py
jseagrave21 Nov 27, 2018
de79875
Update Wallet.py
jseagrave21 Nov 27, 2018
54d5166
Merge pull request #75 from CityOfZion/refactor-prompt
jseagrave21 Nov 27, 2018
86396e0
Update prompt.py
jseagrave21 Nov 27, 2018
fe95dee
Update Send.py
jseagrave21 Nov 27, 2018
d288e4d
Update test_send_command.py
jseagrave21 Nov 27, 2018
d654416
Update Wallet.py
jseagrave21 Nov 27, 2018
01076dc
Update CommandBase.py
jseagrave21 Nov 27, 2018
38e77fb
Update CommandBase.py
jseagrave21 Nov 27, 2018
f6db1fe
Merge pull request #76 from CityOfZion/refactor-prompt
jseagrave21 Nov 27, 2018
5d766cc
Update prompt.py
jseagrave21 Nov 27, 2018
1944217
Update Wallet.py
jseagrave21 Nov 27, 2018
1a390d2
Update CommandBase.py
jseagrave21 Nov 27, 2018
6755c6a
Update Send.py
jseagrave21 Nov 27, 2018
809faab
Update test_send_command.py
jseagrave21 Nov 27, 2018
9f2d7f1
Update Wallet.py
jseagrave21 Nov 27, 2018
18d5ec0
Update CommandBase.py
jseagrave21 Nov 27, 2018
72bf6cb
Update test_send_command.py
jseagrave21 Nov 27, 2018
b2958e6
Update Wallet.py
jseagrave21 Nov 27, 2018
6e1e95c
Update Wallet.py
jseagrave21 Nov 27, 2018
faaa972
Update test_wallet_commands.py
jseagrave21 Nov 27, 2018
963baee
Update test_send_command.py
jseagrave21 Nov 27, 2018
f53f0a8
Update test_wallet_commands.py
jseagrave21 Nov 27, 2018
041ad14
Update Wallet.py
jseagrave21 Nov 27, 2018
9c3b679
Update prompt.py
jseagrave21 Nov 28, 2018
e7c9914
Update Wallet.py
jseagrave21 Nov 28, 2018
d870595
Update PromptData.py
jseagrave21 Nov 28, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions neo/Prompt/CommandBase.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,11 @@ def command_desc(self):
pass

# Raise KeyError exception if the command does not exist
def execute_sub_command(self, id, arguments):
self.__sub_commands[id].execute(arguments)
def execute_sub_command(self, id, arguments=None):
if arguments is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, the original signature execute(self, arguments) should be respected. None could be given as arguments for commands like CommandWalletClose.

self.__sub_commands[id].execute(arguments)
else:
self.__sub_commands[id].execute()

def register_sub_command(self, sub_command, additional_ids=[]):
"""
Expand Down
110 changes: 82 additions & 28 deletions neo/Prompt/Commands/Send.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,73 @@
import json
from prompt_toolkit import prompt
import traceback
from neo.Prompt.PromptData import PromptData
from neo.Prompt.CommandBase import CommandBase, CommandDesc, ParameterDesc
from logzero import logger


class CommandWalletSend(CommandBase):

def __init__(self):
super().__init__()

def execute(self, arguments):
framework = construct_send_basic(PromptData.Wallet, arguments)
if type(framework) is list:
process_transaction(PromptData.Wallet, contract_tx=framework[0], scripthash_from=framework[1], fee=framework[2], owners=framework[3], user_tx_attributes=framework[4])

def command_desc(self):
p1 = ParameterDesc('assetId or name', 'the asset you wish to send')
p2 = ParameterDesc('address', 'the NEO address you will send to')
p3 = ParameterDesc('amount', 'the amount of the asset you wish to send')
p4 = ParameterDesc('--from-addr={addr}', 'specify the NEO address you wish to send from', optional=True)
p5 = ParameterDesc('--fee={priority_fee}', 'attach a fee to give your tx priority (> 0.001)', optional=True)
p6 = ParameterDesc('--owners=[{addr}, ...]', 'specify tx owners', optional=True)
p7 = ParameterDesc('--tx-attr=[{"usage": <value>,"data":"<remark>"}, ...]', 'specify unique tx attributes', optional=True)
params = [p1, p2, p3, p4, p5, p6, p7]
return CommandDesc('send', 'send an asset', params=params)


class CommandWalletSendMany(CommandBase):

def __init__(self):
super().__init__()

def execute(self, arguments):
framework = construct_send_many(PromptData.Wallet, arguments)
if type(framework) is list:
process_transaction(PromptData.Wallet, contract_tx=framework[0], scripthash_from=framework[1], scripthash_change=framework[2], fee=framework[3], owners=framework[4], user_tx_attributes=framework[5])

def command_desc(self):
p1 = ParameterDesc('number of outgoing tx', 'the number of tx you wish to send')
p2 = ParameterDesc('--change-addr={addr}', 'specify the change address', optional=True)
p3 = ParameterDesc('--from-addr={addr}', 'specify the NEO address you wish to send from', optional=True)
p4 = ParameterDesc('--fee={priority_fee}', 'attach a fee to give your tx priority (> 0.001)', optional=True)
p5 = ParameterDesc('--owners=[{addr}, ...]', 'specify tx owners', optional=True)
p6 = ParameterDesc('--tx-attr=[{"usage": <value>,"data":"<remark>"}, ...]', 'specify unique tx attributes', optional=True)
params = [p1, p2, p3, p4, p5, p6]
return CommandDesc('sendmany', 'send multiple contract transactions', params=params)


class CommandWalletSign(CommandBase):

def __init__(self):
super().__init__()

def execute(self, arguments):
jsn = get_arg(arguments)
parse_and_sign(PromptData.Wallet, jsn)

def command_desc(self):
p1 = ParameterDesc('jsn', 'transaction in JSON format')
params = [p1]
return CommandDesc('sign', 'sign multi-sig tx', params=params)


def construct_send_basic(wallet, arguments):
if not wallet:
print("please open a wallet")
return False
if len(arguments) < 3:
print("Not enough arguments")
return False
return
Copy link
Contributor

Choose a reason for hiding this comment

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

Because construct_send_basic can return [contract_tx, scripthash_from, fee, owners, user_tx_attributes] it should return None or False in early returns.

Copy link
Contributor Author

@jseagrave21 jseagrave21 Nov 28, 2018

Choose a reason for hiding this comment

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

@LysanderGG
@ixje has explained to me that it is undesirable to have a function return both a boolean and say a function. Instead if you just leave return blank, it returns none, which will also be interpreted as False, so all my tests pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree for boolean and something else (I suggested False because it was this way before).
I think it should return None explicitly then.

When I see return (nothing) I assume that the function doesn't return anything. It can be misleading imo.

@ixje any thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I'm indifferent when it comes to return or return None. I guess I'm used to the idea that return always returns None under the hood, so to speak. Might as well make it explicit if that clarifies it for all future readers. I'll probably need to slapping on the fingers to remind me of doing that though ;)

Copy link
Member

Choose a reason for hiding this comment

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

The add a few more words regarding return values; at some point I expect to start applying gradual typing. We've had past cases where functions were returning None, bool or a tuple. That's just not workable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the answer, I won't make this as a blocker for this PR, so @jseagrave21 as you want 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to leave it this way so the format is consistent with #716. Consistency is what matters most to me so people will be able to follow our example throughout the code.


arguments, from_address = get_from_addr(arguments)
arguments, priority_fee = get_fee(arguments)
Expand All @@ -35,19 +92,19 @@ def construct_send_basic(wallet, arguments):
assetId = get_asset_id(wallet, to_send)
if assetId is None:
print("Asset id not found")
return False
return

scripthash_to = lookup_addr_str(wallet, address_to)
if scripthash_to is None:
logger.debug("invalid address")
return False
return

scripthash_from = None
if from_address is not None:
scripthash_from = lookup_addr_str(wallet, from_address)
if scripthash_from is None:
logger.debug("invalid address")
return False
return

# if this is a token, we will use a different
# transfer mechanism
Expand All @@ -57,38 +114,35 @@ def construct_send_basic(wallet, arguments):
f8amount = get_asset_amount(amount, assetId)
if f8amount is False:
logger.debug("invalid amount")
return False
return
if float(amount) == 0:
print("amount cannot be 0")
return False
return

fee = Fixed8.Zero()
if priority_fee is not None:
fee = priority_fee
if fee is False:
logger.debug("invalid fee")
return False
return

output = TransactionOutput(AssetId=assetId, Value=f8amount, script_hash=scripthash_to)
contract_tx = ContractTransaction(outputs=[output])
return [contract_tx, scripthash_from, fee, owners, user_tx_attributes]


def construct_send_many(wallet, arguments):
if not wallet:
print("please open a wallet")
return False
if len(arguments) is 0:
print("Not enough arguments")
return False
return
Copy link
Contributor

@LysanderGG LysanderGG Nov 28, 2018

Choose a reason for hiding this comment

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

Should return something here too (and other early returns).

Copy link
Contributor Author

Choose a reason for hiding this comment

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


outgoing = get_arg(arguments, convert_to_int=True)
if outgoing is None:
print("invalid outgoing number")
return False
return
if outgoing < 1:
print("outgoing number must be >= 1")
return False
return

arguments, from_address = get_from_addr(arguments)
arguments, change_address = get_change_addr(arguments)
Expand All @@ -103,23 +157,23 @@ def construct_send_many(wallet, arguments):
assetId = get_asset_id(wallet, to_send)
if assetId is None:
print("Asset id not found")
return False
return
if type(assetId) is NEP5Token:
print('Sendmany does not support NEP5 tokens')
return False
return
address_to = prompt("Address to: ")
scripthash_to = lookup_addr_str(wallet, address_to)
if scripthash_to is None:
logger.debug("invalid address")
return False
return
amount = prompt("Amount to send: ")
f8amount = get_asset_amount(amount, assetId)
if f8amount is False:
logger.debug("invalid amount")
return False
return
if float(amount) == 0:
print("amount cannot be 0")
return False
return
tx_output = TransactionOutput(AssetId=assetId, Value=f8amount, script_hash=scripthash_to)
output.append(tx_output)
contract_tx = ContractTransaction(outputs=output)
Expand All @@ -130,22 +184,22 @@ def construct_send_many(wallet, arguments):
scripthash_from = lookup_addr_str(wallet, from_address)
if scripthash_from is None:
logger.debug("invalid address")
return False
return

scripthash_change = None

if change_address is not None:
scripthash_change = lookup_addr_str(wallet, change_address)
if scripthash_change is None:
logger.debug("invalid address")
return False
return

fee = Fixed8.Zero()
if priority_fee is not None:
fee = priority_fee
if fee is False:
logger.debug("invalid fee")
return False
return

print("sending with fee: %s " % fee.ToString())
return [contract_tx, scripthash_from, scripthash_change, fee, owners, user_tx_attributes]
Expand All @@ -160,13 +214,13 @@ def process_transaction(wallet, contract_tx, scripthash_from=None, scripthash_ch

if tx is None:
logger.debug("insufficient funds")
return False
return

# password prompt
passwd = prompt("[Password]> ", is_password=True)
if not wallet.ValidatePassword(passwd):
print("incorrect password")
return False
return

standard_contract = wallet.GetStandardAddress()

Expand Down Expand Up @@ -216,14 +270,14 @@ def process_transaction(wallet, contract_tx, scripthash_from=None, scripthash_ch
else:
print("Transaction initiated, but the signature is incomplete")
print(json.dumps(context.ToJson(), separators=(',', ':')))
return False
return

except Exception as e:
print("could not send: %s " % e)
traceback.print_stack()
traceback.print_exc()

return False
return


def parse_and_sign(wallet, jsn):
Expand Down
71 changes: 68 additions & 3 deletions neo/Prompt/Commands/Wallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from neo.Implementations.Wallets.peewee.Models import Account
from neo.Prompt.CommandBase import CommandBase, CommandDesc, ParameterDesc
from neo.Prompt.PromptData import PromptData
from neo.Prompt.Commands.Send import CommandWalletSend, CommandWalletSendMany, CommandWalletSign
from neo.logging import log_manager


Expand All @@ -30,9 +31,14 @@ def __init__(self):
super().__init__()

self.register_sub_command(CommandWalletCreate())
self.register_sub_command(CommandWalletOpen())
self.register_sub_command(CommandWalletClose())
self.register_sub_command(CommandWalletVerbose(), ['v', '--v'])
self.register_sub_command(CommandWalletMigrate())
self.register_sub_command(CommandWalletCreateAddress())
self.register_sub_command(CommandWalletSend())
self.register_sub_command(CommandWalletSendMany())
self.register_sub_command(CommandWalletSign())

def command_desc(self):
return CommandDesc('wallet', 'manage wallets')
Expand All @@ -42,7 +48,7 @@ def execute(self, arguments):
item = get_arg(arguments)

# Create and Open must be handled specially.
if item == 'create':
if item in {'create', 'open'}:
self.execute_sub_command(item, arguments[1:])
return

Expand All @@ -55,7 +61,10 @@ def execute(self, arguments):
return

try:
self.execute_sub_command(item, arguments[1:])
if len(arguments) > 1:
self.execute_sub_command(item, arguments[1:])
else:
self.execute_sub_command(item)
except KeyError:
print(f"Wallet: {item} is an invalid parameter")

Expand All @@ -66,6 +75,8 @@ def __init__(self):
super().__init__()

def execute(self, arguments):
if PromptData.Wallet:
CommandWalletClose().execute()
path = get_arg(arguments, 0)

if path:
Expand Down Expand Up @@ -109,12 +120,66 @@ def command_desc(self):
return CommandDesc('create', 'creates a new NEO wallet address', [p1])


class CommandWalletVerbose(CommandBase):
class CommandWalletOpen(CommandBase):

def __init__(self):
super().__init__()

def execute(self, arguments):
if PromptData.Wallet:
CommandWalletClose().execute()

path = get_arg(arguments, 0)

if path:

if not os.path.exists(path):
print("Wallet file not found")
return
Copy link
Contributor

@LysanderGG LysanderGG Nov 28, 2018

Choose a reason for hiding this comment

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

Should return something here too (and other early returns).

Copy link
Contributor Author

Choose a reason for hiding this comment

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


passwd = prompt("[password]> ", is_password=True)
password_key = to_aes_key(passwd)

try:
PromptData.Wallet = UserWallet.Open(path, password_key)

PromptData.Prompt.start_wallet_loop()
print("Opened wallet at %s" % path)
return PromptData.Wallet
except Exception as e:
print("Could not open wallet: %s" % e)

else:
print("Please specify a path")

def command_desc(self):
p1 = ParameterDesc('path', 'path to open the wallet file')
return CommandDesc('open', 'opens a NEO wallet', [p1])


class CommandWalletClose(CommandBase):

def __init__(self):
super().__init__()

def execute(self):
if PromptData.Wallet:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the implementation could be moved to a static method or to PromptData.
It could avoid having to instanciate CommandWalletClose to call execute. (CommandWalletClose().execute() in prompt.py).

What do you think?

Copy link
Contributor Author

@jseagrave21 jseagrave21 Nov 27, 2018

Choose a reason for hiding this comment

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

This is why I originally had it as a class method. But changed it after your review. Couldn't we just keep it as a class method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A static method still required me to instantiate the class

Copy link
Contributor

Choose a reason for hiding this comment

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

I realize that my message was probably confusing, sorry for that.

I was suggesting to keep execute as is (to override execute(self arguments)) and have it call another static method.
Imo a @classmethod should only be used when you want to override it in a child class or when you need to have access to cls.

I think the following should work:

class CommandWalletClose(CommandBase):
...
def execute(self, arguments=None):
    CommandWalletClose.close_wallet()


@staticmethod
def close_wallet():
    if not PromptData.Wallet:
        return False

    path = PromptData.Wallet._path
    PromptData.Prompt.stop_wallet_loop()
    PromptData.Wallet.Close()
    PromptData.Wallet = None
    print("Closed wallet %s" % path)
    return True

It should be possible to call it with CommandWalletClose.close_wallet().
I think it would be even better to have this function in PromptData and call PromptData.close_wallet().

Copy link
Contributor Author

@jseagrave21 jseagrave21 Nov 28, 2018

Choose a reason for hiding this comment

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

@LysanderGG Thank you. I will give it a shot. I will try it your preferred way.

path = PromptData.Wallet._path
PromptData.Prompt.stop_wallet_loop()
PromptData.Wallet.Close()
PromptData.Wallet = None
print("Closed wallet %s" % path)

def command_desc(self):
return CommandDesc('close', 'closes the open NEO wallet')


class CommandWalletVerbose(CommandBase):

def __init__(self):
super().__init__()

def execute(self):
print("Wallet %s " % json.dumps(PromptData.Wallet.ToJson(verbose=True), indent=4))

def command_desc(self):
Expand Down
Loading