CityOfZion / neo-python

Python Node and SDK for the NEO 2.x blockchain. For NEO 3.x go to our successor project neo-mamba
https://neo-python.readthedocs.io/en/latest/
MIT License
313 stars 189 forks source link

Fix simple policy implementation for outgoing transactions #989

Closed jseagrave21 closed 5 years ago

jseagrave21 commented 5 years ago

What current issue(s) does this address, or what feature is it adding? Addresses #960; specifically, https://github.com/CityOfZion/neo-python/issues/960#issuecomment-512248474

How did you solve this problem? implement SimplePolicy check when relaying any tx

How did you make sure your solution works? make test with unittests

Are there any special changes in the code that we should be aware of? Replaced TXFeeError with TXError to incorporate a check for violating the tx maximum size

Please check the following, if applicable:

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.003%) to 85.266% when pulling 5e65d840132f71efc6b956fe543a4e4b3ddcb9ae on jseagrave21:fix-SimplePolicy into 36c2860488311a827898a9cd636467c5eaccb711 on CityOfZion:development.

ixje commented 5 years ago

note to self; gave slack feedback regarding late notification if funds are insufficient

ixje commented 5 years ago

I had another look at the code because it just didn't feel right to have relay() be responsible for policy validation. One of the issues in resolving this in the best way possible is that certain code parts do too much e.g. https://github.com/CityOfZion/neo-python/blob/fe90f62e123d720d4281c79af0598d9df9e776fb/neo/Prompt/Commands/Invoke.py#L48

does not just invoke a contract, it also sign's the TX and this is a problem for reporting the required fees prior to asking if the TX should be send to the network. Instead of resolving the latter now I want to keep that in mind for the NEO3 build.

I moved the policy validation into its own function and out of the relay() code because those 2 should not be tied together. In doing so I could also simplify some of the test logic.

ixje commented 5 years ago

🎉