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

IsWalletTransaction fix #368

Merged
merged 2 commits into from
Apr 1, 2018
Merged

IsWalletTransaction fix #368

merged 2 commits into from
Apr 1, 2018

Conversation

hal0x2328
Copy link
Member

What current issue(s) does this address, or what feature is it adding?

In IsWalletTransaction in neo/Wallets/Wallet.py, as part of evaluating whether a transaction belongs to a wallet or not, neo-python is currently comparing the VerificationScript to all the contract scripthashes in the wallet. However this will never return true, because a scripthash is by definition a hash of a script so it has already gone through the hashing process before the comparison, even if they were the same scripts to start with. As a result, wallet verbose was not showing the complete set of transactions for added contract addresses and watch-only addresses (outbound transactions were missing)

How did you solve this problem?

Changed the comparisons to compare script to script for contracts, and scripthash to scripthash for watch_only addresses where we don't know what the original VerificationScript looks like.

How did you make sure your solution works?

Tested it manually on a local wallet

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)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.407% when pulling 1d63c23 on hal0x2328:development into 0add676 on CityOfZion:development.

@hal0x2328 hal0x2328 changed the title Development IsWalletTransaction fix Apr 1, 2018
@localhuman
Copy link
Collaborator

Looks good, thanks!

@localhuman localhuman merged commit 7c38ef3 into CityOfZion:development Apr 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants