search

CityOfZion / neo-boa

Python compiler for the Neo2 Virtual Machine, see neo3-boa for NEO3
MIT License
69 stars 55 forks source link

Fixes known issues in the crowdsale demo #100

Closed WyattMufson closed 5 years ago

WyattMufson commented 6 years ago

What current issue(s) from Github does this address?

98 #99

What problem does this PR solve? Adds OnTransfer to the minting in the deploy function and fixes total supply. Also makes sure that the nep5 functions are checking that the arguments are in the right format

How did you solve this problem?

98 and #99 are pretty self explanatory. Length check for addresses for the argument formatting issue

How did you make sure your solution works? Standardized it with the standalone NEP5 demo and another contract that I've tested on my private net

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

Is there anything else we should know? The NEP5 demo has slightly different code than the one in the crowdsale demo. They should probably be fully standardized in the future

WyattMufson commented 6 years ago

Looks like some of the tests are failing (like one due to the use of totalSupply over circulation). Fixing these now and will commit the updates tests.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 92.508% when pulling ddc9873b230bbd74427eb54d6ecc684476b247e5 on WyattMufson:master into acd9793e0c77b06203a2f9a0cdf012db3f91034d on CityOfZion:master.

WyattMufson commented 6 years ago

Now also meant to address #101 and #102

WyattMufson commented 5 years ago

Interested in getting feedback on this^

localhuman commented 5 years ago

Looks good, thanks for the changes!

I would only add that the demo is only for compiler testing purposes and shouldn't be confused for something that is a production ready NEP5 token.

WyattMufson commented 5 years ago

@localhuman is there an example of a place with a production ready NEP5 contract? The awesome-neo repo lists these contracts and the ICO-Templates repo is empty so far. Would it make sense to put some of this there?