Add PUSHNULL and ISNULL
#208
Conversation
Codecov Report
@@ Coverage Diff @@
## master #208 +/- ##
==========================================
- Coverage 79.32% 78.87% -0.45%
==========================================
Files 16 17 +1
Lines 1422 1444 +22
==========================================
+ Hits 1128 1139 +11
- Misses 294 305 +11
Continue to review full report at Codecov.
|
|
I'm glad a new Stack Item is not being created... I'll check this. Any way to avoid impacting XDROP? |
It was created xD |
|
Really? Why? |
|
How we can ensure that a byte[0] is an empty array or null? |
Simple. Another possibility is using Interop stack item, which already assumes The point is, we have many options for this. But in the end, what bother me even more, is so many dangerous side changes made here... why? What about these many opcode relocations on basic stack operations? Are you all aware that these things may break virtually everything written down on NeoVM so far? https://neoresearch.io/nvm-learn/#stack-operations |
|
Can we reorder opcodes in another PR, and focus only on the null situation here? I think this helps a quicker agreement. |
|
@erikzhang and @shargon , I reverted stack operation changes. I propose that we create a new PR and reorder everything from scratch. If we redesign all opcode positions, we can get much better distribution, and more clear separation between prefixes. What do you think? |
|
For the next commit, I opened a PR here: #210 [UPDATE]: I tried the ByteArray(null) approach, and it became worse than a Null stackitem. The reason is that, because of the change I made, every time we got a bytearray from the ByteArray stackitem, we would need to ensure it is null or not, which is terrible for bugs. In this sense,although it solves the problem,bug surface increases on ByteArray, which is a fundamental type. Better keep it non-null,and Null constrained to a new type. |
There was a problem hiding this comment.
Reverted unrelated changes, trying another approach on PR #210
[UPDATE]: that didn't work, this approach here is better.
There was a problem hiding this comment.
I tried to make it better, but I couldn't. This approach with a new stack item seems reasonable to me, given the problem it's trying to solve.
Regarding opcode positions, I would prefer in another PR and had reverted some decisions made here... but after that, I re-reverted everything (to the way it was before my participation). This way, you are free to move on. Congratulations for the good work, as always ;)
|
Could be merged? |
|
hi, I wonder how much is the systemfee of these two opcodes? |
Related to: neo-project/neo#1112