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

Nep8 #635

Merged
merged 20 commits into from Sep 26, 2018
Merged

Nep8 #635

merged 20 commits into from Sep 26, 2018

Conversation

localhuman
Copy link
Collaborator

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

This addresses #585, and supersedes #603. It adds in NEP8 functionality from here: CityOfZion/neo-boa#94

Also makes usage of NEP8 compiler features configurable via protocol.json and from the prompt, with the default value being False until NEP8 is on mainnet. From the prompt, you can use config compiler-nep8 on|off

How did you solve this problem?

How did you make sure your solution works?

  • tests

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

  • no. But thanks to @ixje for taking the initiative and getting this important feature implemented!

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)

ixje and others added 19 commits September 10, 2018 11:58
- Restore appending executed script hashes
- apply AltStack replication fixes of neo-project/neo-vm@59f3e66
- fix CheckStackSize: neo-project/neo@e8c5e03
- fix CheckDynamicInvoke: neo-project/neo@b8ad89e
and general update to checkstacksize as it was lagging behind
limit arraysize in 'block.gettransactions' neo-project/neo@9660630
limit itemsize in 'runtime_serialize' neo-project/neo@5894cbd
add description for new stack isolation instructions
@coveralls
Copy link

coveralls commented Sep 25, 2018

Coverage Status

Coverage increased (+0.2%) to 78.91% when pulling e7f3caf on nep8 into 3a518c1 on 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.

Good find on the CALL_E(D). I knew some extra tests would be handy after all :)

neo/Settings.py Outdated
@@ -203,6 +205,9 @@ def setup(self, config_file):
if 'ServiceEnabled' in config:
self.SERVICE_ENABLED = bool(config['ServiceEnabled'])

if 'CompilerNep8' in config:
self.COMPILER_NEP_8 = bool(config['CompilerNep8'])
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to suggest to at least use config.get('CompilerNep8', False) because the AcceptIncomingPeers update showed that quite some users just copy/replace their configs with their old one after they upgraded. Then this call will fail because the key is not there. Ideally we might want to have a utility function that tries to get the key, if not uses a specified default value but also warns the user that the key was not found and is now using the "x" as the default.

@localhuman
Copy link
Collaborator Author

🥇

@localhuman localhuman merged commit 963cce5 into development Sep 26, 2018
@ixje ixje deleted the nep8 branch September 26, 2018 17:13
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