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

Support #888 #911

Merged
merged 32 commits into from
Mar 21, 2019
Merged

Support #888 #911

merged 32 commits into from
Mar 21, 2019

Conversation

jseagrave21
Copy link
Contributor

What current issue(s) does this address, or what feature is it adding?
Supports CityOfZion/neo-python-core#169 which addresses #888.

How did you solve this problem?
See CityOfZion/neo-python-core#169

How did you make sure your solution works?
make test

Are there any special changes in the code that we should be aware of?
No

Please check the following, if applicable:

  • Did you add any tests?
  • Did you run make lint?
  • Did you run make test?
  • Are you making a PR to a feature branch or development rather than master?
  • Did you add an entry to CHANGELOG.rst? (if not, please do)

jseagrave21 and others added 29 commits October 9, 2018 20:38
Merge CoZ Development into jseagrave21 Development
Merge CoZ Development into jseagrave21 Development
Merge CoZ Development into jseagrave21 Development
* Update ExtendedJsonRpcApi.py

- add fix provided by @localhuman so original methods are returned as well as extended methods
…yOfZion#661)

* Add the option -u (unittest-net) to prompt.py

* Add unittest guildeline and add the smart contract source codes (UnitTest-SM.zip) to the fixtures package
for compatibility
* Fix ExtendedJsonRpcApi (CityOfZion#662)

* Update ExtendedJsonRpcApi.py

- add fix provided by @localhuman so original methods are returned as well as extended methods

* Mute expected test stacktrace and clearly identify why an exception is thrown. (CityOfZion#663)

* Add guideline for adding tests to the neo-privnet-unittest image (CityOfZion#661)

* Add the option -u (unittest-net) to prompt.py

* Add unittest guildeline and add the smart contract source codes (UnitTest-SM.zip) to the fixtures package

* Add raw transaction building examples (CityOfZion#665)

* Update neo-boa version to fix core building test
Merge CoZ Development into jseagrave21 Development
Merge CoZ Development into jseagrave21 Development
Merge CoZ Development into jseagrave21 Development
Merge CoZ Development into jseagrave21 development
Merge CoZ development into jseagrave21 development
- add tests for "showallaccounts" and "showallaccountstates"
Merge neo-python development into jseagrave21 development
Merge neo-python development into jseagrave21 development
Merge neo-python development into jseagrave21 development
Merge CoZ development into jseagrave21 development
Gracefully handle NEP-5 balance query failures (CityOfZion#744)
Merge CoZ development into neo-python development
Merge CoZ development into jseagrave21 development
Merge CoZ Development into jseagrave21 development
Merge CoZ/neo-python development into jseagrave21/neo-python development
Copy link
Member

@ixje ixje left a comment

Choose a reason for hiding this comment

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

The made changes all look good and tests pass if I pull your neo-python-core version. A couple of improvements that we can do (needs CityOfZion/neo-python-core#169 (review))

  1. this can now throw a ValueError
    while reader.ReadUInt8() == 33:
    ecpoint = ecdsa.ec.decode_from_hex(binascii.hexlify(reader.ReadBytes(33)).decode())
    points.append(ecpoint)
    ms.close()

    Please update to
try:
  while reader.ReadUInt8() == 33:
     ecpoint = ecdsa.ec.decode_from_hex(binascii.hexlify(reader.ReadBytes(33)).decode())
     points.append(ecpoint)
except ValueError:
   return False
finally:
   ms.close()
  1. return ECDSA.decode_secp256r1(result).G, False

    replace with
try:
   return ECDSA.decode_secp256r1(result).G, False
except ValueError:
   return None, True
  1. pubkey = ECDSA.decode_secp256r1(items[0]).G

    update to capture ValueError and inform user that their public key is invalid

  2. owner = ECDSA.decode_secp256r1(ownerData, unhex=False).G

    update to capture ValueError and return False

  3. point = ECDSA.decode_secp256r1(hashOrPubkey, unhex=False).G

    update to capture ValueError and return False

Note: I skipped the calls in the Blockchain, Contract, ContractParameter classes on purpose as they already have undefined behaviour. I don't have the time to investigate them now and apparently they've worked fine up to now. The above requests have a clear way of dealing with them

@ixje
Copy link
Member

ixje commented Mar 18, 2019

2 small requests;

  1. can you point travis to the development branch of neo-python-core. I just created that and already holds your core exception changes. So this change will allow us to continue working without having to already do a neo-python-core and neo-python release.

Before

- pip install -e git+https://github.com/CityOfZion/neo-python-core@master#egg=neocore

After

 - pip install -e git+https://github.com/CityOfZion/neo-python-core@development#egg=neocore 
  1. please resolve the changelog

@coveralls
Copy link

coveralls commented Mar 18, 2019

Coverage Status

Coverage decreased (-0.04%) to 86.532% when pulling d1261a1 on jseagrave21:support-#888 into 2fd24ce on CityOfZion:development.

@ixje
Copy link
Member

ixje commented Mar 21, 2019

💪

@ixje ixje merged commit 8a08b0d into CityOfZion:development Mar 21, 2019
@ixje
Copy link
Member

ixje commented Mar 21, 2019

200 points jseagrave

@jseagrave21 jseagrave21 deleted the support-#888 branch March 21, 2019 08:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants