This repository was archived by the owner on Nov 15, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 188
Add support for wallet
send
, sendmany
, sign
, open
, and close
#726
Merged
ixje
merged 37 commits into
CityOfZion:refactor-prompt
from
jseagrave21:jseagrave21-send,sendmany,sign,open,close
Nov 28, 2018
Merged
Changes from all commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
ba231b1
Update Send.py
jseagrave21 1529505
Update Wallet.py
jseagrave21 dba3ffb
Update prompt.py
jseagrave21 7d94d3c
Update Send.py
jseagrave21 191b28a
Update Wallet.py
jseagrave21 7d14b07
Update prompt.py
jseagrave21 dd8030f
Update CommandBase.py
jseagrave21 d9f5102
Update Wallet.py
jseagrave21 ad864fc
Update CommandBase.py
jseagrave21 309c594
Update Wallet.py
jseagrave21 0a12c55
Update Wallet.py
jseagrave21 de79875
Update Wallet.py
jseagrave21 54d5166
Merge pull request #75 from CityOfZion/refactor-prompt
jseagrave21 86396e0
Update prompt.py
jseagrave21 fe95dee
Update Send.py
jseagrave21 d288e4d
Update test_send_command.py
jseagrave21 d654416
Update Wallet.py
jseagrave21 01076dc
Update CommandBase.py
jseagrave21 38e77fb
Update CommandBase.py
jseagrave21 f6db1fe
Merge pull request #76 from CityOfZion/refactor-prompt
jseagrave21 5d766cc
Update prompt.py
jseagrave21 1944217
Update Wallet.py
jseagrave21 1a390d2
Update CommandBase.py
jseagrave21 6755c6a
Update Send.py
jseagrave21 809faab
Update test_send_command.py
jseagrave21 9f2d7f1
Update Wallet.py
jseagrave21 18d5ec0
Update CommandBase.py
jseagrave21 72bf6cb
Update test_send_command.py
jseagrave21 b2958e6
Update Wallet.py
jseagrave21 6e1e95c
Update Wallet.py
jseagrave21 faaa972
Update test_wallet_commands.py
jseagrave21 963baee
Update test_send_command.py
jseagrave21 f53f0a8
Update test_wallet_commands.py
jseagrave21 041ad14
Update Wallet.py
jseagrave21 9c3b679
Update prompt.py
jseagrave21 e7c9914
Update Wallet.py
jseagrave21 d870595
Update PromptData.py
jseagrave21 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,16 +13,77 @@ | |
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: | ||
return process_transaction(PromptData.Wallet, contract_tx=framework[0], scripthash_from=framework[1], | ||
fee=framework[2], owners=framework[3], user_tx_attributes=framework[4]) | ||
return framework | ||
|
||
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: | ||
return 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]) | ||
return framework | ||
|
||
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) | ||
return 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 | ||
|
||
arguments, from_address = get_from_addr(arguments) | ||
arguments, priority_fee = get_fee(arguments) | ||
|
@@ -35,60 +96,58 @@ 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 | ||
if type(assetId) is NEP5Token: | ||
return do_token_transfer(assetId, wallet, from_address, address_to, amount_from_string(assetId, amount), tx_attributes=user_tx_attributes) | ||
return do_token_transfer(assetId, wallet, from_address, address_to, amount_from_string(assetId, amount), | ||
tx_attributes=user_tx_attributes) | ||
|
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should return something here too (and other early returns). There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
@@ -103,23 +162,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) | ||
|
@@ -130,22 +189,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] | ||
|
@@ -160,13 +219,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() | ||
|
||
|
@@ -216,14 +275,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): | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Because
construct_send_basic
can return[contract_tx, scripthash_from, fee, owners, user_tx_attributes]
it should returnNone
orFalse
in early returns.Uh oh!
There was an error while loading. Please reload this page.
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.
@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.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.
see #716 (comment)
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 agree for
boolean
and something else (I suggestedFalse
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?
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'm indifferent when it comes to
return
orreturn None
. I guess I'm used to the idea thatreturn
always returnsNone
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 ;)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.
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 atuple
. That's just not workable.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 for the answer, I won't make this as a blocker for this PR, so @jseagrave21 as you want 😄
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 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.