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

Improve parameter parsing [02] #973

Open
wants to merge 17 commits into
base: development
Choose a base branch
from
Open

Improve parameter parsing [02] #973

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
528fb1e
impliment parameter verification function
Jun 28, 2019
485904e
implement user_entry flag to ensure existing logic is not disrupted
Jun 29, 2019
0f1e242
add tests for too few params entered
Jun 29, 2019
10570ba
updated CHANGELOG.rst
Jun 29, 2019
efb6ef4
update tests
Jun 29, 2019
98c8863
remove Ceiling rounding for invocation GAS costs
Jul 3, 2019
b7ffb25
Merge branch 'development' into verify-params
ixje Jul 3, 2019
29e7fc4
restore user_entry flag
ixje Jul 3, 2019
4d32219
fix lint error
ixje Jul 3, 2019
935e526
Merge branch 'development' of https://github.com/CityOfZion/neo-pytho…
Sep 10, 2019
9611d83
update Changelog for 0.9.1-dev
Sep 10, 2019
016f30d
revert removal of `Ceil()` function based on https://github.com/CityO…
Sep 10, 2019
22a3339
update unittest
Sep 10, 2019
19fe4e2
improve per https://github.com/CityOfZion/neo-python/pull/973#pullreq…
Sep 12, 2019
9eddbe2
improve per https://github.com/CityOfZion/neo-python/pull/973#issueco…
Sep 22, 2019
435063d
Merge branch 'development' of https://github.com/CityOfZion/neo-pytho…
Sep 22, 2019
23b50e4
Merge branch 'development' of https://github.com/CityOfZion/neo-pytho…
Sep 30, 2019
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
1 change: 1 addition & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ All notable changes to this project are documented in this file.
- Add workaround for ``Neo.Contract.Create`` SYSCALl accepting invalid ``ContractParameterType``'s until neo-cli fixes it to keep same state
- Fix parsing nested lists `#954 <https://github.com/CityOfZion/neo-python/issues/954>`_
- Fix clearing storage manipulations on failed invocation transaction execution
- Fix param parsing input from command line


[0.8.4] 2019-02-14
Expand Down
7 changes: 4 additions & 3 deletions neo/Prompt/Commands/BuildNRun.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,10 @@ def DoRun(contract_script, arguments, wallet, path, verbose=True,
except Exception:
raise TypeError

tx, result, total_ops, engine = test_deploy_and_invoke(script, i_args, wallet, from_addr,
min_fee, invocation_test_mode, debug_map=debug_map,
invoke_attrs=invoke_attrs, owners=owners, enable_debugger=enable_debugger)
tx, result, total_ops, engine = test_deploy_and_invoke(script, i_args, wallet, from_addr, min_fee,
invocation_test_mode, debug_map=debug_map,
invoke_attrs=invoke_attrs, owners=owners,
enable_debugger=enable_debugger, user_entry=True)
i_args.reverse()

return_type_results = []
Expand Down
49 changes: 38 additions & 11 deletions neo/Prompt/Commands/Invoke.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from neo.SmartContract import TriggerType
from neo.SmartContract.StateMachine import StateMachine
from neo.SmartContract.ContractParameterContext import ContractParametersContext
from neo.SmartContract.ContractParameter import ContractParameterType
from neo.SmartContract.Contract import Contract
from neo.Core.Cryptography.Helper import scripthash_to_address
from neo.Core.Cryptography.Crypto import Crypto
Expand Down Expand Up @@ -163,19 +164,17 @@ def InvokeWithTokenVerificationScript(wallet, tx, token, fee=Fixed8.Zero(), invo
return False


def TestInvokeContract(wallet, args, withdrawal_tx=None, from_addr=None,
min_fee=DEFAULT_MIN_FEE, invoke_attrs=None, owners=None):
def TestInvokeContract(wallet, args, withdrawal_tx=None, from_addr=None, min_fee=DEFAULT_MIN_FEE,
invoke_attrs=None, owners=None, user_entry=False):
BC = GetBlockchain()

contract = BC.GetContract(args[0])

if contract:
#
params = args[1:] if len(args) > 1 else []

params, neo_to_attach, gas_to_attach = PromptUtils.get_asset_attachments(params)
params, parse_addresses = PromptUtils.get_parse_addresses(params)
params.reverse()

if '--i' in params:
params = []
Expand All @@ -185,6 +184,21 @@ def TestInvokeContract(wallet, args, withdrawal_tx=None, from_addr=None,
return None, None, None, None, False
params.append(param)
params.reverse()
elif user_entry:
try:
i_args = []
for index, iarg in enumerate(contract.Code.ParameterList):
param, abort = PromptUtils.verify_params(ContractParameterType(iarg), params[index])
if abort:
return None, None, None, None, False
i_args.append(param)
i_args.reverse()
params = i_args
except IndexError:
print(f"Check params. {len(contract.Code.ParameterList)} params specified and only {len(params)} given.")
return None, None, None, None, False
else:
params.reverse()

sb = ScriptBuilder()

Expand Down Expand Up @@ -385,9 +399,10 @@ def test_invoke(script, wallet, outputs, withdrawal_tx=None,
return None, None, None, None, False


def test_deploy_and_invoke(deploy_script, invoke_args, wallet,
from_addr=None, min_fee=DEFAULT_MIN_FEE, invocation_test_mode=True,
debug_map=None, invoke_attrs=None, owners=None, enable_debugger=False):
def test_deploy_and_invoke(deploy_script, invoke_args, wallet, from_addr=None,
min_fee=DEFAULT_MIN_FEE, invocation_test_mode=True,
debug_map=None, invoke_attrs=None, owners=None,
enable_debugger=False, user_entry=False):
bc = GetBlockchain()

accounts = DBInterface(bc._db, DBPrefix.ST_Account, AccountState)
Expand Down Expand Up @@ -468,16 +483,28 @@ def test_deploy_and_invoke(deploy_script, invoke_args, wallet,
invoke_args, neo_to_attach, gas_to_attach = PromptUtils.get_asset_attachments(invoke_args)
invoke_args, no_parse_addresses = PromptUtils.get_parse_addresses(invoke_args)

invoke_args.reverse()

if '--i' in invoke_args:
invoke_args = []
for index, iarg in enumerate(contract_state.Code.ParameterList):
param, abort = PromptUtils.gather_param(index, iarg)
if abort:
return None, [], 0, None
else:
invoke_args.append(param)
invoke_args.append(param)
invoke_args.reverse()
elif user_entry:
try:
i_args = []
for index, iarg in enumerate(contract_state.Code.ParameterList):
param, abort = PromptUtils.verify_params(ContractParameterType(iarg), invoke_args[index])
if abort:
return None, [], 0, None
i_args.append(param)
i_args.reverse()
invoke_args = i_args
except IndexError:
print(f"Check params. {len(contract_state.Code.ParameterList)} params specified and only {len(invoke_args)} given.")
return None, [], 0, None
else:
invoke_args.reverse()

sb = ScriptBuilder()
Expand Down
3 changes: 2 additions & 1 deletion neo/Prompt/Commands/SC.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ def execute(self, arguments):
logger.debug("invalid fee")
return False

tx, fee, results, num_ops, engine_success = TestInvokeContract(wallet, arguments, from_addr=from_addr, invoke_attrs=invoke_attrs, owners=owners)
tx, fee, results, num_ops, engine_success = TestInvokeContract(wallet, arguments, from_addr=from_addr, invoke_attrs=invoke_attrs,
owners=owners, user_entry=True)
if tx is not None and results is not None:

if return_type is not None:
Expand Down
16 changes: 16 additions & 0 deletions neo/Prompt/Commands/tests/test_sc_commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,15 @@ def test_sc_buildrun(self):
self.assertFalse(tx)
self.assertIn("run `sc build_run help` to see supported queries", mock_print.getvalue())

# test too few args
PromptData.Wallet = self.GetWallet1(recreate=True)
with patch('sys.stdout', new=StringIO()) as mock_print:
args = ['build_run', 'neo/Prompt/Commands/tests/SampleSC.py', 'True', 'False', 'False', '070502', '02', 'add', 'AG4GfwjnvydAZodm4xEDivguCtjCFzLcJy',
] # missing third param
tx, result, total_ops, engine = CommandSC().execute(args)
self.assertFalse(tx)
self.assertIn("Check params. 3 params specified and only 2 given.", mock_print.getvalue())

# test successful build and run
PromptData.Wallet = self.GetWallet1(recreate=True)
with patch('sys.stdout', new=StringIO()) as mock_print:
Expand Down Expand Up @@ -402,6 +411,13 @@ def test_sc_invoke(self):
self.assertFalse(res)
self.assertIn("Error testing contract invoke", mock_print.getvalue())

# test too few args
with patch('sys.stdout', new=StringIO()) as mock_print:
args = ['invoke', token_hash_str, 'name'] # missing second arg
res = CommandSC().execute(args)
self.assertFalse(res)
self.assertIn("Check params. 2 params specified and only 1 given.", mock_print.getvalue())

# test with keyboard interrupt
with patch('sys.stdout', new=StringIO()) as mock_print:
with patch('neo.Prompt.Commands.SC.prompt', side_effect=[KeyboardInterrupt]):
Expand Down
57 changes: 30 additions & 27 deletions neo/Prompt/Utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,35 @@ def get_input_prompt(message):
return prompt(message)


def verify_params(ptype, param):
if ptype == ContractParameterType.String:
return str(param), False
elif ptype == ContractParameterType.Integer:
return int(param), False
elif ptype == ContractParameterType.Boolean:
return bool(param), False
elif ptype == ContractParameterType.PublicKey:
try:
return ECDSA.decode_secp256r1(param).G, False
except ValueError:
return None, True
elif ptype == ContractParameterType.ByteArray:
if isinstance(param, str) and len(param) == 34 and param[0] == 'A':
return Helper.AddrStrToScriptHash(param).Data, False
res = eval(param, {"__builtins__": {'bytearray': bytearray, 'bytes': bytes}}, {})
if isinstance(res, bytes):
return bytearray(res), False
return res, False

elif ptype == ContractParameterType.Array:
res = eval(param)
if isinstance(res, list):
return res, False
raise Exception("Please provide a list")
else:
raise Exception("Unknown param type %s " % ptype.name)


def gather_param(index, param_type, do_continue=True):
ptype = ContractParameterType(param_type)
prompt_message = '[Param %s] %s input: ' % (index, ptype.name)
Expand All @@ -322,33 +351,7 @@ def gather_param(index, param_type, do_continue=True):
return None, True

try:

if ptype == ContractParameterType.String:
return str(result), False
elif ptype == ContractParameterType.Integer:
return int(result), False
elif ptype == ContractParameterType.Boolean:
return bool(result), False
elif ptype == ContractParameterType.PublicKey:
try:
return ECDSA.decode_secp256r1(result).G, False
except ValueError:
return None, True
elif ptype == ContractParameterType.ByteArray:
if isinstance(result, str) and len(result) == 34 and result[0] == 'A':
return Helper.AddrStrToScriptHash(result).Data, False
res = eval(result, {"__builtins__": {'bytearray': bytearray, 'bytes': bytes}}, {})
if isinstance(res, bytes):
return bytearray(res), False
return res, False

elif ptype == ContractParameterType.Array:
res = eval(result)
if isinstance(res, list):
return res, False
raise Exception("Please provide a list")
else:
raise Exception("Unknown param type %s " % ptype.name)
return verify_params(ptype, result)

except KeyboardInterrupt: # Control-C pressed: exit

Expand Down
8 changes: 4 additions & 4 deletions neo/SmartContract/tests/test_gas_costs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_build_contract_3(self):
expected_fee = Fixed8.FromDecimal(.0001)
self.assertEqual(expected_cost, engine.GasConsumed())
self.assertEqual(tx.Gas, expected_fee)
self.assertEqual(result[0].GetByteArray(), bytearray(b'\xab\xab\xab\xab\xab\xab'))
self.assertEqual(result[0].GetByteArray(), bytearray(b'abababababab'))

def test_build_contract_4(self):
"""
Expand All @@ -98,7 +98,7 @@ def test_build_contract_4(self):

tx, result, total_ops, engine = BuildAndRun(arguments, wallet, False)

expected_cost = Fixed8.FromDecimal(2.153)
expected_cost = Fixed8.FromDecimal(3.153)
expected_fee = Fixed8.FromDecimal(.0001)
self.assertEqual(expected_cost, engine.GasConsumed())
self.assertEqual(tx.Gas, expected_fee)
Expand Down Expand Up @@ -126,8 +126,8 @@ def test_build_contract_5(self):

tx, result, total_ops, engine = BuildAndRun(arguments, wallet, False)

expected_cost = Fixed8(1046600000)
expected_gas = Fixed8.FromDecimal(1.0)
expected_cost = Fixed8.FromDecimal(15.466)
expected_gas = Fixed8.FromDecimal(6.0)
Copy link
Member

Choose a reason for hiding this comment

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

Do you know why the GAS costs rises with 5 GAS? That really seems like a lot for (assuming) just an input type parsing difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The difference is the original code pushed the big_str as bytes and the updated code pushes it as a bytearray. Here are the input values just prior to being pushed onto the stack:

Original:

b'abababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababab' Length: 2048
b'6b657931' Length: 8
b'7075745f35' Length: 10

Updated:

bytearray(b'abababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababab') Length: 2048
b'6b657931' Length: 8
b'7075745f35' Length:10

Copy link
Member

Choose a reason for hiding this comment

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

Using the development branch, I get a result of 1.0 GAS used when invoking with b’abababab…' and 6.0 GAS used when invoking with b’ghghghgh…' so 5 GAS is the difference between parsing that size of an argument as a hex string vs parsing it as an ascii string. The changes in the test_gas_costs.py would seem to be correct if the parser is no longer arbitrarily converting strings to binary just because they look like hexadecimal values.

self.assertEqual(expected_cost, engine.GasConsumed())
self.assertEqual(tx.Gas, expected_gas)

Expand Down